Skip to content
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.

Sign up for GitHub

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

Jump to bottom

security/advancedtls: add TlsVersionOption to select desired min/max TLS versions #6007

Merged
merged 1 commit into from Apr 10, 2023
Merged

security/advancedtls: add TlsVersionOption to select desired min/max TLS versions #6007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions security/advancedtls/advancedtls.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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