feat: Support Customer CAS Private CA for server certificates. #2095
Conversation
hessjcg
force-pushed
the
customer-ca-support
branch
2 times, most recently
from
ff8e682 to
17c3bdf
Compare
hessjcg
changed the title
| @@ -312,7 +312,7 @@ private InstanceMetadata fetchMetadata(CloudSqlInstanceName instanceName, AuthTy | |||
| instanceName, | |||
| ipAddrs, | |||
| instanceCaCertificates, | |||
| "GOOGLE_MANAGED_CAS_CA".equals(instanceMetadata.getServerCaMode()), | |||
| isCasManagedCertificate(instanceMetadata), | |||
There was a problem hiding this comment.
Would it simplify things to have this function take in serverCaMode instead of instanceMetadata?
Would make adding a standalone unit test for isCasManagedCertificate easy and reduce having to call .getServerCaMode() twice in the function.
| isCasManagedCertificate(instanceMetadata), | |
| isCasManagedCertificate(instanceMetadata.getServerCaMode()), |
There was a problem hiding this comment.
I've inverted the logic here. Going forward, both GOOGLE_MANAGED_CAS_CA, CUSTOMER_MANAGED_CAS_CA, and future new kinds of CA we will do 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. The code should reflect it:
We don't have a unit test for this method, but this change is covered by this test: com.google.cloud.sql.core.ConnectorTest#create_successfulPublicCasConnection
There was a problem hiding this comment.
I still think it would be simpler for the function to take in serverCaMode instead of instanceMetadata...
hessjcg
force-pushed
the
customer-ca-support
branch
from
17c3bdf to
d19126a
Compare
hessjcg
force-pushed
the
customer-ca-support
branch
from
d19126a to
4f78dae
Compare
hessjcg
force-pushed
the
customer-ca-support
branch
2 times, most recently
from
9b87197 to
5b1fa42
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
| private static boolean isCasManagedCertificate(String serverCaMode) { | ||
| return serverCaMode != null && !"GOOGLE_MANAGED_INTERNAL_CA".equals(serverCaMode); | ||
| } |
There was a problem hiding this comment.
is there a case where serverCaMode could be the empty string?
There was a problem hiding this comment.
Good catch. Updated again for clarity.
This includes the integration test.
hessjcg
force-pushed
the
customer-ca-support
branch
from
5b1fa42 to
f0a1886
Compare
Support instances configured with a Customer managed CAS private CA. This includes the integration test.