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

feat: add mitigation for the d(HE)ater DoS attack via ssh #19803

Closed
wants to merge 2 commits into from

Conversation

wolviecb
Copy link
Contributor

Hi,

This PR adds SSHd configuration to mitigate the ssh d(HE)eater DoS attack, the mitigation consists in disabling a Diffie-Hellman Key Exchange algorithm (diffie-hellman-group1-sha1), more details can be found at https://github.com/Balasys/dheater
It also applies some ssh hardening from https://www.ssh-audit.com/hardening_guides.html namely settings a hardened list of Key Exchange Algorithms, Ciphers, HostKeyAlgorithms and Message Authentication Code Algorithms.

Please let me know if you need any more clarification.

@silverwind
Copy link
Member

silverwind commented May 25, 2022

Having to maintain these cipher/kex lists will be an additional maintenance burden. What is the stance of OpenSSH regarding this CVE? Is it or are they planning a fix? If yes, I suggest we just update openssh via alpine (which is 3.16 now) and leave those ciphers at their default.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 25, 2022
@wolviecb
Copy link
Contributor Author

From what I could find OpenSSH will not fix this in future releases (https://www.mail-archive.com/[email protected]/msg15476.html), I agree that this list is painful, and it could be reduced to just the change in line 26 since the other lines are extra hardening, not related to the CVE (CVE-2002-20001 btw), but will still be painful. Anyway, if you believe keeping just line 26 I'm fine to update the PR if you still believe it's not worth maintaining I can drop the PR too.

Thanks for your time :)

@wxiaoguang
Copy link
Contributor

wxiaoguang commented May 26, 2022

There could be a configurable option like LogLevel ${SSH_LOG_LEVEL} or ${SSH_MAX_SESSIONS} to let users decide, instead of hard-coding the ciphers inside the config file.


Update: since there are quite a few options, maybe it's better to introduce a general config for Include (https://man7.org/linux/man-pages/man5/sshd_config.5.html), allow users to write anything they need in their private config files.

@wolviecb
Copy link
Contributor Author

There could be a configurable option like LogLevel ${SSH_LOG_LEVEL} or ${SSH_MAX_SESSIONS} to let users decide, instead of hard-coding the ciphers inside the config file.

Update: since there are quite a few options, maybe it's better to introduce a general config for Include (https://man7.org/linux/man-pages/man5/sshd_config.5.html), allow users to write anything they need in their private config files.

This looks like an awesome idea, I will take a look at it and update this PR, thanks @wxiaoguang

@wolviecb
Copy link
Contributor Author

Closing this PR as #19842 replaces it

@wolviecb wolviecb closed this May 30, 2022
@wolviecb wolviecb deleted the feat/dheat_mitigation branch May 31, 2022 18:43
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants