Skip to content

kex: build group1-sha1's DH group from its prime - #1521

Open
FloSch62 wants to merge 1 commit into
mscdex:masterfrom
FloSch62:kex-group1-without-named-group
Open

FloSch62 wants to merge 1 commit into
mscdex:masterfrom
FloSch62:kex-group1-without-named-group

Conversation

@FloSch62

@FloSch62 FloSch62 commented Oct 8, 2026

Copy link
Copy Markdown

diffie-hellman-group1-sha1 uses createDiffieHellmanGroup('modp2'), but not every node build has that group. Electron uses BoringSSL instead of OpenSSL, which doesn't have modp1/modp2, so the handshake fails right after KEXINIT:

Error: Unknown DH group
    at new DiffieHellmanGroup (node:internal/crypto/diffiehellman:146:19)
    at createDiffieHellmanGroup (node:crypto:162:10)
    at DHExchange.generateKeys (ssh2/lib/protocol/kex.js:1569:20)
    at DHExchange.getPublicKey (ssh2/lib/protocol/kex.js:1036:12)
    at DHExchange.start (ssh2/lib/protocol/kex.js:1555:29)
    at handleKexInit (ssh2/lib/protocol/kex.js:510:13)
code: ERR_CRYPTO_UNKNOWN_DH_GROUP

A user of our Electron app ran into this with an old Cisco switch (IOS 12.2, Cisco-1.25) that needs group1. Group exchange isn't an option there either because of BAD_DHGEX, so they couldn't connect at all.

The fix builds the group from the RFC 2409 prime and generator 2 with createDiffieHellman. It's the same prime OpenSSL uses for modp2, so nothing changes on regular node. modp14 and up are still available in BoringSSL, so I left those alone.

The new test stubs out modp1/modp2 before loading the lib, so it fails without the change. npm test and npm run lint pass locally.

Original report: FloSch62/muxus#247

BoringSSL, which Electron uses for node's crypto module, does not provide
the 'modp2' named group, so diffie-hellman-group1-sha1 failed with
ERR_CRYPTO_UNKNOWN_DH_GROUP before any keys were exchanged. Construct the
1024-bit Oakley Group 2 from its RFC 2409 prime and generator instead,
which works with both OpenSSL and BoringSSL.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant