Skip to content

Commit

Permalink
advancedtls: add min/max TLS version option
Browse files Browse the repository at this point in the history 
Adding an "TlsVersionOption" for users to select their desired min/max TLS versions, if advanced TLS is used, per request by #5667

RELEASE NOTES:

security/advancedtls: add min/max TLS version selection options
  • Loading branch information
@ZhenLian @joeljeske
ZhenLian authored and joeljeske committed Feb 4, 2023
1 parent 3151e83 commit 23c3143
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 8 deletions.
28 changes: 28 additions & 0 deletions security/advancedtls/advancedtls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,15 @@ type ClientOptions struct {
// RevocationConfig is the configurations for certificate revocation checks.
// It could be nil if such checks are not needed.
RevocationConfig *RevocationConfig
// MinVersion contains the minimum TLS version that is acceptable.
// By default, TLS 1.2 is currently used as the minimum when acting as a
// client, and TLS 1.0 when acting as a server. TLS 1.0 is the minimum
// supported by this package, both as a client and as a server.
MinVersion uint16
// MaxVersion contains the maximum TLS version that is acceptable.
// By default, the maximum version supported by this package is used,
// which is currently TLS 1.3.
MaxVersion uint16
}

// ServerOptions contains the fields needed to be filled by the server.
Expand All @@ -205,6 +214,15 @@ type ServerOptions struct {
// RevocationConfig is the configurations for certificate revocation checks.
// It could be nil if such checks are not needed.
RevocationConfig *RevocationConfig
// MinVersion contains the minimum TLS version that is acceptable.
// By default, TLS 1.2 is currently used as the minimum when acting as a
// client, and TLS 1.0 when acting as a server. TLS 1.0 is the minimum
// supported by this package, both as a client and as a server.
MinVersion uint16
// MaxVersion contains the maximum TLS version that is acceptable.
// By default, the maximum version supported by this package is used,
// which is currently TLS 1.3.
MaxVersion uint16
}

func (o *ClientOptions) config() (*tls.Config, error) {
Expand All @@ -222,11 +240,16 @@ func (o *ClientOptions) config() (*tls.Config, error) {
if o.IdentityOptions.GetIdentityCertificatesForServer != nil {
return nil, fmt.Errorf("GetIdentityCertificatesForServer cannot be specified on the client side")
}
if o.MinVersion > o.MaxVersion {
return nil, fmt.Errorf("the minimum TLS version is larger than the maximum TLS version")
}
config := &tls.Config{
ServerName: o.ServerNameOverride,
// We have to set InsecureSkipVerify to true to skip the default checks and
// use the verification function we built from buildVerifyFunc.
InsecureSkipVerify: true,
MinVersion: o.MinVersion,
MaxVersion: o.MaxVersion,
}
// Propagate root-certificate-related fields in tls.Config.
switch {
Expand Down Expand Up @@ -293,6 +316,9 @@ func (o *ServerOptions) config() (*tls.Config, error) {
if o.IdentityOptions.GetIdentityCertificatesForClient != nil {
return nil, fmt.Errorf("GetIdentityCertificatesForClient cannot be specified on the server side")
}
if o.MinVersion > o.MaxVersion {
return nil, fmt.Errorf("the minimum TLS version is larger than the maximum TLS version")
}
clientAuth := tls.NoClientCert
if o.RequireClientCert {
// We have to set clientAuth to RequireAnyClientCert to force underlying
Expand All @@ -302,6 +328,8 @@ func (o *ServerOptions) config() (*tls.Config, error) {
}
config := &tls.Config{
ClientAuth: clientAuth,
MinVersion: o.MinVersion,
MaxVersion: o.MaxVersion,
}
// Propagate root-certificate-related fields in tls.Config.
switch {
Expand Down
156 changes: 148 additions & 8 deletions security/advancedtls/advancedtls_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,13 +731,12 @@ func (s) TestDefaultHostNameCheck(t *testing.T) {
t.Fatalf("cs.LoadCerts() failed, err: %v", err)
}
for _, test := range []struct {
desc string
clientRoot *x509.CertPool
clientVerifyFunc CustomVerificationFunc
clientVType VerificationType
serverCert []tls.Certificate
serverVType VerificationType
expectError bool
desc string
clientRoot *x509.CertPool
clientVType VerificationType
serverCert []tls.Certificate
serverVType VerificationType
expectError bool
}{
// Client side sets vType to CertAndHostVerification, and will do
// default hostname check. Server uses a cert without "localhost" or
Expand Down Expand Up @@ -787,7 +786,6 @@ func (s) TestDefaultHostNameCheck(t *testing.T) {
pb.RegisterGreeterServer(s, greeterServer{})
go s.Serve(lis)
clientOptions := &ClientOptions{
VerifyPeer: test.clientVerifyFunc,
RootOptions: RootCertificateOptions{
RootCACerts: test.clientRoot,
},
Expand All @@ -811,3 +809,145 @@ func (s) TestDefaultHostNameCheck(t *testing.T) {
})
}
}

func (s) TestTLSVersions(t *testing.T) {
cs := &testutils.CertStore{}
if err := cs.LoadCerts(); err != nil {
t.Fatalf("cs.LoadCerts() failed, err: %v", err)
}
for _, test := range []struct {
desc string
expectError bool
clientMinVersion uint16
clientMaxVersion uint16
serverMinVersion uint16
serverMaxVersion uint16
}{
// Client side sets TLS version that is higher than required from the server side.
{
desc: "Client TLS version higher than server",
clientMinVersion: tls.VersionTLS13,
clientMaxVersion: tls.VersionTLS13,
serverMinVersion: tls.VersionTLS12,
serverMaxVersion: tls.VersionTLS12,
expectError: true,
},
// Server side sets TLS version that is higher than required from the client side.
{
desc: "Server TLS version higher than client",
clientMinVersion: tls.VersionTLS12,
clientMaxVersion: tls.VersionTLS12,
serverMinVersion: tls.VersionTLS13,
serverMaxVersion: tls.VersionTLS13,
expectError: true,
},
// Client and server set proper TLS versions.
{
desc: "Good TLS version settings",
clientMinVersion: tls.VersionTLS12,
clientMaxVersion: tls.VersionTLS13,
serverMinVersion: tls.VersionTLS12,
serverMaxVersion: tls.VersionTLS13,
expectError: false,
},
{
desc: "Client 1.2 - 1.3 and server 1.2",
clientMinVersion: tls.VersionTLS12,
clientMaxVersion: tls.VersionTLS13,
serverMinVersion: tls.VersionTLS12,
serverMaxVersion: tls.VersionTLS12,
expectError: false,
},
{
desc: "Client 1.2 - 1.3 and server 1.1 - 1.2",
clientMinVersion: tls.VersionTLS12,
clientMaxVersion: tls.VersionTLS13,
serverMinVersion: tls.VersionTLS11,
serverMaxVersion: tls.VersionTLS12,
expectError: false,
},
{
desc: "Client 1.2 - 1.3 and server 1.3",
clientMinVersion: tls.VersionTLS12,
clientMaxVersion: tls.VersionTLS13,
serverMinVersion: tls.VersionTLS13,
serverMaxVersion: tls.VersionTLS13,
expectError: false,
},
{
desc: "Client 1.2 - 1.2 and server 1.2 - 1.3",
clientMinVersion: tls.VersionTLS12,
clientMaxVersion: tls.VersionTLS12,
serverMinVersion: tls.VersionTLS12,
serverMaxVersion: tls.VersionTLS13,
expectError: false,
},
{
desc: "Client 1.1 - 1.2 and server 1.2 - 1.3",
clientMinVersion: tls.VersionTLS11,
clientMaxVersion: tls.VersionTLS12,
serverMinVersion: tls.VersionTLS12,
serverMaxVersion: tls.VersionTLS13,
expectError: false,
},
{
desc: "Client 1.3 and server 1.2 - 1.3",
clientMinVersion: tls.VersionTLS13,
clientMaxVersion: tls.VersionTLS13,
serverMinVersion: tls.VersionTLS12,
serverMaxVersion: tls.VersionTLS13,
expectError: false,
},
} {
test := test
t.Run(test.desc, func(t *testing.T) {
// Start a server using ServerOptions in another goroutine.
serverOptions := &ServerOptions{
IdentityOptions: IdentityCertificateOptions{
Certificates: []tls.Certificate{cs.ServerPeerLocalhost1},
},
RequireClientCert: false,
VType: CertAndHostVerification,
MinVersion: test.serverMinVersion,
MaxVersion: test.serverMaxVersion,
}
serverTLSCreds, err := NewServerCreds(serverOptions)
if err != nil {
t.Fatalf("failed to create server creds: %v", err)
}
s := grpc.NewServer(grpc.Creds(serverTLSCreds))
defer s.Stop()
lis, err := net.Listen("tcp", "localhost:0")
if err != nil {
t.Fatalf("failed to listen: %v", err)
}
defer lis.Close()
addr := fmt.Sprintf("localhost:%v", lis.Addr().(*net.TCPAddr).Port)
pb.RegisterGreeterServer(s, greeterServer{})
go s.Serve(lis)
clientOptions := &ClientOptions{
RootOptions: RootCertificateOptions{
RootCACerts: cs.ClientTrust1,
},
VType: CertAndHostVerification,
MinVersion: test.clientMinVersion,
MaxVersion: test.clientMaxVersion,
}
clientTLSCreds, err := NewClientCreds(clientOptions)
if err != nil {
t.Fatalf("clientTLSCreds failed to create: %v", err)
}
shouldFail := false
if test.expectError {
shouldFail = true
}
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
conn, _, err := callAndVerifyWithClientConn(ctx, addr, "rpc call 1", clientTLSCreds, shouldFail)
if err != nil {
t.Fatal(err)
}
defer conn.Close()
})
}
}
30 changes: 30 additions & 0 deletions security/advancedtls/advancedtls_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ func (s) TestClientOptionsConfigErrorCases(t *testing.T) {
clientVType VerificationType
IdentityOptions IdentityCertificateOptions
RootOptions RootCertificateOptions
MinVersion uint16
MaxVersion uint16
}{
{
desc: "Skip default verification and provide no root credentials",
Expand Down Expand Up @@ -122,6 +124,11 @@ func (s) TestClientOptionsConfigErrorCases(t *testing.T) {
},
},
},
{
desc: "Invalid min/max TLS versions",
MinVersion: tls.VersionTLS13,
MaxVersion: tls.VersionTLS12,
},
}
for _, test := range tests {
test := test
Expand All @@ -130,6 +137,8 @@ func (s) TestClientOptionsConfigErrorCases(t *testing.T) {
VType: test.clientVType,
IdentityOptions: test.IdentityOptions,
RootOptions: test.RootOptions,
MinVersion: test.MinVersion,
MaxVersion: test.MaxVersion,
}
_, err := clientOptions.config()
if err == nil {
Expand All @@ -145,6 +154,8 @@ func (s) TestClientOptionsConfigSuccessCases(t *testing.T) {
clientVType VerificationType
IdentityOptions IdentityCertificateOptions
RootOptions RootCertificateOptions
MinVersion uint16
MaxVersion uint16
}{
{
desc: "Use system default if no fields in RootCertificateOptions is specified",
Expand All @@ -159,6 +170,8 @@ func (s) TestClientOptionsConfigSuccessCases(t *testing.T) {
IdentityOptions: IdentityCertificateOptions{
IdentityProvider: fakeProvider{pt: provTypeIdentity},
},
MinVersion: tls.VersionTLS12,
MaxVersion: tls.VersionTLS13,
},
}
for _, test := range tests {
Expand All @@ -168,6 +181,8 @@ func (s) TestClientOptionsConfigSuccessCases(t *testing.T) {
VType: test.clientVType,
IdentityOptions: test.IdentityOptions,
RootOptions: test.RootOptions,
MinVersion: test.MinVersion,
MaxVersion: test.MaxVersion,
}
clientConfig, err := clientOptions.config()
if err != nil {
Expand All @@ -192,6 +207,8 @@ func (s) TestServerOptionsConfigErrorCases(t *testing.T) {
serverVType VerificationType
IdentityOptions IdentityCertificateOptions
RootOptions RootCertificateOptions
MinVersion uint16
MaxVersion uint16
}{
{
desc: "Skip default verification and provide no root credentials",
Expand Down Expand Up @@ -229,6 +246,11 @@ func (s) TestServerOptionsConfigErrorCases(t *testing.T) {
},
},
},
{
desc: "Invalid min/max TLS versions",
MinVersion: tls.VersionTLS13,
MaxVersion: tls.VersionTLS12,
},
}
for _, test := range tests {
test := test
Expand All @@ -238,6 +260,8 @@ func (s) TestServerOptionsConfigErrorCases(t *testing.T) {
RequireClientCert: test.requireClientCert,
IdentityOptions: test.IdentityOptions,
RootOptions: test.RootOptions,
MinVersion: test.MinVersion,
MaxVersion: test.MaxVersion,
}
_, err := serverOptions.config()
if err == nil {
Expand All @@ -254,6 +278,8 @@ func (s) TestServerOptionsConfigSuccessCases(t *testing.T) {
serverVType VerificationType
IdentityOptions IdentityCertificateOptions
RootOptions RootCertificateOptions
MinVersion uint16
MaxVersion uint16
}{
{
desc: "Use system default if no fields in RootCertificateOptions is specified",
Expand All @@ -275,6 +301,8 @@ func (s) TestServerOptionsConfigSuccessCases(t *testing.T) {
return nil, nil
},
},
MinVersion: tls.VersionTLS12,
MaxVersion: tls.VersionTLS13,
},
}
for _, test := range tests {
Expand All @@ -285,6 +313,8 @@ func (s) TestServerOptionsConfigSuccessCases(t *testing.T) {
RequireClientCert: test.requireClientCert,
IdentityOptions: test.IdentityOptions,
RootOptions: test.RootOptions,
MinVersion: test.MinVersion,
MaxVersion: test.MaxVersion,
}
serverConfig, err := serverOptions.config()
if err != nil {
Expand Down

0 comments on commit 23c3143

Please sign in to comment.