Repository navigation
Weak Diffie-Hellman groups provided by crypto module #44539
Description
Activity
-1 to removing modp2 yet, it's still used by older SSH (2.0) implementations and in some places it's the only exchange algorithm offered.
Reacted by 赵旭东(ZHAO Xudong)It seems OpenSSH itself hasn't supported that since 2016:
https://www.openssh.com/txt/release-7.2- ssh(1), sshd(8): increase the minimum modulus size supported for
diffie-hellman-group-exchange to 2048 bits.
- ssh(1), sshd(8): increase the minimum modulus size supported for
- added a commit that references this issue
on Sep 6, 2022 It seems OpenSSH itself hasn't supported that since 2016: https://www.openssh.com/txt/release-7.2
- ssh(1), sshd(8): increase the minimum modulus size supported for
diffie-hellman-group-exchange to 2048 bits.
That's for the group exchange, which is separate from the modp-based algorithms. Specifically, the modp2-based algorithm is called
diffie-hellman-group1-sha1, which is still supported by OpenSSH.- ssh(1), sshd(8): increase the minimum modulus size supported for
I remember being mildly apprehensive when they were added back in 2012, modp1 in particular. I'm feeling vindicated now.
What is an acceptable way forward? Remove modp1 and doc-deprecate (or runtime deprecate?) the other two?
I'm sympathetic to @mscdex's concern w.r.t. ssh but if working on open source has taught me one thing, it's that users never read the documentation.
Reacted by Anderson Bosa- added a commit that references this issue
on Sep 10, 2022 - added a commit that references this issue
on Sep 10, 2022 It appears the list was chosen by exporting every group provided by OpenSSL:
https://github.com/nodejs/node/blob/main/src/crypto/crypto_dh.cc#L222-L229FWIW, that's not entirely accurate. I only recently wrote that part of code to replace a large header file that previously explicitly specified all of these groups. In other words, until recently, the MODP implementation in node did not use any constants provided by OpenSSL :)
What is an acceptable way forward? Remove modp1 and doc-deprecate (or runtime deprecate?) the other two?
Let's add a documentation-only deprecation for all three groups as a first step, which is not a semver-major change and can thus land and be released quickly: #44588
Warning about or outright removing modp1 is not semver-major under the security exception. I don't expect huge ecosystem fallout, there's probably very little software that would be affected.
17 remaining items
- added a commit that references this issue
on Oct 13, 2022 This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.
Node exposes various IKE MODP groups. It appears the list was chosen by exporting every group provided by OpenSSL:
https://github.com/nodejs/node/blob/main/src/crypto/crypto_dh.cc#L222-L229
https://nodejs.org/api/crypto.html#class-diffiehellmangroup
However, some of these groups are too small to be used. See RFC 8247, section 2.4:
These are all exposed by Node as "modp1", "modp2", and "modp5". The documentation should reflect their status and they should be deprecated and removed, especially modp1.