advancedTLS: Combine ClientOptions and ServerOptions to just Options (#7202)

* rename to Options

* added some documentation

* typos

Author: gtcooke94

security/advancedtls/advancedtls.go

@@ -184,66 +184,18 @@ const (
 )
 
 // ClientOptions contains the fields needed to be filled by the client.
-type ClientOptions struct {
-	// IdentityOptions is OPTIONAL on client side. This field only needs to be
-	// set if mutual authentication is required on server side.
-	IdentityOptions IdentityCertificateOptions
-	// AdditionalPeerVerification is a custom verification check after certificate signature
-	// check.
-	// If this is set, we will perform this customized check after doing the
-	// normal check(s) indicated by setting VerificationType.
-	AdditionalPeerVerification PostHandshakeVerificationFunc
-	// VerifyPeer is a custom verification check after certificate signature
-	// check.
-	// If this is set, we will perform this customized check after doing the
-	// normal check(s) indicated by setting VerificationType.
-	//
-	// Deprecated: use AdditionalPeerVerification instead.
-	VerifyPeer PostHandshakeVerificationFunc
-	// RootOptions is OPTIONAL on client side. If not set, we will try to use the
-	// default trust certificates in users' OS system.
-	RootOptions RootCertificateOptions
-	// VerificationType defines what type of server verification is done. See
-	// the `VerificationType` enum for the different options.
-	// Default: CertAndHostVerification
-	VerificationType VerificationType
-	// VType is the verification type on the client side.
-	//
-	// Deprecated: use VerificationType instead.
-	VType VerificationType
-	// RevocationOptions is the configurations for certificate revocation checks.
-	// It could be nil if such checks are not needed.
-	RevocationOptions *RevocationOptions
-	// MinVersion contains the minimum TLS version that is acceptable.
-	//
-	// Deprecated: use MinTLSVersion instead.
-	MinVersion uint16
-	// MaxVersion contains the maximum TLS version that is acceptable.
-	//
-	// 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
-	// verification.
-	serverNameOverride string
-}
+// Deprecated: use Options instead.
+type ClientOptions = Options
 
 // ServerOptions contains the fields needed to be filled by the server.
-type ServerOptions struct {
+// Deprecated: use Options instead.
+type ServerOptions = Options
+
+// Options contains the fields a user can configure when setting up TLS clients
+// and servers
+type Options struct {
+	// IdentityOptions is OPTIONAL on client side. This field only needs to be
+	// set if mutual authentication is required on server side.
 	// IdentityOptions is REQUIRED on server side.
 	IdentityOptions IdentityCertificateOptions
 	// AdditionalPeerVerification is a custom verification check after certificate signature
@@ -261,9 +213,11 @@ type ServerOptions struct {
 	// RootOptions is OPTIONAL on server side. This field only needs to be set if
 	// mutual authentication is required(RequireClientCert is true).
 	RootOptions RootCertificateOptions
-	// If the server want the client to send certificates.
+	// If the server requires the client to send certificates. This value is only
+	// relevant when configuring options for the server. Is not used for
+	// client-side configuration.
 	RequireClientCert bool
-	// VerificationType defines what type of client verification is done. See
+	// VerificationType defines what type of peer verification is done. See
 	// the `VerificationType` enum for the different options.
 	// Default: CertAndHostVerification
 	VerificationType VerificationType
@@ -295,9 +249,14 @@ type ServerOptions struct {
 	// which is currently TLS 1.3.  This default may be changed over time
 	// affecting backwards compatibility.
 	MaxTLSVersion uint16
+	// serverNameOverride is for testing only and only relevant on the client
+	// side. 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 verification.
+	serverNameOverride string
 }
 
-func (o *ClientOptions) config() (*tls.Config, error) {
+func (o *Options) clientConfig() (*tls.Config, error) {
 	// TODO(gtcooke94) Remove this block when o.VerifyPeer is remoed.
 	// VerifyPeer is deprecated, but do this to aid the transitory migration time.
 	if o.AdditionalPeerVerification == nil {
@@ -401,7 +360,7 @@ func (o *ClientOptions) config() (*tls.Config, error) {
 	return config, nil
 }
 
-func (o *ServerOptions) config() (*tls.Config, error) {
+func (o *Options) serverConfig() (*tls.Config, error) {
 	// TODO(gtcooke94) Remove this block when o.VerifyPeer is remoed.
 	// VerifyPeer is deprecated, but do this to aid the transitory migration time.
 	if o.AdditionalPeerVerification == nil {
@@ -696,8 +655,8 @@ func buildVerifyFunc(c *advancedTLSCreds,
 
 // NewClientCreds uses ClientOptions to construct a TransportCredentials based
 // on TLS.
-func NewClientCreds(o *ClientOptions) (credentials.TransportCredentials, error) {
-	conf, err := o.config()
+func NewClientCreds(o *Options) (credentials.TransportCredentials, error) {
+	conf, err := o.clientConfig()
 	if err != nil {
 		return nil, err
 	}
@@ -715,8 +674,8 @@ func NewClientCreds(o *ClientOptions) (credentials.TransportCredentials, error)
 
 // NewServerCreds uses ServerOptions to construct a TransportCredentials based
 // on TLS.
-func NewServerCreds(o *ServerOptions) (credentials.TransportCredentials, error) {
-	conf, err := o.config()
+func NewServerCreds(o *Options) (credentials.TransportCredentials, error) {
+	conf, err := o.serverConfig()
 	if err != nil {
 		return nil, err
 	}

security/advancedtls/advancedtls_integration_test.go

@@ -336,7 +336,7 @@ func (s) TestEnd2End(t *testing.T) {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
 			// Start a server using ServerOptions in another goroutine.
-			serverOptions := &ServerOptions{
+			serverOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					Certificates:                     test.serverCert,
 					GetIdentityCertificatesForServer: test.serverGetCert,
@@ -363,7 +363,7 @@ func (s) TestEnd2End(t *testing.T) {
 			addr := fmt.Sprintf("localhost:%v", lis.Addr().(*net.TCPAddr).Port)
 			pb.RegisterGreeterServer(s, greeterServer{})
 			go s.Serve(lis)
-			clientOptions := &ClientOptions{
+			clientOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					Certificates:                     test.clientCert,
 					GetIdentityCertificatesForClient: test.clientGetCert,
@@ -627,7 +627,7 @@ func (s) TestPEMFileProviderEnd2End(t *testing.T) {
 			defer serverIdentityProvider.Close()
 			defer serverRootProvider.Close()
 			// Start a server and create a client using advancedtls API with Provider.
-			serverOptions := &ServerOptions{
+			serverOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					IdentityProvider: serverIdentityProvider,
 				},
@@ -654,7 +654,7 @@ func (s) TestPEMFileProviderEnd2End(t *testing.T) {
 			addr := fmt.Sprintf("localhost:%v", lis.Addr().(*net.TCPAddr).Port)
 			pb.RegisterGreeterServer(s, greeterServer{})
 			go s.Serve(lis)
-			clientOptions := &ClientOptions{
+			clientOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					IdentityProvider: clientIdentityProvider,
 				},
@@ -764,7 +764,7 @@ func (s) TestDefaultHostNameCheck(t *testing.T) {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
 			// Start a server using ServerOptions in another goroutine.
-			serverOptions := &ServerOptions{
+			serverOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					Certificates: test.serverCert,
 				},
@@ -785,7 +785,7 @@ func (s) TestDefaultHostNameCheck(t *testing.T) {
 			addr := fmt.Sprintf("localhost:%v", lis.Addr().(*net.TCPAddr).Port)
 			pb.RegisterGreeterServer(s, greeterServer{})
 			go s.Serve(lis)
-			clientOptions := &ClientOptions{
+			clientOptions := &Options{
 				RootOptions: RootCertificateOptions{
 					RootCACerts: test.clientRoot,
 				},
@@ -902,7 +902,7 @@ func (s) TestTLSVersions(t *testing.T) {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
 			// Start a server using ServerOptions in another goroutine.
-			serverOptions := &ServerOptions{
+			serverOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					Certificates: []tls.Certificate{cs.ServerPeerLocalhost1},
 				},
@@ -925,7 +925,7 @@ func (s) TestTLSVersions(t *testing.T) {
 			addr := fmt.Sprintf("localhost:%v", lis.Addr().(*net.TCPAddr).Port)
 			pb.RegisterGreeterServer(s, greeterServer{})
 			go s.Serve(lis)
-			clientOptions := &ClientOptions{
+			clientOptions := &Options{
 				RootOptions: RootCertificateOptions{
 					RootCACerts: cs.ClientTrust1,
 				},

security/advancedtls/advancedtls_test.go

@@ -134,14 +134,14 @@ func (s) TestClientOptionsConfigErrorCases(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
-			clientOptions := &ClientOptions{
+			clientOptions := &Options{
 				VerificationType: test.clientVerificationType,
 				IdentityOptions:  test.IdentityOptions,
 				RootOptions:      test.RootOptions,
 				MinTLSVersion:    test.MinVersion,
 				MaxTLSVersion:    test.MaxVersion,
 			}
-			_, err := clientOptions.config()
+			_, err := clientOptions.clientConfig()
 			if err == nil {
 				t.Fatalf("ClientOptions{%v}.config() returns no err, wantErr != nil", clientOptions)
 			}
@@ -154,10 +154,10 @@ func (s) TestClientOptionsConfigErrorCases(t *testing.T) {
 // VerificationType. This should error because one cannot skip default
 // verification and provide no root credentials",
 func (s) TestClientOptionsWithDeprecatedVType(t *testing.T) {
-	clientOptions := &ClientOptions{
+	clientOptions := &Options{
 		VType: SkipVerification,
 	}
-	_, err := clientOptions.config()
+	_, err := clientOptions.clientConfig()
 	if err == nil {
 		t.Fatalf("ClientOptions{%v}.config() returns no err, wantErr != nil", clientOptions)
 	}
@@ -192,14 +192,14 @@ func (s) TestClientOptionsConfigSuccessCases(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
-			clientOptions := &ClientOptions{
+			clientOptions := &Options{
 				VerificationType: test.clientVerificationType,
 				IdentityOptions:  test.IdentityOptions,
 				RootOptions:      test.RootOptions,
 				MinTLSVersion:    test.MinVersion,
 				MaxTLSVersion:    test.MaxVersion,
 			}
-			clientConfig, err := clientOptions.config()
+			clientConfig, err := clientOptions.clientConfig()
 			if err != nil {
 				t.Fatalf("ClientOptions{%v}.config() = %v, wantErr == nil", clientOptions, err)
 			}
@@ -288,17 +288,17 @@ func (s) TestServerOptionsConfigErrorCases(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
-			serverOptions := &ServerOptions{
+			serverOptions := &Options{
 				VerificationType:  test.serverVerificationType,
 				RequireClientCert: test.requireClientCert,
 				IdentityOptions:   test.IdentityOptions,
 				RootOptions:       test.RootOptions,
 				MinTLSVersion:     test.MinVersion,
 				MaxTLSVersion:     test.MaxVersion,
 			}
-			_, err := serverOptions.config()
+			_, err := serverOptions.serverConfig()
 			if err == nil {
-				t.Fatalf("ServerOptions{%v}.config() returns no err, wantErr != nil", serverOptions)
+				t.Fatalf("ServerOptions{%v}.serverConfig() returns no err, wantErr != nil", serverOptions)
 			}
 		})
 	}
@@ -309,10 +309,10 @@ func (s) TestServerOptionsConfigErrorCases(t *testing.T) {
 // VerificationType. This should error because one cannot skip default
 // verification and provide no root credentials",
 func (s) TestServerOptionsWithDeprecatedVType(t *testing.T) {
-	serverOptions := &ServerOptions{
+	serverOptions := &Options{
 		VType: SkipVerification,
 	}
-	_, err := serverOptions.config()
+	_, err := serverOptions.serverConfig()
 	if err == nil {
 		t.Fatalf("ClientOptions{%v}.config() returns no err, wantErr != nil", serverOptions)
 	}
@@ -355,15 +355,15 @@ func (s) TestServerOptionsConfigSuccessCases(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
-			serverOptions := &ServerOptions{
+			serverOptions := &Options{
 				VerificationType:  test.serverVerificationType,
 				RequireClientCert: test.requireClientCert,
 				IdentityOptions:   test.IdentityOptions,
 				RootOptions:       test.RootOptions,
 				MinTLSVersion:     test.MinVersion,
 				MaxTLSVersion:     test.MaxVersion,
 			}
-			serverConfig, err := serverOptions.config()
+			serverConfig, err := serverOptions.serverConfig()
 			if err != nil {
 				t.Fatalf("ServerOptions{%v}.config() = %v, wantErr == nil", serverOptions, err)
 			}
@@ -829,7 +829,7 @@ func (s) TestClientServerHandshake(t *testing.T) {
 				t.Fatalf("Failed to listen: %v", err)
 			}
 			// Start a server using ServerOptions in another goroutine.
-			serverOptions := &ServerOptions{
+			serverOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					Certificates:                     test.serverCert,
 					GetIdentityCertificatesForServer: test.serverGetCert,
@@ -845,7 +845,7 @@ func (s) TestClientServerHandshake(t *testing.T) {
 				VerificationType:           test.serverVerificationType,
 				RevocationOptions:          test.serverRevocationOptions,
 			}
-			go func(done chan credentials.AuthInfo, lis net.Listener, serverOptions *ServerOptions) {
+			go func(done chan credentials.AuthInfo, lis net.Listener, serverOptions *Options) {
 				serverRawConn, err := lis.Accept()
 				if err != nil {
 					close(done)
@@ -873,7 +873,7 @@ func (s) TestClientServerHandshake(t *testing.T) {
 				t.Fatalf("Client failed to connect to %s. Error: %v", lisAddr, err)
 			}
 			defer conn.Close()
-			clientOptions := &ClientOptions{
+			clientOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					Certificates:                     test.clientCert,
 					GetIdentityCertificatesForClient: test.clientGetCert,
@@ -944,7 +944,7 @@ func (s) TestAdvancedTLSOverrideServerName(t *testing.T) {
 	if err := cs.LoadCerts(); err != nil {
 		t.Fatalf("cs.LoadCerts() failed, err: %v", err)
 	}
-	clientOptions := &ClientOptions{
+	clientOptions := &Options{
 		RootOptions: RootCertificateOptions{
 			RootCACerts: cs.ClientTrust1,
 		},
@@ -988,16 +988,16 @@ func (s) TestGetCertificatesSNI(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
-			serverOptions := &ServerOptions{
+			serverOptions := &Options{
 				IdentityOptions: IdentityCertificateOptions{
 					GetIdentityCertificatesForServer: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) {
 						return []*tls.Certificate{&cs.ServerCert1, &cs.ServerCert2, &cs.ServerPeer3}, nil
 					},
 				},
 			}
-			serverConfig, err := serverOptions.config()
+			serverConfig, err := serverOptions.serverConfig()
 			if err != nil {
-				t.Fatalf("serverOptions.config() failed: %v", err)
+				t.Fatalf("serverOptions.serverConfig() failed: %v", err)
 			}
 			pointFormatUncompressed := uint8(0)
 			clientHello := &tls.ClientHelloInfo{

security/advancedtls/sni.go

@@ -25,7 +25,7 @@ import (
 
 // buildGetCertificates returns the certificate that matches the SNI field
 // for the given ClientHelloInfo, defaulting to the first element of o.GetCertificates.
-func buildGetCertificates(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) {
+func buildGetCertificates(clientHello *tls.ClientHelloInfo, o *Options) (*tls.Certificate, error) {
 	if o.IdentityOptions.GetIdentityCertificatesForServer == nil {
 		return nil, fmt.Errorf("function GetCertificates must be specified")
 	}