Skip to content

Commit

Permalink
advancedTLS: Rename {Min/Max}Version to {Min/Max}TLSVersion (#7173)
Browse files Browse the repository at this point in the history 
* rename `MinVersion` and `MaxVersion` to `MinTLSVersion` and `MaxTLSVersion`
  • Loading branch information
@gtcooke94
gtcooke94 authored May 6, 2024
1 parent f2d6421 commit befc29d
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 28 deletions.
90 changes: 74 additions & 16 deletions security/advancedtls/advancedtls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,26 @@ type ClientOptions struct {
// It could be nil if such checks are not needed.
RevocationOptions *RevocationOptions
// 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.
//
// Deprecated: use MinTLSVersion instead.
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.
//
// Deprecated: use MaxTLSVersion instead.
MaxVersion uint16
// MinTLSVersion contains the minimum TLS version that is acceptable.
// The value should be set using tls.VersionTLSxx from https://pkg.go.dev/crypto/tls
// 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. This
// default may be changed over time affecting backwards compatibility.
MinTLSVersion uint16
// MaxTLSVersion contains the maximum TLS version that is acceptable.
// The value should be set using tls.VersionTLSxx from https://pkg.go.dev/crypto/tls
// By default, the maximum version supported by this package is used,
// which is currently TLS 1.3. This default may be changed over time
// affecting backwards compatibility.
MaxTLSVersion uint16
// serverNameOverride is for testing only. If set to a non-empty string, it
// will override the virtual host name of authority (e.g. :authority header
// field) in requests and the target hostname used during server cert
Expand Down Expand Up @@ -263,14 +275,26 @@ type ServerOptions struct {
// It could be nil if such checks are not needed.
RevocationOptions *RevocationOptions
// 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.
//
// Deprecated: use MinTLSVersion instead.
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.
//
// Deprecated: use MaxTLSVersion instead.
MaxVersion uint16
// MinTLSVersion contains the minimum TLS version that is acceptable.
// The value should be set using tls.VersionTLSxx from https://pkg.go.dev/crypto/tls
// 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. This
// default may be changed over time affecting backwards compatibility.
MinTLSVersion uint16
// MaxTLSVersion contains the maximum TLS version that is acceptable.
// The value should be set using tls.VersionTLSxx from https://pkg.go.dev/crypto/tls
// By default, the maximum version supported by this package is used,
// which is currently TLS 1.3. This default may be changed over time
// affecting backwards compatibility.
MaxTLSVersion uint16
}

func (o *ClientOptions) config() (*tls.Config, error) {
Expand All @@ -285,6 +309,15 @@ func (o *ClientOptions) config() (*tls.Config, error) {
if o.VType != CertAndHostVerification {
o.VerificationType = o.VType
}
// TODO(gtcooke94) MinVersion and MaxVersion are deprected, eventually
// remove this block. This is a temporary fallback to ensure that if the
// refactored names aren't set we use the old names.
if o.MinTLSVersion == 0 {
o.MinTLSVersion = o.MinVersion
}
if o.MaxTLSVersion == 0 {
o.MaxTLSVersion = o.MaxVersion
}
if o.VerificationType == SkipVerification && o.AdditionalPeerVerification == nil {
return nil, fmt.Errorf("client needs to provide custom verification mechanism if choose to skip default verification")
}
Expand All @@ -299,16 +332,24 @@ 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 {
if o.MinTLSVersion > o.MaxTLSVersion {
return nil, fmt.Errorf("the minimum TLS version is larger than the maximum TLS version")
}
// If the MinTLSVersion isn't set, default to 1.2
if o.MinTLSVersion == 0 {
o.MinTLSVersion = tls.VersionTLS12
}
// If the MaxTLSVersion isn't set, default to 1.3
if o.MaxTLSVersion == 0 {
o.MaxTLSVersion = tls.VersionTLS13
}
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,
MinVersion: o.MinTLSVersion,
MaxVersion: o.MaxTLSVersion,
}
// Propagate root-certificate-related fields in tls.Config.
switch {
Expand Down Expand Up @@ -372,6 +413,15 @@ func (o *ServerOptions) config() (*tls.Config, error) {
if o.VType != CertAndHostVerification {
o.VerificationType = o.VType
}
// TODO(gtcooke94) MinVersion and MaxVersion are deprected, eventually
// remove this block. This is a temporary fallback to ensure that if the
// refactored names aren't set we use the old names.
if o.MinTLSVersion == 0 {
o.MinTLSVersion = o.MinVersion
}
if o.MaxTLSVersion == 0 {
o.MaxTLSVersion = o.MaxVersion
}
if o.RequireClientCert && o.VerificationType == SkipVerification && o.AdditionalPeerVerification == nil {
return nil, fmt.Errorf("server needs to provide custom verification mechanism if choose to skip default verification, but require client certificate(s)")
}
Expand All @@ -386,7 +436,7 @@ 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 {
if o.MinTLSVersion > o.MaxTLSVersion {
return nil, fmt.Errorf("the minimum TLS version is larger than the maximum TLS version")
}
clientAuth := tls.NoClientCert
Expand All @@ -396,10 +446,18 @@ func (o *ServerOptions) config() (*tls.Config, error) {
// buildVerifyFunc.
clientAuth = tls.RequireAnyClientCert
}
// If the MinTLSVersion isn't set, default to 1.2
if o.MinTLSVersion == 0 {
o.MinTLSVersion = tls.VersionTLS12
}
// If the MaxTLSVersion isn't set, default to 1.3
if o.MaxTLSVersion == 0 {
o.MaxTLSVersion = tls.VersionTLS13
}
config := &tls.Config{
ClientAuth: clientAuth,
MinVersion: o.MinVersion,
MaxVersion: o.MaxVersion,
MinVersion: o.MinTLSVersion,
MaxVersion: o.MaxTLSVersion,
}
// Propagate root-certificate-related fields in tls.Config.
switch {
Expand Down
8 changes: 4 additions & 4 deletions security/advancedtls/advancedtls_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,8 @@ func (s) TestTLSVersions(t *testing.T) {
},
RequireClientCert: false,
VerificationType: CertAndHostVerification,
MinVersion: test.serverMinVersion,
MaxVersion: test.serverMaxVersion,
MinTLSVersion: test.serverMinVersion,
MaxTLSVersion: test.serverMaxVersion,
}
serverTLSCreds, err := NewServerCreds(serverOptions)
if err != nil {
Expand All @@ -930,8 +930,8 @@ func (s) TestTLSVersions(t *testing.T) {
RootCACerts: cs.ClientTrust1,
},
VerificationType: CertAndHostVerification,
MinVersion: test.clientMinVersion,
MaxVersion: test.clientMaxVersion,
MinTLSVersion: test.clientMinVersion,
MaxTLSVersion: test.clientMaxVersion,
}
clientTLSCreds, err := NewClientCreds(clientOptions)
if err != nil {
Expand Down
34 changes: 26 additions & 8 deletions security/advancedtls/advancedtls_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ func (s) TestClientOptionsConfigErrorCases(t *testing.T) {
VerificationType: test.clientVerificationType,
IdentityOptions: test.IdentityOptions,
RootOptions: test.RootOptions,
MinVersion: test.MinVersion,
MaxVersion: test.MaxVersion,
MinTLSVersion: test.MinVersion,
MaxTLSVersion: test.MaxVersion,
}
_, err := clientOptions.config()
if err == nil {
Expand Down Expand Up @@ -196,8 +196,8 @@ func (s) TestClientOptionsConfigSuccessCases(t *testing.T) {
VerificationType: test.clientVerificationType,
IdentityOptions: test.IdentityOptions,
RootOptions: test.RootOptions,
MinVersion: test.MinVersion,
MaxVersion: test.MaxVersion,
MinTLSVersion: test.MinVersion,
MaxTLSVersion: test.MaxVersion,
}
clientConfig, err := clientOptions.config()
if err != nil {
Expand All @@ -211,6 +211,24 @@ func (s) TestClientOptionsConfigSuccessCases(t *testing.T) {
t.Fatalf("Failed to assign system-provided certificates on the client side.")
}
}
if test.MinVersion != 0 {
if clientConfig.MinVersion != test.MinVersion {
t.Fatalf("Failed to assign min tls version.")
}
} else {
if clientConfig.MinVersion != tls.VersionTLS12 {
t.Fatalf("Default min tls version not set correctly")
}
}
if test.MaxVersion != 0 {
if clientConfig.MaxVersion != test.MaxVersion {
t.Fatalf("Failed to assign max tls version.")
}
} else {
if clientConfig.MaxVersion != tls.VersionTLS13 {
t.Fatalf("Default max tls version not set correctly")
}
}
})
}
}
Expand Down Expand Up @@ -275,8 +293,8 @@ func (s) TestServerOptionsConfigErrorCases(t *testing.T) {
RequireClientCert: test.requireClientCert,
IdentityOptions: test.IdentityOptions,
RootOptions: test.RootOptions,
MinVersion: test.MinVersion,
MaxVersion: test.MaxVersion,
MinTLSVersion: test.MinVersion,
MaxTLSVersion: test.MaxVersion,
}
_, err := serverOptions.config()
if err == nil {
Expand Down Expand Up @@ -342,8 +360,8 @@ func (s) TestServerOptionsConfigSuccessCases(t *testing.T) {
RequireClientCert: test.requireClientCert,
IdentityOptions: test.IdentityOptions,
RootOptions: test.RootOptions,
MinVersion: test.MinVersion,
MaxVersion: test.MaxVersion,
MinTLSVersion: test.MinVersion,
MaxTLSVersion: test.MaxVersion,
}
serverConfig, err := serverOptions.config()
if err != nil {
Expand Down

0 comments on commit befc29d

Please sign in to comment.