From f4aaf049130857a3d50c8882011fc6c14e1909fb Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Thu, 18 Apr 2024 20:23:12 +0000 Subject: [PATCH 1/8] Rename RevocationConfig --- security/advancedtls/advancedtls.go | 6 +++--- security/advancedtls/advancedtls_test.go | 12 ++++++------ security/advancedtls/crl.go | 23 ++++++++++++++--------- security/advancedtls/crl_test.go | 12 ++++++------ 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 2aff11e7e41c..e83264520e11 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -179,7 +179,7 @@ type ClientOptions struct { VType VerificationType // RevocationConfig is the configurations for certificate revocation checks. // It could be nil if such checks are not needed. - RevocationConfig *RevocationConfig + RevocationConfig *RevocationOptions // MinVersion contains the minimum TLS version that is acceptable. // By default, TLS 1.2 is currently used as the minimum when acting as a // client, and TLS 1.0 when acting as a server. TLS 1.0 is the minimum @@ -214,7 +214,7 @@ type ServerOptions struct { VType VerificationType // RevocationConfig is the configurations for certificate revocation checks. // It could be nil if such checks are not needed. - RevocationConfig *RevocationConfig + RevocationConfig *RevocationOptions // MinVersion contains the minimum TLS version that is acceptable. // By default, TLS 1.2 is currently used as the minimum when acting as a // client, and TLS 1.0 when acting as a server. TLS 1.0 is the minimum @@ -396,7 +396,7 @@ type advancedTLSCreds struct { getRootCAs func(params *GetRootCAsParams) (*GetRootCAsResults, error) isClient bool vType VerificationType - revocationConfig *RevocationConfig + revocationConfig *RevocationOptions } func (c advancedTLSCreds) Info() credentials.ProtocolInfo { diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index f33a21a45cb3..c47b6a8a6b01 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -381,13 +381,13 @@ func (s) TestClientServerHandshake(t *testing.T) { return &GetRootCAsResults{TrustCerts: cs.ServerTrust3}, nil } - makeStaticCRLRevocationConfig := func(crlPath string, allowUndetermined bool) *RevocationConfig { + makeStaticCRLRevocationConfig := func(crlPath string, allowUndetermined bool) *RevocationOptions { rawCRL, err := os.ReadFile(crlPath) if err != nil { t.Fatalf("readFile(%v) failed err = %v", crlPath, err) } cRLProvider := NewStaticCRLProvider([][]byte{rawCRL}) - return &RevocationConfig{ + return &RevocationOptions{ AllowUndetermined: allowUndetermined, CRLProvider: cRLProvider, } @@ -407,7 +407,7 @@ func (s) TestClientServerHandshake(t *testing.T) { clientVType VerificationType clientRootProvider certprovider.Provider clientIdentityProvider certprovider.Provider - clientRevocationConfig *RevocationConfig + clientRevocationConfig *RevocationOptions clientExpectHandshakeError bool serverMutualTLS bool serverCert []tls.Certificate @@ -418,7 +418,7 @@ func (s) TestClientServerHandshake(t *testing.T) { serverVType VerificationType serverRootProvider certprovider.Provider serverIdentityProvider certprovider.Provider - serverRevocationConfig *RevocationConfig + serverRevocationConfig *RevocationOptions serverExpectError bool }{ // Client: nil setting except verifyFuncGood @@ -711,7 +711,7 @@ func (s) TestClientServerHandshake(t *testing.T) { clientGetRoot: getRootCAsForClient, clientVerifyFunc: clientVerifyFuncGood, clientVType: CertVerification, - clientRevocationConfig: &RevocationConfig{ + clientRevocationConfig: &RevocationOptions{ RootDir: testdata.Path("crl"), AllowUndetermined: true, Cache: cache, @@ -720,7 +720,7 @@ func (s) TestClientServerHandshake(t *testing.T) { serverCert: []tls.Certificate{cs.ServerCert1}, serverGetRoot: getRootCAsForServer, serverVType: CertVerification, - serverRevocationConfig: &RevocationConfig{ + serverRevocationConfig: &RevocationOptions{ RootDir: testdata.Path("crl"), AllowUndetermined: true, Cache: cache, diff --git a/security/advancedtls/crl.go b/security/advancedtls/crl.go index efd404c89737..dde75e1ca317 100644 --- a/security/advancedtls/crl.go +++ b/security/advancedtls/crl.go @@ -55,8 +55,9 @@ type Cache interface { Get(key any) (value any, ok bool) } -// RevocationConfig contains options for CRL lookup. -type RevocationConfig struct { +// RevocationOptions allows a user to configure revocation using certificate +// revocation lists (CRLs) +type RevocationOptions struct { // RootDir is the directory to search for CRL files. // Directory format must match OpenSSL X509_LOOKUP_hash_dir(3). // Deprecated: use CRLProvider instead. @@ -75,6 +76,10 @@ type RevocationConfig struct { CRLProvider CRLProvider } +// DEPRECATED: Renamed to `RevocationOptions` +// RevocationOptions contains options for CRL lookup. +type RevocationConfig = RevocationOptions + // revocationStatus is the revocation status for a certificate or chain. type revocationStatus int @@ -196,13 +201,13 @@ func x509NameHash(r pkix.RDNSequence) string { // - Delta CRL files are not supported. // - Certificate CRLDistributionPoint must be URLs, but are then ignored and converted into a file path. // - CRL checks are done after path building, which goes against RFC4158. -func checkRevocation(conn tls.ConnectionState, cfg RevocationConfig) error { +func checkRevocation(conn tls.ConnectionState, cfg RevocationOptions) error { return checkChainRevocation(conn.VerifiedChains, cfg) } // checkChainRevocation checks the verified certificate chain // for revoked certificates based on RFC5280. -func checkChainRevocation(verifiedChains [][]*x509.Certificate, cfg RevocationConfig) error { +func checkChainRevocation(verifiedChains [][]*x509.Certificate, cfg RevocationOptions) error { // Iterate the verified chains looking for one that is RevocationUnrevoked. // A single RevocationUnrevoked chain is enough to allow the connection, and a single RevocationRevoked // chain does not mean the connection should fail. @@ -232,7 +237,7 @@ func checkChainRevocation(verifiedChains [][]*x509.Certificate, cfg RevocationCo // 1. If any certificate is RevocationRevoked, return RevocationRevoked. // 2. If any certificate is RevocationUndetermined, return RevocationUndetermined. // 3. If all certificates are RevocationUnrevoked, return RevocationUnrevoked. -func checkChain(chain []*x509.Certificate, cfg RevocationConfig) revocationStatus { +func checkChain(chain []*x509.Certificate, cfg RevocationOptions) revocationStatus { chainStatus := RevocationUnrevoked for _, c := range chain { switch checkCert(c, chain, cfg) { @@ -269,7 +274,7 @@ func cachedCrl(rawIssuer []byte, cache Cache) (*CRL, bool) { } // fetchIssuerCRL fetches and verifies the CRL for rawIssuer from disk or cache if configured in cfg. -func fetchIssuerCRL(rawIssuer []byte, crlVerifyCrt []*x509.Certificate, cfg RevocationConfig) (*CRL, error) { +func fetchIssuerCRL(rawIssuer []byte, crlVerifyCrt []*x509.Certificate, cfg RevocationOptions) (*CRL, error) { if cfg.Cache != nil { if crl, ok := cachedCrl(rawIssuer, cfg.Cache); ok { return crl, nil @@ -290,7 +295,7 @@ func fetchIssuerCRL(rawIssuer []byte, crlVerifyCrt []*x509.Certificate, cfg Revo return crl, nil } -func fetchCRL(c *x509.Certificate, crlVerifyCrt []*x509.Certificate, cfg RevocationConfig) (*CRL, error) { +func fetchCRL(c *x509.Certificate, crlVerifyCrt []*x509.Certificate, cfg RevocationOptions) (*CRL, error) { if cfg.CRLProvider != nil { crl, err := cfg.CRLProvider.CRL(c) if err != nil { @@ -314,7 +319,7 @@ func fetchCRL(c *x509.Certificate, crlVerifyCrt []*x509.Certificate, cfg Revocat // RevocationUndetermined. // c is the certificate to check. // crlVerifyCrt is the group of possible certificates to verify the crl. -func checkCert(c *x509.Certificate, crlVerifyCrt []*x509.Certificate, cfg RevocationConfig) revocationStatus { +func checkCert(c *x509.Certificate, crlVerifyCrt []*x509.Certificate, cfg RevocationOptions) revocationStatus { crl, err := fetchCRL(c, crlVerifyCrt, cfg) if err != nil { // We couldn't load any valid CRL files for the certificate, so we don't @@ -486,7 +491,7 @@ func parseCRLExtensions(c *x509.RevocationList) (*CRL, error) { return certList, nil } -func fetchCRLOpenSSLHashDir(rawIssuer []byte, cfg RevocationConfig) (*CRL, error) { +func fetchCRLOpenSSLHashDir(rawIssuer []byte, cfg RevocationOptions) (*CRL, error) { var parsedCRL *CRL // 6.3.3 (a) (1) (ii) // According to X509_LOOKUP_hash_dir the format is issuer_hash.rN where N is an increasing number. diff --git a/security/advancedtls/crl_test.go b/security/advancedtls/crl_test.go index 3e06a6336e21..8d1eb39fa1eb 100644 --- a/security/advancedtls/crl_test.go +++ b/security/advancedtls/crl_test.go @@ -426,7 +426,7 @@ func TestGetIssuerCRLCache(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { cache.Purge() - _, err := fetchIssuerCRL(tt.rawIssuer, tt.certs, RevocationConfig{ + _, err := fetchIssuerCRL(tt.rawIssuer, tt.certs, RevocationOptions{ RootDir: testdata.Path("."), Cache: cache, }) @@ -601,7 +601,7 @@ func TestRevokedCert(t *testing.T) { for _, tt := range revocationTests { t.Run(fmt.Sprintf("%v with x509 crl hash dir", tt.desc), func(t *testing.T) { - err := checkRevocation(tt.in, RevocationConfig{ + err := checkRevocation(tt.in, RevocationOptions{ RootDir: testdata.Path("crl"), AllowUndetermined: tt.allowUndetermined, Cache: cache, @@ -614,7 +614,7 @@ func TestRevokedCert(t *testing.T) { } }) t.Run(fmt.Sprintf("%v with static provider", tt.desc), func(t *testing.T) { - err := checkRevocation(tt.in, RevocationConfig{ + err := checkRevocation(tt.in, RevocationOptions{ AllowUndetermined: tt.allowUndetermined, CRLProvider: cRLProvider, }) @@ -739,7 +739,7 @@ func TestVerifyConnection(t *testing.T) { cliCfg := tls.Config{ RootCAs: cp, VerifyConnection: func(cs tls.ConnectionState) error { - return checkRevocation(cs, RevocationConfig{RootDir: dir}) + return checkRevocation(cs, RevocationOptions{RootDir: dir}) }, } conn, err := tls.Dial(lis.Addr().Network(), lis.Addr().String(), &cliCfg) @@ -762,7 +762,7 @@ func TestIssuerNonPrintableString(t *testing.T) { if err != nil { t.Fatalf("failed to decode issuer: %s", err) } - _, err = fetchCRLOpenSSLHashDir(rawIssuer, RevocationConfig{RootDir: testdata.Path("crl")}) + _, err = fetchCRLOpenSSLHashDir(rawIssuer, RevocationOptions{RootDir: testdata.Path("crl")}) if err != nil { t.Fatalf("fetchCRL failed: %s", err) } @@ -792,7 +792,7 @@ func TestCRLCacheExpirationReloading(t *testing.T) { crl.certList.RevokedCertificates = nil crl.certList.NextUpdate = time.Now().Add(time.Hour) cache.Add(hex.EncodeToString(rawIssuer), crl) - var cfg = RevocationConfig{RootDir: testdata.Path("crl"), Cache: cache} + var cfg = RevocationOptions{RootDir: testdata.Path("crl"), Cache: cache} revocationStatus := checkChain(certs, cfg) if revocationStatus != RevocationUnrevoked { t.Fatalf("Certificate check should be RevocationUnrevoked, was %v", revocationStatus) From 151d843651ac238877b3854a3acb2289e2048209 Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Thu, 18 Apr 2024 21:01:04 +0000 Subject: [PATCH 2/8] replace usages --- security/advancedtls/advancedtls.go | 12 ++-- security/advancedtls/advancedtls_test.go | 80 ++++++++++++------------ security/advancedtls/crl.go | 2 +- security/advancedtls/crl_provider.go | 2 +- 4 files changed, 48 insertions(+), 48 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index e83264520e11..c14096b14661 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -177,9 +177,9 @@ type ClientOptions struct { RootOptions RootCertificateOptions // VType is the verification type on the client side. VType VerificationType - // RevocationConfig is the configurations for certificate revocation checks. + // RevocationOptions is the configurations for certificate revocation checks. // It could be nil if such checks are not needed. - RevocationConfig *RevocationOptions + RevocationOptions *RevocationOptions // MinVersion contains the minimum TLS version that is acceptable. // By default, TLS 1.2 is currently used as the minimum when acting as a // client, and TLS 1.0 when acting as a server. TLS 1.0 is the minimum @@ -212,9 +212,9 @@ type ServerOptions struct { RequireClientCert bool // VType is the verification type on the server side. VType VerificationType - // RevocationConfig is the configurations for certificate revocation checks. + // RevocationOptions is the configurations for certificate revocation checks. // It could be nil if such checks are not needed. - RevocationConfig *RevocationOptions + RevocationOptions *RevocationOptions // MinVersion contains the minimum TLS version that is acceptable. // By default, TLS 1.2 is currently used as the minimum when acting as a // client, and TLS 1.0 when acting as a server. TLS 1.0 is the minimum @@ -580,7 +580,7 @@ func NewClientCreds(o *ClientOptions) (credentials.TransportCredentials, error) getRootCAs: o.RootOptions.GetRootCertificates, verifyFunc: o.VerifyPeer, vType: o.VType, - revocationConfig: o.RevocationConfig, + revocationConfig: o.RevocationOptions, } tc.config.NextProtos = credinternal.AppendH2ToNextProtos(tc.config.NextProtos) return tc, nil @@ -599,7 +599,7 @@ func NewServerCreds(o *ServerOptions) (credentials.TransportCredentials, error) getRootCAs: o.RootOptions.GetRootCertificates, verifyFunc: o.VerifyPeer, vType: o.VType, - revocationConfig: o.RevocationConfig, + revocationConfig: o.RevocationOptions, } tc.config.NextProtos = credinternal.AppendH2ToNextProtos(tc.config.NextProtos) return tc, nil diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index c47b6a8a6b01..7a239a9ffccd 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -381,7 +381,7 @@ func (s) TestClientServerHandshake(t *testing.T) { return &GetRootCAsResults{TrustCerts: cs.ServerTrust3}, nil } - makeStaticCRLRevocationConfig := func(crlPath string, allowUndetermined bool) *RevocationOptions { + makeStaticCRLRevocationOptions := func(crlPath string, allowUndetermined bool) *RevocationOptions { rawCRL, err := os.ReadFile(crlPath) if err != nil { t.Fatalf("readFile(%v) failed err = %v", crlPath, err) @@ -407,7 +407,7 @@ func (s) TestClientServerHandshake(t *testing.T) { clientVType VerificationType clientRootProvider certprovider.Provider clientIdentityProvider certprovider.Provider - clientRevocationConfig *RevocationOptions + clientRevocationOptions *RevocationOptions clientExpectHandshakeError bool serverMutualTLS bool serverCert []tls.Certificate @@ -418,7 +418,7 @@ func (s) TestClientServerHandshake(t *testing.T) { serverVType VerificationType serverRootProvider certprovider.Provider serverIdentityProvider certprovider.Provider - serverRevocationConfig *RevocationOptions + serverRevocationOptions *RevocationOptions serverExpectError bool }{ // Client: nil setting except verifyFuncGood @@ -711,7 +711,7 @@ func (s) TestClientServerHandshake(t *testing.T) { clientGetRoot: getRootCAsForClient, clientVerifyFunc: clientVerifyFuncGood, clientVType: CertVerification, - clientRevocationConfig: &RevocationOptions{ + clientRevocationOptions: &RevocationOptions{ RootDir: testdata.Path("crl"), AllowUndetermined: true, Cache: cache, @@ -720,7 +720,7 @@ func (s) TestClientServerHandshake(t *testing.T) { serverCert: []tls.Certificate{cs.ServerCert1}, serverGetRoot: getRootCAsForServer, serverVType: CertVerification, - serverRevocationConfig: &RevocationOptions{ + serverRevocationOptions: &RevocationOptions{ RootDir: testdata.Path("crl"), AllowUndetermined: true, Cache: cache, @@ -730,49 +730,49 @@ func (s) TestClientServerHandshake(t *testing.T) { // Server: set valid credentials with the revocation config // Expected Behavior: success, because none of the certificate chains sent in the connection are revoked { - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; Client uses CRL; mutualTLS", - clientCert: []tls.Certificate{cs.ClientCertForCRL}, - clientGetRoot: getRootCAsForClientCRL, - clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, - clientRevocationConfig: makeStaticCRLRevocationConfig(testdata.Path("crl/provider_crl_empty.pem"), true), - serverMutualTLS: true, - serverCert: []tls.Certificate{cs.ServerCertForCRL}, - serverGetRoot: getRootCAsForServerCRL, - serverVType: CertVerification, + desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; Client uses CRL; mutualTLS", + clientCert: []tls.Certificate{cs.ClientCertForCRL}, + clientGetRoot: getRootCAsForClientCRL, + clientVerifyFunc: clientVerifyFuncGood, + clientVType: CertVerification, + clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_crl_empty.pem"), true), + serverMutualTLS: true, + serverCert: []tls.Certificate{cs.ServerCertForCRL}, + serverGetRoot: getRootCAsForServerCRL, + serverVType: CertVerification, }, // Client: set valid credentials with the revocation config // Server: set revoked credentials with the revocation config // Expected Behavior: fail, server creds are revoked { - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets revoked cert; Client uses CRL; mutualTLS", - clientCert: []tls.Certificate{cs.ClientCertForCRL}, - clientGetRoot: getRootCAsForClientCRL, - clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, - clientRevocationConfig: makeStaticCRLRevocationConfig(testdata.Path("crl/provider_crl_server_revoked.pem"), true), - serverMutualTLS: true, - serverCert: []tls.Certificate{cs.ServerCertForCRL}, - serverGetRoot: getRootCAsForServerCRL, - serverVType: CertVerification, - serverExpectError: true, + desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets revoked cert; Client uses CRL; mutualTLS", + clientCert: []tls.Certificate{cs.ClientCertForCRL}, + clientGetRoot: getRootCAsForClientCRL, + clientVerifyFunc: clientVerifyFuncGood, + clientVType: CertVerification, + clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_crl_server_revoked.pem"), true), + serverMutualTLS: true, + serverCert: []tls.Certificate{cs.ServerCertForCRL}, + serverGetRoot: getRootCAsForServerCRL, + serverVType: CertVerification, + serverExpectError: true, }, // Client: set valid credentials with the revocation config // Server: set valid credentials with the revocation config // Expected Behavior: fail, because CRL is issued by the malicious CA. It // can't be properly processed, and we don't allow RevocationUndetermined. { - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; Client uses CRL; mutualTLS", - clientCert: []tls.Certificate{cs.ClientCertForCRL}, - clientGetRoot: getRootCAsForClientCRL, - clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, - clientRevocationConfig: makeStaticCRLRevocationConfig(testdata.Path("crl/provider_malicious_crl_empty.pem"), false), - serverMutualTLS: true, - serverCert: []tls.Certificate{cs.ServerCertForCRL}, - serverGetRoot: getRootCAsForServerCRL, - serverVType: CertVerification, - serverExpectError: true, + desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; Client uses CRL; mutualTLS", + clientCert: []tls.Certificate{cs.ClientCertForCRL}, + clientGetRoot: getRootCAsForClientCRL, + clientVerifyFunc: clientVerifyFuncGood, + clientVType: CertVerification, + clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_malicious_crl_empty.pem"), false), + serverMutualTLS: true, + serverCert: []tls.Certificate{cs.ServerCertForCRL}, + serverGetRoot: getRootCAsForServerCRL, + serverVType: CertVerification, + serverExpectError: true, }, } { test := test @@ -797,7 +797,7 @@ func (s) TestClientServerHandshake(t *testing.T) { RequireClientCert: test.serverMutualTLS, VerifyPeer: test.serverVerifyFunc, VType: test.serverVType, - RevocationConfig: test.serverRevocationConfig, + RevocationOptions: test.serverRevocationOptions, } go func(done chan credentials.AuthInfo, lis net.Listener, serverOptions *ServerOptions) { serverRawConn, err := lis.Accept() @@ -839,8 +839,8 @@ func (s) TestClientServerHandshake(t *testing.T) { GetRootCertificates: test.clientGetRoot, RootProvider: test.clientRootProvider, }, - VType: test.clientVType, - RevocationConfig: test.clientRevocationConfig, + VType: test.clientVType, + RevocationOptions: test.clientRevocationOptions, } clientTLS, err := NewClientCreds(clientOptions) if err != nil { diff --git a/security/advancedtls/crl.go b/security/advancedtls/crl.go index dde75e1ca317..ce1c71b9a657 100644 --- a/security/advancedtls/crl.go +++ b/security/advancedtls/crl.go @@ -77,7 +77,7 @@ type RevocationOptions struct { } // DEPRECATED: Renamed to `RevocationOptions` -// RevocationOptions contains options for CRL lookup. +// RevocationConfig contains options for CRL lookup. type RevocationConfig = RevocationOptions // revocationStatus is the revocation status for a certificate or chain. diff --git a/security/advancedtls/crl_provider.go b/security/advancedtls/crl_provider.go index 6777be0c5554..47dae3d049e4 100644 --- a/security/advancedtls/crl_provider.go +++ b/security/advancedtls/crl_provider.go @@ -34,7 +34,7 @@ const minCRLRefreshDuration = 1 * time.Minute // // The interface defines how gRPC gets CRLs from the provider during handshakes, // but doesn't prescribe a specific way to load and store CRLs. Such -// implementations can be used in RevocationConfig of advancedtls.ClientOptions +// implementations can be used in RevocationOptions of advancedtls.ClientOptions // and/or advancedtls.ServerOptions. // Please note that checking CRLs is directly on the path of connection // establishment, so implementations of the CRL function need to be fast, and From b6874338bce4070c7bcb4d6aa16a28da32f33de7 Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Fri, 19 Apr 2024 19:39:58 +0000 Subject: [PATCH 3/8] fix tests --- security/advancedtls/advancedtls_test.go | 77 +++--------------------- 1 file changed, 9 insertions(+), 68 deletions(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index ab796a76e4d9..99cbf4cff478 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -734,38 +734,21 @@ func (s) TestClientServerHandshake(t *testing.T) { // Server: set valid credentials with the revocation config // Expected Behavior: success, because none of the certificate chains sent in the connection are revoked { -<<<<<<< HEAD desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", clientCert: []tls.Certificate{cs.ClientCert1}, clientGetRoot: getRootCAsForClient, clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, - clientRevocationOptions: &RevocationOptions{ -======= - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", - clientCert: []tls.Certificate{cs.ClientCert1}, - clientGetRoot: getRootCAsForClient, - clientVerifyFunc: clientVerifyFuncGood, clientVerificationType: CertVerification, - clientRevocationConfig: &RevocationConfig{ ->>>>>>> master + clientRevocationOptions: &RevocationOptions{ RootDir: testdata.Path("crl"), AllowUndetermined: true, Cache: cache, }, -<<<<<<< HEAD serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCert1}, serverGetRoot: getRootCAsForServer, - serverVType: CertVerification, - serverRevocationOptions: &RevocationOptions{ -======= - serverMutualTLS: true, - serverCert: []tls.Certificate{cs.ServerCert1}, - serverGetRoot: getRootCAsForServer, serverVerificationType: CertVerification, - serverRevocationConfig: &RevocationConfig{ ->>>>>>> master + serverRevocationOptions: &RevocationOptions{ RootDir: testdata.Path("crl"), AllowUndetermined: true, Cache: cache, @@ -775,90 +758,49 @@ func (s) TestClientServerHandshake(t *testing.T) { // Server: set valid credentials with the revocation config // Expected Behavior: success, because none of the certificate chains sent in the connection are revoked { -<<<<<<< HEAD desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; Client uses CRL; mutualTLS", clientCert: []tls.Certificate{cs.ClientCertForCRL}, clientGetRoot: getRootCAsForClientCRL, clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, + clientVerificationType: CertVerification, clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_crl_empty.pem"), true), serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCertForCRL}, serverGetRoot: getRootCAsForServerCRL, - serverVType: CertVerification, -======= - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; Client uses CRL; mutualTLS", - clientCert: []tls.Certificate{cs.ClientCertForCRL}, - clientGetRoot: getRootCAsForClientCRL, - clientVerifyFunc: clientVerifyFuncGood, - clientVerificationType: CertVerification, - clientRevocationConfig: makeStaticCRLRevocationConfig(testdata.Path("crl/provider_crl_empty.pem"), true), - serverMutualTLS: true, - serverCert: []tls.Certificate{cs.ServerCertForCRL}, - serverGetRoot: getRootCAsForServerCRL, - serverVerificationType: CertVerification, ->>>>>>> master + serverVerificationType: CertVerification, }, // Client: set valid credentials with the revocation config // Server: set revoked credentials with the revocation config // Expected Behavior: fail, server creds are revoked { -<<<<<<< HEAD desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets revoked cert; Client uses CRL; mutualTLS", clientCert: []tls.Certificate{cs.ClientCertForCRL}, clientGetRoot: getRootCAsForClientCRL, clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, + clientVerificationType: CertVerification, clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_crl_server_revoked.pem"), true), serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCertForCRL}, serverGetRoot: getRootCAsForServerCRL, - serverVType: CertVerification, - serverExpectError: true, -======= - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets revoked cert; Client uses CRL; mutualTLS", - clientCert: []tls.Certificate{cs.ClientCertForCRL}, - clientGetRoot: getRootCAsForClientCRL, - clientVerifyFunc: clientVerifyFuncGood, - clientVerificationType: CertVerification, - clientRevocationConfig: makeStaticCRLRevocationConfig(testdata.Path("crl/provider_crl_server_revoked.pem"), true), - serverMutualTLS: true, - serverCert: []tls.Certificate{cs.ServerCertForCRL}, - serverGetRoot: getRootCAsForServerCRL, serverVerificationType: CertVerification, - serverExpectError: true, ->>>>>>> master + serverExpectError: true, }, // Client: set valid credentials with the revocation config // Server: set valid credentials with the revocation config // Expected Behavior: fail, because CRL is issued by the malicious CA. It // can't be properly processed, and we don't allow RevocationUndetermined. { -<<<<<<< HEAD desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; Client uses CRL; mutualTLS", clientCert: []tls.Certificate{cs.ClientCertForCRL}, clientGetRoot: getRootCAsForClientCRL, clientVerifyFunc: clientVerifyFuncGood, - clientVType: CertVerification, + clientVerificationType: CertVerification, clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_malicious_crl_empty.pem"), false), serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCertForCRL}, serverGetRoot: getRootCAsForServerCRL, - serverVType: CertVerification, - serverExpectError: true, -======= - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; Client uses CRL; mutualTLS", - clientCert: []tls.Certificate{cs.ClientCertForCRL}, - clientGetRoot: getRootCAsForClientCRL, - clientVerifyFunc: clientVerifyFuncGood, - clientVerificationType: CertVerification, - clientRevocationConfig: makeStaticCRLRevocationConfig(testdata.Path("crl/provider_malicious_crl_empty.pem"), false), - serverMutualTLS: true, - serverCert: []tls.Certificate{cs.ServerCertForCRL}, - serverGetRoot: getRootCAsForServerCRL, serverVerificationType: CertVerification, - serverExpectError: true, ->>>>>>> master + serverExpectError: true, }, } { test := test @@ -883,10 +825,9 @@ func (s) TestClientServerHandshake(t *testing.T) { RequireClientCert: test.serverMutualTLS, VerifyPeer: test.serverVerifyFunc, <<<<<<< HEAD - VType: test.serverVType, + VerificationType: test.serverVerificationType, RevocationOptions: test.serverRevocationOptions, ======= - VerificationType: test.serverVerificationType, RevocationConfig: test.serverRevocationConfig, >>>>>>> master } From 6bb3f5c90e3cf546c78a57f1620d39869ba05d5a Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Fri, 19 Apr 2024 19:49:44 +0000 Subject: [PATCH 4/8] didn't actually save the file before committing --- security/advancedtls/advancedtls_test.go | 37 +++++++++--------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index 99cbf4cff478..8b1e4b7332f3 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -734,19 +734,19 @@ func (s) TestClientServerHandshake(t *testing.T) { // Server: set valid credentials with the revocation config // Expected Behavior: success, because none of the certificate chains sent in the connection are revoked { - desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", - clientCert: []tls.Certificate{cs.ClientCert1}, - clientGetRoot: getRootCAsForClient, - clientVerifyFunc: clientVerifyFuncGood, + desc: "Client sets peer cert, reload root function with verifyFuncGood; Server sets peer cert, reload root function; mutualTLS", + clientCert: []tls.Certificate{cs.ClientCert1}, + clientGetRoot: getRootCAsForClient, + clientVerifyFunc: clientVerifyFuncGood, clientVerificationType: CertVerification, clientRevocationOptions: &RevocationOptions{ RootDir: testdata.Path("crl"), AllowUndetermined: true, Cache: cache, }, - serverMutualTLS: true, - serverCert: []tls.Certificate{cs.ServerCert1}, - serverGetRoot: getRootCAsForServer, + serverMutualTLS: true, + serverCert: []tls.Certificate{cs.ServerCert1}, + serverGetRoot: getRootCAsForServer, serverVerificationType: CertVerification, serverRevocationOptions: &RevocationOptions{ RootDir: testdata.Path("crl"), @@ -762,12 +762,12 @@ func (s) TestClientServerHandshake(t *testing.T) { clientCert: []tls.Certificate{cs.ClientCertForCRL}, clientGetRoot: getRootCAsForClientCRL, clientVerifyFunc: clientVerifyFuncGood, - clientVerificationType: CertVerification, + clientVerificationType: CertVerification, clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_crl_empty.pem"), true), serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCertForCRL}, serverGetRoot: getRootCAsForServerCRL, - serverVerificationType: CertVerification, + serverVerificationType: CertVerification, }, // Client: set valid credentials with the revocation config // Server: set revoked credentials with the revocation config @@ -777,12 +777,12 @@ func (s) TestClientServerHandshake(t *testing.T) { clientCert: []tls.Certificate{cs.ClientCertForCRL}, clientGetRoot: getRootCAsForClientCRL, clientVerifyFunc: clientVerifyFuncGood, - clientVerificationType: CertVerification, + clientVerificationType: CertVerification, clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_crl_server_revoked.pem"), true), serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCertForCRL}, serverGetRoot: getRootCAsForServerCRL, - serverVerificationType: CertVerification, + serverVerificationType: CertVerification, serverExpectError: true, }, // Client: set valid credentials with the revocation config @@ -794,12 +794,12 @@ func (s) TestClientServerHandshake(t *testing.T) { clientCert: []tls.Certificate{cs.ClientCertForCRL}, clientGetRoot: getRootCAsForClientCRL, clientVerifyFunc: clientVerifyFuncGood, - clientVerificationType: CertVerification, + clientVerificationType: CertVerification, clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_malicious_crl_empty.pem"), false), serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCertForCRL}, serverGetRoot: getRootCAsForServerCRL, - serverVerificationType: CertVerification, + serverVerificationType: CertVerification, serverExpectError: true, }, } { @@ -824,12 +824,8 @@ func (s) TestClientServerHandshake(t *testing.T) { }, RequireClientCert: test.serverMutualTLS, VerifyPeer: test.serverVerifyFunc, -<<<<<<< HEAD VerificationType: test.serverVerificationType, RevocationOptions: test.serverRevocationOptions, -======= - RevocationConfig: test.serverRevocationConfig, ->>>>>>> master } go func(done chan credentials.AuthInfo, lis net.Listener, serverOptions *ServerOptions) { serverRawConn, err := lis.Accept() @@ -871,13 +867,8 @@ func (s) TestClientServerHandshake(t *testing.T) { GetRootCertificates: test.clientGetRoot, RootProvider: test.clientRootProvider, }, -<<<<<<< HEAD - VType: test.clientVType, + VerificationType: test.clientVerificationType, RevocationOptions: test.clientRevocationOptions, -======= - VerificationType: test.clientVerificationType, - RevocationConfig: test.clientRevocationConfig, ->>>>>>> master } clientTLS, err := NewClientCreds(clientOptions) if err != nil { From f18fe5fff32f71c38dfe85b339d584249165416b Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Mon, 22 Apr 2024 18:57:45 +0000 Subject: [PATCH 5/8] fix deprecation notice --- security/advancedtls/crl.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/advancedtls/crl.go b/security/advancedtls/crl.go index ce1c71b9a657..fb802fe4e986 100644 --- a/security/advancedtls/crl.go +++ b/security/advancedtls/crl.go @@ -76,8 +76,9 @@ type RevocationOptions struct { CRLProvider CRLProvider } -// DEPRECATED: Renamed to `RevocationOptions` // RevocationConfig contains options for CRL lookup. +// +// Deprecated: use RevocationOptions instead. type RevocationConfig = RevocationOptions // revocationStatus is the revocation status for a certificate or chain. From 78a7818ab2abced7aad0f8fd62aabc34ca41e578 Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Tue, 23 Apr 2024 15:51:47 +0000 Subject: [PATCH 6/8] reword comment --- security/advancedtls/crl.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/security/advancedtls/crl.go b/security/advancedtls/crl.go index fb802fe4e986..fc01fa9e60b3 100644 --- a/security/advancedtls/crl.go +++ b/security/advancedtls/crl.go @@ -55,8 +55,7 @@ type Cache interface { Get(key any) (value any, ok bool) } -// RevocationOptions allows a user to configure revocation using certificate -// revocation lists (CRLs) +// RevocationOptions allows a user to configure certificate revocation behavior. type RevocationOptions struct { // RootDir is the directory to search for CRL files. // Directory format must match OpenSSL X509_LOOKUP_hash_dir(3). From 6cd5be4b668e68fe544c7ed8e7e115c6a390dfae Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Tue, 23 Apr 2024 20:07:37 +0000 Subject: [PATCH 7/8] didn't save merge conflict resolution --- security/advancedtls/advancedtls.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index d0915103fd09..e2542d39bc7e 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -459,7 +459,7 @@ func (o *ServerOptions) config() (*tls.Config, error) { // using TLS. type advancedTLSCreds struct { config *tls.Config - verifyFunc PostHandshakeVerificationFunc + verifyFunc PostHandshakeVerificationFunc getRootCAs func(params *GetRootCAsParams) (*GetRootCAsResults, error) isClient bool revocationOptions *RevocationOptions @@ -645,7 +645,7 @@ func NewClientCreds(o *ClientOptions) (credentials.TransportCredentials, error) config: conf, isClient: true, getRootCAs: o.RootOptions.GetRootCertificates, - verifyFunc: o.AdditionalPeerVerification, + verifyFunc: o.AdditionalPeerVerification, revocationOptions: o.RevocationOptions, verificationType: o.VerificationType, } @@ -661,20 +661,12 @@ func NewServerCreds(o *ServerOptions) (credentials.TransportCredentials, error) return nil, err } tc := &advancedTLSCreds{ -<<<<<<< HEAD config: conf, isClient: false, getRootCAs: o.RootOptions.GetRootCertificates, - verifyFunc: o.AdditionalPeerVerification, + verifyFunc: o.AdditionalPeerVerification, revocationOptions: o.RevocationOptions, verificationType: o.VerificationType, -======= - config: conf, - isClient: false, - getRootCAs: o.RootOptions.GetRootCertificates, - verificationType: o.VerificationType, - revocationConfig: o.RevocationConfig, ->>>>>>> master } tc.config.NextProtos = credinternal.AppendH2ToNextProtos(tc.config.NextProtos) return tc, nil From 3d0fbc4b4f8c318fed65f513c6bf9740c4691365 Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Tue, 23 Apr 2024 20:11:22 +0000 Subject: [PATCH 8/8] fix vet error --- security/advancedtls/advancedtls.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index e2542d39bc7e..2e8efe51521d 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -67,6 +67,8 @@ type VerificationFuncParams = HandshakeVerificationInfo // future to include more information. type PostHandshakeVerificationResults struct{} +// VerificationResults contains the information about results of +// PostHandshakeVerificationFunc. // Deprecated: use PostHandshakeVerificationResults instead. type VerificationResults = PostHandshakeVerificationResults