From 0650ecb28cbed2ede9b968d60d3b3db9de3db2ea Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Mon, 15 Jun 2020 20:49:27 -0700 Subject: [PATCH 01/27] Add GetCertificateWithSNI func --- security/advancedtls/advancedtls.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index db78a566262..8c578ed2a12 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -230,6 +230,7 @@ type advancedTLSCreds struct { getRootCAs func(params *GetRootCAsParams) (*GetRootCAsResults, error) isClient bool vType VerificationType + doesSNI bool } func (c advancedTLSCreds) Info() credentials.ProtocolInfo { @@ -475,3 +476,25 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { } return cfg.Clone() } + +// GetCertificateWithSNI returns the certificate that matches the SNI field +// based on the given ClientHelloInfo. It should only be called when the +// client supplies SNI information. +func (c *advancedTLSCreds) GetCertificateWithSNI(clientHello tls.ClientHelloInfo) ([]tls.Certificate, error) { + if len(c.config.Certificates) == 0 { + return nil, fmt.Errorf("No certificates") + } + + if len(c.config.Certificates) == 1 || !c.doesSNI { + return c.config.Certificates, nil + } + + for _, cert := range c.config.Certificates { + if err := clientHello.SupportsCertificate(&cert); err == nil { + return []tls.Certificate{cert}, nil + } + } + + // If nothing matches, return the first certificate. + return c.config.Certificates[0:1], nil +} From c358cff66fe04d76d6281bf1d761d32f750ffa24 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Tue, 16 Jun 2020 10:11:48 -0700 Subject: [PATCH 02/27] Add comments in GetCertificateWithSNI --- security/advancedtls/advancedtls.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 8c578ed2a12..2aa7433b06b 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -25,6 +25,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "errors" "fmt" "net" "syscall" @@ -477,18 +478,22 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { return cfg.Clone() } +var errNoCertificates = errors.New("No certificates configured") + // GetCertificateWithSNI returns the certificate that matches the SNI field // based on the given ClientHelloInfo. It should only be called when the // client supplies SNI information. func (c *advancedTLSCreds) GetCertificateWithSNI(clientHello tls.ClientHelloInfo) ([]tls.Certificate, error) { if len(c.config.Certificates) == 0 { - return nil, fmt.Errorf("No certificates") + return nil, errNoCertificates } if len(c.config.Certificates) == 1 || !c.doesSNI { + // Return a list of certificates return c.config.Certificates, nil } + // Choose the best certificate if users set doesSNI for _, cert := range c.config.Certificates { if err := clientHello.SupportsCertificate(&cert); err == nil { return []tls.Certificate{cert}, nil From 1f0088b3f046a8fae3ef68303abb9d1de6a16da6 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Tue, 16 Jun 2020 10:23:55 -0700 Subject: [PATCH 03/27] Modify comments in GetCertificateWithSNI --- security/advancedtls/advancedtls.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 2aa7433b06b..f66b825483f 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -481,8 +481,8 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { var errNoCertificates = errors.New("No certificates configured") // GetCertificateWithSNI returns the certificate that matches the SNI field -// based on the given ClientHelloInfo. It should only be called when the -// client supplies SNI information. +// based on the given ClientHelloInfo if doesSNI is set to True +// Otherwise it returns the list of Certificates func (c *advancedTLSCreds) GetCertificateWithSNI(clientHello tls.ClientHelloInfo) ([]tls.Certificate, error) { if len(c.config.Certificates) == 0 { return nil, errNoCertificates From edada75c5213589cb308aaa417d5c9f84534506e Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Wed, 17 Jun 2020 22:16:53 -0700 Subject: [PATCH 04/27] Restructure SNI --- security/advancedtls/advancedtls.go | 40 +++++++++++++++-------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index f66b825483f..70070e1f87c 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -161,11 +161,11 @@ type ServerOptions struct { // The server will use Certificates every time when asked for a certificate, // without performing certificate reloading. Certificates []tls.Certificate - // If GetClientCertificate is set and Certificates is nil, the server will + // If GetClientCertificate is set and Certificates is nil, the client will // invoke this function every time asked to present certificates to the - // client when a new connection is established. This is known as peer + // server when a new connection is established. This is known as peer // certificate reloading. - GetCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error) + GetCertificate func(tls.ClientHelloInfo) ([]tls.Certificate, error) // VerifyPeer is a custom verification check after certificate signature // check. // If this is set, we will perform this customized check after doing the @@ -212,10 +212,13 @@ func (o *ServerOptions) config() (*tls.Config, error) { // buildVerifyFunc. clientAuth = tls.RequireAnyClientCert } + getCertificateWithSNI := func(clientHelloInfo *tls.ClientHelloInfo) (*tls.Certificate, error) { + return o.GetCertificateWithSNI(clientHelloInfo) + } config := &tls.Config{ ClientAuth: clientAuth, Certificates: o.Certificates, - GetCertificate: o.GetCertificate, + GetCertificate: getCertificateWithSNI, } if o.RootCACerts != nil { config.ClientCAs = o.RootCACerts @@ -231,7 +234,6 @@ type advancedTLSCreds struct { getRootCAs func(params *GetRootCAsParams) (*GetRootCAsResults, error) isClient bool vType VerificationType - doesSNI bool } func (c advancedTLSCreds) Info() credentials.ProtocolInfo { @@ -481,25 +483,25 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { var errNoCertificates = errors.New("No certificates configured") // GetCertificateWithSNI returns the certificate that matches the SNI field -// based on the given ClientHelloInfo if doesSNI is set to True -// Otherwise it returns the list of Certificates -func (c *advancedTLSCreds) GetCertificateWithSNI(clientHello tls.ClientHelloInfo) ([]tls.Certificate, error) { - if len(c.config.Certificates) == 0 { +// based on the given ClientHelloInfo if GetCertificate returns a list of +// certificates +func (o *ServerOptions) GetCertificateWithSNI(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { + certificates, err := o.GetCertificate(*clientHello) + if err != nil { + return nil, err + } + if len(certificates) == 0 { return nil, errNoCertificates } - - if len(c.config.Certificates) == 1 || !c.doesSNI { - // Return a list of certificates - return c.config.Certificates, nil + // If users pass in only one certificate, return that certificate + if len(certificates) == 1 { + return &certificates[0], nil } - - // Choose the best certificate if users set doesSNI - for _, cert := range c.config.Certificates { + for _, cert := range certificates { if err := clientHello.SupportsCertificate(&cert); err == nil { - return []tls.Certificate{cert}, nil + return &cert, nil } } - // If nothing matches, return the first certificate. - return c.config.Certificates[0:1], nil + return &certificates[0], nil } From cda3e2dc1a35938f39b34ad3f5370bcfa3e07239 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Wed, 17 Jun 2020 22:25:49 -0700 Subject: [PATCH 05/27] Modified comments --- security/advancedtls/advancedtls.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 70070e1f87c..9988ffac086 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -25,7 +25,7 @@ import ( "context" "crypto/tls" "crypto/x509" - "errors" + "fmt" "net" "syscall" @@ -161,9 +161,9 @@ type ServerOptions struct { // The server will use Certificates every time when asked for a certificate, // without performing certificate reloading. Certificates []tls.Certificate - // If GetClientCertificate is set and Certificates is nil, the client will + // If GetClientCertificate is set and Certificates is nil, the server will // invoke this function every time asked to present certificates to the - // server when a new connection is established. This is known as peer + // client when a new connection is established. This is known as peer // certificate reloading. GetCertificate func(tls.ClientHelloInfo) ([]tls.Certificate, error) // VerifyPeer is a custom verification check after certificate signature @@ -480,8 +480,6 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { return cfg.Clone() } -var errNoCertificates = errors.New("No certificates configured") - // GetCertificateWithSNI returns the certificate that matches the SNI field // based on the given ClientHelloInfo if GetCertificate returns a list of // certificates @@ -491,7 +489,7 @@ func (o *ServerOptions) GetCertificateWithSNI(clientHello *tls.ClientHelloInfo) return nil, err } if len(certificates) == 0 { - return nil, errNoCertificates + return nil, fmt.Errorf("No certificates configured") } // If users pass in only one certificate, return that certificate if len(certificates) == 1 { From 40eadfcb0ab95ebe9a535f070d532f43085bdc8a Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Wed, 17 Jun 2020 22:34:15 -0700 Subject: [PATCH 06/27] Fixed arguments name --- security/advancedtls/advancedtls.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 9988ffac086..925e92d52d8 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -212,8 +212,8 @@ func (o *ServerOptions) config() (*tls.Config, error) { // buildVerifyFunc. clientAuth = tls.RequireAnyClientCert } - getCertificateWithSNI := func(clientHelloInfo *tls.ClientHelloInfo) (*tls.Certificate, error) { - return o.GetCertificateWithSNI(clientHelloInfo) + getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { + return o.GetCertificateWithSNI(clientHello) } config := &tls.Config{ ClientAuth: clientAuth, From 9e74bb776e6b22efb0c10c262cfcedd0d6744f0c Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Thu, 18 Jun 2020 12:04:48 -0700 Subject: [PATCH 07/27] Review Comments #1 --- security/advancedtls/advancedtls.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 925e92d52d8..81152a1c0b4 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -25,7 +25,6 @@ import ( "context" "crypto/tls" "crypto/x509" - "fmt" "net" "syscall" @@ -165,7 +164,7 @@ type ServerOptions struct { // invoke this function every time asked to present certificates to the // client when a new connection is established. This is known as peer // certificate reloading. - GetCertificate func(tls.ClientHelloInfo) ([]tls.Certificate, error) + GetCertificate func(*tls.ClientHelloInfo) (*[]tls.Certificate, error) // VerifyPeer is a custom verification check after certificate signature // check. // If this is set, we will perform this customized check after doing the @@ -213,7 +212,7 @@ func (o *ServerOptions) config() (*tls.Config, error) { clientAuth = tls.RequireAnyClientCert } getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { - return o.GetCertificateWithSNI(clientHello) + return o.getCertificateWithSNI(clientHello) } config := &tls.Config{ ClientAuth: clientAuth, @@ -480,26 +479,27 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { return cfg.Clone() } -// GetCertificateWithSNI returns the certificate that matches the SNI field -// based on the given ClientHelloInfo if GetCertificate returns a list of -// certificates -func (o *ServerOptions) GetCertificateWithSNI(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { - certificates, err := o.GetCertificate(*clientHello) +// GetCertificateWithSNI is a helper function that returns the certificate +// that matches the SNI field based on the given ClientHelloInfo +// if GetCertificate returns a list of certificates +func (o *ServerOptions) getCertificateWithSNI(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { + certificates, err := o.GetCertificate(clientHello) if err != nil { return nil, err } - if len(certificates) == 0 { + if len(*certificates) == 0 { return nil, fmt.Errorf("No certificates configured") } // If users pass in only one certificate, return that certificate - if len(certificates) == 1 { - return &certificates[0], nil + if len(*certificates) == 1 { + return &(*certificates)[0], nil } - for _, cert := range certificates { + // Choose the SNI certificate using SupportsCertificate + for _, cert := range *certificates { if err := clientHello.SupportsCertificate(&cert); err == nil { return &cert, nil } } // If nothing matches, return the first certificate. - return &certificates[0], nil + return &(*certificates)[0], nil } From 4adb63c352bfdae45f390aa495b5b405fe34f034 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Thu, 18 Jun 2020 15:23:17 -0700 Subject: [PATCH 08/27] Review comments #2 --- security/advancedtls/advancedtls.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 81152a1c0b4..f552ab7f692 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -164,7 +164,7 @@ type ServerOptions struct { // invoke this function every time asked to present certificates to the // client when a new connection is established. This is known as peer // certificate reloading. - GetCertificate func(*tls.ClientHelloInfo) (*[]tls.Certificate, error) + GetCertificate func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) // VerifyPeer is a custom verification check after certificate signature // check. // If this is set, we will perform this customized check after doing the @@ -212,7 +212,7 @@ func (o *ServerOptions) config() (*tls.Config, error) { clientAuth = tls.RequireAnyClientCert } getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { - return o.getCertificateWithSNI(clientHello) + return buildGetCertificateFunc(clientHello, o) } config := &tls.Config{ ClientAuth: clientAuth, @@ -479,27 +479,27 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { return cfg.Clone() } -// GetCertificateWithSNI is a helper function that returns the certificate +// buildGetCertificateFunc is a helper function that returns the certificate // that matches the SNI field based on the given ClientHelloInfo // if GetCertificate returns a list of certificates -func (o *ServerOptions) getCertificateWithSNI(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { +func buildGetCertificateFunc(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { certificates, err := o.GetCertificate(clientHello) if err != nil { return nil, err } - if len(*certificates) == 0 { - return nil, fmt.Errorf("No certificates configured") + if len(certificates) == 0 { + return nil, fmt.Errorf("no certificates configured") } // If users pass in only one certificate, return that certificate - if len(*certificates) == 1 { - return &(*certificates)[0], nil + if len(certificates) == 1 { + return certificates[0], nil } // Choose the SNI certificate using SupportsCertificate - for _, cert := range *certificates { - if err := clientHello.SupportsCertificate(&cert); err == nil { - return &cert, nil + for _, cert := range certificates { + if err := clientHello.SupportsCertificate(cert); err == nil { + return cert, nil } } // If nothing matches, return the first certificate. - return &(*certificates)[0], nil + return certificates[0], nil } From 62c0f229a3ad010c91b32fe4dba63360dceebbd7 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Sat, 20 Jun 2020 12:17:24 -0700 Subject: [PATCH 09/27] Made tests compiled after changing the func signature --- security/advancedtls/advancedtls.go | 3 ++ .../advancedtls_integration_test.go | 18 +++++---- security/advancedtls/advancedtls_test.go | 38 +++++++++++++------ 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index f552ab7f692..4bdffc61165 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -483,6 +483,9 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { // that matches the SNI field based on the given ClientHelloInfo // if GetCertificate returns a list of certificates func buildGetCertificateFunc(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { + if o.GetCertificate == nil { + return nil, fmt.Errorf("function GetCertificate must be specified") + } certificates, err := o.GetCertificate(clientHello) if err != nil { return nil, err diff --git a/security/advancedtls/advancedtls_integration_test.go b/security/advancedtls/advancedtls_integration_test.go index 5f8c25f681b..1ebed0ce7f1 100644 --- a/security/advancedtls/advancedtls_integration_test.go +++ b/security/advancedtls/advancedtls_integration_test.go @@ -197,7 +197,7 @@ func TestEnd2End(t *testing.T) { clientVerifyFunc CustomVerificationFunc clientVType VerificationType serverCert []tls.Certificate - serverGetCert func(*tls.ClientHelloInfo) (*tls.Certificate, error) + serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) serverRoot *x509.CertPool serverGetRoot func(params *GetRootCAsParams) (*GetRootCAsResults, error) serverVerifyFunc CustomVerificationFunc @@ -271,12 +271,14 @@ func TestEnd2End(t *testing.T) { return &VerificationResults{}, nil }, clientVType: CertVerification, - serverGetCert: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + serverGetCert: func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) { switch stage.read() { case 0: - return &cs.serverPeer1, nil + // return &cs.serverPeer1, nil + return []*tls.Certificate{&cs.serverPeer1}, nil default: - return &cs.serverPeer2, nil + // return &cs.serverPeer2, nil + return []*tls.Certificate{&cs.serverPeer2}, nil } }, serverRoot: cs.serverTrust1, @@ -336,12 +338,14 @@ func TestEnd2End(t *testing.T) { return nil, fmt.Errorf("custom authz check fails") }, clientVType: CertVerification, - serverGetCert: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + serverGetCert: func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) { switch stage.read() { case 0: - return &cs.serverPeer1, nil + // return &cs.serverPeer1, nil + return []*tls.Certificate{&cs.serverPeer1}, nil default: - return &cs.serverPeer2, nil + // return &cs.serverPeer2, nil + return []*tls.Certificate{&cs.serverPeer2}, nil } }, serverRoot: cs.serverTrust1, diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index ab6ba59068a..1bcbba8f002 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -102,13 +102,20 @@ func TestClientServerHandshake(t *testing.T) { clientExpectHandshakeError bool serverMutualTLS bool serverCert []tls.Certificate - serverGetCert func(*tls.ClientHelloInfo) (*tls.Certificate, error) + serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) serverRoot *x509.CertPool serverGetRoot func(params *GetRootCAsParams) (*GetRootCAsResults, error) serverVerifyFunc CustomVerificationFunc serverVType VerificationType serverExpectError bool }{ + { + desc: "Client has no trust cert with verifyFuncGood; server sends peer cert", + clientVerifyFunc: clientVerifyFuncGood, + clientVType: SkipVerification, + serverCert: []tls.Certificate{serverPeerCert}, + serverVType: CertAndHostVerification, + }, // Client: nil setting // Server: only set serverCert with mutual TLS off // Expected Behavior: server side failure @@ -129,6 +136,7 @@ func TestClientServerHandshake(t *testing.T) { // Expected Behavior: success // Reason: we will use verifyFuncGood to verify the server, // if either clientCert or clientGetCert is not set + // ERROR { desc: "Client has no trust cert with verifyFuncGood; server sends peer cert", clientVerifyFunc: clientVerifyFuncGood, @@ -169,6 +177,7 @@ func TestClientServerHandshake(t *testing.T) { // Client: set clientGetRoot and clientVerifyFunc // Server: only set serverCert with mutual TLS off // Expected Behavior: success + // ERROR { desc: "Client sets reload root function with verifyFuncGood; server sends peer cert", clientGetRoot: getRootCAsForClient, @@ -206,6 +215,7 @@ func TestClientServerHandshake(t *testing.T) { // Client: set clientGetRoot, clientVerifyFunc and clientCert // Server: set serverRoot and serverCert with mutual TLS on // Expected Behavior: success + // ERROR { desc: "Client sets peer cert, reload root function with verifyFuncGood; server sets peer cert and root cert; mutualTLS", clientCert: []tls.Certificate{clientPeerCert}, @@ -239,6 +249,7 @@ func TestClientServerHandshake(t *testing.T) { // Client: set clientGetRoot, clientVerifyFunc and clientCert // Server: set serverGetRoot and serverCert with mutual TLS on // Expected Behavior: success + // ERROR { desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", clientCert: []tls.Certificate{clientPeerCert}, @@ -279,8 +290,9 @@ func TestClientServerHandshake(t *testing.T) { clientVerifyFunc: clientVerifyFuncGood, clientVType: CertVerification, serverMutualTLS: true, - serverGetCert: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - return &serverPeerCert, nil + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + // return &serverPeerCert, nil + return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForServer, serverVerifyFunc: serverVerifyFunc, @@ -300,8 +312,9 @@ func TestClientServerHandshake(t *testing.T) { clientVerifyFunc: clientVerifyFuncGood, clientVType: CertVerification, serverMutualTLS: true, - serverGetCert: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - return &serverPeerCert, nil + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + // return &serverPeerCert, nil + return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForServer, serverVerifyFunc: serverVerifyFunc, @@ -322,8 +335,9 @@ func TestClientServerHandshake(t *testing.T) { clientVType: CertVerification, clientExpectHandshakeError: true, serverMutualTLS: true, - serverGetCert: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - return &serverPeerCert, nil + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + // return &serverPeerCert, nil + return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForServer, serverVerifyFunc: serverVerifyFunc, @@ -344,8 +358,9 @@ func TestClientServerHandshake(t *testing.T) { clientVerifyFunc: clientVerifyFuncGood, clientVType: CertVerification, serverMutualTLS: true, - serverGetCert: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - return &clientPeerCert, nil + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + // return &clientPeerCert, nil + return []*tls.Certificate{&clientPeerCert}, nil }, serverGetRoot: getRootCAsForServer, serverVerifyFunc: serverVerifyFunc, @@ -366,8 +381,9 @@ func TestClientServerHandshake(t *testing.T) { clientVType: CertVerification, clientExpectHandshakeError: true, serverMutualTLS: true, - serverGetCert: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - return &serverPeerCert, nil + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + // return &serverPeerCert, nil + return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForClient, serverVerifyFunc: serverVerifyFunc, From d2343cc38008a20c928ae0516d633815159d3365 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Sat, 20 Jun 2020 22:48:22 -0700 Subject: [PATCH 10/27] Cleaned up comments --- security/advancedtls/advancedtls_test.go | 85 +++++++++++------------- 1 file changed, 37 insertions(+), 48 deletions(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index 1bcbba8f002..d29e715d593 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -109,13 +109,6 @@ func TestClientServerHandshake(t *testing.T) { serverVType VerificationType serverExpectError bool }{ - { - desc: "Client has no trust cert with verifyFuncGood; server sends peer cert", - clientVerifyFunc: clientVerifyFuncGood, - clientVType: SkipVerification, - serverCert: []tls.Certificate{serverPeerCert}, - serverVType: CertAndHostVerification, - }, // Client: nil setting // Server: only set serverCert with mutual TLS off // Expected Behavior: server side failure @@ -137,13 +130,13 @@ func TestClientServerHandshake(t *testing.T) { // Reason: we will use verifyFuncGood to verify the server, // if either clientCert or clientGetCert is not set // ERROR - { - desc: "Client has no trust cert with verifyFuncGood; server sends peer cert", - clientVerifyFunc: clientVerifyFuncGood, - clientVType: SkipVerification, - serverCert: []tls.Certificate{serverPeerCert}, - serverVType: CertAndHostVerification, - }, + // { + // desc: "Client has no trust cert with verifyFuncGood; server sends peer cert", + // clientVerifyFunc: clientVerifyFuncGood, + // clientVType: SkipVerification, + // serverCert: []tls.Certificate{serverPeerCert}, + // serverVType: CertAndHostVerification, + // }, // Client: only set clientRoot // Server: only set serverCert with mutual TLS off // Expected Behavior: server side failure and client handshake failure @@ -178,14 +171,14 @@ func TestClientServerHandshake(t *testing.T) { // Server: only set serverCert with mutual TLS off // Expected Behavior: success // ERROR - { - desc: "Client sets reload root function with verifyFuncGood; server sends peer cert", - clientGetRoot: getRootCAsForClient, - clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, - serverCert: []tls.Certificate{serverPeerCert}, - serverVType: CertAndHostVerification, - }, + // { + // desc: "Client sets reload root function with verifyFuncGood; server sends peer cert", + // clientGetRoot: getRootCAsForClient, + // clientVerifyFunc: clientVerifyFuncGood, + // clientVType: CertVerification, + // serverCert: []tls.Certificate{serverPeerCert}, + // serverVType: CertAndHostVerification, + // }, // Client: set clientGetRoot and bad clientVerifyFunc function // Server: only set serverCert with mutual TLS off // Expected Behavior: server side failure and client handshake failure @@ -216,17 +209,17 @@ func TestClientServerHandshake(t *testing.T) { // Server: set serverRoot and serverCert with mutual TLS on // Expected Behavior: success // ERROR - { - desc: "Client sets peer cert, reload root function with verifyFuncGood; server sets peer cert and root cert; mutualTLS", - clientCert: []tls.Certificate{clientPeerCert}, - clientGetRoot: getRootCAsForClient, - clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, - serverMutualTLS: true, - serverCert: []tls.Certificate{serverPeerCert}, - serverRoot: serverTrustPool, - serverVType: CertVerification, - }, + // { + // desc: "Client sets peer cert, reload root function with verifyFuncGood; server sets peer cert and root cert; mutualTLS", + // clientCert: []tls.Certificate{clientPeerCert}, + // clientGetRoot: getRootCAsForClient, + // clientVerifyFunc: clientVerifyFuncGood, + // clientVType: CertVerification, + // serverMutualTLS: true, + // serverCert: []tls.Certificate{serverPeerCert}, + // serverRoot: serverTrustPool, + // serverVType: CertVerification, + // }, // Client: set clientGetRoot, clientVerifyFunc and clientCert // Server: set serverCert, but not setting any of serverRoot, serverGetRoot // or serverVerifyFunc, with mutual TLS on @@ -250,17 +243,17 @@ func TestClientServerHandshake(t *testing.T) { // Server: set serverGetRoot and serverCert with mutual TLS on // Expected Behavior: success // ERROR - { - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", - clientCert: []tls.Certificate{clientPeerCert}, - clientGetRoot: getRootCAsForClient, - clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, - serverMutualTLS: true, - serverCert: []tls.Certificate{serverPeerCert}, - serverGetRoot: getRootCAsForServer, - serverVType: CertVerification, - }, + // { + // desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", + // clientCert: []tls.Certificate{clientPeerCert}, + // clientGetRoot: getRootCAsForClient, + // clientVerifyFunc: clientVerifyFuncGood, + // clientVType: CertVerification, + // serverMutualTLS: true, + // serverCert: []tls.Certificate{serverPeerCert}, + // serverGetRoot: getRootCAsForServer, + // serverVType: CertVerification, + // }, // Client: set clientGetRoot, clientVerifyFunc and clientCert // Server: set serverGetRoot returning error and serverCert with mutual // TLS on @@ -291,7 +284,6 @@ func TestClientServerHandshake(t *testing.T) { clientVType: CertVerification, serverMutualTLS: true, serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - // return &serverPeerCert, nil return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForServer, @@ -313,7 +305,6 @@ func TestClientServerHandshake(t *testing.T) { clientVType: CertVerification, serverMutualTLS: true, serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - // return &serverPeerCert, nil return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForServer, @@ -336,7 +327,6 @@ func TestClientServerHandshake(t *testing.T) { clientExpectHandshakeError: true, serverMutualTLS: true, serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - // return &serverPeerCert, nil return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForServer, @@ -359,7 +349,6 @@ func TestClientServerHandshake(t *testing.T) { clientVType: CertVerification, serverMutualTLS: true, serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - // return &clientPeerCert, nil return []*tls.Certificate{&clientPeerCert}, nil }, serverGetRoot: getRootCAsForServer, From 292f2272f99fe9eb09a55c0a3a761b9a4eee8677 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Sun, 21 Jun 2020 17:09:59 -0700 Subject: [PATCH 11/27] Fixed advancedtls unit test --- security/advancedtls/advancedtls.go | 14 +++-- security/advancedtls/advancedtls_test.go | 78 +++++++++++------------- 2 files changed, 45 insertions(+), 47 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 4bdffc61165..0a2fe5a4be7 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -211,13 +211,15 @@ func (o *ServerOptions) config() (*tls.Config, error) { // buildVerifyFunc. clientAuth = tls.RequireAnyClientCert } - getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { - return buildGetCertificateFunc(clientHello, o) - } config := &tls.Config{ - ClientAuth: clientAuth, - Certificates: o.Certificates, - GetCertificate: getCertificateWithSNI, + ClientAuth: clientAuth, + Certificates: o.Certificates, + } + if o.GetCertificate != nil { + getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { + return buildGetCertificateFunc(clientHello, o) + } + config.GetCertificate = getCertificateWithSNI } if o.RootCACerts != nil { config.ClientCAs = o.RootCACerts diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index d29e715d593..c9d422a7411 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -129,14 +129,13 @@ func TestClientServerHandshake(t *testing.T) { // Expected Behavior: success // Reason: we will use verifyFuncGood to verify the server, // if either clientCert or clientGetCert is not set - // ERROR - // { - // desc: "Client has no trust cert with verifyFuncGood; server sends peer cert", - // clientVerifyFunc: clientVerifyFuncGood, - // clientVType: SkipVerification, - // serverCert: []tls.Certificate{serverPeerCert}, - // serverVType: CertAndHostVerification, - // }, + { + desc: "Client has no trust cert with verifyFuncGood; server sends peer cert", + clientVerifyFunc: clientVerifyFuncGood, + clientVType: SkipVerification, + serverCert: []tls.Certificate{serverPeerCert}, + serverVType: CertAndHostVerification, + }, // Client: only set clientRoot // Server: only set serverCert with mutual TLS off // Expected Behavior: server side failure and client handshake failure @@ -170,15 +169,14 @@ func TestClientServerHandshake(t *testing.T) { // Client: set clientGetRoot and clientVerifyFunc // Server: only set serverCert with mutual TLS off // Expected Behavior: success - // ERROR - // { - // desc: "Client sets reload root function with verifyFuncGood; server sends peer cert", - // clientGetRoot: getRootCAsForClient, - // clientVerifyFunc: clientVerifyFuncGood, - // clientVType: CertVerification, - // serverCert: []tls.Certificate{serverPeerCert}, - // serverVType: CertAndHostVerification, - // }, + { + desc: "Client sets reload root function with verifyFuncGood; server sends peer cert", + clientGetRoot: getRootCAsForClient, + clientVerifyFunc: clientVerifyFuncGood, + clientVType: CertVerification, + serverCert: []tls.Certificate{serverPeerCert}, + serverVType: CertAndHostVerification, + }, // Client: set clientGetRoot and bad clientVerifyFunc function // Server: only set serverCert with mutual TLS off // Expected Behavior: server side failure and client handshake failure @@ -208,18 +206,17 @@ func TestClientServerHandshake(t *testing.T) { // Client: set clientGetRoot, clientVerifyFunc and clientCert // Server: set serverRoot and serverCert with mutual TLS on // Expected Behavior: success - // ERROR - // { - // desc: "Client sets peer cert, reload root function with verifyFuncGood; server sets peer cert and root cert; mutualTLS", - // clientCert: []tls.Certificate{clientPeerCert}, - // clientGetRoot: getRootCAsForClient, - // clientVerifyFunc: clientVerifyFuncGood, - // clientVType: CertVerification, - // serverMutualTLS: true, - // serverCert: []tls.Certificate{serverPeerCert}, - // serverRoot: serverTrustPool, - // serverVType: CertVerification, - // }, + { + desc: "Client sets peer cert, reload root function with verifyFuncGood; server sets peer cert and root cert; mutualTLS", + clientCert: []tls.Certificate{clientPeerCert}, + clientGetRoot: getRootCAsForClient, + clientVerifyFunc: clientVerifyFuncGood, + clientVType: CertVerification, + serverMutualTLS: true, + serverCert: []tls.Certificate{serverPeerCert}, + serverRoot: serverTrustPool, + serverVType: CertVerification, + }, // Client: set clientGetRoot, clientVerifyFunc and clientCert // Server: set serverCert, but not setting any of serverRoot, serverGetRoot // or serverVerifyFunc, with mutual TLS on @@ -242,18 +239,17 @@ func TestClientServerHandshake(t *testing.T) { // Client: set clientGetRoot, clientVerifyFunc and clientCert // Server: set serverGetRoot and serverCert with mutual TLS on // Expected Behavior: success - // ERROR - // { - // desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", - // clientCert: []tls.Certificate{clientPeerCert}, - // clientGetRoot: getRootCAsForClient, - // clientVerifyFunc: clientVerifyFuncGood, - // clientVType: CertVerification, - // serverMutualTLS: true, - // serverCert: []tls.Certificate{serverPeerCert}, - // serverGetRoot: getRootCAsForServer, - // serverVType: CertVerification, - // }, + { + desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", + clientCert: []tls.Certificate{clientPeerCert}, + clientGetRoot: getRootCAsForClient, + clientVerifyFunc: clientVerifyFuncGood, + clientVType: CertVerification, + serverMutualTLS: true, + serverCert: []tls.Certificate{serverPeerCert}, + serverGetRoot: getRootCAsForServer, + serverVType: CertVerification, + }, // Client: set clientGetRoot, clientVerifyFunc and clientCert // Server: set serverGetRoot returning error and serverCert with mutual // TLS on From 7eeb1d6bea156823a3a5ab63073952e97383a3d0 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Mon, 22 Jun 2020 09:17:21 -0700 Subject: [PATCH 12/27] Clean up comments --- security/advancedtls/advancedtls_integration_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/security/advancedtls/advancedtls_integration_test.go b/security/advancedtls/advancedtls_integration_test.go index 1ebed0ce7f1..69d7952af04 100644 --- a/security/advancedtls/advancedtls_integration_test.go +++ b/security/advancedtls/advancedtls_integration_test.go @@ -274,10 +274,8 @@ func TestEnd2End(t *testing.T) { serverGetCert: func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) { switch stage.read() { case 0: - // return &cs.serverPeer1, nil return []*tls.Certificate{&cs.serverPeer1}, nil default: - // return &cs.serverPeer2, nil return []*tls.Certificate{&cs.serverPeer2}, nil } }, @@ -341,10 +339,8 @@ func TestEnd2End(t *testing.T) { serverGetCert: func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) { switch stage.read() { case 0: - // return &cs.serverPeer1, nil return []*tls.Certificate{&cs.serverPeer1}, nil default: - // return &cs.serverPeer2, nil return []*tls.Certificate{&cs.serverPeer2}, nil } }, From b76410b33dcc4c4215e6346a28a3ba6451e48c26 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Mon, 22 Jun 2020 09:18:59 -0700 Subject: [PATCH 13/27] Clean up comments --- security/advancedtls/advancedtls_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index c9d422a7411..572a44182f5 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -367,7 +367,6 @@ func TestClientServerHandshake(t *testing.T) { clientExpectHandshakeError: true, serverMutualTLS: true, serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - // return &serverPeerCert, nil return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForClient, From 3519dc6502340090d530dc90af9d558dd644a9e5 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Mon, 29 Jun 2020 22:03:31 -0700 Subject: [PATCH 14/27] Added unit test for getCertificateWithSNI --- .../advancedtls_integration_test.go | 4 - security/advancedtls/advancedtls_test.go | 103 +++++++++++++++++- .../advancedtls/testdata/server_cert_3.pem | 22 ++++ .../advancedtls/testdata/server_key_3.pem | 28 +++++ 4 files changed, 152 insertions(+), 5 deletions(-) create mode 100644 security/advancedtls/testdata/server_cert_3.pem create mode 100644 security/advancedtls/testdata/server_key_3.pem diff --git a/security/advancedtls/advancedtls_integration_test.go b/security/advancedtls/advancedtls_integration_test.go index 1ebed0ce7f1..69d7952af04 100644 --- a/security/advancedtls/advancedtls_integration_test.go +++ b/security/advancedtls/advancedtls_integration_test.go @@ -274,10 +274,8 @@ func TestEnd2End(t *testing.T) { serverGetCert: func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) { switch stage.read() { case 0: - // return &cs.serverPeer1, nil return []*tls.Certificate{&cs.serverPeer1}, nil default: - // return &cs.serverPeer2, nil return []*tls.Certificate{&cs.serverPeer2}, nil } }, @@ -341,10 +339,8 @@ func TestEnd2End(t *testing.T) { serverGetCert: func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) { switch stage.read() { case 0: - // return &cs.serverPeer1, nil return []*tls.Certificate{&cs.serverPeer1}, nil default: - // return &cs.serverPeer2, nil return []*tls.Certificate{&cs.serverPeer2}, nil } }, diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index c9d422a7411..65b2dc388fa 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -367,7 +367,6 @@ func TestClientServerHandshake(t *testing.T) { clientExpectHandshakeError: true, serverMutualTLS: true, serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - // return &serverPeerCert, nil return []*tls.Certificate{&serverPeerCert}, nil }, serverGetRoot: getRootCAsForClient, @@ -624,3 +623,105 @@ func TestWrapSyscallConn(t *testing.T) { wrapConn) } } + +const ( + TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA uint16 = 0xc012 +) + +const ( + CurveP256 tls.CurveID = 23 +) + +const ( + pointFormatUncompressed uint8 = 0 +) + +const ( + VersionTLS10 uint16 = 0x0301 +) + +func TestGetCertificateSNI(t *testing.T) { + serverPeerCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), + testdata.Path("server_key_1.pem")) + if err != nil { + t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + } + serverPeerCert2, err := tls.LoadX509KeyPair(testdata.Path("server_cert_2.pem"), + testdata.Path("server_key_2.pem")) + if err != nil { + t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + } + serverPeerCert3, err := tls.LoadX509KeyPair(testdata.Path("server_cert_3.pem"), + testdata.Path("server_key_3.pem")) + if err != nil { + t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + } + tests := []struct { + desc string + serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) + }{ + { + desc: "Select the certificate that matches the server name provided in clientHello", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil + }, + }, + } + for _, test := range tests { + test := test + t.Run(test.desc, func(t *testing.T) { + serverOptions := &ServerOptions{ + GetCertificate: test.serverGetCert, + } + serverConfig, err := serverOptions.config() + if err != nil { + t.Fatalf("Unable to generate serverConfig. Error: %v", err) + } + // "foo.bar.com" is the common name on server certificate server_cert_1.pem. + clientHello1 := &tls.ClientHelloInfo{ + CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA}, + ServerName: "foo.bar.com", + SupportedCurves: []tls.CurveID{CurveP256}, + SupportedPoints: []uint8{pointFormatUncompressed}, + SupportedVersions: []uint16{VersionTLS10}, + } + got1, err := serverConfig.GetCertificate(clientHello1) + if err != nil { + t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + } + if !reflect.DeepEqual(*got1, serverPeerCert1) { + t.Errorf("GetCertificate() = %v, want %v", got1, serverPeerCert1) + } + // "foo.bar.server2.com" is the common name on server certificate server_cert_2.pem. + clientHello2 := &tls.ClientHelloInfo{ + CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA}, + ServerName: "foo.bar.server2.com", + SupportedCurves: []tls.CurveID{CurveP256}, + SupportedPoints: []uint8{pointFormatUncompressed}, + SupportedVersions: []uint16{VersionTLS10}, + } + got2, err := serverConfig.GetCertificate(clientHello2) + if err != nil { + t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + } + if !reflect.DeepEqual(*got2, serverPeerCert2) { + t.Errorf("GetCertificate() = %v, want %v", got2, serverPeerCert2) + } + // "localhost" is the common name on server certificate server_cert_3.pem. + clientHello3 := &tls.ClientHelloInfo{ + CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA}, + ServerName: "localhost", + SupportedCurves: []tls.CurveID{CurveP256}, + SupportedPoints: []uint8{pointFormatUncompressed}, + SupportedVersions: []uint16{VersionTLS10}, + } + got3, err := serverConfig.GetCertificate(clientHello3) + if err != nil { + t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + } + if !reflect.DeepEqual(*got3, serverPeerCert3) { + t.Errorf("GetCertificate() = %v, want %v", got3, serverPeerCert3) + } + }) + } +} diff --git a/security/advancedtls/testdata/server_cert_3.pem b/security/advancedtls/testdata/server_cert_3.pem new file mode 100644 index 00000000000..68b84644e27 --- /dev/null +++ b/security/advancedtls/testdata/server_cert_3.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDjzCCAncCFHFDpmSYUQBXce6Fz6xsA1+t00l7MA0GCSqGSIb3DQEBCwUAMIGD +MQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ0ExETAPBgNVBAcMCFNhbiBKb3NlMQ8w +DQYDVQQKDAZHb29nbGUxCzAJBgNVBAsMAklUMRIwEAYDVQQDDAlsb2NhbGhvc3Qx +IjAgBgkqhkiG9w0BCQEWE2NpbmR5eHVlQGdvb2dsZS5jb20wHhcNMjAwNjI5MDA0 +NDU2WhcNMjEwNjI5MDA0NDU2WjCBgzELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNB +MREwDwYDVQQHDAhTYW4gSm9zZTEPMA0GA1UECgwGR29vZ2xlMQswCQYDVQQLDAJJ +VDESMBAGA1UEAwwJbG9jYWxob3N0MSIwIAYJKoZIhvcNAQkBFhNjaW5keXh1ZUBn +b29nbGUuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsFT6w0At +sk90S5Scu+jvScOHlGyKQJxU8P0r6roLPVEDZtjMoozBJ318LQNIzX123kTRixii +dIGeiGteb9AeY0rivbmU/UbaAtoobLVbd4M1BqhSZzgpu1IDvs3OIlU1//E2UQxc +n9JxumyWrDfXu+i0qIRCiABKcUrnyfZ5pmY6M4KAtzMkOrVreUD0Egf6ateEYeok +yE8UOOQfgIc5Ak97GIlMnE4fIuY3rIXcVkfFgG5vBH63Bhz2JnOT+3aPvM7Ft0wP +eij515dlMpuusLcluDUZAAD1T5Zi36lA/X+1JAZiAa3zzU3rCjD2kWZ2C6OPN39t +l5ghHerEwvloJwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBG3A0L3Wl7yymPOFiq +rDCNdqb1NRNFVr3WkjJaVd6bTk+7L0MIT81aTydv8w3s5CJ07GA7EQzTT1w3KNld +mJq7/xLBNRX1DWMLFTzwx+ig93WrwiQLscWRf26SPKzB8x5eVY/LNJaS3zObDcOv +LULfgIyiTAOfFBoYbzpNVWUW7swD2aBB0f0Zk8ZfmuCvyKy9sxRK6oyV1ot64tFy +YSUpJxSBcaeTt5HXLA7QgTpHMYOUttZCaVs1/4WcrOC9wh24r1mXlPvWWMp5dAZq +aVz8ZXd00OLz6v8ag4QEq/wxTeLdf1U7kJkJ0cj9/nPMLXBERQ54hVvuj5ea3oOv +FY7W +-----END CERTIFICATE----- diff --git a/security/advancedtls/testdata/server_key_3.pem b/security/advancedtls/testdata/server_key_3.pem new file mode 100644 index 00000000000..6ef2ee3457d --- /dev/null +++ b/security/advancedtls/testdata/server_key_3.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQCwVPrDQC2yT3RL +lJy76O9Jw4eUbIpAnFTw/Svqugs9UQNm2MyijMEnfXwtA0jNfXbeRNGLGKJ0gZ6I +a15v0B5jSuK9uZT9RtoC2ihstVt3gzUGqFJnOCm7UgO+zc4iVTX/8TZRDFyf0nG6 +bJasN9e76LSohEKIAEpxSufJ9nmmZjozgoC3MyQ6tWt5QPQSB/pq14Rh6iTITxQ4 +5B+AhzkCT3sYiUycTh8i5jeshdxWR8WAbm8EfrcGHPYmc5P7do+8zsW3TA96KPnX +l2Uym66wtyW4NRkAAPVPlmLfqUD9f7UkBmIBrfPNTesKMPaRZnYLo483f22XmCEd +6sTC+WgnAgMBAAECggEBAKAaOKlpr1QUYmJxqDHR2Nqf541zU8BAcbtyFBsHG0ds +NGuAc6Uvqf1iKYpxTfwObAx6bcwe7Ppd6oSxAEkDrWO4TdG4HDgvyakHTecOb+M+ +xbmqwU8pJnMHj5ECKqTvu50M9aV3VO5kVJDRIaly9rWOH+JWXFZB9VWSjozI90GG +U11E5iSZI/ngUZL72eVqr5gOOD8lTaaLy8jHPnxOMpjZFAscPp1zy8y5DIf+Fgxc +kPzbhckxX8cpk2ATcdRK/UWNi3e/cyMxfeJvst6NwCgmXmDmfkyXMbU8wcEzEWPT +n4uf8gAaCR7fEJQa6Xo3s7q9WP86xnntaoer7V27KtkCgYEA5Y7uL2p2YcjrKfZ9 +6VMPmJyDTH6G08/zyOjFu1P4UYOIS6L5CeEdQj7rw2em313dZOtVQN/uhE7CzqS1 +0wpZ1Qbl1oVFd5On6r8Qz9Dhf+bRVbikPAtvwdETWpqHqXtYD++BIYpfyJXi2nnR +6TrWBWl/QT/uDYEKlZNdRWm8qu0CgYEAxKSLN3wSoepxXxC5A5/vrtvXxdLRZpxT +w5Tn4/Ihx8nB64XkrcdivsyLjVDmyseKVQvnk+/8Y5G5VT6bV1MweDJl4cNHqOMJ +xubNm0dnbGITYyXxEknyGyNHO3l7DGPTF+iENln5GwYHVIHZci87rLyn22yDszYe +Sg2p0p0yOOMCgYEAhMZA+efoBPMDIchSV2wVbz3Hq6zbKxPye1g3VfxhejRL1wOy +a4ZrN+/QybrgB/3wmfiL3PQorxonDwKxsBkikFZnBccbwOgEjYBppum4JBRLK/uZ +8qjiwQW+3f7XTK3s53quA8pXUFtxVNB1GyNOut6kHgduFx12E8Gihw17dJkCgYAJ +Jsc729BaLLj9/Z8+pfDXqG+QS1Fnfxr+3S01lI0x6RfXSDHuTHsx+f78oqk7ArJT +ZuxuHBsY3y1K5FECbWKyFVZcfWQWXgqUcSVcdqQ/jQjt8lQXz80uqiOkhvDNENBA +KpgMl39aXJt2uVxPThdu4JDHS3ONoZUjSSOAI8S0lQKBgQC1llqqsrVR88XWwrCj +BucenaOL0ej0ShBrY/mphRSXO8X20pn/sh7vAWrAltwGLjWkBalpmYLoRigPdZft +CwlIPnmwNeEOW83eAgc6uToUdagUjolO7Dc+vDLBVXj6DulYfsEhwH/2Y66V2Kd5 +8TLhmj77CVDskX6lhRhYGS0FBA== +-----END PRIVATE KEY----- From df72ccd216de0efb76ec490cf34a423383ff14fb Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Sun, 5 Jul 2020 17:46:17 -0700 Subject: [PATCH 15/27] Separate out buildGetCertificateFunc for go1.9 --- security/advancedtls/advancedtls.go | 32 +--------------- security/advancedtls/sni_110.go | 53 ++++++++++++++++++++++++++ security/advancedtls/sni_before_110.go | 41 ++++++++++++++++++++ 3 files changed, 96 insertions(+), 30 deletions(-) create mode 100644 security/advancedtls/sni_110.go create mode 100644 security/advancedtls/sni_before_110.go diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 8c0988ad743..c210e39ef9a 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -1,5 +1,3 @@ -// +build go1.10 - /* * * Copyright 2019 gRPC authors. @@ -239,6 +237,8 @@ func (o *ServerOptions) config() (*tls.Config, error) { ClientAuth: clientAuth, Certificates: o.Certificates, } + // The function getCertificateWithSNI is only able to perform SNI logic for go1.10 and above. + // It will return the first certificate in o.GetCertificate for go1.9. if o.GetCertificate != nil { getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { return buildGetCertificateFunc(clientHello, o) @@ -504,31 +504,3 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { } return cfg.Clone() } - -// buildGetCertificateFunc is a helper function that returns the certificate -// that matches the SNI field based on the given ClientHelloInfo -// if GetCertificate returns a list of certificates -func buildGetCertificateFunc(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { - if o.GetCertificate == nil { - return nil, fmt.Errorf("function GetCertificate must be specified") - } - certificates, err := o.GetCertificate(clientHello) - if err != nil { - return nil, err - } - if len(certificates) == 0 { - return nil, fmt.Errorf("no certificates configured") - } - // If users pass in only one certificate, return that certificate - if len(certificates) == 1 { - return certificates[0], nil - } - // Choose the SNI certificate using SupportsCertificate - for _, cert := range certificates { - if err := clientHello.SupportsCertificate(cert); err == nil { - return cert, nil - } - } - // If nothing matches, return the first certificate. - return certificates[0], nil -} diff --git a/security/advancedtls/sni_110.go b/security/advancedtls/sni_110.go new file mode 100644 index 00000000000..929b1eb86ae --- /dev/null +++ b/security/advancedtls/sni_110.go @@ -0,0 +1,53 @@ +// +build go1.10 + +/* + * + * Copyright 2019 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package advancedtls + +import ( + "crypto/tls" + "fmt" +) + +// The function buildGetCertificateFunc returns the certificate that matches the SNI field +// for the given ClientHelloInfo, defaulting to the first element of o.GetCertificate. +func buildGetCertificateFunc(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { + if o.GetCertificate == nil { + return nil, fmt.Errorf("function GetCertificate must be specified") + } + certificates, err := o.GetCertificate(clientHello) + if err != nil { + return nil, err + } + if len(certificates) == 0 { + return nil, fmt.Errorf("no certificates configured") + } + // If users pass in only one certificate, return that certificate. + if len(certificates) == 1 { + return certificates[0], nil + } + // Choose the SNI certificate using SupportsCertificate. + for _, cert := range certificates { + if err := clientHello.SupportsCertificate(cert); err == nil { + return cert, nil + } + } + // If nothing matches, return the first certificate. + return certificates[0], nil +} diff --git a/security/advancedtls/sni_before_110.go b/security/advancedtls/sni_before_110.go new file mode 100644 index 00000000000..e7bb8ffd884 --- /dev/null +++ b/security/advancedtls/sni_before_110.go @@ -0,0 +1,41 @@ +// +build !go1.10 + +/* + * + * Copyright 2019 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package advancedtls + +import ( + "crypto/tls" + "fmt" +) + +// The function buildGetCertificateFunc returns the first element of o.GetCertificate. +func buildGetCertificateFunc(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { + if o.GetCertificate == nil { + return nil, fmt.Errorf("function GetCertificate must be specified") + } + certificates, err := o.GetCertificate(clientHello) + if err != nil { + return nil, err + } + if len(certificates) == 0 { + return nil, fmt.Errorf("no certificates configured") + } + return certificates[0], nil +} From b066d6ad40e4467a0105296271137f1f3e43a7ec Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Fri, 10 Jul 2020 12:30:57 -0700 Subject: [PATCH 16/27] Review comments #3 --- security/advancedtls/advancedtls_test.go | 91 ++++++++++-------------- security/advancedtls/testdata/README.md | 15 ++++ 2 files changed, 51 insertions(+), 55 deletions(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index 1f572384373..e826d1d2ac9 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -682,23 +682,21 @@ func TestOptionsConfig(t *testing.T) { } } -const ( - tlsEcdheRsaWith3desEdeCbcSha uint16 = 0xc012 -) - -const ( - CurveP256 tls.CurveID = 23 -) - -const ( - pointFormatUncompressed uint8 = 0 -) - -const ( - VersionTLS10 uint16 = 0x0301 -) - func TestGetCertificateSNI(t *testing.T) { + // Define const variables needed to set up ClientHelloInfo for testing. + const ( + tlsEcdheRsaWith3desEdeCbcSha uint16 = 0xc012 + ) + const ( + CurveP256 tls.CurveID = 23 + ) + const ( + pointFormatUncompressed uint8 = 0 + ) + const ( + VersionTLS10 uint16 = 0x0301 + ) + // Load server certificates for setting the serverGetCert callback function. serverPeerCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) if err != nil { @@ -715,14 +713,28 @@ func TestGetCertificateSNI(t *testing.T) { t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) } tests := []struct { - desc string - serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) + desc string + serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) + serverName string + expectedCertificate tls.Certificate }{ { - desc: "Select the certificate that matches the server name provided in clientHello", + desc: "Selected certificate by SNI should be serverPeerCert1", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil + }, + // "foo.bar.com" is the common name on server certificate server_cert_1.pem. + serverName: "foo.bar.com", + expectedCertificate: serverPeerCert1, + }, + { + desc: "Selected certificate by SNI should be serverPeerCert2", serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil }, + // "foo.bar.server2.com" is the common name on server certificate server_cert_2.pem. + serverName: "foo.bar.server2.com", + expectedCertificate: serverPeerCert2, }, } for _, test := range tests { @@ -735,50 +747,19 @@ func TestGetCertificateSNI(t *testing.T) { if err != nil { t.Fatalf("Unable to generate serverConfig. Error: %v", err) } - // "foo.bar.com" is the common name on server certificate server_cert_1.pem. - clientHello1 := &tls.ClientHelloInfo{ - CipherSuites: []uint16{tlsEcdheRsaWith3desEdeCbcSha}, - ServerName: "foo.bar.com", - SupportedCurves: []tls.CurveID{CurveP256}, - SupportedPoints: []uint8{pointFormatUncompressed}, - SupportedVersions: []uint16{VersionTLS10}, - } - got1, err := serverConfig.GetCertificate(clientHello1) - if err != nil { - t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) - } - if !reflect.DeepEqual(*got1, serverPeerCert1) { - t.Errorf("GetCertificate() = %v, want %v", got1, serverPeerCert1) - } - // "foo.bar.server2.com" is the common name on server certificate server_cert_2.pem. - clientHello2 := &tls.ClientHelloInfo{ - CipherSuites: []uint16{tlsEcdheRsaWith3desEdeCbcSha}, - ServerName: "foo.bar.server2.com", - SupportedCurves: []tls.CurveID{CurveP256}, - SupportedPoints: []uint8{pointFormatUncompressed}, - SupportedVersions: []uint16{VersionTLS10}, - } - got2, err := serverConfig.GetCertificate(clientHello2) - if err != nil { - t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) - } - if !reflect.DeepEqual(*got2, serverPeerCert2) { - t.Errorf("GetCertificate() = %v, want %v", got2, serverPeerCert2) - } - // "localhost" is the common name on server certificate server_cert_3.pem. - clientHello3 := &tls.ClientHelloInfo{ + clientHello := &tls.ClientHelloInfo{ CipherSuites: []uint16{tlsEcdheRsaWith3desEdeCbcSha}, - ServerName: "localhost", + ServerName: test.serverName, SupportedCurves: []tls.CurveID{CurveP256}, SupportedPoints: []uint8{pointFormatUncompressed}, SupportedVersions: []uint16{VersionTLS10}, } - got3, err := serverConfig.GetCertificate(clientHello3) + gotCertificate, err := serverConfig.GetCertificate(clientHello) if err != nil { t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) } - if !reflect.DeepEqual(*got3, serverPeerCert3) { - t.Errorf("GetCertificate() = %v, want %v", got3, serverPeerCert3) + if !reflect.DeepEqual(*gotCertificate, test.expectedCertificate) { + t.Errorf("GetCertificate() = %v, want %v", gotCertificate, test.expectedCertificate) } }) } diff --git a/security/advancedtls/testdata/README.md b/security/advancedtls/testdata/README.md index 48a1c135c44..203adc1426a 100644 --- a/security/advancedtls/testdata/README.md +++ b/security/advancedtls/testdata/README.md @@ -40,3 +40,18 @@ commands we run: $ openssl verify -verbose -CAfile ca_cert.pem subject_cert.pem ``` + +Appendix +------------- +Documents the attributes for generated certificates. +1. `server_cert_1.pem` + + Common name: `"foo.bar.com"` + +2. `server_cert_2.pem` + + Common name: `"foo.bar.server2.com"` + +2. `server_cert_3.pem` + + Common name: `"localhost"` From ec327deeb6d87de32b0c17d011fa076962cb688b Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Thu, 16 Jul 2020 09:17:29 -0700 Subject: [PATCH 17/27] Review comments #4 --- security/advancedtls/advancedtls_test.go | 16 +++------------- security/advancedtls/sni_110.go | 2 +- security/advancedtls/sni_before_110.go | 2 +- security/advancedtls/testdata/README.md | 15 --------------- 4 files changed, 5 insertions(+), 30 deletions(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index e826d1d2ac9..dd055fbc857 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -683,19 +683,9 @@ func TestOptionsConfig(t *testing.T) { } func TestGetCertificateSNI(t *testing.T) { - // Define const variables needed to set up ClientHelloInfo for testing. - const ( - tlsEcdheRsaWith3desEdeCbcSha uint16 = 0xc012 - ) - const ( - CurveP256 tls.CurveID = 23 - ) const ( pointFormatUncompressed uint8 = 0 ) - const ( - VersionTLS10 uint16 = 0x0301 - ) // Load server certificates for setting the serverGetCert callback function. serverPeerCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) @@ -748,11 +738,11 @@ func TestGetCertificateSNI(t *testing.T) { t.Fatalf("Unable to generate serverConfig. Error: %v", err) } clientHello := &tls.ClientHelloInfo{ - CipherSuites: []uint16{tlsEcdheRsaWith3desEdeCbcSha}, + CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA}, ServerName: test.serverName, - SupportedCurves: []tls.CurveID{CurveP256}, + SupportedCurves: []tls.CurveID{tls.CurveP256}, SupportedPoints: []uint8{pointFormatUncompressed}, - SupportedVersions: []uint16{VersionTLS10}, + SupportedVersions: []uint16{tls.VersionTLS10}, } gotCertificate, err := serverConfig.GetCertificate(clientHello) if err != nil { diff --git a/security/advancedtls/sni_110.go b/security/advancedtls/sni_110.go index 929b1eb86ae..e49a0a0ceba 100644 --- a/security/advancedtls/sni_110.go +++ b/security/advancedtls/sni_110.go @@ -2,7 +2,7 @@ /* * - * Copyright 2019 gRPC authors. + * Copyright 2020 gRPC authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/security/advancedtls/sni_before_110.go b/security/advancedtls/sni_before_110.go index e7bb8ffd884..ba8249a6861 100644 --- a/security/advancedtls/sni_before_110.go +++ b/security/advancedtls/sni_before_110.go @@ -2,7 +2,7 @@ /* * - * Copyright 2019 gRPC authors. + * Copyright 2020 gRPC authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/security/advancedtls/testdata/README.md b/security/advancedtls/testdata/README.md index 203adc1426a..48a1c135c44 100644 --- a/security/advancedtls/testdata/README.md +++ b/security/advancedtls/testdata/README.md @@ -40,18 +40,3 @@ commands we run: $ openssl verify -verbose -CAfile ca_cert.pem subject_cert.pem ``` - -Appendix -------------- -Documents the attributes for generated certificates. -1. `server_cert_1.pem` - - Common name: `"foo.bar.com"` - -2. `server_cert_2.pem` - - Common name: `"foo.bar.server2.com"` - -2. `server_cert_3.pem` - - Common name: `"localhost"` From cc37b1da042fb150d8414a583ac709d181d63d06 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Thu, 16 Jul 2020 10:35:03 -0700 Subject: [PATCH 18/27] Regenerated certificate and added a test case to test DNS field for SNI --- security/advancedtls/advancedtls_test.go | 9 ++++ .../advancedtls/testdata/server_cert_3.pem | 41 ++++++++------- .../advancedtls/testdata/server_key_3.pem | 52 +++++++++---------- 3 files changed, 56 insertions(+), 46 deletions(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index dd055fbc857..1c9ba77fb2f 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -726,6 +726,15 @@ func TestGetCertificateSNI(t *testing.T) { serverName: "foo.bar.server2.com", expectedCertificate: serverPeerCert2, }, + { + desc: "Selected certificate by SNI should be serverPeerCert3", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil + }, + // "google.com" is one of the DNS names on server certificate server_cert_3.pem. + serverName: "google.com", + expectedCertificate: serverPeerCert3, + }, } for _, test := range tests { test := test diff --git a/security/advancedtls/testdata/server_cert_3.pem b/security/advancedtls/testdata/server_cert_3.pem index 68b84644e27..bfd2d095b22 100644 --- a/security/advancedtls/testdata/server_cert_3.pem +++ b/security/advancedtls/testdata/server_cert_3.pem @@ -1,22 +1,23 @@ -----BEGIN CERTIFICATE----- -MIIDjzCCAncCFHFDpmSYUQBXce6Fz6xsA1+t00l7MA0GCSqGSIb3DQEBCwUAMIGD -MQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ0ExETAPBgNVBAcMCFNhbiBKb3NlMQ8w -DQYDVQQKDAZHb29nbGUxCzAJBgNVBAsMAklUMRIwEAYDVQQDDAlsb2NhbGhvc3Qx -IjAgBgkqhkiG9w0BCQEWE2NpbmR5eHVlQGdvb2dsZS5jb20wHhcNMjAwNjI5MDA0 -NDU2WhcNMjEwNjI5MDA0NDU2WjCBgzELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNB -MREwDwYDVQQHDAhTYW4gSm9zZTEPMA0GA1UECgwGR29vZ2xlMQswCQYDVQQLDAJJ -VDESMBAGA1UEAwwJbG9jYWxob3N0MSIwIAYJKoZIhvcNAQkBFhNjaW5keXh1ZUBn -b29nbGUuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsFT6w0At -sk90S5Scu+jvScOHlGyKQJxU8P0r6roLPVEDZtjMoozBJ318LQNIzX123kTRixii -dIGeiGteb9AeY0rivbmU/UbaAtoobLVbd4M1BqhSZzgpu1IDvs3OIlU1//E2UQxc -n9JxumyWrDfXu+i0qIRCiABKcUrnyfZ5pmY6M4KAtzMkOrVreUD0Egf6ateEYeok -yE8UOOQfgIc5Ak97GIlMnE4fIuY3rIXcVkfFgG5vBH63Bhz2JnOT+3aPvM7Ft0wP -eij515dlMpuusLcluDUZAAD1T5Zi36lA/X+1JAZiAa3zzU3rCjD2kWZ2C6OPN39t -l5ghHerEwvloJwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBG3A0L3Wl7yymPOFiq -rDCNdqb1NRNFVr3WkjJaVd6bTk+7L0MIT81aTydv8w3s5CJ07GA7EQzTT1w3KNld -mJq7/xLBNRX1DWMLFTzwx+ig93WrwiQLscWRf26SPKzB8x5eVY/LNJaS3zObDcOv -LULfgIyiTAOfFBoYbzpNVWUW7swD2aBB0f0Zk8ZfmuCvyKy9sxRK6oyV1ot64tFy -YSUpJxSBcaeTt5HXLA7QgTpHMYOUttZCaVs1/4WcrOC9wh24r1mXlPvWWMp5dAZq -aVz8ZXd00OLz6v8ag4QEq/wxTeLdf1U7kJkJ0cj9/nPMLXBERQ54hVvuj5ea3oOv -FY7W +MIIDyTCCArGgAwIBAgIUeoNdEiqhXVkpcYsmHaKiVS5W/tQwDQYJKoZIhvcNAQEL +BQAwPjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMREwDwYDVQQHDAhTYW4gSm9z +ZTEPMA0GA1UECgwGR29vZ2xlMB4XDTIwMDcxNjE2NTMxOVoXDTQwMDcxMTE2NTMx +OVowgZMxCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTERMA8GA1UEBwwIU2FuIEpv +c2UxEjAQBgNVBAoMCUVuZCBQb2ludDEOMAwGA1UECwwFSW5mcmExIjAgBgkqhkiG +9w0BCQEWE2NpbmR5eHVlQGdvb2dsZS5jb20xHDAaBgNVBAMME2Zvby5iYXIuc2Vy +dmVyMy5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDcIyJEt3ZA +xPn7H5f/IwXS8NcoAXWP8L6rWndcg+EWayx7W+wmUsFKGSFGzrFPPCFmKO8MrQqp +8LSAxHAVtOC6Uw+INWJJw9BRlx2nvV7hfbqu3OnPkPVkN/siUQCqnEKJQHliNT9X +Dl4/Mav75uQSWb3Vfi3KtG7mzPFNNbbe4yfHyGbC4e9RtKkGimDSJ413s3m4+scD +vtpCcCXj9XXZNdCwD1CL3kNdmOdhgfkDBP+AMLBFKZKqpCo6m0s4JJTiej13dc27 +wTrnkFm1CP77SV+kQlWg5DAcVXYJkN9FqNqExqIPS/SxMk7H+3qSQACbttQK9UmC +n3qR3pGbwqzNAgMBAAGjaTBnMB8GA1UdIwQYMBaAFG4bi8k0dOd7jSpPQQ6YUDAU +ARaxMAkGA1UdEwQCMAAwCwYDVR0PBAQDAgTwMCwGA1UdEQQlMCOCCmdvb2dsZS5j +b22CCWFwcGxlLmNvbYIKYW1hem9uLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAn5aW +HEHNTDmcgC25oEtCj+IkoAslgFze4ZqkSz0HCzx76vj3AfMmIEvqB0W74wKqeZgm +V0D7I0xHkM3ILH4RjoCotpol3nLooIPFflA6Z1ILTRZl8mE5kfBSHzKdPS0egOf6 +kgrNYgJjBEtGNsmq8RKxAHVVAPgH88di0JnQDN5LcV9ZBKTQM2R7EM6a8eWD/Jsi +uujbNtdNERssSBV+Oil93MbsEcOT1RSKKxAiVvkHkR+45GRB889xBnqelcDVqcMK +Vcdp6X7aD5/Bu/4fq9AZlcHSEQDixNtjp/pQR0B5FsCGrb5OAz0B2t9jykDiIyj4 +4lxhQz8ykXf7ue0/ag== -----END CERTIFICATE----- diff --git a/security/advancedtls/testdata/server_key_3.pem b/security/advancedtls/testdata/server_key_3.pem index 6ef2ee3457d..ae8ff1a75b4 100644 --- a/security/advancedtls/testdata/server_key_3.pem +++ b/security/advancedtls/testdata/server_key_3.pem @@ -1,28 +1,28 @@ -----BEGIN PRIVATE KEY----- -MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQCwVPrDQC2yT3RL -lJy76O9Jw4eUbIpAnFTw/Svqugs9UQNm2MyijMEnfXwtA0jNfXbeRNGLGKJ0gZ6I -a15v0B5jSuK9uZT9RtoC2ihstVt3gzUGqFJnOCm7UgO+zc4iVTX/8TZRDFyf0nG6 -bJasN9e76LSohEKIAEpxSufJ9nmmZjozgoC3MyQ6tWt5QPQSB/pq14Rh6iTITxQ4 -5B+AhzkCT3sYiUycTh8i5jeshdxWR8WAbm8EfrcGHPYmc5P7do+8zsW3TA96KPnX -l2Uym66wtyW4NRkAAPVPlmLfqUD9f7UkBmIBrfPNTesKMPaRZnYLo483f22XmCEd -6sTC+WgnAgMBAAECggEBAKAaOKlpr1QUYmJxqDHR2Nqf541zU8BAcbtyFBsHG0ds -NGuAc6Uvqf1iKYpxTfwObAx6bcwe7Ppd6oSxAEkDrWO4TdG4HDgvyakHTecOb+M+ -xbmqwU8pJnMHj5ECKqTvu50M9aV3VO5kVJDRIaly9rWOH+JWXFZB9VWSjozI90GG -U11E5iSZI/ngUZL72eVqr5gOOD8lTaaLy8jHPnxOMpjZFAscPp1zy8y5DIf+Fgxc -kPzbhckxX8cpk2ATcdRK/UWNi3e/cyMxfeJvst6NwCgmXmDmfkyXMbU8wcEzEWPT -n4uf8gAaCR7fEJQa6Xo3s7q9WP86xnntaoer7V27KtkCgYEA5Y7uL2p2YcjrKfZ9 -6VMPmJyDTH6G08/zyOjFu1P4UYOIS6L5CeEdQj7rw2em313dZOtVQN/uhE7CzqS1 -0wpZ1Qbl1oVFd5On6r8Qz9Dhf+bRVbikPAtvwdETWpqHqXtYD++BIYpfyJXi2nnR -6TrWBWl/QT/uDYEKlZNdRWm8qu0CgYEAxKSLN3wSoepxXxC5A5/vrtvXxdLRZpxT -w5Tn4/Ihx8nB64XkrcdivsyLjVDmyseKVQvnk+/8Y5G5VT6bV1MweDJl4cNHqOMJ -xubNm0dnbGITYyXxEknyGyNHO3l7DGPTF+iENln5GwYHVIHZci87rLyn22yDszYe -Sg2p0p0yOOMCgYEAhMZA+efoBPMDIchSV2wVbz3Hq6zbKxPye1g3VfxhejRL1wOy -a4ZrN+/QybrgB/3wmfiL3PQorxonDwKxsBkikFZnBccbwOgEjYBppum4JBRLK/uZ -8qjiwQW+3f7XTK3s53quA8pXUFtxVNB1GyNOut6kHgduFx12E8Gihw17dJkCgYAJ -Jsc729BaLLj9/Z8+pfDXqG+QS1Fnfxr+3S01lI0x6RfXSDHuTHsx+f78oqk7ArJT -ZuxuHBsY3y1K5FECbWKyFVZcfWQWXgqUcSVcdqQ/jQjt8lQXz80uqiOkhvDNENBA -KpgMl39aXJt2uVxPThdu4JDHS3ONoZUjSSOAI8S0lQKBgQC1llqqsrVR88XWwrCj -BucenaOL0ej0ShBrY/mphRSXO8X20pn/sh7vAWrAltwGLjWkBalpmYLoRigPdZft -CwlIPnmwNeEOW83eAgc6uToUdagUjolO7Dc+vDLBVXj6DulYfsEhwH/2Y66V2Kd5 -8TLhmj77CVDskX6lhRhYGS0FBA== +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDcIyJEt3ZAxPn7 +H5f/IwXS8NcoAXWP8L6rWndcg+EWayx7W+wmUsFKGSFGzrFPPCFmKO8MrQqp8LSA +xHAVtOC6Uw+INWJJw9BRlx2nvV7hfbqu3OnPkPVkN/siUQCqnEKJQHliNT9XDl4/ +Mav75uQSWb3Vfi3KtG7mzPFNNbbe4yfHyGbC4e9RtKkGimDSJ413s3m4+scDvtpC +cCXj9XXZNdCwD1CL3kNdmOdhgfkDBP+AMLBFKZKqpCo6m0s4JJTiej13dc27wTrn +kFm1CP77SV+kQlWg5DAcVXYJkN9FqNqExqIPS/SxMk7H+3qSQACbttQK9UmCn3qR +3pGbwqzNAgMBAAECggEAU0Te84tKKdnYjUs4HYRL8ay0VienJpl0JjEEMXSZMfe8 +TbVJsH1hK/wxgC0zGLuwDoqxUeQqwnmQbZzgoPVYhGJi360BztFI/XPh/c8+EqGS +eg6KSr+UcyJR1ns5e0+8Q1qmD6YAnZeLwu+xFIoT/3T+v8EI5UI3KQqgxAnrcIdc +RWqIwWLkkPm1QVRYsvRaTvmgFd2LcIT9AdIruP/VsqF3GEzvEQSr0lgmwOe0izLb +HKfZr+2JwOppwTLGhKo1wUyUUsglXCBOcFYAA03xdviQWBuCeKXamNrdB7M0T4zZ +THheNRQq2g7bTYnncCYKcrNa5VY5wmHY767mf6ahAQKBgQD+AGk21CsRgiKCGzUI +wTRynutAMkX55U1bud2+OMpzH9omvnccX5ezq5WTw/jZfz+jrUBF8YKSTtb3InUg +yXcpJ/XjRDFMZp1Avy/44rOlYg1QYMD4JK96f8bbd1yej5NVw45V9synaJHuvoDV +bkbZu00S0X8Pgvlh354MUH3CLQKBgQDd3oQdKMZgsX8gtmcQfQoEZ0VvlOYmTM5W +Kw+24iGrBkfBt+NuKx8qm1CpoFGx4G2+TgttMywjF9RG3R2uGqbJZbCOAXzjRMQJ +L9PuTiGAdYD3fA5cTmnrrNEhPydtRhF2M3p8FFeQtwsEBYreXux25rbmVOYTFMgJ +hVUW9RdZIQKBgQDwEYdgMQw70hm3iuuHSMS/iQCkfl+xH08MYRH6FkcSpIpVkDOX +96m0QXpwXQs41pJZqwhSkz9r9WQr1L+Lq58aoRBAK1XE9j+u0IUQ4YQVziTzUV9R +qarJRze2eoxpuR3yM5C2IzuvBqDXW+r8zuvcIrFoFeXXzVzTar1AulsCSQKBgQCa +UD+3QDrp2co/6F26vB0RfvpuZzPEA7undv/RBWrBVvblp46Je3iL28a4lAb+Hsh1 +ijasVuEl71b3iqcwBt1mSlIIEsTYFWX7tcZDgxgODqwKdcBPN0K4ZlR2OUSk3g0b +Fybj0gotXwJMY8Z4b7Er6b/gZ8A2GUggRxotg34fwQKBgEh6a88SgTyu1KcR2yzN +Zbs8MEfZ8hqfUj+GL0+6y9KQd4uSIngyHWGNETE9dCObNz5HEdJIpUNvB3vfN0Vk +f1EEPDQLQKJjii7jZ9U9XfPVaUhqIVH3Aupmb30H3AQw7XOF23g63k9Yg1FWtDT6 +4CuBsUvjXywL4yHrWK+BuSXF -----END PRIVATE KEY----- From 3891625f6059578f20dffb8a5a9ba4a1af1c0a36 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Wed, 22 Jul 2020 09:18:17 -0700 Subject: [PATCH 19/27] Removed const var in function --- security/advancedtls/advancedtls_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index 1c9ba77fb2f..dd8d45af0fd 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -683,9 +683,6 @@ func TestOptionsConfig(t *testing.T) { } func TestGetCertificateSNI(t *testing.T) { - const ( - pointFormatUncompressed uint8 = 0 - ) // Load server certificates for setting the serverGetCert callback function. serverPeerCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) @@ -746,6 +743,7 @@ func TestGetCertificateSNI(t *testing.T) { if err != nil { t.Fatalf("Unable to generate serverConfig. Error: %v", err) } + pointFormatUncompressed := uint8(0) clientHello := &tls.ClientHelloInfo{ CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA}, ServerName: test.serverName, From ec83efb825aa0c2435729098032d678aa8a844a0 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Fri, 24 Jul 2020 10:06:17 -0700 Subject: [PATCH 20/27] Renamed GetCertificate and buildGetCertificateFunc --- security/advancedtls/advancedtls.go | 14 +++++++------- .../advancedtls/advancedtls_integration_test.go | 4 ++-- security/advancedtls/advancedtls_test.go | 10 +++++----- security/advancedtls/sni_110.go | 12 ++++++------ security/advancedtls/sni_before_110.go | 10 +++++----- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index c210e39ef9a..5ed699d0a83 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -156,7 +156,7 @@ type ClientOptions struct { // Certificates or GetClientCertificate indicates the certificates sent from // the server to the client to prove server's identities. The rules for setting // these two fields are: -// Either Certificates or GetCertificate must be set; the other will be ignored. +// Either Certificates or GetCertificates must be set; the other will be ignored. type ServerOptions struct { // If field Certificates is set, field GetClientCertificate will be ignored. // The server will use Certificates every time when asked for a certificate, @@ -166,7 +166,7 @@ type ServerOptions struct { // invoke this function every time asked to present certificates to the // client when a new connection is established. This is known as peer // certificate reloading. - GetCertificate func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) + GetCertificates func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) // VerifyPeer is a custom verification check after certificate signature // check. // If this is set, we will perform this customized check after doing the @@ -210,8 +210,8 @@ func (o *ClientOptions) config() (*tls.Config, error) { } func (o *ServerOptions) config() (*tls.Config, error) { - if o.Certificates == nil && o.GetCertificate == nil { - return nil, fmt.Errorf("either Certificates or GetCertificate must be specified") + if o.Certificates == nil && o.GetCertificates == nil { + return nil, fmt.Errorf("either Certificates or GetCertificates must be specified") } if o.RequireClientCert && o.VType == SkipVerification && o.VerifyPeer == nil { return nil, fmt.Errorf( @@ -238,10 +238,10 @@ func (o *ServerOptions) config() (*tls.Config, error) { Certificates: o.Certificates, } // The function getCertificateWithSNI is only able to perform SNI logic for go1.10 and above. - // It will return the first certificate in o.GetCertificate for go1.9. - if o.GetCertificate != nil { + // It will return the first certificate in o.GetCertificates for go1.9. + if o.GetCertificates != nil { getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { - return buildGetCertificateFunc(clientHello, o) + return buildGetCertificates(clientHello, o) } config.GetCertificate = getCertificateWithSNI } diff --git a/security/advancedtls/advancedtls_integration_test.go b/security/advancedtls/advancedtls_integration_test.go index e35205f4ca5..d593e5ec4fe 100644 --- a/security/advancedtls/advancedtls_integration_test.go +++ b/security/advancedtls/advancedtls_integration_test.go @@ -390,8 +390,8 @@ func TestEnd2End(t *testing.T) { t.Run(test.desc, func(t *testing.T) { // Start a server using ServerOptions in another goroutine. serverOptions := &ServerOptions{ - Certificates: test.serverCert, - GetCertificate: test.serverGetCert, + Certificates: test.serverCert, + GetCertificates: test.serverGetCert, RootCertificateOptions: RootCertificateOptions{ RootCACerts: test.serverRoot, GetRootCAs: test.serverGetRoot, diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index dd8d45af0fd..af21a52f520 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -404,8 +404,8 @@ func TestClientServerHandshake(t *testing.T) { } // Start a server using ServerOptions in another goroutine. serverOptions := &ServerOptions{ - Certificates: test.serverCert, - GetCertificate: test.serverGetCert, + Certificates: test.serverCert, + GetCertificates: test.serverGetCert, RootCertificateOptions: RootCertificateOptions{ RootCACerts: test.serverRoot, GetRootCAs: test.serverGetRoot, @@ -682,7 +682,7 @@ func TestOptionsConfig(t *testing.T) { } } -func TestGetCertificateSNI(t *testing.T) { +func TestGetCertificatesSNI(t *testing.T) { // Load server certificates for setting the serverGetCert callback function. serverPeerCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) @@ -737,7 +737,7 @@ func TestGetCertificateSNI(t *testing.T) { test := test t.Run(test.desc, func(t *testing.T) { serverOptions := &ServerOptions{ - GetCertificate: test.serverGetCert, + GetCertificates: test.serverGetCert, } serverConfig, err := serverOptions.config() if err != nil { @@ -756,7 +756,7 @@ func TestGetCertificateSNI(t *testing.T) { t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) } if !reflect.DeepEqual(*gotCertificate, test.expectedCertificate) { - t.Errorf("GetCertificate() = %v, want %v", gotCertificate, test.expectedCertificate) + t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.expectedCertificate) } }) } diff --git a/security/advancedtls/sni_110.go b/security/advancedtls/sni_110.go index e49a0a0ceba..ac3900b64d1 100644 --- a/security/advancedtls/sni_110.go +++ b/security/advancedtls/sni_110.go @@ -25,13 +25,13 @@ import ( "fmt" ) -// The function buildGetCertificateFunc returns the certificate that matches the SNI field -// for the given ClientHelloInfo, defaulting to the first element of o.GetCertificate. -func buildGetCertificateFunc(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { - if o.GetCertificate == nil { - return nil, fmt.Errorf("function GetCertificate must be specified") +// The function 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) { + if o.GetCertificates == nil { + return nil, fmt.Errorf("function GetCertificates must be specified") } - certificates, err := o.GetCertificate(clientHello) + certificates, err := o.GetCertificates(clientHello) if err != nil { return nil, err } diff --git a/security/advancedtls/sni_before_110.go b/security/advancedtls/sni_before_110.go index ba8249a6861..50b99b82f3a 100644 --- a/security/advancedtls/sni_before_110.go +++ b/security/advancedtls/sni_before_110.go @@ -25,12 +25,12 @@ import ( "fmt" ) -// The function buildGetCertificateFunc returns the first element of o.GetCertificate. -func buildGetCertificateFunc(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { - if o.GetCertificate == nil { - return nil, fmt.Errorf("function GetCertificate must be specified") +// The function buildGetCertificates returns the first element of o.GetCertificates. +func buildGetCertificates(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { + if o.GetCertificates == nil { + return nil, fmt.Errorf("function GetCertificates must be specified") } - certificates, err := o.GetCertificate(clientHello) + certificates, err := o.GetCertificates(clientHello) if err != nil { return nil, err } From 789296a562a8fb4efc62ec02bc5892cc3d05343c Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Fri, 24 Jul 2020 10:24:56 -0700 Subject: [PATCH 21/27] Resolved Easwars' comments --- security/advancedtls/advancedtls.go | 4 +- security/advancedtls/advancedtls_test.go | 52 ++++++++++++------------ 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 5ed699d0a83..183678c952b 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -237,12 +237,12 @@ func (o *ServerOptions) config() (*tls.Config, error) { ClientAuth: clientAuth, Certificates: o.Certificates, } - // The function getCertificateWithSNI is only able to perform SNI logic for go1.10 and above. - // It will return the first certificate in o.GetCertificates for go1.9. if o.GetCertificates != nil { getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { return buildGetCertificates(clientHello, o) } + // GetCertificate is only able to perform SNI logic for go1.10 and above. + // It will return the first certificate in o.GetCertificates for go1.9. config.GetCertificate = getCertificateWithSNI } if clientCAs != nil { diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index af21a52f520..cc8d206ff25 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -683,54 +683,52 @@ func TestOptionsConfig(t *testing.T) { } func TestGetCertificatesSNI(t *testing.T) { - // Load server certificates for setting the serverGetCert callback function. - serverPeerCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), - testdata.Path("server_key_1.pem")) + // Load server peer certificates for setting the serverGetCert callback function. + serverPeerCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) if err != nil { - t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + t.Fatalf("tls.LoadX509KeyPair(server_cert_1.pem, server_key_1.pem) failed: %v", err) } - serverPeerCert2, err := tls.LoadX509KeyPair(testdata.Path("server_cert_2.pem"), - testdata.Path("server_key_2.pem")) + serverPeerCert2, err := tls.LoadX509KeyPair(testdata.Path("server_cert_2.pem"), testdata.Path("server_key_2.pem")) if err != nil { - t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + t.Fatalf("tls.LoadX509KeyPair(server_cert_2.pem, server_key_2.pem) failed: %v", err) } - serverPeerCert3, err := tls.LoadX509KeyPair(testdata.Path("server_cert_3.pem"), - testdata.Path("server_key_3.pem")) + serverPeerCert3, err := tls.LoadX509KeyPair(testdata.Path("server_cert_3.pem"), testdata.Path("server_key_3.pem")) if err != nil { - t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + t.Fatalf("tls.LoadX509KeyPair(server_cert_3.pem, server_key_3.pem) failed: %v", err) } + tests := []struct { - desc string - serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) - serverName string - expectedCertificate tls.Certificate + desc string + serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) + serverName string + wantCert tls.Certificate }{ { - desc: "Selected certificate by SNI should be serverPeerCert1", + desc: "Select serverPeerCert1", serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil }, // "foo.bar.com" is the common name on server certificate server_cert_1.pem. - serverName: "foo.bar.com", - expectedCertificate: serverPeerCert1, + serverName: "foo.bar.com", + wantCert: serverPeerCert1, }, { - desc: "Selected certificate by SNI should be serverPeerCert2", + desc: "Select serverPeerCert2", serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil }, // "foo.bar.server2.com" is the common name on server certificate server_cert_2.pem. - serverName: "foo.bar.server2.com", - expectedCertificate: serverPeerCert2, + serverName: "foo.bar.server2.com", + wantCert: serverPeerCert2, }, { - desc: "Selected certificate by SNI should be serverPeerCert3", + desc: "Select serverPeerCert3", serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil }, // "google.com" is one of the DNS names on server certificate server_cert_3.pem. - serverName: "google.com", - expectedCertificate: serverPeerCert3, + serverName: "google.com", + wantCert: serverPeerCert3, }, } for _, test := range tests { @@ -741,7 +739,7 @@ func TestGetCertificatesSNI(t *testing.T) { } serverConfig, err := serverOptions.config() if err != nil { - t.Fatalf("Unable to generate serverConfig. Error: %v", err) + t.Fatalf("serverOptions.config() failed: %v", err) } pointFormatUncompressed := uint8(0) clientHello := &tls.ClientHelloInfo{ @@ -753,10 +751,10 @@ func TestGetCertificatesSNI(t *testing.T) { } gotCertificate, err := serverConfig.GetCertificate(clientHello) if err != nil { - t.Fatalf("Server is unable to parse peer certificates. Error: %v", err) + t.Fatalf("serverConfig.GetCertificate(clientHello) failed: %v", err) } - if !reflect.DeepEqual(*gotCertificate, test.expectedCertificate) { - t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.expectedCertificate) + if !reflect.DeepEqual(*gotCertificate, test.wantCert) { + t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.wantCert) } }) } From 8ef1de71b527a430805304a061271cbf8e2cdca2 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Fri, 24 Jul 2020 12:17:54 -0700 Subject: [PATCH 22/27] Added server_cert_3.txt --- .../advancedtls/testdata/server_cert_3.txt | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 security/advancedtls/testdata/server_cert_3.txt diff --git a/security/advancedtls/testdata/server_cert_3.txt b/security/advancedtls/testdata/server_cert_3.txt new file mode 100644 index 00000000000..e62c99cbf58 --- /dev/null +++ b/security/advancedtls/testdata/server_cert_3.txt @@ -0,0 +1,60 @@ +Certificate: + Data: + Version: 3 (0x2) + Serial Number: + 7a:83:5d:12:2a:a1:5d:59:29:71:8b:26:1d:a2:a2:55:2e:56:fe:d4 + Signature Algorithm: sha256WithRSAEncryption + Issuer: C = US, ST = CA, L = San Jose, O = Google + Validity + Not Before: Jul 16 16:53:19 2020 GMT + Not After : Jul 11 16:53:19 2040 GMT + Subject: C = US, ST = CA, L = San Jose, O = End Point, OU = Infra, emailAddress = cindyxue@google.com, CN = foo.bar.server3.com + Subject Public Key Info: + Public Key Algorithm: rsaEncryption + RSA Public-Key: (2048 bit) + Modulus: + 00:dc:23:22:44:b7:76:40:c4:f9:fb:1f:97:ff:23: + 05:d2:f0:d7:28:01:75:8f:f0:be:ab:5a:77:5c:83: + e1:16:6b:2c:7b:5b:ec:26:52:c1:4a:19:21:46:ce: + b1:4f:3c:21:66:28:ef:0c:ad:0a:a9:f0:b4:80:c4: + 70:15:b4:e0:ba:53:0f:88:35:62:49:c3:d0:51:97: + 1d:a7:bd:5e:e1:7d:ba:ae:dc:e9:cf:90:f5:64:37: + fb:22:51:00:aa:9c:42:89:40:79:62:35:3f:57:0e: + 5e:3f:31:ab:fb:e6:e4:12:59:bd:d5:7e:2d:ca:b4: + 6e:e6:cc:f1:4d:35:b6:de:e3:27:c7:c8:66:c2:e1: + ef:51:b4:a9:06:8a:60:d2:27:8d:77:b3:79:b8:fa: + c7:03:be:da:42:70:25:e3:f5:75:d9:35:d0:b0:0f: + 50:8b:de:43:5d:98:e7:61:81:f9:03:04:ff:80:30: + b0:45:29:92:aa:a4:2a:3a:9b:4b:38:24:94:e2:7a: + 3d:77:75:cd:bb:c1:3a:e7:90:59:b5:08:fe:fb:49: + 5f:a4:42:55:a0:e4:30:1c:55:76:09:90:df:45:a8: + da:84:c6:a2:0f:4b:f4:b1:32:4e:c7:fb:7a:92:40: + 00:9b:b6:d4:0a:f5:49:82:9f:7a:91:de:91:9b:c2: + ac:cd + Exponent: 65537 (0x10001) + X509v3 extensions: + X509v3 Authority Key Identifier: + keyid:6E:1B:8B:C9:34:74:E7:7B:8D:2A:4F:41:0E:98:50:30:14:01:16:B1 + + X509v3 Basic Constraints: + CA:FALSE + X509v3 Key Usage: + Digital Signature, Non Repudiation, Key Encipherment, Data Encipherment + X509v3 Subject Alternative Name: + DNS:google.com, DNS:apple.com, DNS:amazon.com + Signature Algorithm: sha256WithRSAEncryption + 9f:96:96:1c:41:cd:4c:39:9c:80:2d:b9:a0:4b:42:8f:e2:24: + a0:0b:25:80:5c:de:e1:9a:a4:4b:3d:07:0b:3c:7b:ea:f8:f7: + 01:f3:26:20:4b:ea:07:45:bb:e3:02:aa:79:98:26:57:40:fb: + 23:4c:47:90:cd:c8:2c:7e:11:8e:80:a8:b6:9a:25:de:72:e8: + a0:83:c5:7e:50:3a:67:52:0b:4d:16:65:f2:61:39:91:f0:52: + 1f:32:9d:3d:2d:1e:80:e7:fa:92:0a:cd:62:02:63:04:4b:46: + 36:c9:aa:f1:12:b1:00:75:55:00:f8:07:f3:c7:62:d0:99:d0: + 0c:de:4b:71:5f:59:04:a4:d0:33:64:7b:10:ce:9a:f1:e5:83: + fc:9b:22:ba:e8:db:36:d7:4d:11:1b:2c:48:15:7e:3a:29:7d: + dc:c6:ec:11:c3:93:d5:14:8a:2b:10:22:56:f9:07:91:1f:b8: + e4:64:41:f3:cf:71:06:7a:9e:95:c0:d5:a9:c3:0a:55:c7:69: + e9:7e:da:0f:9f:c1:bb:fe:1f:ab:d0:19:95:c1:d2:11:00:e2: + c4:db:63:a7:fa:50:47:40:79:16:c0:86:ad:be:4e:03:3d:01: + da:df:63:ca:40:e2:23:28:f8:e2:5c:61:43:3f:32:91:77:fb: + b9:ed:3f:6a From c70c90b42843704726234829bbdf8ee92e803438 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Fri, 24 Jul 2020 14:10:46 -0700 Subject: [PATCH 23/27] Renamed serverPeerCert to serverCert --- security/advancedtls/advancedtls_test.go | 26 ++++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index cc8d206ff25..23e39dcc663 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -683,16 +683,16 @@ func TestOptionsConfig(t *testing.T) { } func TestGetCertificatesSNI(t *testing.T) { - // Load server peer certificates for setting the serverGetCert callback function. - serverPeerCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) + // Load server certificates for setting the serverGetCert callback function. + serverCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) if err != nil { t.Fatalf("tls.LoadX509KeyPair(server_cert_1.pem, server_key_1.pem) failed: %v", err) } - serverPeerCert2, err := tls.LoadX509KeyPair(testdata.Path("server_cert_2.pem"), testdata.Path("server_key_2.pem")) + serverCert2, err := tls.LoadX509KeyPair(testdata.Path("server_cert_2.pem"), testdata.Path("server_key_2.pem")) if err != nil { t.Fatalf("tls.LoadX509KeyPair(server_cert_2.pem, server_key_2.pem) failed: %v", err) } - serverPeerCert3, err := tls.LoadX509KeyPair(testdata.Path("server_cert_3.pem"), testdata.Path("server_key_3.pem")) + serverCert3, err := tls.LoadX509KeyPair(testdata.Path("server_cert_3.pem"), testdata.Path("server_key_3.pem")) if err != nil { t.Fatalf("tls.LoadX509KeyPair(server_cert_3.pem, server_key_3.pem) failed: %v", err) } @@ -704,31 +704,31 @@ func TestGetCertificatesSNI(t *testing.T) { wantCert tls.Certificate }{ { - desc: "Select serverPeerCert1", + desc: "Select serverCert1", serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil }, // "foo.bar.com" is the common name on server certificate server_cert_1.pem. serverName: "foo.bar.com", - wantCert: serverPeerCert1, + wantCert: serverCert1, }, { - desc: "Select serverPeerCert2", + desc: "Select serverCert2", serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil }, // "foo.bar.server2.com" is the common name on server certificate server_cert_2.pem. serverName: "foo.bar.server2.com", - wantCert: serverPeerCert2, + wantCert: serverCert2, }, { - desc: "Select serverPeerCert3", + desc: "Select serverCert3", serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - return []*tls.Certificate{&serverPeerCert1, &serverPeerCert2, &serverPeerCert3}, nil + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil }, // "google.com" is one of the DNS names on server certificate server_cert_3.pem. serverName: "google.com", - wantCert: serverPeerCert3, + wantCert: serverCert3, }, } for _, test := range tests { From fea90e17b51d7f7c4b8d72f140ea5ef267d79a3c Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Fri, 24 Jul 2020 14:20:31 -0700 Subject: [PATCH 24/27] Added server_cert_1.txt and server_cert_2.txt --- .../advancedtls_integration_test.go | 2 - .../advancedtls/testdata/server_cert_1.txt | 90 ++++++++++++++++++ .../advancedtls/testdata/server_cert_2.txt | 91 +++++++++++++++++++ 3 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 security/advancedtls/testdata/server_cert_1.txt create mode 100644 security/advancedtls/testdata/server_cert_2.txt diff --git a/security/advancedtls/advancedtls_integration_test.go b/security/advancedtls/advancedtls_integration_test.go index d593e5ec4fe..3f9f6218035 100644 --- a/security/advancedtls/advancedtls_integration_test.go +++ b/security/advancedtls/advancedtls_integration_test.go @@ -1,5 +1,3 @@ -// +build go1.10 - /* * * Copyright 2020 gRPC authors. diff --git a/security/advancedtls/testdata/server_cert_1.txt b/security/advancedtls/testdata/server_cert_1.txt new file mode 100644 index 00000000000..5367569d052 --- /dev/null +++ b/security/advancedtls/testdata/server_cert_1.txt @@ -0,0 +1,90 @@ +Certificate: + Data: + Version: 3 (0x2) + Serial Number: 3 (0x3) + Signature Algorithm: sha256WithRSAEncryption + Issuer: C = US, ST = CA, L = SVL, O = Internet Widgits Pty Ltd + Validity + Not Before: Nov 4 21:43:00 2019 GMT + Not After : Aug 18 21:43:00 2293 GMT + Subject: C = US, ST = CA, L = DUMMYCITY, O = Internet Widgits Pty Ltd, CN = foo.bar.com + Subject Public Key Info: + Public Key Algorithm: rsaEncryption + RSA Public-Key: (4096 bit) + Modulus: + 00:ec:3f:24:2d:91:3a:bd:c3:fc:15:72:42:b3:fb: + 28:e6:04:a3:be:26:20:e6:ea:30:a8:aa:48:78:36: + 0e:0b:99:29:3b:4b:f9:f1:d5:bf:bd:0c:13:7c:ea: + 52:06:f4:bc:34:9e:2b:c0:b4:82:2c:87:fa:2f:e2: + cd:7c:d7:b9:e1:8f:04:71:6d:85:77:ae:18:40:e4: + b1:3a:4a:6b:e5:33:bf:3e:65:db:cf:94:64:87:1a: + 20:46:c0:37:3a:9f:93:3f:d4:4f:ac:c4:e4:e0:28: + b6:0f:28:53:2a:cf:b9:fe:50:f2:ef:47:dc:7e:b6: + 60:c2:47:85:b8:cb:ca:48:5b:fa:9f:8a:97:30:01: + f4:b3:51:0f:68:e1:60:ab:2f:a0:ad:fc:f0:10:4f: + 60:e1:92:db:be:83:04:5c:40:87:ce:51:3e:9a:9e: + d6:1c:1b:19:cb:8c:c2:6c:57:74:6f:7b:af:94:3d: + 53:ad:17:a5:99:69:7c:41:f5:3e:7a:5b:48:c7:78: + ff:d7:3b:a8:1f:f7:30:e7:83:26:78:e2:cb:a2:8f: + 58:92:61:cd:ca:e9:b8:d1:80:c0:40:58:e9:d8:d3: + 42:64:82:8f:e4:0c:b9:b1:36:db:9f:65:3f:3f:5b: + 24:59:31:b3:60:0c:fa:41:5a:1b:b8:9d:ec:99:37: + 90:fa:b5:e7:3f:cb:7c:e0:f9:ed:ea:27:ce:15:24: + c7:77:3b:45:45:2d:19:8e:2e:7f:65:0e:85:df:66: + 50:69:24:2c:a4:6a:07:e5:3f:eb:28:84:53:94:4d: + 5f:9c:a8:65:a6:50:4c:c0:35:06:40:6a:a5:62:b1: + 93:60:e5:1c:85:28:34:9b:29:81:6f:e2:4f:cd:15: + 30:b9:19:d7:4b:bb:30:0c:4b:2d:64:fe:3b:dd:0e: + a4:25:2c:4a:5c:de:d7:74:1f:5e:93:7b:1c:e8:c8: + fa:72:1f:4a:eb:8d:3f:98:e4:55:98:b8:e0:8a:29: + 92:33:af:75:6b:05:84:05:d3:0c:2c:07:78:bc:0e: + b2:6d:a7:00:35:c4:53:1f:7b:e6:ba:07:72:a8:24: + c1:0a:a7:c4:46:e6:f2:6f:3a:79:23:00:0b:b8:e5: + 1f:e0:e2:ee:c6:13:a3:57:d9:86:1a:95:f7:a3:04: + f1:46:d5:5f:21:d2:aa:d2:30:fb:f6:cb:e0:da:24: + c6:c3:30:2f:d2:1f:21:fe:bc:0f:99:ac:ac:9b:65: + 9b:e4:83:9a:00:b8:2f:40:fc:3b:42:d3:7a:e8:b7: + 52:d7:f4:67:2a:a5:f7:eb:78:f1:0a:56:8b:56:12: + d5:48:d8:48:70:ab:b8:69:5a:21:d3:71:b0:59:9d: + 17:b4:4b + Exponent: 65537 (0x10001) + X509v3 extensions: + X509v3 Subject Key Identifier: + C0:82:DA:FA:69:46:30:AE:FF:6F:CD:BB:93:49:94:A6:D0:E2:17:EB + X509v3 Authority Key Identifier: + keyid:5A:A5:DA:B1:99:D4:E5:0E:E6:1E:94:EA:FF:FC:62:E2:ED:09:F1:06 + + X509v3 Basic Constraints: + CA:FALSE + X509v3 Key Usage: + Digital Signature, Key Encipherment + Signature Algorithm: sha256WithRSAEncryption + 36:fd:cf:ec:f5:20:4b:52:dc:2e:38:f3:92:b1:e4:b6:a1:06: + 86:aa:2d:c0:e6:f5:0a:58:97:a9:e3:be:13:09:61:79:ed:d4: + 41:83:26:ad:ee:0b:43:83:d1:dd:19:1a:e8:7b:b2:1f:fe:d4: + c1:57:7d:6d:6b:d4:42:ea:7d:cd:34:8c:a4:1f:5b:3b:fa:de: + bb:2f:ae:56:b6:18:e5:53:a9:a3:99:58:ad:36:be:19:54:61: + 0d:52:b6:a7:53:fc:60:e5:ff:f5:7f:82:3f:c1:49:06:cd:b2: + af:25:ee:de:bd:e0:e5:5e:ad:0b:dc:2e:b1:ec:7a:52:6f:9d: + e0:b9:84:18:db:49:53:ee:df:93:ee:8b:9d:9b:8e:3b:2a:82: + 86:7f:45:c8:dd:d1:b0:40:17:ed:63:52:a1:5b:6e:d3:5c:a2: + 72:05:fb:3a:39:71:0d:b4:2c:9d:15:23:1b:1f:8d:ac:89:dc: + c9:56:f2:19:c7:f3:2f:bb:d5:de:40:17:f1:52:ea:e8:93:ff: + 56:43:f5:1d:cb:c0:51:52:25:d7:b0:81:a9:0e:4d:92:24:e7: + 10:81:c7:31:26:ac:cb:66:c1:3f:f6:5f:69:7b:74:87:0d:b0: + 8c:27:d4:24:29:59:e9:5b:a2:cb:0c:c0:f5:9b:1d:42:38:6b: + e3:c3:43:1e:ba:df:b1:51:0a:b7:33:55:26:39:01:2f:9f:c7: + 88:ac:2f:4a:89:f3:69:de:72:43:48:49:08:59:36:86:84:09: + db:6a:82:84:3e:71:6a:9d:f9:bd:d8:b5:1e:7c:2c:29:e1:27: + 45:4c:47:5b:88:b8:e6:fa:9d:9b:ff:d4:e9:8d:2d:5e:64:7f: + 27:87:b2:8c:d8:7e:f5:52:3c:c4:d8:30:03:24:d7:ac:f8:53: + 91:80:98:42:24:5a:6b:cb:34:48:57:e0:82:ac:96:d9:55:6c: + c2:c3:8c:19:7c:56:39:0a:a8:f1:b8:77:64:70:83:a8:04:c8: + 3a:5d:0b:00:4c:e5:ba:f1:40:e5:57:cd:d9:67:48:21:e9:9c: + d3:f2:b8:01:b8:d1:c0:d1:3a:44:c0:97:db:e6:bc:8f:2e:33: + d5:e2:38:3d:d7:7b:50:13:01:36:28:61:cc:28:98:3c:f8:21: + 5d:8c:fe:f5:d0:ab:e0:60:ec:36:22:8d:0b:71:30:1b:3d:56: + ae:96:e9:d2:89:c2:43:8b:ef:25:b7:d6:0d:82:e6:5a:c6:91: + 8a:ad:8c:28:2a:2b:5c:4e:a1:de:cb:7d:cb:29:11:a2:66:c8: + a1:33:35:75:16:fe:28:0b:78:31:0a:1f:fa:d0:a8:f4:f1:69: + c7:97:1e:5d:fb:53:08:b5 diff --git a/security/advancedtls/testdata/server_cert_2.txt b/security/advancedtls/testdata/server_cert_2.txt new file mode 100644 index 00000000000..8962204ff34 --- /dev/null +++ b/security/advancedtls/testdata/server_cert_2.txt @@ -0,0 +1,91 @@ +Certificate: + Data: + Version: 3 (0x2) + Serial Number: 7 (0x7) + Signature Algorithm: sha256WithRSAEncryption + Issuer: C = US, ST = CA, O = Internet Widgits Pty Ltd, CN = foo.bar.client2.trust.com + Validity + Not Before: Jan 9 22:51:54 2020 GMT + Not After : Oct 23 22:51:54 2293 GMT + Subject: C = US, ST = CA, O = Internet Widgits Pty Ltd, CN = foo.bar.server2.com + Subject Public Key Info: + Public Key Algorithm: rsaEncryption + RSA Public-Key: (4096 bit) + Modulus: + 00:b1:0b:d3:7e:5b:61:30:db:b0:5f:3f:6d:d2:e0: + 3b:c6:4c:88:95:f5:7e:fd:cd:aa:20:5d:08:b9:6e: + 41:db:c4:ed:0d:f8:bc:cb:b4:ee:c5:87:11:05:a0: + ac:12:3b:4e:0b:4c:e4:43:e4:17:89:c1:ae:b4:13: + 58:1c:31:58:6a:f2:01:ed:df:66:e9:f9:2e:9c:c5: + 85:e6:02:db:36:f4:f3:07:39:75:30:f1:b5:55:5b: + 46:2f:87:b0:d4:a0:ab:57:df:30:45:ae:bd:b0:49: + 9a:fc:ba:5e:bc:d0:5d:86:f4:24:45:4a:d5:4d:5b: + b6:ba:e8:b7:a1:3b:c3:2f:46:2e:b3:ad:2c:63:03: + df:cb:f4:56:62:91:bd:bc:23:00:af:a2:7a:3d:6f: + f1:33:81:60:0e:bc:20:f5:8a:49:5f:ec:58:bc:64: + d5:47:36:a0:2b:b8:1f:76:25:01:89:3e:ff:52:69: + 95:03:8f:bb:14:2f:1a:38:a3:9f:c1:45:20:22:77: + 70:97:5e:25:51:b8:3d:5d:89:7a:bb:15:12:cd:1d: + 96:d2:9c:72:67:12:85:72:6e:27:7a:ef:25:da:af: + 49:26:8d:eb:a0:34:a4:4d:64:c3:63:33:77:5d:ad: + 53:c7:ee:51:32:7b:cc:43:bb:86:8d:f9:52:ba:35: + 23:0e:30:5d:dc:3b:25:63:c1:e3:5f:4b:b2:02:fc: + fe:5b:18:7f:84:aa:f3:71:e4:16:b5:98:bc:73:c5: + 58:13:41:38:eb:f3:a2:fa:8c:98:bd:f1:10:ee:b6: + fe:7e:a5:81:c7:5e:f2:72:54:8e:db:09:f0:35:42: + ca:b7:86:c2:48:b2:c6:18:08:ac:d1:f0:5d:de:b0: + b8:25:8b:3b:bd:61:48:0f:71:3f:ed:97:72:02:c9: + 44:5d:0c:00:fc:30:ca:5d:1c:e5:13:1b:3a:d0:ce: + d9:36:a0:db:f5:c2:ad:a6:95:26:4e:7b:29:2d:fc: + c4:04:1d:47:6e:03:59:68:1e:7a:20:6d:e8:a8:e1: + 3c:57:59:f8:3d:2f:16:61:7e:24:e5:13:ca:48:0a: + e6:f0:60:a3:2d:93:0b:8f:93:eb:b5:d1:06:26:52: + c0:63:1f:fc:9b:73:fe:91:c3:04:40:32:8d:09:d5: + 9e:c4:f6:0b:61:3d:9f:a1:d7:94:a2:e1:3d:b6:bb: + 60:26:74:89:33:25:18:0f:c3:88:db:10:5e:a0:5b: + f4:ee:d0:18:ab:36:50:c5:44:9b:6d:ba:ea:e2:6e: + 52:3a:55:49:a3:72:ae:04:af:1d:f6:f2:83:27:17: + 8b:9a:98:0a:f5:44:b1:c8:f2:a9:c8:ed:b0:75:ca: + 52:25:f3 + Exponent: 65537 (0x10001) + X509v3 extensions: + X509v3 Subject Key Identifier: + 74:BD:18:0B:32:AF:D0:51:8E:4C:4C:8D:B2:F6:4E:B8:6D:AB:BD:BA + X509v3 Authority Key Identifier: + keyid:01:74:A9:44:61:3D:7A:BB:C2:32:CD:D0:ED:20:DA:3A:C4:C6:02:E8 + + X509v3 Basic Constraints: + CA:FALSE + X509v3 Key Usage: + Digital Signature, Key Encipherment + Signature Algorithm: sha256WithRSAEncryption + b5:63:0c:d8:ed:af:74:2d:4c:94:36:41:05:2a:f2:ef:45:e5: + 6a:0c:76:0c:f3:90:25:e0:54:56:f3:26:23:95:7e:24:74:6b: + fd:02:0a:bc:33:ba:e8:e8:8f:a3:b3:85:2e:59:4c:cf:e3:85: + 1a:d6:70:5c:7c:86:e2:7a:11:99:a8:fa:43:9a:bf:50:54:00: + 9e:6a:7b:72:7f:c5:20:89:6e:18:6c:46:64:ce:44:44:47:4d: + 87:b5:fc:cf:f3:b9:9f:45:a3:cb:b0:91:00:96:2d:29:68:8b: + ff:c7:e0:f1:b7:8d:31:c2:01:be:5b:51:1d:af:42:b1:17:22: + bc:91:e4:d9:b9:96:6d:64:40:79:6c:71:ed:f6:e5:49:16:0a: + e3:bc:18:95:2e:89:ba:c4:a5:ce:ba:ab:3a:32:eb:bc:d8:91: + cd:f2:ee:d1:fc:67:3a:51:00:92:bd:b8:68:0b:54:04:d5:07: + 0b:97:11:2c:42:64:7c:47:c1:68:b4:eb:21:c4:e4:ad:17:a7: + 16:b9:e0:e6:cd:04:c6:89:36:40:d4:4b:c3:f7:7e:26:6b:3a: + d7:68:b3:b2:da:00:65:13:c8:fa:d0:1c:2e:10:ba:71:3e:0f: + aa:8b:d0:ff:b7:3e:83:9c:bc:b3:d1:52:0c:9f:3f:21:4a:10: + dc:8f:ab:38:45:d4:2c:2a:15:2d:71:45:fe:91:a2:d8:d9:dd: + 0c:dc:a7:d9:cd:1b:f5:35:fe:14:ba:c5:1f:ed:ee:fb:87:cc: + 87:a1:08:c2:2e:ff:5d:af:b3:3d:6e:11:94:79:0b:28:e6:83: + 4e:fc:28:8f:7f:00:85:79:7f:3a:d1:07:ee:6e:fa:94:c4:0b: + 4b:2c:05:b1:68:00:e8:37:bc:b8:b2:03:5c:5a:ca:13:f2:68: + 57:df:ac:fc:da:be:27:24:7e:6d:c4:a9:53:2d:f2:43:0e:30: + 9c:82:d5:fb:f1:a2:0a:83:e0:a5:d8:9f:09:3e:99:c8:39:d6: + 69:6d:d6:c2:27:70:59:05:3c:3c:7d:d6:41:6a:b4:9c:1f:70: + 7e:3e:ee:6f:67:de:95:1d:eb:31:8b:11:c8:0d:a1:25:4e:08: + ef:3a:11:2d:a7:98:0d:a1:d9:30:2d:da:d2:a0:05:6b:34:38: + a6:87:b2:bd:0f:9c:51:cc:e0:2e:a2:1b:a3:a0:a6:eb:1f:0a: + 22:70:59:f0:0b:c9:bd:94:4e:1d:65:3b:99:5d:8e:6c:18:82: + 1d:b5:cc:6f:14:21:c4:89:07:9b:81:1d:9a:79:ff:bf:fd:ce: + e4:77:11:0f:47:21:dc:d9:79:f3:40:26:56:5c:b4:86:32:8e: + 28:b9:14:e7:b3:fe:86:47 + \ No newline at end of file From 783ab2945538de078764cc08e4a9bb2644eaeabf Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Fri, 24 Jul 2020 15:55:48 -0700 Subject: [PATCH 25/27] Separated unit test for go1.10+ and go1.9 --- security/advancedtls/advancedtls_test.go | 80 --------------- security/advancedtls/sni_test_110.go | 108 ++++++++++++++++++++ security/advancedtls/sni_test_before_110.go | 108 ++++++++++++++++++++ 3 files changed, 216 insertions(+), 80 deletions(-) create mode 100644 security/advancedtls/sni_test_110.go create mode 100644 security/advancedtls/sni_test_before_110.go diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index 23e39dcc663..b2514547c17 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -1,5 +1,3 @@ -// +build go1.10 - /* * * Copyright 2019 gRPC authors. @@ -681,81 +679,3 @@ func TestOptionsConfig(t *testing.T) { }) } } - -func TestGetCertificatesSNI(t *testing.T) { - // Load server certificates for setting the serverGetCert callback function. - serverCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) - if err != nil { - t.Fatalf("tls.LoadX509KeyPair(server_cert_1.pem, server_key_1.pem) failed: %v", err) - } - serverCert2, err := tls.LoadX509KeyPair(testdata.Path("server_cert_2.pem"), testdata.Path("server_key_2.pem")) - if err != nil { - t.Fatalf("tls.LoadX509KeyPair(server_cert_2.pem, server_key_2.pem) failed: %v", err) - } - serverCert3, err := tls.LoadX509KeyPair(testdata.Path("server_cert_3.pem"), testdata.Path("server_key_3.pem")) - if err != nil { - t.Fatalf("tls.LoadX509KeyPair(server_cert_3.pem, server_key_3.pem) failed: %v", err) - } - - tests := []struct { - desc string - serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) - serverName string - wantCert tls.Certificate - }{ - { - desc: "Select serverCert1", - serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil - }, - // "foo.bar.com" is the common name on server certificate server_cert_1.pem. - serverName: "foo.bar.com", - wantCert: serverCert1, - }, - { - desc: "Select serverCert2", - serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil - }, - // "foo.bar.server2.com" is the common name on server certificate server_cert_2.pem. - serverName: "foo.bar.server2.com", - wantCert: serverCert2, - }, - { - desc: "Select serverCert3", - serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { - return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil - }, - // "google.com" is one of the DNS names on server certificate server_cert_3.pem. - serverName: "google.com", - wantCert: serverCert3, - }, - } - for _, test := range tests { - test := test - t.Run(test.desc, func(t *testing.T) { - serverOptions := &ServerOptions{ - GetCertificates: test.serverGetCert, - } - serverConfig, err := serverOptions.config() - if err != nil { - t.Fatalf("serverOptions.config() failed: %v", err) - } - pointFormatUncompressed := uint8(0) - clientHello := &tls.ClientHelloInfo{ - CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA}, - ServerName: test.serverName, - SupportedCurves: []tls.CurveID{tls.CurveP256}, - SupportedPoints: []uint8{pointFormatUncompressed}, - SupportedVersions: []uint16{tls.VersionTLS10}, - } - gotCertificate, err := serverConfig.GetCertificate(clientHello) - if err != nil { - t.Fatalf("serverConfig.GetCertificate(clientHello) failed: %v", err) - } - if !reflect.DeepEqual(*gotCertificate, test.wantCert) { - t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.wantCert) - } - }) - } -} diff --git a/security/advancedtls/sni_test_110.go b/security/advancedtls/sni_test_110.go new file mode 100644 index 00000000000..44e1abc1038 --- /dev/null +++ b/security/advancedtls/sni_test_110.go @@ -0,0 +1,108 @@ +// +build go1.10 + +/* + * + * Copyright 2019 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package advancedtls + +import ( + "crypto/tls" + "reflect" + "testing" + + "google.golang.org/grpc/security/advancedtls/testdata" +) + +// TestGetCertificatesSNI tests SNI logic for go1.10 and above. +func TestGetCertificatesSNI(t *testing.T) { + // Load server certificates for setting the serverGetCert callback function. + serverCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(server_cert_1.pem, server_key_1.pem) failed: %v", err) + } + serverCert2, err := tls.LoadX509KeyPair(testdata.Path("server_cert_2.pem"), testdata.Path("server_key_2.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(server_cert_2.pem, server_key_2.pem) failed: %v", err) + } + serverCert3, err := tls.LoadX509KeyPair(testdata.Path("server_cert_3.pem"), testdata.Path("server_key_3.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(server_cert_3.pem, server_key_3.pem) failed: %v", err) + } + + tests := []struct { + desc string + serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) + serverName string + wantCert tls.Certificate + }{ + { + desc: "Select serverCert1", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil + }, + // "foo.bar.com" is the common name on server certificate server_cert_1.pem. + serverName: "foo.bar.com", + wantCert: serverCert1, + }, + { + desc: "Select serverCert2", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil + }, + // "foo.bar.server2.com" is the common name on server certificate server_cert_2.pem. + serverName: "foo.bar.server2.com", + wantCert: serverCert2, + }, + { + desc: "Select serverCert3", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil + }, + // "google.com" is one of the DNS names on server certificate server_cert_3.pem. + serverName: "google.com", + wantCert: serverCert3, + }, + } + for _, test := range tests { + test := test + t.Run(test.desc, func(t *testing.T) { + serverOptions := &ServerOptions{ + GetCertificates: test.serverGetCert, + } + serverConfig, err := serverOptions.config() + if err != nil { + t.Fatalf("serverOptions.config() failed: %v", err) + } + pointFormatUncompressed := uint8(0) + clientHello := &tls.ClientHelloInfo{ + CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA}, + ServerName: test.serverName, + SupportedCurves: []tls.CurveID{tls.CurveP256}, + SupportedPoints: []uint8{pointFormatUncompressed}, + SupportedVersions: []uint16{tls.VersionTLS10}, + } + gotCertificate, err := serverConfig.GetCertificate(clientHello) + if err != nil { + t.Fatalf("serverConfig.GetCertificate(clientHello) failed: %v", err) + } + if !reflect.DeepEqual(*gotCertificate, test.wantCert) { + t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.wantCert) + } + }) + } +} diff --git a/security/advancedtls/sni_test_before_110.go b/security/advancedtls/sni_test_before_110.go new file mode 100644 index 00000000000..6c5ac80dd46 --- /dev/null +++ b/security/advancedtls/sni_test_before_110.go @@ -0,0 +1,108 @@ +// +build !go1.10 + +/* + * + * Copyright 2019 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package advancedtls + +import ( + "crypto/tls" + "reflect" + "testing" + + "google.golang.org/grpc/security/advancedtls/testdata" +) + +// TestGetCertificatesSNI tests SNI logic for go1.9. +func TestGetCertificatesSNI(t *testing.T) { + // Load server certificates for setting the serverGetCert callback function. + serverCert1, err := tls.LoadX509KeyPair(testdata.Path("server_cert_1.pem"), testdata.Path("server_key_1.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(server_cert_1.pem, server_key_1.pem) failed: %v", err) + } + serverCert2, err := tls.LoadX509KeyPair(testdata.Path("server_cert_2.pem"), testdata.Path("server_key_2.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(server_cert_2.pem, server_key_2.pem) failed: %v", err) + } + serverCert3, err := tls.LoadX509KeyPair(testdata.Path("server_cert_3.pem"), testdata.Path("server_key_3.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(server_cert_3.pem, server_key_3.pem) failed: %v", err) + } + + tests := []struct { + desc string + serverGetCert func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) + serverName string + wantCert tls.Certificate + }{ + { + desc: "Select serverCert1", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil + }, + // "foo.bar.com" is the common name on server certificate server_cert_1.pem. + serverName: "foo.bar.com", + wantCert: serverCert1, + }, + { + desc: "Select serverCert2", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil + }, + // "foo.bar.server2.com" is the common name on server certificate server_cert_2.pem. + serverName: "foo.bar.server2.com", + wantCert: serverCert1, + }, + { + desc: "Select serverCert3", + serverGetCert: func(info *tls.ClientHelloInfo) ([]*tls.Certificate, error) { + return []*tls.Certificate{&serverCert1, &serverCert2, &serverCert3}, nil + }, + // "google.com" is one of the DNS names on server certificate server_cert_3.pem. + serverName: "google.com", + wantCert: serverCert1, + }, + } + for _, test := range tests { + test := test + t.Run(test.desc, func(t *testing.T) { + serverOptions := &ServerOptions{ + GetCertificates: test.serverGetCert, + } + serverConfig, err := serverOptions.config() + if err != nil { + t.Fatalf("serverOptions.config() failed: %v", err) + } + pointFormatUncompressed := uint8(0) + clientHello := &tls.ClientHelloInfo{ + CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA}, + ServerName: test.serverName, + SupportedCurves: []tls.CurveID{tls.CurveP256}, + SupportedPoints: []uint8{pointFormatUncompressed}, + SupportedVersions: []uint16{tls.VersionTLS10}, + } + gotCertificate, err := serverConfig.GetCertificate(clientHello) + if err != nil { + t.Fatalf("serverConfig.GetCertificate(clientHello) failed: %v", err) + } + if !reflect.DeepEqual(*gotCertificate, test.wantCert) { + t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.wantCert) + } + }) + } +} From 18570bbdc641cc3fe8c3b36d25eb7a9badeaa142 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Fri, 24 Jul 2020 20:26:56 -0700 Subject: [PATCH 26/27] Changed comparing method to cmp.Equal --- security/advancedtls/go.mod | 2 +- security/advancedtls/sni_test_110.go | 4 ++-- security/advancedtls/sni_test_before_110.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/security/advancedtls/go.mod b/security/advancedtls/go.mod index 392985d7446..f21b428ef4c 100644 --- a/security/advancedtls/go.mod +++ b/security/advancedtls/go.mod @@ -4,7 +4,7 @@ go 1.13 require ( github.com/golang/protobuf v1.3.5 // indirect - github.com/google/go-cmp v0.4.0 // indirect + github.com/google/go-cmp v0.4.0 golang.org/x/net v0.0.0-20200602114024-627f9648deb9 // indirect golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980 // indirect golang.org/x/text v0.3.3 // indirect diff --git a/security/advancedtls/sni_test_110.go b/security/advancedtls/sni_test_110.go index 44e1abc1038..b5049f6e957 100644 --- a/security/advancedtls/sni_test_110.go +++ b/security/advancedtls/sni_test_110.go @@ -22,9 +22,9 @@ package advancedtls import ( "crypto/tls" - "reflect" "testing" + "github.com/google/go-cmp/cmp" "google.golang.org/grpc/security/advancedtls/testdata" ) @@ -100,7 +100,7 @@ func TestGetCertificatesSNI(t *testing.T) { if err != nil { t.Fatalf("serverConfig.GetCertificate(clientHello) failed: %v", err) } - if !reflect.DeepEqual(*gotCertificate, test.wantCert) { + if !cmp.Equal(gotCertificate, test.wantCert, cmp.AllowUnexported(tls.Certificate{}, tls.Certificate{})) { t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.wantCert) } }) diff --git a/security/advancedtls/sni_test_before_110.go b/security/advancedtls/sni_test_before_110.go index 6c5ac80dd46..53fc4eb96ba 100644 --- a/security/advancedtls/sni_test_before_110.go +++ b/security/advancedtls/sni_test_before_110.go @@ -22,9 +22,9 @@ package advancedtls import ( "crypto/tls" - "reflect" "testing" + "github.com/google/go-cmp/cmp" "google.golang.org/grpc/security/advancedtls/testdata" ) @@ -100,7 +100,7 @@ func TestGetCertificatesSNI(t *testing.T) { if err != nil { t.Fatalf("serverConfig.GetCertificate(clientHello) failed: %v", err) } - if !reflect.DeepEqual(*gotCertificate, test.wantCert) { + if !cmp.Equal(gotCertificate, test.wantCert, cmp.AllowUnexported(tls.Certificate{}, tls.Certificate{})) { t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.wantCert) } }) From d1206c6aa2b8973bb99e0d9a8608462f8b9db646 Mon Sep 17 00:00:00 2001 From: Cindy Xue Date: Mon, 27 Jul 2020 20:17:33 -0700 Subject: [PATCH 27/27] Resolved minor issues --- security/advancedtls/advancedtls.go | 7 +++---- security/advancedtls/sni_110.go | 2 +- security/advancedtls/sni_before_110.go | 2 +- security/advancedtls/sni_test_110.go | 2 +- security/advancedtls/sni_test_before_110.go | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 183678c952b..e028c623ba4 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -238,12 +238,11 @@ func (o *ServerOptions) config() (*tls.Config, error) { Certificates: o.Certificates, } if o.GetCertificates != nil { - getCertificateWithSNI := func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { - return buildGetCertificates(clientHello, o) - } // GetCertificate is only able to perform SNI logic for go1.10 and above. // It will return the first certificate in o.GetCertificates for go1.9. - config.GetCertificate = getCertificateWithSNI + config.GetCertificate = func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { + return buildGetCertificates(clientHello, o) + } } if clientCAs != nil { config.ClientCAs = clientCAs diff --git a/security/advancedtls/sni_110.go b/security/advancedtls/sni_110.go index ac3900b64d1..5c9a6ae13a0 100644 --- a/security/advancedtls/sni_110.go +++ b/security/advancedtls/sni_110.go @@ -25,7 +25,7 @@ import ( "fmt" ) -// The function buildGetCertificates returns the certificate that matches the SNI field +// 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) { if o.GetCertificates == nil { diff --git a/security/advancedtls/sni_before_110.go b/security/advancedtls/sni_before_110.go index 50b99b82f3a..180e3a05d49 100644 --- a/security/advancedtls/sni_before_110.go +++ b/security/advancedtls/sni_before_110.go @@ -25,7 +25,7 @@ import ( "fmt" ) -// The function buildGetCertificates returns the first element of o.GetCertificates. +// buildGetCertificates returns the first element of o.GetCertificates. func buildGetCertificates(clientHello *tls.ClientHelloInfo, o *ServerOptions) (*tls.Certificate, error) { if o.GetCertificates == nil { return nil, fmt.Errorf("function GetCertificates must be specified") diff --git a/security/advancedtls/sni_test_110.go b/security/advancedtls/sni_test_110.go index b5049f6e957..130ccde5905 100644 --- a/security/advancedtls/sni_test_110.go +++ b/security/advancedtls/sni_test_110.go @@ -100,7 +100,7 @@ func TestGetCertificatesSNI(t *testing.T) { if err != nil { t.Fatalf("serverConfig.GetCertificate(clientHello) failed: %v", err) } - if !cmp.Equal(gotCertificate, test.wantCert, cmp.AllowUnexported(tls.Certificate{}, tls.Certificate{})) { + if !cmp.Equal(gotCertificate, test.wantCert, cmp.AllowUnexported(tls.Certificate{})) { t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.wantCert) } }) diff --git a/security/advancedtls/sni_test_before_110.go b/security/advancedtls/sni_test_before_110.go index 53fc4eb96ba..e31e2e6ee75 100644 --- a/security/advancedtls/sni_test_before_110.go +++ b/security/advancedtls/sni_test_before_110.go @@ -100,7 +100,7 @@ func TestGetCertificatesSNI(t *testing.T) { if err != nil { t.Fatalf("serverConfig.GetCertificate(clientHello) failed: %v", err) } - if !cmp.Equal(gotCertificate, test.wantCert, cmp.AllowUnexported(tls.Certificate{}, tls.Certificate{})) { + if !cmp.Equal(gotCertificate, test.wantCert, cmp.AllowUnexported(tls.Certificate{})) { t.Errorf("GetCertificates() = %v, want %v", gotCertificate, test.wantCert) } })