Skip to content

advancedTLS: Combine ClientOptions and ServerOptions to just Options #7202

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

Merged
merged 3 commits into from
May 6, 2024
Merged

advancedTLS: Combine ClientOptions and ServerOptions to just Options #7202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5792085
rename to Options
gtcooke94 May 6, 2024
e5dae8c
added some documentation
gtcooke94 May 6, 2024
7ac5c04
typos
gtcooke94 May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 25 additions & 66 deletions security/advancedtls/advancedtls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions security/advancedtls/advancedtls_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
},
Expand All @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand All @@ -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,
},
Expand Down Expand Up @@ -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},
},
Expand All @@ -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,
},
Expand Down
40 changes: 20 additions & 20 deletions security/advancedtls/advancedtls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
})
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion security/advancedtls/sni.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down