Skip to content

Commit

Permalink
advancedtls: Rename RevocationConfig (#7151)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gtcooke94
gtcooke94 committed Apr 30, 2024
1 parent 5ab1c1a commit b433b94
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 83 deletions.
51 changes: 25 additions & 26 deletions security/advancedtls/advancedtls.go
View file
Expand Up @@ -68,8 +68,7 @@ type VerificationFuncParams = HandshakeVerificationInfo
type PostHandshakeVerificationResults struct{}

// VerificationResults contains the information about results of
// CustomVerificationFunc.
//
// PostHandshakeVerificationFunc.
// Deprecated: use PostHandshakeVerificationResults instead.
type VerificationResults = PostHandshakeVerificationResults

Expand Down Expand Up @@ -212,9 +211,9 @@ type ClientOptions struct {
//
// Deprecated: use VerificationType instead.
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 *RevocationConfig
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
Expand Down Expand Up @@ -260,9 +259,9 @@ type ServerOptions struct {
//
// Deprecated: use VerificationType instead.
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 *RevocationConfig
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
Expand Down Expand Up @@ -461,12 +460,12 @@ func (o *ServerOptions) config() (*tls.Config, error) {
// advancedTLSCreds is the credentials required for authenticating a connection
// using TLS.
type advancedTLSCreds struct {
config *tls.Config
verifyFunc PostHandshakeVerificationFunc
getRootCAs func(params *GetRootCAsParams) (*GetRootCAsResults, error)
isClient bool
verificationType VerificationType
revocationConfig *RevocationConfig
config *tls.Config
verifyFunc PostHandshakeVerificationFunc
getRootCAs func(params *GetRootCAsParams) (*GetRootCAsResults, error)
isClient bool
revocationOptions *RevocationOptions
verificationType VerificationType
}

func (c advancedTLSCreds) Info() credentials.ProtocolInfo {
Expand Down Expand Up @@ -614,12 +613,12 @@ func buildVerifyFunc(c *advancedTLSCreds,
leafCert = rawCertList[0]
}
// Perform certificate revocation check if specified.
if c.revocationConfig != nil {
if c.revocationOptions != nil {
verifiedChains := chains
if verifiedChains == nil {
verifiedChains = [][]*x509.Certificate{rawCertList}
}
if err := checkChainRevocation(verifiedChains, *c.revocationConfig); err != nil {
if err := checkChainRevocation(verifiedChains, *c.revocationOptions); err != nil {
return err
}
}
Expand All @@ -645,12 +644,12 @@ func NewClientCreds(o *ClientOptions) (credentials.TransportCredentials, error)
return nil, err
}
tc := &advancedTLSCreds{
config: conf,
isClient: true,
getRootCAs: o.RootOptions.GetRootCertificates,
verifyFunc: o.AdditionalPeerVerification,
verificationType: o.VerificationType,
revocationConfig: o.RevocationConfig,
config: conf,
isClient: true,
getRootCAs: o.RootOptions.GetRootCertificates,
verifyFunc: o.AdditionalPeerVerification,
revocationOptions: o.RevocationOptions,
verificationType: o.VerificationType,
}
tc.config.NextProtos = credinternal.AppendH2ToNextProtos(tc.config.NextProtos)
return tc, nil
Expand All @@ -664,12 +663,12 @@ func NewServerCreds(o *ServerOptions) (credentials.TransportCredentials, error)
return nil, err
}
tc := &advancedTLSCreds{
config: conf,
isClient: false,
getRootCAs: o.RootOptions.GetRootCertificates,
verifyFunc: o.AdditionalPeerVerification,
verificationType: o.VerificationType,
revocationConfig: o.RevocationConfig,
config: conf,
isClient: false,
getRootCAs: o.RootOptions.GetRootCertificates,
verifyFunc: o.AdditionalPeerVerification,
revocationOptions: o.RevocationOptions,
verificationType: o.VerificationType,
}
tc.config.NextProtos = credinternal.AppendH2ToNextProtos(tc.config.NextProtos)
return tc, nil
Expand Down
82 changes: 41 additions & 41 deletions security/advancedtls/advancedtls_test.go
View file
Expand Up @@ -409,13 +409,13 @@ func (s) TestClientServerHandshake(t *testing.T) {
return &GetRootCAsResults{TrustCerts: cs.ServerTrust3}, nil
}

makeStaticCRLRevocationConfig := func(crlPath string, allowUndetermined bool) *RevocationConfig {
makeStaticCRLRevocationOptions := 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,
}
Expand All @@ -435,7 +435,7 @@ func (s) TestClientServerHandshake(t *testing.T) {
clientVerificationType VerificationType
clientRootProvider certprovider.Provider
clientIdentityProvider certprovider.Provider
clientRevocationConfig *RevocationConfig
clientRevocationOptions *RevocationOptions
clientExpectHandshakeError bool
serverMutualTLS bool
serverCert []tls.Certificate
Expand All @@ -446,7 +446,7 @@ func (s) TestClientServerHandshake(t *testing.T) {
serverVerificationType VerificationType
serverRootProvider certprovider.Provider
serverIdentityProvider certprovider.Provider
serverRevocationConfig *RevocationConfig
serverRevocationOptions *RevocationOptions
serverExpectError bool
}{
// Client: nil setting except verifyFuncGood
Expand Down Expand Up @@ -739,7 +739,7 @@ func (s) TestClientServerHandshake(t *testing.T) {
clientGetRoot: getRootCAsForClient,
clientVerifyFunc: clientVerifyFuncGood,
clientVerificationType: CertVerification,
clientRevocationConfig: &RevocationConfig{
clientRevocationOptions: &RevocationOptions{
RootDir: testdata.Path("crl"),
AllowUndetermined: true,
Cache: cache,
Expand All @@ -748,7 +748,7 @@ func (s) TestClientServerHandshake(t *testing.T) {
serverCert: []tls.Certificate{cs.ServerCert1},
serverGetRoot: getRootCAsForServer,
serverVerificationType: CertVerification,
serverRevocationConfig: &RevocationConfig{
serverRevocationOptions: &RevocationOptions{
RootDir: testdata.Path("crl"),
AllowUndetermined: true,
Cache: cache,
Expand All @@ -758,49 +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
{
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,
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,
clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_crl_empty.pem"), true),
serverMutualTLS: true,
serverCert: []tls.Certificate{cs.ServerCertForCRL},
serverGetRoot: getRootCAsForServerCRL,
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
{
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,
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,
clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_crl_server_revoked.pem"), true),
serverMutualTLS: true,
serverCert: []tls.Certificate{cs.ServerCertForCRL},
serverGetRoot: getRootCAsForServerCRL,
serverVerificationType: 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,
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,
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,
clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_malicious_crl_empty.pem"), false),
serverMutualTLS: true,
serverCert: []tls.Certificate{cs.ServerCertForCRL},
serverGetRoot: getRootCAsForServerCRL,
serverVerificationType: CertVerification,
serverExpectError: true,
},
} {
test := test
Expand All @@ -825,7 +825,7 @@ func (s) TestClientServerHandshake(t *testing.T) {
RequireClientCert: test.serverMutualTLS,
AdditionalPeerVerification: test.serverVerifyFunc,
VerificationType: test.serverVerificationType,
RevocationConfig: test.serverRevocationConfig,
RevocationOptions: test.serverRevocationOptions,
}
go func(done chan credentials.AuthInfo, lis net.Listener, serverOptions *ServerOptions) {
serverRawConn, err := lis.Accept()
Expand Down Expand Up @@ -867,8 +867,8 @@ func (s) TestClientServerHandshake(t *testing.T) {
GetRootCertificates: test.clientGetRoot,
RootProvider: test.clientRootProvider,
},
VerificationType: test.clientVerificationType,
RevocationConfig: test.clientRevocationConfig,
VerificationType: test.clientVerificationType,
RevocationOptions: test.clientRevocationOptions,
}
clientTLS, err := NewClientCreds(clientOptions)
if err != nil {
Expand Down
23 changes: 14 additions & 9 deletions security/advancedtls/crl.go
View file
Expand Up @@ -55,8 +55,8 @@ 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 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).
// Deprecated: use CRLProvider instead.
Expand All @@ -75,6 +75,11 @@ type RevocationConfig struct {
CRLProvider CRLProvider
}

// RevocationConfig contains options for CRL lookup.
//
// Deprecated: use RevocationOptions instead.
type RevocationConfig = RevocationOptions

// revocationStatus is the revocation status for a certificate or chain.
type revocationStatus int

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion security/advancedtls/crl_provider.go
View file
Expand Up @@ -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
Expand Down

0 comments on commit b433b94

Please sign in to comment.