Skip to content

Commit 3126f9a

Browse files
hessjcghessjcg
committed
refactor: Use new ConnectSettings.DnsNames field to validate the server certificate's server name.
1 parent 26dcf57 commit 3126f9a

File tree

9 files changed

+201
-124
lines changed

9 files changed

+201
-124
lines changed

‎dialer_test.go‎

Lines changed: 77 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,8 @@ func (r *fakeResolver) Resolve(_ context.Context, name string) (instance.ConnNam
970970
func TestDialerSuccessfullyDialsDnsTxtRecord(t *testing.T) {
971971
inst := mock.NewFakeCSQLInstance(
972972
"my-project", "my-region", "my-instance",
973+
mock.WithDNSMapping("db.example.com", "INSTANCE", "CUSTOM_SAN"),
974+
mock.WithDNSMapping("db2.example.com", "INSTANCE", "CUSTOM_SAN"),
973975
)
974976
wantName, _ := instance.ParseConnNameWithDomainName("my-project:my-region:my-instance", "db.example.com")
975977
wantName2, _ := instance.ParseConnNameWithDomainName("my-project:my-region:my-instance", "db2.example.com")
@@ -1046,9 +1048,11 @@ func TestDialerUpdatesAutomaticallyAfterDnsChange(t *testing.T) {
10461048
// SRV record and connect to the correct instance.
10471049
inst := mock.NewFakeCSQLInstance(
10481050
"my-project", "my-region", "my-instance",
1051+
mock.WithDNS("update.example.com"),
10491052
)
10501053
inst2 := mock.NewFakeCSQLInstance(
10511054
"my-project", "my-region", "my-instance2",
1055+
mock.WithDNS("update.example.com"),
10521056
)
10531057
r := &changingResolver{
10541058
stage: new(int32),
@@ -1104,42 +1108,85 @@ func TestDialerUpdatesAutomaticallyAfterDnsChange(t *testing.T) {
11041108

11051109
func TestDialerChecksSubjectAlternativeNameAndSucceeds(t *testing.T) {
11061110

1107-
// Create an instance with custom SAN 'db.example.com'
1108-
inst := mock.NewFakeCSQLInstanceWithSan(
1109-
"my-project", "my-region", "my-instance", []string{"db.example.com"},
1110-
mock.WithDNS("db.example.com"),
1111-
mock.WithServerCAMode("GOOGLE_MANAGED_CAS_CA"),
1112-
)
1111+
tcs := []struct {
1112+
name string
1113+
legacy bool
1114+
icn string
1115+
dn string
1116+
}{{
1117+
name: "domainName DnsName older",
1118+
legacy: true,
1119+
icn: "my-project:my-region:my-instance",
1120+
}, {
1121+
name: "domainName DnsNames newer",
1122+
legacy: false,
1123+
icn: "my-project:my-region:my-instance",
1124+
},
1125+
{
1126+
name: "InstanceConnectionName DnsName older",
1127+
legacy: true,
1128+
icn: "my-project:my-region:my-instance",
1129+
dn: "db.example.com",
1130+
}, {
1131+
name: "InstanceConnectionName DnsNames newer",
1132+
legacy: false,
1133+
icn: "my-project:my-region:my-instance",
1134+
dn: "db.example.com",
1135+
}}
1136+
for _, tc := range tcs {
1137+
t.Run(tc.name, func(t *testing.T) {
1138+
// Create an instance with custom SAN 'db.example.com'
1139+
var inst mock.FakeCSQLInstance
1140+
if tc.legacy || tc.dn == "" {
1141+
inst = mock.NewFakeCSQLInstance(
1142+
"my-project", "my-region", "my-instance",
1143+
mock.WithDNS("db.example.com"),
1144+
mock.WithServerCAMode("GOOGLE_MANAGED_CAS_CA"),
1145+
)
1146+
} else {
1147+
inst = mock.NewFakeCSQLInstance(
1148+
"my-project", "my-region", "my-instance",
1149+
mock.WithDNSMapping("db.example.com", "INSTANCE", "CUSTOM_SAN"),
1150+
mock.WithServerCAMode("GOOGLE_MANAGED_CAS_CA"),
1151+
)
1152+
}
11131153

1114-
wantName, _ := instance.ParseConnNameWithDomainName("my-project:my-region:my-instance", "db.example.com")
1115-
d := setupDialer(t, setupConfig{
1116-
testInstance: inst,
1117-
reqs: []*mock.Request{
1118-
mock.InstanceGetSuccess(inst, 1),
1119-
mock.CreateEphemeralSuccess(inst, 1),
1120-
},
1121-
dialerOptions: []Option{
1122-
WithTokenSource(mock.EmptyTokenSource{}),
1123-
WithResolver(&fakeResolver{
1124-
entries: map[string]instance.ConnName{
1125-
"db.example.com": wantName,
1154+
wantName, _ := instance.ParseConnNameWithDomainName(tc.icn, tc.dn)
1155+
d := setupDialer(t, setupConfig{
1156+
testInstance: inst,
1157+
reqs: []*mock.Request{
1158+
mock.InstanceGetSuccess(inst, 1),
1159+
mock.CreateEphemeralSuccess(inst, 1),
11261160
},
1127-
}),
1128-
},
1129-
})
1161+
dialerOptions: []Option{
1162+
WithTokenSource(mock.EmptyTokenSource{}),
1163+
WithResolver(&fakeResolver{
1164+
entries: map[string]instance.ConnName{
1165+
"db.example.com": wantName,
1166+
"my-project:my-region:my-instance": wantName,
1167+
},
1168+
}),
1169+
},
1170+
})
1171+
dnOrIcn := tc.icn
1172+
if tc.dn != "" {
1173+
dnOrIcn = tc.dn
1174+
}
11301175

1131-
// Dial db.example.com
1132-
testSuccessfulDial(
1133-
context.Background(), t, d,
1134-
"db.example.com",
1135-
)
1176+
// Dial db.example.com
1177+
testSuccessfulDial(
1178+
context.Background(), t, d,
1179+
dnOrIcn,
1180+
)
1181+
})
1182+
}
11361183
}
11371184

11381185
func TestDialerChecksSubjectAlternativeNameAndFails(t *testing.T) {
11391186

11401187
// Create an instance with custom SAN 'db.example.com'
1141-
inst := mock.NewFakeCSQLInstanceWithSan(
1142-
"my-project", "my-region", "my-instance", []string{"db.example.com"},
1188+
inst := mock.NewFakeCSQLInstance(
1189+
"my-project", "my-region", "my-instance",
11431190
mock.WithDNS("db.example.com"),
11441191
mock.WithServerCAMode("GOOGLE_MANAGED_CAS_CA"),
11451192
)
@@ -1207,8 +1254,8 @@ func TestDialerRefreshesAfterRotateCACerts(t *testing.T) {
12071254
}
12081255
for _, tc := range tcs {
12091256
t.Run(tc.desc, func(t *testing.T) {
1210-
inst := mock.NewFakeCSQLInstanceWithSan(
1211-
"my-project", "my-region", "my-instance", []string{"db.example.com"},
1257+
inst := mock.NewFakeCSQLInstance(
1258+
"my-project", "my-region", "my-instance",
12121259
mock.WithDNS("db.example.com"),
12131260
mock.WithServerCAMode("GOOGLE_MANAGED_CAS_CA"),
12141261
)

‎e2e_mysql_test.go‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,12 @@
1515
package cloudsqlconn_test
1616

1717
import (
18-
"context"
1918
"database/sql"
20-
"fmt"
2119
"os"
2220
"testing"
2321
"time"
2422

2523
"cloud.google.com/go/cloudsqlconn"
26-
"cloud.google.com/go/cloudsqlconn/instance"
2724
"cloud.google.com/go/cloudsqlconn/mysql/mysql"
2825
gomysql "github.com/go-sql-driver/mysql"
2926
)
@@ -55,16 +52,6 @@ func requireMySQLVars(t *testing.T) {
5552
}
5653
}
5754

58-
type mockResolver struct {
59-
}
60-
61-
func (r *mockResolver) Resolve(_ context.Context, name string) (instanceName instance.ConnName, err error) {
62-
if name == "mysql.example.com" {
63-
return instance.ParseConnNameWithDomainName(mysqlConnName, "mysql.example.com")
64-
}
65-
return instance.ConnName{}, fmt.Errorf("no resolution for %v", name)
66-
}
67-
6855
func TestMySQLDriver(t *testing.T) {
6956
if testing.Short() {
7057
t.Skip("skipping MySQL integration tests")
@@ -94,14 +81,6 @@ func TestMySQLDriver(t *testing.T) {
9481
user: mysqlIAMUser,
9582
password: "password",
9683
},
97-
{
98-
desc: "with dns",
99-
driverName: "cloudsql-mysql-dns",
100-
opts: []cloudsqlconn.Option{cloudsqlconn.WithResolver(&mockResolver{})},
101-
instanceName: "mysql.example.com",
102-
user: mysqlUser,
103-
password: mysqlPass,
104-
},
10584
}
10685

10786
for _, tc := range tcs {

‎go.mod‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ module cloud.google.com/go/cloudsqlconn
22

33
go 1.23.0
44

5-
toolchain go1.23.7
6-
75
require (
86
cloud.google.com/go/auth v0.15.0
97
cloud.google.com/go/auth/oauth2adapt v0.2.7
@@ -16,8 +14,8 @@ require (
1614
golang.org/x/net v0.37.0
1715
golang.org/x/oauth2 v0.28.0
1816
golang.org/x/time v0.11.0
19-
google.golang.org/api v0.224.0
20-
google.golang.org/genproto/googleapis/rpc v0.0.0-20250227231956-55c901821b1e
17+
google.golang.org/api v0.225.0
18+
google.golang.org/genproto/googleapis/rpc v0.0.0-20250303144028-a0af3efb3deb
2119
google.golang.org/grpc v1.71.0
2220
)
2321

‎go.sum‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -305,17 +305,17 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T
305305
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
306306
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
307307
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
308-
google.golang.org/api v0.224.0 h1:Ir4UPtDsNiwIOHdExr3fAj4xZ42QjK7uQte3lORLJwU=
309-
google.golang.org/api v0.224.0/go.mod h1:3V39my2xAGkodXy0vEqcEtkqgw2GtrFL5WuBZlCTCOQ=
308+
google.golang.org/api v0.225.0 h1:+4/IVqBQm0MV5S+JW3kdEGC1WtOmM2mXN1LKH1LdNlw=
309+
google.golang.org/api v0.225.0/go.mod h1:WP/0Xm4LVvMOCldfvOISnWquSRWbG2kArDZcg+W2DbY=
310310
google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
311311
google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
312312
google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
313313
google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc=
314314
google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo=
315315
google.golang.org/genproto/googleapis/api v0.0.0-20250106144421-5f5ef82da422 h1:GVIKPyP/kLIyVOgOnTwFOrvQaQUzOzGMCxgFUOEmm24=
316316
google.golang.org/genproto/googleapis/api v0.0.0-20250106144421-5f5ef82da422/go.mod h1:b6h1vNKhxaSoEI+5jc3PJUCustfli/mRab7295pY7rw=
317-
google.golang.org/genproto/googleapis/rpc v0.0.0-20250227231956-55c901821b1e h1:YA5lmSs3zc/5w+xsRcHqpETkaYyK63ivEPzNTcUUlSA=
318-
google.golang.org/genproto/googleapis/rpc v0.0.0-20250227231956-55c901821b1e/go.mod h1:LuRYeWDFV6WOn90g357N17oMCaxpgCnbi/44qJvDn2I=
317+
google.golang.org/genproto/googleapis/rpc v0.0.0-20250303144028-a0af3efb3deb h1:TLPQVbx1GJ8VKZxz52VAxl1EBgKXXbTiU9Fc5fZeLn4=
318+
google.golang.org/genproto/googleapis/rpc v0.0.0-20250303144028-a0af3efb3deb/go.mod h1:LuRYeWDFV6WOn90g357N17oMCaxpgCnbi/44qJvDn2I=
319319
google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
320320
google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg=
321321
google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=

‎internal/cloudsql/instance.go‎

Lines changed: 44 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,14 @@ type RefreshAheadCache struct {
124124

125125
// NewRefreshAheadCache initializes a new Instance given an instance connection name
126126
func NewRefreshAheadCache(
127-
cn instance.ConnName,
128-
l debug.ContextLogger,
129-
client *sqladmin.Service,
130-
key *rsa.PrivateKey,
131-
refreshTimeout time.Duration,
132-
tp auth.TokenProvider,
133-
dialerID string,
134-
useIAMAuthNDial bool,
127+
cn instance.ConnName,
128+
l debug.ContextLogger,
129+
client *sqladmin.Service,
130+
key *rsa.PrivateKey,
131+
refreshTimeout time.Duration,
132+
tp auth.TokenProvider,
133+
dialerID string,
134+
useIAMAuthNDial bool,
135135
) *RefreshAheadCache {
136136
ctx, cancel := context.WithCancel(context.Background())
137137
i := &RefreshAheadCache{
@@ -188,13 +188,13 @@ type ConnectionInfo struct {
188188

189189
// NewConnectionInfo initializes a ConnectionInfo struct.
190190
func NewConnectionInfo(
191-
cn instance.ConnName,
192-
dnsName string,
193-
serverCAMode string,
194-
version string,
195-
ipAddrs map[string]string,
196-
serverCACert []*x509.Certificate,
197-
clientCert tls.Certificate,
191+
cn instance.ConnName,
192+
dnsName string,
193+
serverCAMode string,
194+
version string,
195+
ipAddrs map[string]string,
196+
serverCACert []*x509.Certificate,
197+
clientCert tls.Certificate,
198198
) ConnectionInfo {
199199
return ConnectionInfo{
200200
addrs: ipAddrs,
@@ -242,43 +242,44 @@ func (c ConnectionInfo) TLSConfig() *tls.Config {
242242
pool.AddCert(caCert)
243243
}
244244

245-
// For CAS instances, we can rely on the DNS name to verify the server identity.
246-
if c.ServerCAMode != "" && c.ServerCAMode != "GOOGLE_MANAGED_INTERNAL_CA" {
247-
// By default, use Standard TLS hostname verification name to
248-
// verify the server identity.
249-
250-
// If the connector was configured with a domain name, use that domain name
251-
// to validate the certificate. Otherwise, use the DNS name from the
252-
// instance ConnectionInfo API response.
253-
serverName := c.ConnectionName.DomainName()
254-
if serverName == "" {
255-
serverName = c.DNSName
256-
}
245+
// If the connector was configured with a domain name, use that domain name
246+
// to validate the certificate. Otherwise, use the DNS name from the
247+
// instance metadata retrieved from the ConnectSettings API endpoint.
248+
serverName := c.ConnectionName.DomainName()
249+
if serverName == "" {
250+
serverName = c.DNSName
251+
}
257252

253+
// If the instance metadata does not contain a domain name, use the legacy
254+
// validation checking the CN field for the instance connection name.
255+
if c.DNSName == "" {
258256
return &tls.Config{
259-
ServerName: serverName,
257+
ServerName: c.ConnectionName.String(),
260258
Certificates: []tls.Certificate{c.ClientCertificate},
261259
RootCAs: pool,
262-
MinVersion: tls.VersionTLS13,
260+
// We need to set InsecureSkipVerify to true due to
261+
// https://github.com/GoogleCloudPlatform/cloudsql-proxy/issues/194
262+
// https://tip.golang.org/doc/go1.11#crypto/x509
263+
//
264+
// Since we have a secure channel to the Cloud SQL API which we use to
265+
// retrieve the certificates, we instead need to implement our own
266+
// VerifyPeerCertificate function that will verify that the certificate
267+
// is OK.
268+
InsecureSkipVerify: true,
269+
VerifyPeerCertificate: verifyPeerCertificateFunc(c.ConnectionName, pool),
270+
MinVersion: tls.VersionTLS13,
263271
}
264272
}
265-
// For legacy instances use the custom TLS validation
273+
274+
// By default, use Standard TLS hostname verification name to
275+
// verify the server identity.
266276
return &tls.Config{
267-
ServerName: c.ConnectionName.String(),
277+
ServerName: serverName,
268278
Certificates: []tls.Certificate{c.ClientCertificate},
269279
RootCAs: pool,
270-
// We need to set InsecureSkipVerify to true due to
271-
// https://github.com/GoogleCloudPlatform/cloudsql-proxy/issues/194
272-
// https://tip.golang.org/doc/go1.11#crypto/x509
273-
//
274-
// Since we have a secure channel to the Cloud SQL API which we use to
275-
// retrieve the certificates, we instead need to implement our own
276-
// VerifyPeerCertificate function that will verify that the certificate
277-
// is OK.
278-
InsecureSkipVerify: true,
279-
VerifyPeerCertificate: verifyPeerCertificateFunc(c.ConnectionName, pool),
280-
MinVersion: tls.VersionTLS13,
280+
MinVersion: tls.VersionTLS13,
281281
}
282+
282283
}
283284

284285
// verifyPeerCertificateFunc creates a VerifyPeerCertificate func that
@@ -287,7 +288,7 @@ func (c ConnectionInfo) TLSConfig() *tls.Config {
287288
// my-project:my-instance) instead of a valid domain name for the certificate's
288289
// Common Name.
289290
func verifyPeerCertificateFunc(
290-
cn instance.ConnName, pool *x509.CertPool,
291+
cn instance.ConnName, pool *x509.CertPool,
291292
) func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
292293
return func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
293294
if len(rawCerts) == 0 {

0 commit comments

Comments
 (0)