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

Continued Use of Insecure Default Parameters #12135

Open
Anderson-Xia opened this issue Apr 24, 2024 · 8 comments
Open

Continued Use of Insecure Default Parameters #12135

Anderson-Xia opened this issue Apr 24, 2024 · 8 comments
Labels

Comments

@Anderson-Xia
Copy link

Hi authors, I've noticed that some files, like this one, still use insecure default values such as 1024-bit DSA keys and MD5 hashing.
These are not recommended by modern cryptographic standards due to security concerns.
Are there plans to update to more secure alternatives?

@adiroiban
Copy link
Member

Hi,

Thanks for the report.

We would be happy to review and accept a PR with a fix for this.

I guess that for DSA we can use 2048 as the default.

Can you please suggest what other parts need to be updated?

Thanks!

@Anderson-Xia
Copy link
Author

Sure, I'll prepare a small PR to update the DSA key length shortly.
Regarding the MD5 issue, I noticed it's already being discussed in issue #11866. It seems like resolving this might require a more comprehensive approach.

@glyph
Copy link
Member

glyph commented Apr 24, 2024

Just to calibrate on the severity of this issue, ssh-keygen -t dsa from OpenSSH 9.7p1, released last month, still defaults to 1024 bits as well. Not to say we shouldn't upgrade size-wise but mostly people just shouldn't be using DSA keys :) (Unlike ssh-keygen, ckeygen doesn't have a default type, it just spits out --help when run with no arguments. Maybe it should have a default type?)

@cjwatson
Copy link
Contributor

Also ssh-keygen(1) says "DSA keys must be exactly 1024 bits as specified by FIPS 186-2".

@Anderson-Xia
Copy link
Author

Sounds like a good solution.
Security standards are always evolving, as a student studying computer security, I often find myself questioning whether advocating for these upgrades ultimately benefits or burdens the community.

@cjwatson
Copy link
Contributor

I would suggest that the most sensible approach here with least burden to the community would be not to change the default DSA key length (which I suspect might cause interoperability issues) but to figure out a plan to deprecate DSA keys, just as OpenSSH is doing. This shouldn't be an abrupt removal, per the Twisted compatibility policy, but I think it would make sense to at least begin adding deprecation warnings to the DSA bits in Conch.

@Anderson-Xia
Copy link
Author

I would explain that my concerns are partly based on NIST SP 800-57, where it says:

This revision updates cryptographic requirements for the protocols and applications in the
document so that the required security strengths as specified in [SP 800-131A] can be
achieved. This revision also adds the security-related updates from the protocols
addressed in the document. In addition, this revision adds a new section for Secure Shell
(SSH). Specific changes in this revision include the following.
Section 2:

  1. Additional clarification about the recommended algorithms and key sizes for
    digital signature and key establishment certificates has been added.
  2. 1024-bit RSA and DSA are no longer approved for generating digital signatures
    after 2013.
    ...

@glyph
Copy link
Member

glyph commented Apr 24, 2024

at least begin adding deprecation warnings to the DSA bits in Conch

Since you mention it, I should bring up that we have been deprecating stuff in Conch in fairly useless and annoying ways thus far, and we need to both (A) clean that up, and (B) take care to only actually emit deprecation warnings to useful places when you actually use stuff. Consider that conch --version does this on trunk:

$ conch --version    
/Users/glyph/Projects/Twisted/src/twisted/conch/ssh/transport.py:106: CryptographyDeprecationWarning: Blowfish has been deprecated and will be removed in a future release
  b"blowfish-cbc": (algorithms.Blowfish, 16, modes.CBC),
/Users/glyph/Projects/Twisted/src/twisted/conch/ssh/transport.py:110: CryptographyDeprecationWarning: CAST5 has been deprecated and will be removed in a future release
  b"cast128-cbc": (algorithms.CAST5, 16, modes.CBC),
/Users/glyph/Projects/Twisted/src/twisted/conch/ssh/transport.py:115: CryptographyDeprecationWarning: Blowfish has been deprecated and will be removed in a future release
  b"blowfish-ctr": (algorithms.Blowfish, 16, modes.CTR),
/Users/glyph/Projects/Twisted/src/twisted/conch/ssh/transport.py:116: CryptographyDeprecationWarning: CAST5 has been deprecated and will be removed in a future release
  b"cast128-ctr": (algorithms.CAST5, 16, modes.CTR),
Twisted version: 24.3.0.post0

This doesn't help anyone; I didn't ask for any of those algorithms and I shouldn't be seeing ugly warnings just for launching conch itself. Similarly if you start the server, the warnings just go to stderr of twist on startup, rather than to a log where an admin might actually have a hope of doing something about them.

Fixing this is certainly out of scope for this particular issue, but we should not make this problem worse if we can avoid it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants