Closed alexjurkiewicz closed 7 months ago
Thanks @alexjurkiewicz ! I'm thinking to merge your PR now but in the future potentially create a separate repo under https://github.com/mysqljs and move profiles to its own package ( possibly as an optional dependency to save the bundle size when you don't need to connect to RDS ) If optional this has to be a major release
I'm curious why the need for 115 certs - can we just have the 4 root instead?
I didn't check the cert tree very closely but I don't think there are four roots. Each certificate in this bundle is signed only by itself.
For what it's worth, most mysql-client implementations don't validate the server's identity. They only use the certificate for encryption. Reducing default ssl mode strictness might be an even simpler (albeit still major semver) change for you.
also there are GovCloud 10 certs in https://truststore.pki.us-gov-west-1.rds.amazonaws.com/global/global-bundle.pem Not sure if we should include them as well
The following certificate chain is used for connections with aurora serveless and RDS proxy.
https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.SSL-certificate-rotation.html
Amazon RDS Proxy uses certificates from the AWS Certificate Manager (ACM).
example
C = US, O = "Starfield Technologies, Inc.", OU = Starfield Class 2 Certification Authority
C = US, ST = Arizona, L = Scottsdale, O = "Starfield Technologies, Inc.", CN = Starfield Services Root Certificate Authority - G2
C = US, O = Amazon, CN = Amazon Root CA 1
C = US, O = Amazon, CN = Amazon RSA 2048 M02
CN = *.cluster-ctapflxgnyle.ap-northeast-1.rds.amazonaws.com
Currently, I am able to verify with C = US, O = Amazon, CN = Amazon Root CA 1
.
so please do not remove the following certificate.
https://github.com/sidorares/node-mysql2/blob/master/lib/constants/ssl_profiles.js#L1089-L1107
Guys, it will be great if someone with access to the original mysql lib (https://github.com/mysqljs/mysql/issues) can report this also there. I'm unable to open an issue in that project, as I'm not in the collaboration list. Not sure if still maintained as has not been updated in 4 years, but worth trying.
Cheers.
Hi @jhbarrantes I am updating the certs there too. I'll have a release with them soon. Apologies there was some spam to the repo so it was temp measure.
Hi @jhbarrantes I am updating the certs there too. I'll have a release with them soon. Apologies there was some spam to the repo so it was temp measure.
Nothing to apologize, glad to hear you're already patching the other one as well.
Thank you very much @dougwilson
Hey guys, thanks a lot for this maintenance work! Any ETA on when approximately we could expect a release with the CA certs?
Can this be merged in soon? I've been waiting to update my RDS instances for the new certs to be merged in. The deadline is a few months out. https://aws.amazon.com/blogs/aws/rotate-your-ssl-tls-certificates-now-amazon-rds-and-amazon-aurora-expire-in-2024/
@lbadger I'll try to find some time soon to review and potentially merge, but you can always just download certs manually and use your own ssl config
Hello! Commenting here to give more visibility on AWS timelines, as this can be time sensitive for some people using IAM authentication for RDS databases:
For now, I've linked both #2119, #2130 and this PR as related links in the examples that use RDS SSL.
our development environment just stopped working because of this. It would be great if this can be addressed soon 🙏🏽
Same here. After getting errors, we bundle the new eu-central-1-bundle.pem RDS certificate manually with our app instead of relying on the Amazon RDS
defaults.
Can this be merged?
@wellwelwel I'm thinking to merge this, though the better solution would be to extract profiles to separate repo and use it as a dependency ( here and mysqljs/mysql ) and then later as a semver major release make it optional dependency and add documentation on how to use it ( and explain in details in the error message when ssl: 'Amazon RDS'
config is selected but no dependency installed.
I guess this should be a semver minor release?
@sidorares, we can merge this PR in a similar way to what was done when deciding the "typeCast for execute".
though the better solution would be to extract profiles to separate repo and use it as a dependency
About this, I believe this could be done in a further patch
version, since the solution would already be working due to this PR. Then, everyone who update to this new version would already have this additional dependency naturally (as a refactor for this PR).
About minor or patch:
Amazon RDS
is supposed to work, I see it more as a fix
(patch) than a feat
(minor).then later as a semver major release make it optional dependency and add documentation on how to use it
Yes, agreed 🙋🏻♂️
Released in v3.9.3 🚀
Hello, this release has broken my connection to my Aurora MySQL instance:
ERROR Invoke Error {
"errorType": "Error",
"errorMessage": "unable to get local issuer certificate",
"code": "HANDSHAKE_SSL_ERROR",
"fatal": true,
"stack": [
"Error: unable to get local issuer certificate",
" at TLSSocket.onConnectSecure (node:_tls_wrap:1674:34)",
" at TLSSocket.emit (node:events:518:28)",
" at TLSSocket._finishInit (node:_tls_wrap:1085:8)",
" at ssl.onhandshakedone (node:_tls_wrap:871:12)"
]
}
I was able to resolve this issue by reverting back to v3.9.2
Hello, this release has broken my connection to my Aurora MySQL instance:
ERROR Invoke Error { "errorType": "Error", "errorMessage": "unable to get local issuer certificate", "code": "HANDSHAKE_SSL_ERROR", "fatal": true, "stack": [ "Error: unable to get local issuer certificate", " at TLSSocket.onConnectSecure (node:_tls_wrap:1674:34)", " at TLSSocket.emit (node:events:518:28)", " at TLSSocket._finishInit (node:_tls_wrap:1085:8)", " at ssl.onhandshakedone (node:_tls_wrap:871:12)" ] }
I was able to resolve this issue by reverting back to v3.9.2
Same for me, after 2 days of testing, debugging, etc, we just went for a temporary:
ssl: {
rejectUnauthorized: false
}
We are unsure on whether we should just bundle the certificates by ourselves at this point...
@ennioVisco looks like they have a fix ready for testing if you want to give that a try. #2542
@ennioVisco looks like they have a fix ready for testing if you want to give that a try. #2542
it does not seem to fix for us :/. I will investigate, but this seems to break our node_modules, so even before reaching the lines where we actually use the certificates...
Fixes #2130.
The new cert chain is significantly longer, there are 115 certs now. This reflects the fact there are four CA root types with per-region unique certificates.