-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: Support Private CA for server certificates. #408
Conversation
2bb7dde
to
eba701f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make sure to add the integration test to all 3 flavors: .ts
, .mjs
and .cjs
files
a39c7fb
to
2724b5e
Compare
2724b5e
to
b3e86ef
Compare
The mjs & cjs system tests are no longer required thanks to #412 |
b3e86ef
to
183ff64
Compare
183ff64
to
64a3d17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ✅
Looks like tests are not happy... |
src/socket.ts
Outdated
@@ -36,23 +36,26 @@ export function validateCertificate( | |||
dnsName: string | |||
) { | |||
return (hostname: string, cert: tls.PeerCertificate): Error | undefined => { | |||
if (serverCaMode === 'GOOGLE_MANAGED_CAS_CA') { | |||
if (!serverCaMode || serverCaMode !== 'GOOGLE_MANAGED_INTERNAL_CA') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder if for this being undefined is causing issues potentially...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests were failing due to a logic error. I fixed it: !serverCaMode || serverCaMode === 'GOOGLE_MANAGED_INTERNAL_CA'
test: Integration test for Customer Private CA CAS instance.
64a3d17
to
c4a031c
Compare
…om CA validation (#910) Going forward, both GOOGLE_MANAGED_CAS_CA, CUSTOMER_MANAGED_CAS_CA, and future new kinds of CA will use standard TLS domain name validation using the server certificate SAN records. The certificate validation logic for the original GOOGLE_MANAGED_INTERNAL_CA is now the exception. See implementation in other connectors: feat: Support Private CA for server certificates. GoogleCloudPlatform/cloud-sql-nodejs-connector#408 feat: Support Customer CAS Private CA for server certificates. GoogleCloudPlatform/cloud-sql-jdbc-socket-factory#2095
Support TLS validation on instances configured with a customer-managed private CA. Includes integration test.