Skip to content

Conversation

@Gusted
Copy link
Contributor

@Gusted Gusted commented Feb 9, 2022

  • Use a better and more curated list of Ciphers and KeyExchanges, these roughly follows OpenSSH's default.
  • Remove some cryptography values which were deprecated.

⚠️ BREAKING ⚠️

I'm removing some cryptography values which I'd really don't like to set as default value, so it's able that special older devices will have trouble SSH'n to the server:

  • "hmac-sha1-96", SHA1 is a phasing out hash algorithm, this variant is even truncating it to 96(this was for compatibility issues with older devices), all should be able to support "hmac-sha1".
  • "diffie-hellman-group1-sha1", "group1" is important here, and the deciding factor why this is being removed, as it is corresponding to a specific prime that would be used for p(as in mod p, see a example), the prime is defined here and is 1024-bit long. While 512-bit primes have been attacked and has caused real-world harm it's better to remove to this completely as well, not secure, not safe and definitely not should be preferred.
  • "arcfour{128,256}", Arcfour is actually RC4 a old steam cipher, which anything to go by the security section of wikipedia is a pretty good cipher to secure your top secret files with sarcasm.

- Use a better and more curated list of Ciphers and KeyExchanges, these roughly follows OpenSSH's default.
- Remove some cryptography values which were deprecated.
@Gusted Gusted added pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! labels Feb 9, 2022
@Gusted Gusted added this to the 1.17.0 milestone Feb 9, 2022
@techknowlogick techknowlogick changed the title Update SSH Server crypto settings Remove deprecated SSH ciphers from default Feb 10, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 10, 2022
@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 Feb 10, 2022
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@0c70b4c). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #18697   +/-   ##
=======================================
  Coverage        ?   46.44%           
=======================================
  Files           ?      851           
  Lines           ?   121925           
  Branches        ?        0           
=======================================
  Hits            ?    56628           
  Misses          ?    58418           
  Partials        ?     6879           
Impacted Files Coverage Δ
modules/setting/setting.go 45.77% <ø> (ø)

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 0c70b4c...c70190e. Read the comment docs.

@6543 6543 merged commit 581d29e into go-gitea:main Feb 10, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 11, 2022
* giteaofficial/main:
  Prevent double encoding of branch names in delete branch (go-gitea#18714)
  [skip ci] Updated translations via Crowdin
  Attempt to improve docs (yet again) (go-gitea#18687)
  Make the proformas clearer that we need DEBUG logs (go-gitea#18700)
  Update SSH Server crypto settings (go-gitea#18697)
  Fix bug for version update hint (go-gitea#18701)
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
- Use a better and more curated list of Ciphers and KeyExchanges, these roughly follows OpenSSH's default.
- Remove some cryptography values which were deprecated.
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
@Gusted Gusted deleted the update-ssh-settings branch June 18, 2022 17:10
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/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.

6 participants