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

[Feature Request]: Re-add support for modp1 and modp2 Diffie-Hellman groups following Node update #39603

Closed
3 tasks done
Eugeny opened this issue Aug 21, 2023 · 2 comments
Closed
3 tasks done

Comments

@Eugeny
Copy link

Eugeny commented Aug 21, 2023

Preflight Checklist

Problem Description

After nodejs/node#43896 and the related Electron patch, Electron has lost the ability to use modp1 and modp2 DH groups in node crypto (used e.g. in the ssh2 Node module to connect to Cisco hardware).

The reason for removal was that BoringSSL doesn't have these two constants while OpenSSL does and thus official node crypto module does too.

Proposed Solution

Based on my limited understanding, it should be trivial to copy-paste these group constants into the Electron patch and remove #ifdefs to bring back support for it: https://github.com/openssl/openssl/blob/master/crypto/bn/bn_const.c#L25-L74

@codebytere
Copy link
Member

See nodejs/node#44539 (comment)

They've been deprecated in Node.js since Sept 2022 for justifiable security reasons and BoringSSL is strongly against supporting them upstream, so I apologize but this is something we're going to defer to BoringSSL and better security practices on.

@codebytere codebytere closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2023
@Eugeny
Copy link
Author

Eugeny commented Aug 22, 2023

BoringSSL still includes an MD5 implementation. Should we really remove support for technology that is still being used, without any available alternatives (it's the only key exchange that's available when connecting to real, physical hardware) instead of just discouraging its use in new implementations?

Especially when it's just about a numeric constant

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

No branches or pull requests

2 participants