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

x/crypto/ssh: proposal to remove weak (rc4 based) ciphers arcfour{128,256} from default cipher list #23173

Closed
psanford opened this issue Dec 19, 2017 · 6 comments
Milestone

Comments

@psanford
Copy link

@psanford psanford commented Dec 19, 2017

x/crypto/ssh has "arcfour256", "arcfour128" in its list of default ciphers. These ciphers are now considered weak. I seems like it might be time to disable these by default as the documentation says:

// The allowed cipher algorithms. If unspecified then a sensible
// default is used.
Ciphers []string

These two ciphers are rc4 variants. They do discard the first 1536 bytes of the cipher stream so they are better than plain rc4 but they are still considered weak.

OpenSSH disabled them by default in 7.2 (2016-02-29) and removed them completely in release 7.6 (2017-10-03).

RC4 was disabled by default in crypto/tls in 2015: #10094.

I was dinged in a security audit because these ciphers were enabled. I can obviously remove them from my config, but with the OpenSSH changes it seemed like it might be more appropriate to update the defaults.

If this change is acceptable I'm happy to something for code review.

@gopherbot gopherbot added this to the Unreleased milestone Dec 19, 2017
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Dec 19, 2017

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Dec 19, 2017

Thanks for bringing this up. The OpenSSH changes suggest there is no sizable compatibility loss, so I’m strongly in favor. (Even if I think SSH has strong downgrade protection, so they wouldn’t affect most connections.)

@hanwen
Copy link
Contributor

@hanwen hanwen commented Dec 19, 2017

OK to remove them from the default list, but they must be available if specified explicitly.

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 21, 2017

Change https://golang.org/cl/85155 mentions this issue: ssh: remove arcfour{128,256} from default cipher list

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 10, 2018

Change https://golang.org/cl/86955 mentions this issue: ssh: remove arcfour ciphers from the default preference list

@hanwen hanwen closed this Jan 22, 2018
@golang golang locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.