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

no shared cipher error if cert is loaded later #49549

Closed
kumarrishav opened this issue Sep 8, 2023 · 6 comments
Closed

no shared cipher error if cert is loaded later #49549

kumarrishav opened this issue Sep 8, 2023 · 6 comments

Comments

@kumarrishav
Copy link
Contributor

kumarrishav commented Sep 8, 2023

Version

v16.20.0

Platform

Darwin xxxx 22.6.0 Darwin Kernel Version 22.6.0: Wed Jul 5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000 x86_64

Subsystem

No response

What steps will reproduce the bug?

create securityContext and load the cert later

        ...
       const tlsOptions = {}
        tlsOptions.ciphers = options.ciphers;
	const secureContext = Tls.createSecureContext(tlsOptions);
	secureContext.context.setCert(cert);
        options.secureContext = secureContext;
        .....
       delete options.ciphers
       delete options.cert
       Https.createServer(options)

#46515

#36655 (comment)

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

No response

What do you see instead?

TLS 40052: server emit tlsClientError: Error: 8138940544:error:1417A0C1:SSL routines:tls_post_process_client_hello:no shared cipher:../deps/openssl/openssl/ssl/statem/statem_srvr.c:2313:

    at TLSWrap.loadSession [as onclienthello] (node:_tls_wrap:205:19)
    at TLSWrap.callbackTrampoline (node:internal/async_hooks:130:17) {
  library: 'SSL routines',
  function: 'tls_post_process_client_hello',
  reason: 'no shared cipher',
  code: 'ERR_SSL_NO_SHARED_CIPHER'
}
9/7/2023, 7:46:47 PM UNCAUGHTEXCEPTION Error: write EPROTO 8138940544:error:14094410:SSL routines:ssl3_read_bytes:sslv3 alert handshake failure:../deps/openssl/openssl/ssl/record/rec_layer_s3.c:1565:SSL alert number 40

Additional information

No response

@bnoordhuis
Copy link
Member

Can you try with the latest v18 or v20 release? v16 is going EOL in three days, any bugs that aren't critical security vulnerabilities aren't going to get fixed.

@kumarrishav
Copy link
Contributor Author

kumarrishav commented Sep 8, 2023

Yes, First I tried with v18 and did similar to this #36655 (comment) because I have to use SECLEVEL=0 #46515 in node 18 (as our certs are sha1 signed and it will few months to upgrade it).

with node 18, it threw similar error mentioned here #36655 (comment)

adding SECLEVEL=0 using the above mechanism (i.e load cert later after setting cipher) works fine when node.js is acting as a client. but when it's acting as a server (like the above explained), it throws the error (even after following above mechanism).

CC: @bnoordhuis

@bnoordhuis
Copy link
Member

Can you post a complete example that throws (with v18 or v20) the exception you reported? I need precise steps to reproduce if you want me to help you.

@kumarrishav
Copy link
Contributor Author

kumarrishav commented Sep 8, 2023

Yeah. I am trying to build a test case (and might need help) that could be publicly shared. I created this cert (sha1 signed and 1024 bit - trying to match my error scenario) , I am surprised that while using following cert (Created via openssl), it doesn't throw the error Error: error:0A00018E:SSL routines::ca md too weak which I am getting (while using node 18). What could be the reason?

-----BEGIN CERTIFICATE-----
MIICDDCCAXUCFB4xZf2O05R9YzYAy/jQAiTGih0BMA0GCSqGSIb3DQEBBQUAMEUx
CzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRl
cm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMjMwOTA4MjExOTE5WhcNMjQwOTA3MjEx
OTE5WjBFMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UE
CgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GN
ADCBiQKBgQDvhOBCcUEtN3/Vh7ruuvG3stlLelbyzit+6OFVqJYaXFdSJxKaYe63
cfh/RdNGfIkpg7Vf20MRAQzltohyGmxxKUGFnk6F3LJ92yReBDBkd33YJCFt1uix
abWB+y0exN4Q3AgAGrrW8zmUJIDI2tCLyfY8A1cRb4laSwVm39cg3wIDAQABMA0G
CSqGSIb3DQEBBQUAA4GBAGJS3JbBS9X4YPphuqNO1A51Q8X3kVGu7b/0tDtR4bQe
7nDtkGtsbmmF5z4Ovm/qoZV7f/lkeCSCHSHfjTDLp/+fH5U8BNcp9UQG5eayEhRH
KmTpOHd7SpJsj5sIWyxWP70fi8MT/RdTVzyRRhPEIWVZy4PteSJkYb8EIE2e17b0
-----END CERTIFICATE-----

@bnoordhuis
Copy link
Member

Since there's been no follow-up I'll go ahead and assume this is working for you now.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Sep 27, 2023
@kumarrishav
Copy link
Contributor Author

kumarrishav commented Oct 13, 2023

Sorry, got busy with another task and hence, couldn't follow up.

So, here are the steps to reproduce.

const https = require('https');
const fs = require('fs');
const Tls = require('tls');

const cert = fs.readFileSync('cert.pem');
const key = fs.readFileSync('key.pem');
const options = {
    cipher: 'DEFAULT:@SECLEVEL=0'
}

const secureContext = Tls.createSecureContext(options);
secureContext.context.setCert(cert);
secureContext.context.setKey(key);
options.secureContext = secureContext;

https.createServer(options, (req, res) => {
    res.writeHead(200);
    res.end('hello world\n');
}
).listen(8000);
console.log('server is running');

key.pem

-----BEGIN PRIVATE KEY-----
MIICdQIBADANBgkqhkiG9w0BAQEFAASCAl8wggJbAgEAAoGBAOyrKupVqbRDB7FF
Q/hqQxhPaXFPH71sJE6K0j2yh/bKAKLu/rxnKFSziH+Y5xNaqJKMlTIhA1DLPhwK
469xgOWtjPPOCf/yXMo58cF66lQ3y2WYUvv59KnIz6CKsgRj5I6GCo2Ircuep30h
nPkUeWyqfY8GVEY9ey5GN5UyXkcXAgMBAAECgYAsHzUe/t1mh+CVQe6MD3N2wsdL
oo6uR8z0/5h2fCQw9DPuLFQ9V5YDRH08HItn5kzrFV5zPAhcNafcnoMYIDcBPvG8
mYZWR34bFdpf8mmJ9gqWMmPvt6y0rt9yoK41A99b/Zp73axZJ+JYkVsvAi866qzE
xEszEVTHU9pb4XwzwQJBAPnq8tY03LEAE5blojlC+2FFWCqxHduNbupbT0W+RIOw
aXCuN0PDF5Vj6q3aJ448yUqEkzOgn4w4TElH2KGu9ZECQQDybaxrIvXKotnWmi9/
z2aXfIjx4gXE9+E9/vNwxIdNRotP+fRKhxOozuXkXbFwJdGOfoGFWFpJrKNeP1IL
gf4nAkB/ez9/0Ns6VoWnlV9YwjgkeBDvRgWq9sw7M3SOaO3eFuDSH6wFHsEq7RrK
r9s5jPxIzLOhxC9egWE4iDOS1gJhAkBNQnKOekfG2nt+DwlQPDK/2Gp0W8nm6fCY
p2XB7IAKOo8vX8Ng9Qdo5vcQ/mMMEKFmPh7c9jlf/WrbIGbvT/BFAkB/QXbpLfht
54+7b7ObsDHWqQMgXrh3Lh5dEwOd96F105a2kUcn6hQw9i2APyLQgNpkVOeHzMho
Fb74WWClnKD0
-----END PRIVATE KEY-----

cert.pem

-----BEGIN CERTIFICATE-----
MIICDDCCAXUCFC+OlOVU/xbxd+UojjRssw3gPDACMA0GCSqGSIb3DQEBCwUAMEUx
CzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRl
cm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMjMxMDEzMTUwMzUzWhcNMjQxMDEyMTUw
MzUzWjBFMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UE
CgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GN
ADCBiQKBgQDsqyrqVam0QwexRUP4akMYT2lxTx+9bCROitI9sof2ygCi7v68ZyhU
s4h/mOcTWqiSjJUyIQNQyz4cCuOvcYDlrYzzzgn/8lzKOfHBeupUN8tlmFL7+fSp
yM+girIEY+SOhgqNiK3Lnqd9IZz5FHlsqn2PBlRGPXsuRjeVMl5HFwIDAQABMA0G
CSqGSIb3DQEBCwUAA4GBAEGU6oe2hWKst89772QAARPrNrBzdraC1g1ngZTJgpmZ
aIWEzM+ClovyGz+d5BFcX5JngsMneg+dJwN48UJd6lkW2EDQvIX+x8VWJBKMJJdK
xTQLdi17tj5abhgeO+pLvBWzFFF1hK8uWzvs0uFUoHJgUI8ia52KvlF34PueIymd
-----END CERTIFICATE-----

Why load the cert later? : due to this #36655 (comment)

Do we know why is this not working?
I get the following error

This site can’t provide a secure connection and localhost uses an unsupported protocol.
ERR_SSL_VERSION_OR_CIPHER_MISMATCH

kumarrishav added a commit to kumarrishav/node that referenced this issue Oct 14, 2023



Set the cipher list and cipher suite before anything else because @SECLEVEL=<n> changes the security level and that affects subsequent operations.

Fixes: nodejs#36655 nodejs#49549

Refs: https://github.com/orgs/nodejs/discussions/49634 https://github.com/orgs/nodejs/discussions/46545
kumarrishav added a commit to kumarrishav/node that referenced this issue Oct 16, 2023
Set the cipher list and cipher suite before anything else because
@SECLEVEL=<n>changes the security level and that affects
subsequent operations.

Fixes: nodejs#36655
nodejs#49549

Refs: https://github.com/orgs/nodejs/discussions/49634
https://github.com/orgs/nodejs/discussions/46545
kumarrishav added a commit to kumarrishav/node that referenced this issue Oct 16, 2023
richardlau pushed a commit that referenced this issue Nov 16, 2023
Set the cipher list and cipher suite before anything else
because @SECLEVEL=<n> changes the security level and
that affects subsequent operations.

Fixes: #36655
Fixes: #49549
Refs: https://github.com/orgs/nodejs/discussions/49634
Refs: https://github.com/orgs/nodejs/discussions/46545
Refs: https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_security_level.html
PR-URL: #50186
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
targos pushed a commit that referenced this issue Nov 23, 2023
Set the cipher list and cipher suite before anything else
because @SECLEVEL=<n> changes the security level and
that affects subsequent operations.

Fixes: #36655
Fixes: #49549
Refs: https://github.com/orgs/nodejs/discussions/49634
Refs: https://github.com/orgs/nodejs/discussions/46545
Refs: https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_security_level.html
PR-URL: #50186
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
martenrichter pushed a commit to martenrichter/node that referenced this issue Nov 26, 2023
Set the cipher list and cipher suite before anything else
because @SECLEVEL=<n> changes the security level and
that affects subsequent operations.

Fixes: nodejs#36655
Fixes: nodejs#49549
Refs: https://github.com/orgs/nodejs/discussions/49634
Refs: https://github.com/orgs/nodejs/discussions/46545
Refs: https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_security_level.html
PR-URL: nodejs#50186
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
UlisesGascon pushed a commit that referenced this issue Dec 11, 2023
Set the cipher list and cipher suite before anything else
because @SECLEVEL=<n> changes the security level and
that affects subsequent operations.

Fixes: #36655
Fixes: #49549
Refs: https://github.com/orgs/nodejs/discussions/49634
Refs: https://github.com/orgs/nodejs/discussions/46545
Refs: https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_security_level.html
PR-URL: #50186
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
UlisesGascon pushed a commit that referenced this issue Dec 19, 2023
Set the cipher list and cipher suite before anything else
because @SECLEVEL=<n> changes the security level and
that affects subsequent operations.

Fixes: #36655
Fixes: #49549
Refs: https://github.com/orgs/nodejs/discussions/49634
Refs: https://github.com/orgs/nodejs/discussions/46545
Refs: https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_security_level.html
PR-URL: #50186
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
richardlau pushed a commit that referenced this issue Mar 19, 2024
Set the cipher list and cipher suite before anything else
because @SECLEVEL=<n> changes the security level and
that affects subsequent operations.

Fixes: #36655
Fixes: #49549
Refs: https://github.com/orgs/nodejs/discussions/49634
Refs: https://github.com/orgs/nodejs/discussions/46545
Refs: https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_security_level.html
PR-URL: #50186
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
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 a pull request may close this issue.

2 participants