Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

advancedtls: Rename RevocationConfig #7151

Merged
merged 11 commits into from Apr 30, 2024
Merged

advancedtls: Rename RevocationConfig #7151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f4aaf04
Rename RevocationConfig
gtcooke94 Apr 18, 2024
151d843
replace usages
gtcooke94 Apr 18, 2024
2d0c093
merge master
gtcooke94 Apr 19, 2024
b687433
fix tests
gtcooke94 Apr 19, 2024
6bb3f5c
didn't actually save the file before committing
gtcooke94 Apr 19, 2024
f18fe5f
fix deprecation notice
gtcooke94 Apr 22, 2024
78a7818
reword comment
gtcooke94 Apr 23, 2024
7e3cfcd
merge master
gtcooke94 Apr 23, 2024
6cd5be4
didn't save merge conflict resolution
gtcooke94 Apr 23, 2024
3d0fbc4
fix vet error
gtcooke94 Apr 23, 2024
8ea9495
fix merge conflict
gtcooke94 Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 25 additions & 26 deletions security/advancedtls/advancedtls.go
View file
Open in desktop
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
erm-g marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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
Open in desktop
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