grpc
/
grpc-go
mirror of https://github.com/grpc/grpc-go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

advancedtls: unexport parts of API not meant to be public (#7118)

This commit is contained in:
Gregory Cooke 2024-04-17 11:32:56 -04:00 committed by GitHub
parent 006e2bad13
commit fc8da03081
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 26 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
security/advancedtls/advancedtls.go
View File

@ -172,10 +172,6 @@ type ClientOptions struct {
// If this is set, we will perform this customized check after doing the // If this is set, we will perform this customized check after doing the
// normal check(s) indicated by setting VType. // normal check(s) indicated by setting VType.
VerifyPeer CustomVerificationFunc VerifyPeer CustomVerificationFunc
// ServerNameOverride is for testing only. If set to a non-empty string,
// it will override the virtual host name of authority (e.g. :authority
// header field) in requests.
ServerNameOverride string
// RootOptions is OPTIONAL on client side. If not set, we will try to use the // RootOptions is OPTIONAL on client side. If not set, we will try to use the
// default trust certificates in users' OS system. // default trust certificates in users' OS system.
RootOptions RootCertificateOptions RootOptions RootCertificateOptions
@ -193,6 +189,11 @@ type ClientOptions struct {
// By default, the maximum version supported by this package is used, // By default, the maximum version supported by this package is used,
// which is currently TLS 1.3. // which is currently TLS 1.3.
MaxVersion uint16 MaxVersion uint16
// serverNameOverride is for testing only. If set to a non-empty string, it
// will override the virtual host name of authority (e.g. :authority header
// field) in requests and the target hostname used during server cert
// verification.
serverNameOverride string
} }
// ServerOptions contains the fields needed to be filled by the server. // ServerOptions contains the fields needed to be filled by the server.
@ -244,7 +245,7 @@ func (o *ClientOptions) config() (*tls.Config, error) {
return nil, fmt.Errorf("the minimum TLS version is larger than the maximum TLS version") return nil, fmt.Errorf("the minimum TLS version is larger than the maximum TLS version")
} }
config := &tls.Config{ config := &tls.Config{
ServerName: o.ServerNameOverride, ServerName: o.serverNameOverride,
// We have to set InsecureSkipVerify to true to skip the default checks and // We have to set InsecureSkipVerify to true to skip the default checks and
// use the verification function we built from buildVerifyFunc. // use the verification function we built from buildVerifyFunc.
InsecureSkipVerify: true, InsecureSkipVerify: true,
@ -548,7 +549,7 @@ func buildVerifyFunc(c *advancedTLSCreds,
if verifiedChains == nil { if verifiedChains == nil {
verifiedChains = [][]*x509.Certificate{rawCertList} verifiedChains = [][]*x509.Certificate{rawCertList}
} }
if err := CheckChainRevocation(verifiedChains, *c.revocationConfig); err != nil { if err := checkChainRevocation(verifiedChains, *c.revocationConfig); err != nil {
return err return err
} }
} }

2
security/advancedtls/advancedtls_test.go
View File

@ -902,7 +902,7 @@ func (s) TestAdvancedTLSOverrideServerName(t *testing.T) {
RootOptions: RootCertificateOptions{ RootOptions: RootCertificateOptions{
RootCACerts: cs.ClientTrust1, RootCACerts: cs.ClientTrust1,
}, },
ServerNameOverride: expectedServerName, serverNameOverride: expectedServerName,
} }
c, err := NewClientCreds(clientOptions) c, err := NewClientCreds(clientOptions)
if err != nil { if err != nil {

28
security/advancedtls/crl.go
View File

@ -75,22 +75,18 @@ type RevocationConfig struct {
CRLProvider CRLProvider CRLProvider CRLProvider
} }
// RevocationStatus is the revocation status for a certificate or chain. // revocationStatus is the revocation status for a certificate or chain.
type RevocationStatus int type revocationStatus int
const ( const (
// RevocationUndetermined means we couldn't find or verify a CRL for the cert. // RevocationUndetermined means we couldn't find or verify a CRL for the cert.
RevocationUndetermined RevocationStatus = iota RevocationUndetermined revocationStatus = iota
// RevocationUnrevoked means we found the CRL for the cert and the cert is not revoked. // RevocationUnrevoked means we found the CRL for the cert and the cert is not revoked.
RevocationUnrevoked RevocationUnrevoked
// RevocationRevoked means we found the CRL and the cert is revoked. // RevocationRevoked means we found the CRL and the cert is revoked.
RevocationRevoked RevocationRevoked
) )
func (s RevocationStatus) String() string {
return [...]string{"RevocationUndetermined", "RevocationUnrevoked", "RevocationRevoked"}[s]
}
// CRL contains a pkix.CertificateList and parsed extensions that aren't // CRL contains a pkix.CertificateList and parsed extensions that aren't
// provided by the golang CRL parser. // provided by the golang CRL parser.
// All CRLs should be loaded using NewCRL() for bytes directly or ReadCRLFile() // All CRLs should be loaded using NewCRL() for bytes directly or ReadCRLFile()
@ -192,7 +188,7 @@ func x509NameHash(r pkix.RDNSequence) string {
return fmt.Sprintf("%08x", fileHash) return fmt.Sprintf("%08x", fileHash)
} }
// CheckRevocation checks the connection for revoked certificates based on RFC5280. // checkRevocation checks the connection for revoked certificates based on RFC5280.
// This implementation has the following major limitations: // This implementation has the following major limitations:
// - Indirect CRL files are not supported. // - Indirect CRL files are not supported.
// - CRL loading is only supported from directories in the X509_LOOKUP_hash_dir format. // - CRL loading is only supported from directories in the X509_LOOKUP_hash_dir format.
@ -200,17 +196,17 @@ func x509NameHash(r pkix.RDNSequence) string {
// - Delta CRL files are not supported. // - Delta CRL files are not supported.
// - Certificate CRLDistributionPoint must be URLs, but are then ignored and converted into a file path. // - 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. // - 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 RevocationConfig) error {
return CheckChainRevocation(conn.VerifiedChains, cfg) return checkChainRevocation(conn.VerifiedChains, cfg)
} }
// CheckChainRevocation checks the verified certificate chain // checkChainRevocation checks the verified certificate chain
// for revoked certificates based on RFC5280. // for revoked certificates based on RFC5280.
func CheckChainRevocation(verifiedChains [][]*x509.Certificate, cfg RevocationConfig) error { func checkChainRevocation(verifiedChains [][]*x509.Certificate, cfg RevocationConfig) error {
// Iterate the verified chains looking for one that is RevocationUnrevoked. // Iterate the verified chains looking for one that is RevocationUnrevoked.
// A single RevocationUnrevoked chain is enough to allow the connection, and a single RevocationRevoked // A single RevocationUnrevoked chain is enough to allow the connection, and a single RevocationRevoked
// chain does not mean the connection should fail. // chain does not mean the connection should fail.
count := make(map[RevocationStatus]int) count := make(map[revocationStatus]int)
for _, chain := range verifiedChains { for _, chain := range verifiedChains {
switch checkChain(chain, cfg) { switch checkChain(chain, cfg) {
case RevocationUnrevoked: case RevocationUnrevoked:
@ -236,7 +232,7 @@ func CheckChainRevocation(verifiedChains [][]*x509.Certificate, cfg RevocationCo
// 1. If any certificate is RevocationRevoked, return RevocationRevoked. // 1. If any certificate is RevocationRevoked, return RevocationRevoked.
// 2. If any certificate is RevocationUndetermined, return RevocationUndetermined. // 2. If any certificate is RevocationUndetermined, return RevocationUndetermined.
// 3. If all certificates are RevocationUnrevoked, return RevocationUnrevoked. // 3. If all certificates are RevocationUnrevoked, return RevocationUnrevoked.
func checkChain(chain []*x509.Certificate, cfg RevocationConfig) RevocationStatus { func checkChain(chain []*x509.Certificate, cfg RevocationConfig) revocationStatus {
chainStatus := RevocationUnrevoked chainStatus := RevocationUnrevoked
for _, c := range chain { for _, c := range chain {
switch checkCert(c, chain, cfg) { switch checkCert(c, chain, cfg) {
@ -318,7 +314,7 @@ func fetchCRL(c *x509.Certificate, crlVerifyCrt []*x509.Certificate, cfg Revocat
// RevocationUndetermined. // RevocationUndetermined.
// c is the certificate to check. // c is the certificate to check.
// crlVerifyCrt is the group of possible certificates to verify the crl. // 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 RevocationConfig) revocationStatus {
crl, err := fetchCRL(c, crlVerifyCrt, cfg) crl, err := fetchCRL(c, crlVerifyCrt, cfg)
if err != nil { if err != nil {
// We couldn't load any valid CRL files for the certificate, so we don't // We couldn't load any valid CRL files for the certificate, so we don't
@ -343,7 +339,7 @@ func checkCert(c *x509.Certificate, crlVerifyCrt []*x509.Certificate, cfg Revoca
return revocation return revocation
} }
func checkCertRevocation(c *x509.Certificate, crl *CRL) (RevocationStatus, error) { func checkCertRevocation(c *x509.Certificate, crl *CRL) (revocationStatus, error) {
// Per section 5.3.3 we prime the certificate issuer with the CRL issuer. // Per section 5.3.3 we prime the certificate issuer with the CRL issuer.
// Subsequent entries use the previous entry's issuer. // Subsequent entries use the previous entry's issuer.
rawEntryIssuer := crl.rawIssuer rawEntryIssuer := crl.rawIssuer

12
security/advancedtls/crl_test.go
View File

@ -229,7 +229,7 @@ qsSIp8gfxSyzkJP+Ngkm2DdLjlJQCZ9R0MZP9Xj4
var revocationTests = []struct { var revocationTests = []struct {
desc string desc string
in x509.Certificate in x509.Certificate
revoked RevocationStatus revoked revocationStatus
}{ }{
{ {
desc: "Single revoked", desc: "Single revoked",
@ -601,12 +601,12 @@ func TestRevokedCert(t *testing.T) {
for _, tt := range revocationTests { for _, tt := range revocationTests {
t.Run(fmt.Sprintf("%v with x509 crl hash dir", tt.desc), func(t *testing.T) { 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, RevocationConfig{
RootDir: testdata.Path("crl"), RootDir: testdata.Path("crl"),
AllowUndetermined: tt.allowUndetermined, AllowUndetermined: tt.allowUndetermined,
Cache: cache, Cache: cache,
}) })
t.Logf("CheckRevocation err = %v", err) t.Logf("checkRevocation err = %v", err)
if tt.revoked && err == nil { if tt.revoked && err == nil {
t.Error("Revoked certificate chain was allowed") t.Error("Revoked certificate chain was allowed")
} else if !tt.revoked && err != nil { } else if !tt.revoked && err != nil {
@ -614,11 +614,11 @@ func TestRevokedCert(t *testing.T) {
} }
}) })
t.Run(fmt.Sprintf("%v with static provider", tt.desc), func(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, RevocationConfig{
AllowUndetermined: tt.allowUndetermined, AllowUndetermined: tt.allowUndetermined,
CRLProvider: cRLProvider, CRLProvider: cRLProvider,
}) })
t.Logf("CheckRevocation err = %v", err) t.Logf("checkRevocation err = %v", err)
if tt.revoked && err == nil { if tt.revoked && err == nil {
t.Error("Revoked certificate chain was allowed") t.Error("Revoked certificate chain was allowed")
} else if !tt.revoked && err != nil { } else if !tt.revoked && err != nil {
@ -739,7 +739,7 @@ func TestVerifyConnection(t *testing.T) {
cliCfg := tls.Config{ cliCfg := tls.Config{
RootCAs: cp, RootCAs: cp,
VerifyConnection: func(cs tls.ConnectionState) error { VerifyConnection: func(cs tls.ConnectionState) error {
return CheckRevocation(cs, RevocationConfig{RootDir: dir}) return checkRevocation(cs, RevocationConfig{RootDir: dir})
}, },
} }
conn, err := tls.Dial(lis.Addr().Network(), lis.Addr().String(), &cliCfg) conn, err := tls.Dial(lis.Addr().Network(), lis.Addr().String(), &cliCfg)