Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable DSA ssh keys by default #13056

Merged
merged 8 commits into from
Oct 9, 2020

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Oct 6, 2020

OpenSSH has disabled DSA keys since version 7.0

DSA keys are considered insecure - we should therefore disable these by default. This means we need to turn on minimum key size checks by default too.

Fix #11417

Signed-off-by: Andrew Thornton art27@cantab.net

OpenSSH has disabled DSA keys since version 7.0

As the docker runs openssh > v7.0 we should just disable
DSA keys by default.

Refers to go-gitea#11417

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the topic/distribution This PR changes something about the packaging of Gitea label Oct 6, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 6, 2020
@zeripath zeripath changed the title Disable DSA ssh keys by default Disable DSA ssh keys by default in docker Oct 6, 2020
@zeripath
Copy link
Contributor Author

zeripath commented Oct 6, 2020

We could just go even further and disable DSA keys by default for Gitea

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath changed the title Disable DSA ssh keys by default in docker Disable DSA ssh keys by default Oct 6, 2020
@zeripath zeripath added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Oct 6, 2020
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@codecov-io
Copy link

codecov-io commented Oct 6, 2020

Codecov Report

Merging #13056 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13056      +/-   ##
==========================================
+ Coverage   42.59%   42.63%   +0.03%     
==========================================
  Files         672      672              
  Lines       73785    73787       +2     
==========================================
+ Hits        31428    31458      +30     
+ Misses      37260    37233      -27     
+ Partials     5097     5096       -1     
Impacted Files Coverage Δ
modules/setting/setting.go 47.61% <100.00%> (ø)
modules/queue/unique_queue_disk_channel.go 53.84% <0.00%> (-1.54%) ⬇️
models/error.go 34.85% <0.00%> (+0.50%) ⬆️
modules/git/repo.go 46.70% <0.00%> (+0.50%) ⬆️
cmd/serv.go 2.72% <0.00%> (+0.65%) ⬆️
modules/log/event.go 58.96% <0.00%> (+1.41%) ⬆️
models/ssh_key.go 54.32% <0.00%> (+1.94%) ⬆️
modules/git/utils.go 77.04% <0.00%> (+3.27%) ⬆️
modules/indexer/stats/queue.go 76.47% <0.00%> (+11.76%) ⬆️
modules/log/console.go 66.66% <0.00%> (+13.88%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d492422...390c528. Read the comment docs.

@CirnoT
Copy link
Contributor

CirnoT commented Oct 7, 2020

Tests need updating TestAddReadOnlyDeployKey

@lafriks lafriks added this to the 1.13.0 milestone Oct 9, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 9, 2020
@lafriks lafriks merged commit ea69ec6 into go-gitea:master Oct 9, 2020
@zeripath zeripath deleted the disable-DSA-keys-in-docker branch October 9, 2020 07:02
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@6543 6543 added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Mar 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! topic/distribution This PR changes something about the packaging of Gitea topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manage SSH keys will accept DSA keys even if opensshd is refusing them
7 participants