diff --git a/revocation/crl/fetcher.go b/revocation/crl/fetcher.go index f0baacc4..44ba4dd1 100644 --- a/revocation/crl/fetcher.go +++ b/revocation/crl/fetcher.go @@ -18,6 +18,7 @@ package crl import ( "context" "crypto/x509" + "crypto/x509/pkix" "encoding/asn1" "errors" "fmt" @@ -25,6 +26,9 @@ import ( "net/http" "net/url" "time" + + "golang.org/x/crypto/cryptobyte" + cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" ) // oidFreshestCRL is the object identifier for the distribution point @@ -44,6 +48,15 @@ type Fetcher interface { Fetch(ctx context.Context, url string) (*Bundle, error) } +// FetcherWithCertificateExtensions is an interface that specifies methods used +// for fetching CRL from the given URL with certificate extensions to identify +// the Freshest CRL extension (Delta CRL) from certificate. +type FetcherWithCertificateExtensions interface { + // FetchWithCertificateExtensions retrieves the CRL from the given URL with + // certificate extensions. + FetchWithCertificateExtensions(ctx context.Context, url string, certificateExtensions *[]pkix.Extension) (*Bundle, error) +} + // HTTPFetcher is a Fetcher implementation that fetches CRL from the given URL type HTTPFetcher struct { // Cache stores fetched CRLs and reuses them until the CRL reaches the @@ -77,6 +90,10 @@ func NewHTTPFetcher(httpClient *http.Client) (*HTTPFetcher, error) { // (e.g. cache miss), it will download the CRL from the URL and store it to the // cache. func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*Bundle, error) { + return f.FetchWithCertificateExtensions(ctx, url, nil) +} + +func (f *HTTPFetcher) FetchWithCertificateExtensions(ctx context.Context, url string, certificateExtensions *[]pkix.Extension) (*Bundle, error) { if url == "" { return nil, errors.New("CRL URL cannot be empty") } @@ -84,9 +101,9 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*Bundle, error) { if f.Cache != nil { bundle, err := f.Cache.Get(ctx, url) if err == nil { - // check expiry - nextUpdate := bundle.BaseCRL.NextUpdate - if !nextUpdate.IsZero() && !time.Now().After(nextUpdate) { + // check expiry of base CRL and delta CRL + if (bundle.BaseCRL != nil && isEffective(bundle.BaseCRL)) && + (bundle.DeltaCRL == nil || isEffective(bundle.DeltaCRL)) { return bundle, nil } } else if !errors.Is(err, ErrCacheMiss) && !f.DiscardCacheError { @@ -94,7 +111,7 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*Bundle, error) { } } - bundle, err := f.fetch(ctx, url) + bundle, err := f.fetch(ctx, url, certificateExtensions) if err != nil { return nil, fmt.Errorf("failed to retrieve CRL: %w", err) } @@ -109,27 +126,113 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*Bundle, error) { return bundle, nil } +// isEffective checks if the CRL is effective by checking the NextUpdate time. +func isEffective(crl *x509.RevocationList) bool { + return !crl.NextUpdate.IsZero() && !time.Now().After(crl.NextUpdate) +} + // fetch downloads the CRL from the given URL. -func (f *HTTPFetcher) fetch(ctx context.Context, url string) (*Bundle, error) { +func (f *HTTPFetcher) fetch(ctx context.Context, url string, certificateExtensions *[]pkix.Extension) (*Bundle, error) { // fetch base CRL base, err := fetchCRL(ctx, url, f.httpClient) if err != nil { return nil, err } - // check delta CRL - // TODO: support delta CRL https://github.com/notaryproject/notation-core-go/issues/228 - for _, ext := range base.Extensions { - if ext.Id.Equal(oidFreshestCRL) { - return nil, errors.New("delta CRL is not supported") + // fetch delta CRL from CRL extension + deltaCRL, err := f.processDeltaCRL(&base.Extensions) + if err != nil { + return nil, err + } + if certificateExtensions != nil && deltaCRL == nil { + // fallback to fetch delta CRL from certificate extension + deltaCRL, err = f.processDeltaCRL(certificateExtensions) + if err != nil { + return nil, err } } return &Bundle{ - BaseCRL: base, + BaseCRL: base, + DeltaCRL: deltaCRL, }, nil } +func (f *HTTPFetcher) processDeltaCRL(extensions *[]pkix.Extension) (*x509.RevocationList, error) { + for _, ext := range *extensions { + if ext.Id.Equal(oidFreshestCRL) { + cdp, err := parseFreshestCRL(ext) + if err != nil { + return nil, fmt.Errorf("failed to parse Freshest CRL extension: %w", err) + } + if len(cdp) == 0 { + return nil, nil + } + if len(cdp) > 1 { + // to simplify the implementation, we only support one + // Freshest CRL URL for now which is the common case + return nil, errors.New("multiple Freshest CRL distribution points are not supported") + } + return fetchCRL(context.Background(), cdp[0], f.httpClient) + } + } + return nil, nil +} + +// parseFreshestCRL parses the Freshest CRL extension and returns the CRL URLs +func parseFreshestCRL(ext pkix.Extension) ([]string, error) { + var cdp []string + // RFC 5280, 4.2.1.15 + // id-ce-freshestCRL OBJECT IDENTIFIER ::= { id-ce 46 } + // + // FreshestCRL ::= CRLDistributionPoints + // + // RFC 5280, 4.2.1.13 + // + // CRLDistributionPoints ::= SEQUENCE SIZE (1..MAX) OF DistributionPoint + // + // DistributionPoint ::= SEQUENCE { + // distributionPoint [0] DistributionPointName OPTIONAL, + // reasons [1] ReasonFlags OPTIONAL, + // cRLIssuer [2] GeneralNames OPTIONAL } + // + // DistributionPointName ::= CHOICE { + // fullName [0] GeneralNames, + // nameRelativeToCRLIssuer [1] RelativeDistinguishedName } + val := cryptobyte.String(ext.Value) + if !val.ReadASN1(&val, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: invalid CRL distribution points") + } + for !val.Empty() { + var dpDER cryptobyte.String + if !val.ReadASN1(&dpDER, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: invalid CRL distribution point") + } + var dpNameDER cryptobyte.String + var dpNamePresent bool + if !dpDER.ReadOptionalASN1(&dpNameDER, &dpNamePresent, cryptobyte_asn1.Tag(0).Constructed().ContextSpecific()) { + return nil, errors.New("x509: invalid CRL distribution point") + } + if !dpNamePresent { + continue + } + if !dpNameDER.ReadASN1(&dpNameDER, cryptobyte_asn1.Tag(0).Constructed().ContextSpecific()) { + return nil, errors.New("x509: invalid CRL distribution point") + } + for !dpNameDER.Empty() { + if !dpNameDER.PeekASN1Tag(cryptobyte_asn1.Tag(6).ContextSpecific()) { + break + } + var uri cryptobyte.String + if !dpNameDER.ReadASN1(&uri, cryptobyte_asn1.Tag(6).ContextSpecific()) { + return nil, errors.New("x509: invalid CRL distribution point") + } + cdp = append(cdp, string(uri)) + } + } + return cdp, nil +} + func fetchCRL(ctx context.Context, crlURL string, client *http.Client) (*x509.RevocationList, error) { // validate URL parsedURL, err := url.Parse(crlURL) diff --git a/revocation/crl/fetcher_test.go b/revocation/crl/fetcher_test.go index 634fbc3a..87fc0dd6 100644 --- a/revocation/crl/fetcher_test.go +++ b/revocation/crl/fetcher_test.go @@ -19,11 +19,13 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" + "encoding/pem" "errors" "fmt" "io" "math/big" "net/http" + "os" "strings" "sync" "testing" @@ -195,39 +197,6 @@ func TestFetch(t *testing.T) { } }) - t.Run("delta CRL is not supported", func(t *testing.T) { - c := &memoryCache{} - // prepare a CRL with refresh CRL extension - certChain := testhelper.GetRevokableRSAChainWithRevocations(2, false, true) - expiredCRLBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ - Number: big.NewInt(1), - NextUpdate: time.Now().Add(-1 * time.Hour), - ExtraExtensions: []pkix.Extension{ - { - Id: oidFreshestCRL, - Value: []byte{0x01, 0x02, 0x03}, - }, - }, - }, certChain[1].Cert, certChain[1].PrivateKey) - if err != nil { - t.Fatalf("failed to create base CRL: %v", err) - } - - httpClient := &http.Client{ - Transport: expectedRoundTripperMock{Body: expiredCRLBytes}, - } - f, err := NewHTTPFetcher(httpClient) - if err != nil { - t.Errorf("NewHTTPFetcher() error = %v, want nil", err) - } - f.Cache = c - f.DiscardCacheError = true - _, err = f.Fetch(context.Background(), uncachedURL) - if !strings.Contains(err.Error(), "delta CRL is not supported") { - t.Errorf("Fetcher.Fetch() error = %v, want delta CRL is not supported", err) - } - }) - t.Run("Set cache error", func(t *testing.T) { c := &errorCache{ GetError: ErrCacheMiss, @@ -291,6 +260,212 @@ func TestFetch(t *testing.T) { }) } +func TestParseFreshestCRL(t *testing.T) { + loadExtentsion := func(certPath string) pkix.Extension { + certData, err := os.ReadFile(certPath) + if err != nil { + t.Fatalf("failed to read certificate: %v", err) + } + + block, _ := pem.Decode(certData) + if block == nil { + t.Fatalf("failed to decode PEM block") + } + + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatalf("failed to parse certificate: %v", err) + } + + for _, ext := range cert.Extensions { + if ext.Id.Equal([]int{2, 5, 29, 46}) { // id-ce-freshestCRL + return ext + } + } + + t.Fatalf("freshestCRL extension not found") + return pkix.Extension{} + } + + t.Run("valid 1 delta CRL URL", func(t *testing.T) { + certPath := "testdata/certificateWithDeltaCRL.cer" + freshestCRLExtension := loadExtentsion(certPath) + urls, err := parseFreshestCRL(freshestCRLExtension) + if err != nil { + t.Fatalf("failed to parse freshest CRL: %v", err) + } + + if len(urls) != 1 { + t.Fatalf("expected 1 URL, got %d", len(urls)) + } + + if !strings.HasPrefix(urls[0], "http://localhost:80") { + t.Fatalf("unexpected URL: %s", urls[0]) + } + }) + + t.Run("empty extension", func(t *testing.T) { + _, err := parseFreshestCRL(pkix.Extension{}) + if err == nil { + t.Fatalf("expected error") + } + }) + + t.Run("URL doesn't exist", func(t *testing.T) { + certPath := "testdata/certificateWithZeroDeltaCRLURL.cer" + freshestCRLExtension := loadExtentsion(certPath) + url, err := parseFreshestCRL(freshestCRLExtension) + if err != nil { + t.Fatalf("failed to parse freshest CRL: %v", err) + } + if len(url) != 0 { + t.Fatalf("expected 0 URL, got %d", len(url)) + } + }) + + t.Run("non URI freshest CRL extension", func(t *testing.T) { + certPath := "testdata/certificateWithNonURIDeltaCRL.cer" + freshestCRLExtension := loadExtentsion(certPath) + url, err := parseFreshestCRL(freshestCRLExtension) + if err != nil { + t.Fatalf("failed to parse freshest CRL: %v", err) + } + if len(url) != 0 { + t.Fatalf("expected 0 URL, got %d", len(url)) + } + }) + + t.Run("certificate with incomplete freshest CRL extension", func(t *testing.T) { + certPath := "testdata/certificateWithIncompleteFreshestCRL.cer" + freshestCRLExtension := loadExtentsion(certPath) + _, err := parseFreshestCRL(freshestCRLExtension) + expectErrorMsg := "x509: invalid CRL distribution point" + if err == nil || err.Error() != expectErrorMsg { + t.Fatalf("expected error %q, got %v", expectErrorMsg, err) + } + }) + + t.Run("certificate with incomplete freshest CRL extension2", func(t *testing.T) { + certPath := "testdata/certificateWithIncompleteFreshestCRL2.cer" + freshestCRLExtension := loadExtentsion(certPath) + url, err := parseFreshestCRL(freshestCRLExtension) + if err != nil { + t.Fatalf("failed to parse freshest CRL: %v", err) + } + if len(url) != 0 { + t.Fatalf("expected 0 URL, got %d", len(url)) + } + }) +} + +func TestProcessDeltaCRL(t *testing.T) { + loadExtentsion := func(certPath string) *[]pkix.Extension { + certData, err := os.ReadFile(certPath) + if err != nil { + t.Fatalf("failed to read certificate: %v", err) + } + + block, _ := pem.Decode(certData) + if block == nil { + t.Fatalf("failed to decode PEM block") + } + + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatalf("failed to parse certificate: %v", err) + } + + return &cert.Extensions + } + + deltaCRL, err := os.ReadFile("testdata/delta.crl") + if err != nil { + t.Fatalf("failed to read delta CRL: %v", err) + } + + fetcher, err := NewHTTPFetcher(&http.Client{ + Transport: expectedRoundTripperMock{Body: deltaCRL}, + }) + if err != nil { + t.Fatalf("failed to create fetcher: %v", err) + } + + t.Run("parse freshest CRL failed", func(t *testing.T) { + certPath := "testdata/certificateWithIncompleteFreshestCRL.cer" + extensions := loadExtentsion(certPath) + _, err := fetcher.processDeltaCRL(extensions) + expectedErrorMsg := "failed to parse Freshest CRL extension: x509: invalid CRL distribution point" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) + } + }) + + t.Run("zero freshest CRL URL", func(t *testing.T) { + certPath := "testdata/certificateWithZeroDeltaCRLURL.cer" + extensions := loadExtentsion(certPath) + deltaCRL, err := fetcher.processDeltaCRL(extensions) + if err != nil { + t.Fatalf("failed to process delta CRL: %v", err) + } + if deltaCRL != nil { + t.Fatalf("expected nil delta CRL, got %v", deltaCRL) + } + }) + + t.Run("one freshest CRL URL", func(t *testing.T) { + certPath := "testdata/certificateWithDeltaCRL.cer" + extensions := loadExtentsion(certPath) + deltaCRL, err := fetcher.processDeltaCRL(extensions) + if err != nil { + t.Fatalf("failed to process delta CRL: %v", err) + } + if deltaCRL == nil { + t.Fatalf("expected non-nil delta CRL") + } + }) + + t.Run("multiple freshest CRL URLs", func(t *testing.T) { + certPath := "testdata/certificateWith2DeltaCRL.cer" + extensions := loadExtentsion(certPath) + _, err := fetcher.processDeltaCRL(extensions) + expectedErrorMsg := "multiple Freshest CRL distribution points are not supported" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) + } + }) + + t.Run("process delta crl from certificate extension failed", func(t *testing.T) { + certPath := "testdata/certificateWithIncompleteFreshestCRL.cer" + extensions := loadExtentsion(certPath) + _, err := fetcher.fetch(context.Background(), "http://localhost.test", extensions) + expectedErrorMsg := "failed to parse Freshest CRL extension: x509: invalid CRL distribution point" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) + } + }) + + crlFile, err := os.ReadFile("testdata/crlWithMultipleFreshestCRLs.crl") + if err != nil { + t.Fatalf("failed to read CRL file: %v", err) + } + fetcher, err = NewHTTPFetcher(&http.Client{ + Transport: expectedRoundTripperMock{Body: crlFile}, + }) + if err != nil { + t.Fatalf("failed to create fetcher: %v", err) + } + + t.Run("fetch delta CRL from CRL failed", func(t *testing.T) { + certPath := "testdata/certificateWithDeltaCRL.cer" + extensions := loadExtentsion(certPath) + _, err := fetcher.fetch(context.Background(), "http://localhost.test", extensions) + expectErrorMsg := "multiple Freshest CRL distribution points are not supported" + if err == nil || err.Error() != expectErrorMsg { + t.Fatalf("expected error %q, got %v", expectErrorMsg, err) + } + }) +} + func TestDownload(t *testing.T) { t.Run("parse url error", func(t *testing.T) { _, err := fetchCRL(context.Background(), ":", http.DefaultClient) diff --git a/revocation/crl/testdata/certificateWith2DeltaCRL.cer b/revocation/crl/testdata/certificateWith2DeltaCRL.cer new file mode 100644 index 00000000..3d029b2a --- /dev/null +++ b/revocation/crl/testdata/certificateWith2DeltaCRL.cer @@ -0,0 +1,32 @@ +-----BEGIN CERTIFICATE----- +MIIFdTCCA92gAwIBAgIUaHnHWrIVx0qzAZIwvELkQUEs+3cwDQYJKoZIhvcNAQEL +BQAwYTEjMCEGCgmSJomT8ixkAQEME2MtMG53d296cXZhd3ZhbzNlY2gxFTATBgNV +BAMMDE1hbmFnZW1lbnRDQTEjMCEGA1UECgwaRUpCQ0EgQ29udGFpbmVyIFF1aWNr +c3RhcnQwHhcNMjQxMTI1MDc1NzI3WhcNMjYxMTI1MDc1NzI2WjAYMRYwFAYDVQQD +DA1Ob3RhdGlvblRlc3QzMIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQAuJlXlrTm +VhEiLz75HgJlZGm0TE7W/7mYn0m03vV+9tBEA/ZV50ACkMDY0ewxxh4Ko6UsGq71 +E466hSVggiaYaE4AdbL5kAolsnEm9/EDfYQNfgiQw0BI7axri9tJ19yZhj4Es31+ +j4RJHAydB4i/qJi6T8cITdT6ViyzWM6AWGl7yRajggJ0MIICcDAMBgNVHRMBAf8E +AjAAMB8GA1UdIwQYMBaAFFPceeG3G4d5oezFSybFwehynbPqMIIBTQYDVR0uBIIB +RDCCAUAwggE8oIIBOKCCATSGgZdodHRwOi8vbG9jYWxob3N0OjgwL2VqYmNhL3B1 +YmxpY3dlYi93ZWJkaXN0L2NlcnRkaXN0P2NtZD1kZWx0YWNybCZpc3N1ZXI9VUlE +JTNEYy0wbnd3b3pxdmF3dmFvM2VjaCUyQ0NOJTNETWFuYWdlbWVudENBJTJDTyUz +REVKQkNBK0NvbnRhaW5lcitRdWlja3N0YXJ0hoGXaHR0cDovL2xvY2FsaG9zdDo4 +MC9lamJjYS9wdWJsaWN3ZWIvd2ViZGlzdC9jZXJ0ZGlzdD9jbWQ9ZGVsdGFjcmwm +aXNzdWVyPVVJRCUzRGMtMG53d296cXZhd3ZhbzNlY2glMkNDTiUzRE1hbmFnZW1l +bnRDQSUyQ08lM0RFSkJDQStDb250YWluZXIrUXVpY2tzdGFydDATBgNVHSUEDDAK +BggrBgEFBQcDATCBqQYDVR0fBIGhMIGeMIGboIGYoIGVhoGSaHR0cDovL2xvY2Fs +aG9zdDo4MC9lamJjYS9wdWJsaWN3ZWIvd2ViZGlzdC9jZXJ0ZGlzdD9jbWQ9Y3Js +Jmlzc3Vlcj1VSUQlM0RjLTBud3dvenF2YXd2YW8zZWNoJTJDQ04lM0RNYW5hZ2Vt +ZW50Q0ElMkNPJTNERUpCQ0ErQ29udGFpbmVyK1F1aWNrc3RhcnQwHQYDVR0OBBYE +FDHE82/06xOocYbvMIyGt2gofk88MA4GA1UdDwEB/wQEAwIFoDANBgkqhkiG9w0B +AQsFAAOCAYEAV7G7WMPn3tQNqB8RxYATV3eVhB3WC5BxqBbQzp2loNycDRmX95fa +7EV5xcPIUv42B+TzLu/ann9FLOMkqEhA+F5zsEomikUA+L4cuIWLXUhwIWwE2I/p +fgHJ61JtMMxv3rWQHyo6YpzpIAG23oxGXzrlN4/oNfWzWMIYlcl4xiHxC2vOKnNO +wId3Ck3jsJE10tImdD/tQYXh7h5ueESyPUZtqM/g2QPap+tEHArpgfAQdpEvRj1v +ZWAotcEIr+a5popE56UaE4a29DspVTA1rVchhKYl2gpDxieSQgQr61fWHXzRoKcd +FZ+NgqJuwd9CxrXbkl6EKDpefivwz9G4b3b6R8lMl+wmeTgPvSUYO34c8GsF143H +V/VKNoBvoz44QyLUf+1+XiHuUjfHaXCtXmDOoQ64M3d9gGrz23G5KJAUBubcbcdu +7Ah3D5zqvieGgoYt8qye1nsIVYC1KrYP2Kp5jWCadLvIwu2B0j7eA+LwN2MBWdlh +dRTSOVkICjWU +-----END CERTIFICATE----- diff --git a/revocation/crl/testdata/certificateWithDeltaCRL.cer b/revocation/crl/testdata/certificateWithDeltaCRL.cer new file mode 100644 index 00000000..8b06be8d --- /dev/null +++ b/revocation/crl/testdata/certificateWithDeltaCRL.cer @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIIE1TCCAz2gAwIBAgIUaHnHWrIVx0qzAZIwvELkQUEs+3cwDQYJKoZIhvcNAQEL +BQAwYTEjMCEGCgmSJomT8ixkAQEME2MtMG53d296cXZhd3ZhbzNlY2gxFTATBgNV +BAMMDE1hbmFnZW1lbnRDQTEjMCEGA1UECgwaRUpCQ0EgQ29udGFpbmVyIFF1aWNr +c3RhcnQwHhcNMjQxMTI1MDc1NzI3WhcNMjYxMTI1MDc1NzI2WjAYMRYwFAYDVQQD +DA1Ob3RhdGlvblRlc3QzMIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQAuJlXlrTm +VhEiLz75HgJlZGm0TE7W/7mYn0m03vV+9tBEA/ZV50ACkMDY0ewxxh4Ko6UsGq71 +E466hSVggiaYaE4AdbL5kAolsnEm9/EDfYQNfgiQw0BI7axri9tJ19yZhj4Es31+ +j4RJHAydB4i/qJi6T8cITdT6ViyzWM6AWGl7yRajggHUMIIB0DAMBgNVHRMBAf8E +AjAAMB8GA1UdIwQYMBaAFFPceeG3G4d5oezFSybFwehynbPqMIGuBgNVHS4EgaYw +gaMwgaCggZ2ggZqGgZdodHRwOi8vbG9jYWxob3N0OjgwL2VqYmNhL3B1YmxpY3dl +Yi93ZWJkaXN0L2NlcnRkaXN0P2NtZD1kZWx0YWNybCZpc3N1ZXI9VUlEJTNEYy0w +bnd3b3pxdmF3dmFvM2VjaCUyQ0NOJTNETWFuYWdlbWVudENBJTJDTyUzREVKQkNB +K0NvbnRhaW5lcitRdWlja3N0YXJ0MBMGA1UdJQQMMAoGCCsGAQUFBwMBMIGpBgNV +HR8EgaEwgZ4wgZuggZiggZWGgZJodHRwOi8vbG9jYWxob3N0OjgwL2VqYmNhL3B1 +YmxpY3dlYi93ZWJkaXN0L2NlcnRkaXN0P2NtZD1jcmwmaXNzdWVyPVVJRCUzRGMt +MG53d296cXZhd3ZhbzNlY2glMkNDTiUzRE1hbmFnZW1lbnRDQSUyQ08lM0RFSkJD +QStDb250YWluZXIrUXVpY2tzdGFydDAdBgNVHQ4EFgQUMcTzb/TrE6hxhu8wjIa3 +aCh+TzwwDgYDVR0PAQH/BAQDAgWgMA0GCSqGSIb3DQEBCwUAA4IBgQBXsbtYw+fe +1A2oHxHFgBNXd5WEHdYLkHGoFtDOnaWg3JwNGZf3l9rsRXnFw8hS/jYH5PMu79qe +f0Us4ySoSED4XnOwSiaKRQD4vhy4hYtdSHAhbATYj+l+AcnrUm0wzG/etZAfKjpi +nOkgAbbejEZfOuU3j+g19bNYwhiVyXjGIfELa84qc07Ah3cKTeOwkTXS0iZ0P+1B +heHuHm54RLI9Rm2oz+DZA9qn60QcCumB8BB2kS9GPW9lYCi1wQiv5rmmikTnpRoT +hrb0OylVMDWtVyGEpiXaCkPGJ5JCBCvrV9YdfNGgpx0Vn42Com7B30LGtduSXoQo +Ol5+K/DP0bhvdvpHyUyX7CZ5OA+9JRg7fhzwawXXjcdX9Uo2gG+jPjhDItR/7X5e +Ie5SN8dpcK1eYM6hDrgzd32AavPbcbkokBQG5txtx27sCHcPnOq+J4aChi3yrJ7W +ewhVgLUqtg/YqnmNYJp0u8jC7YHSPt4D4vA3YwFZ2WF1FNI5WQgKNZQ= +-----END CERTIFICATE----- diff --git a/revocation/crl/testdata/certificateWithIncompleteFreshestCRL.cer b/revocation/crl/testdata/certificateWithIncompleteFreshestCRL.cer new file mode 100644 index 00000000..35776859 --- /dev/null +++ b/revocation/crl/testdata/certificateWithIncompleteFreshestCRL.cer @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDmTCCAgGgAwIBAgIUaHnHWrIVx0qzAZIwvELkQUEs+3cwDQYJKoZIhvcNAQEL +BQAwYTEjMCEGCgmSJomT8ixkAQEME2MtMG53d296cXZhd3ZhbzNlY2gxFTATBgNV +BAMMDE1hbmFnZW1lbnRDQTEjMCEGA1UECgwaRUpCQ0EgQ29udGFpbmVyIFF1aWNr +c3RhcnQwHhcNMjQxMTI1MDc1NzI3WhcNMjYxMTI1MDc1NzI2WjAYMRYwFAYDVQQD +DA1Ob3RhdGlvblRlc3QzMIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQAuJlXlrTm +VhEiLz75HgJlZGm0TE7W/7mYn0m03vV+9tBEA/ZV50ACkMDY0ewxxh4Ko6UsGq71 +E466hSVggiaYaE4AdbL5kAolsnEm9/EDfYQNfgiQw0BI7axri9tJ19yZhj4Es31+ +j4RJHAydB4i/qJi6T8cITdT6ViyzWM6AWGl7yRajgZkwgZYwDAYDVR0TAQH/BAIw +ADAfBgNVHSMEGDAWgBRT3HnhtxuHeaHsxUsmxcHocp2z6jANBgNVHS4EBjAEMAKg +ADATBgNVHSUEDDAKBggrBgEFBQcDATASBgNVHR8ECzAJMAegBaADhQH/MB0GA1Ud +DgQWBBQxxPNv9OsTqHGG7zCMhrdoKH5PPDAOBgNVHQ8BAf8EBAMCBaAwDQYJKoZI +hvcNAQELBQADggGBAFexu1jD597UDagfEcWAE1d3lYQd1guQcagW0M6dpaDcnA0Z +l/eX2uxFecXDyFL+Ngfk8y7v2p5/RSzjJKhIQPhec7BKJopFAPi+HLiFi11IcCFs +BNiP6X4ByetSbTDMb961kB8qOmKc6SABtt6MRl865TeP6DX1s1jCGJXJeMYh8Qtr +zipzTsCHdwpN47CRNdLSJnQ/7UGF4e4ebnhEsj1GbajP4NkD2qfrRBwK6YHwEHaR +L0Y9b2VgKLXBCK/muaaKROelGhOGtvQ7KVUwNa1XIYSmJdoKQ8YnkkIEK+tX1h18 +0aCnHRWfjYKibsHfQsa125JehCg6Xn4r8M/RuG92+kfJTJfsJnk4D70lGDt+HPBr +BdeNx1f1SjaAb6M+OEMi1H/tfl4h7lI3x2lwrV5gzqEOuDN3fYBq89txuSiQFAbm +3G3HbuwIdw+c6r4nhoKGLfKsntZ7CFWAtSq2D9iqeY1gmnS7yMLtgdI+3gPi8Ddj +AVnZYXUU0jlZCAo1lA== +-----END CERTIFICATE----- diff --git a/revocation/crl/testdata/certificateWithIncompleteFreshestCRL2.cer b/revocation/crl/testdata/certificateWithIncompleteFreshestCRL2.cer new file mode 100644 index 00000000..8ef955fc --- /dev/null +++ b/revocation/crl/testdata/certificateWithIncompleteFreshestCRL2.cer @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDlzCCAf+gAwIBAgIUaHnHWrIVx0qzAZIwvELkQUEs+3cwDQYJKoZIhvcNAQEL +BQAwYTEjMCEGCgmSJomT8ixkAQEME2MtMG53d296cXZhd3ZhbzNlY2gxFTATBgNV +BAMMDE1hbmFnZW1lbnRDQTEjMCEGA1UECgwaRUpCQ0EgQ29udGFpbmVyIFF1aWNr +c3RhcnQwHhcNMjQxMTI1MDc1NzI3WhcNMjYxMTI1MDc1NzI2WjAYMRYwFAYDVQQD +DA1Ob3RhdGlvblRlc3QzMIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQAuJlXlrTm +VhEiLz75HgJlZGm0TE7W/7mYn0m03vV+9tBEA/ZV50ACkMDY0ewxxh4Ko6UsGq71 +E466hSVggiaYaE4AdbL5kAolsnEm9/EDfYQNfgiQw0BI7axri9tJ19yZhj4Es31+ +j4RJHAydB4i/qJi6T8cITdT6ViyzWM6AWGl7yRajgZcwgZQwDAYDVR0TAQH/BAIw +ADAfBgNVHSMEGDAWgBRT3HnhtxuHeaHsxUsmxcHocp2z6jALBgNVHS4EBDACMAAw +EwYDVR0lBAwwCgYIKwYBBQUHAwEwEgYDVR0fBAswCTAHoAWgA4UB/zAdBgNVHQ4E +FgQUMcTzb/TrE6hxhu8wjIa3aCh+TzwwDgYDVR0PAQH/BAQDAgWgMA0GCSqGSIb3 +DQEBCwUAA4IBgQBXsbtYw+fe1A2oHxHFgBNXd5WEHdYLkHGoFtDOnaWg3JwNGZf3 +l9rsRXnFw8hS/jYH5PMu79qef0Us4ySoSED4XnOwSiaKRQD4vhy4hYtdSHAhbATY +j+l+AcnrUm0wzG/etZAfKjpinOkgAbbejEZfOuU3j+g19bNYwhiVyXjGIfELa84q +c07Ah3cKTeOwkTXS0iZ0P+1BheHuHm54RLI9Rm2oz+DZA9qn60QcCumB8BB2kS9G +PW9lYCi1wQiv5rmmikTnpRoThrb0OylVMDWtVyGEpiXaCkPGJ5JCBCvrV9YdfNGg +px0Vn42Com7B30LGtduSXoQoOl5+K/DP0bhvdvpHyUyX7CZ5OA+9JRg7fhzwawXX +jcdX9Uo2gG+jPjhDItR/7X5eIe5SN8dpcK1eYM6hDrgzd32AavPbcbkokBQG5txt +x27sCHcPnOq+J4aChi3yrJ7WewhVgLUqtg/YqnmNYJp0u8jC7YHSPt4D4vA3YwFZ +2WF1FNI5WQgKNZQ= +-----END CERTIFICATE----- diff --git a/revocation/crl/testdata/certificateWithNonURIDeltaCRL.cer b/revocation/crl/testdata/certificateWithNonURIDeltaCRL.cer new file mode 100644 index 00000000..128bbbe9 --- /dev/null +++ b/revocation/crl/testdata/certificateWithNonURIDeltaCRL.cer @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDnjCCAgagAwIBAgIUaHnHWrIVx0qzAZIwvELkQUEs+3cwDQYJKoZIhvcNAQEL +BQAwYTEjMCEGCgmSJomT8ixkAQEME2MtMG53d296cXZhd3ZhbzNlY2gxFTATBgNV +BAMMDE1hbmFnZW1lbnRDQTEjMCEGA1UECgwaRUpCQ0EgQ29udGFpbmVyIFF1aWNr +c3RhcnQwHhcNMjQxMTI1MDc1NzI3WhcNMjYxMTI1MDc1NzI2WjAYMRYwFAYDVQQD +DA1Ob3RhdGlvblRlc3QzMIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQAuJlXlrTm +VhEiLz75HgJlZGm0TE7W/7mYn0m03vV+9tBEA/ZV50ACkMDY0ewxxh4Ko6UsGq71 +E466hSVggiaYaE4AdbL5kAolsnEm9/EDfYQNfgiQw0BI7axri9tJ19yZhj4Es31+ +j4RJHAydB4i/qJi6T8cITdT6ViyzWM6AWGl7yRajgZ4wgZswDAYDVR0TAQH/BAIw +ADAfBgNVHSMEGDAWgBRT3HnhtxuHeaHsxUsmxcHocp2z6jASBgNVHS4ECzAJMAeg +BaADhQH/MBMGA1UdJQQMMAoGCCsGAQUFBwMBMBIGA1UdHwQLMAkwB6AFoAOFAf8w +HQYDVR0OBBYEFDHE82/06xOocYbvMIyGt2gofk88MA4GA1UdDwEB/wQEAwIFoDAN +BgkqhkiG9w0BAQsFAAOCAYEAV7G7WMPn3tQNqB8RxYATV3eVhB3WC5BxqBbQzp2l +oNycDRmX95fa7EV5xcPIUv42B+TzLu/ann9FLOMkqEhA+F5zsEomikUA+L4cuIWL +XUhwIWwE2I/pfgHJ61JtMMxv3rWQHyo6YpzpIAG23oxGXzrlN4/oNfWzWMIYlcl4 +xiHxC2vOKnNOwId3Ck3jsJE10tImdD/tQYXh7h5ueESyPUZtqM/g2QPap+tEHArp +gfAQdpEvRj1vZWAotcEIr+a5popE56UaE4a29DspVTA1rVchhKYl2gpDxieSQgQr +61fWHXzRoKcdFZ+NgqJuwd9CxrXbkl6EKDpefivwz9G4b3b6R8lMl+wmeTgPvSUY +O34c8GsF143HV/VKNoBvoz44QyLUf+1+XiHuUjfHaXCtXmDOoQ64M3d9gGrz23G5 +KJAUBubcbcdu7Ah3D5zqvieGgoYt8qye1nsIVYC1KrYP2Kp5jWCadLvIwu2B0j7e +A+LwN2MBWdlhdRTSOVkICjWU +-----END CERTIFICATE----- diff --git a/revocation/crl/testdata/certificateWithZeroDeltaCRLURL.cer b/revocation/crl/testdata/certificateWithZeroDeltaCRLURL.cer new file mode 100644 index 00000000..6f4de65e --- /dev/null +++ b/revocation/crl/testdata/certificateWithZeroDeltaCRLURL.cer @@ -0,0 +1,25 @@ +-----BEGIN CERTIFICATE----- +MIIENTCCAp2gAwIBAgIUaHnHWrIVx0qzAZIwvELkQUEs+3cwDQYJKoZIhvcNAQEL +BQAwYTEjMCEGCgmSJomT8ixkAQEME2MtMG53d296cXZhd3ZhbzNlY2gxFTATBgNV +BAMMDE1hbmFnZW1lbnRDQTEjMCEGA1UECgwaRUpCQ0EgQ29udGFpbmVyIFF1aWNr +c3RhcnQwHhcNMjQxMTI1MDc1NzI3WhcNMjYxMTI1MDc1NzI2WjAYMRYwFAYDVQQD +DA1Ob3RhdGlvblRlc3QzMIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQAuJlXlrTm +VhEiLz75HgJlZGm0TE7W/7mYn0m03vV+9tBEA/ZV50ACkMDY0ewxxh4Ko6UsGq71 +E466hSVggiaYaE4AdbL5kAolsnEm9/EDfYQNfgiQw0BI7axri9tJ19yZhj4Es31+ +j4RJHAydB4i/qJi6T8cITdT6ViyzWM6AWGl7yRajggE0MIIBMDAMBgNVHRMBAf8E +AjAAMB8GA1UdIwQYMBaAFFPceeG3G4d5oezFSybFwehynbPqMA8GA1UdLgQIMAYw +BKACoAAwEwYDVR0lBAwwCgYIKwYBBQUHAwEwgakGA1UdHwSBoTCBnjCBm6CBmKCB +lYaBkmh0dHA6Ly9sb2NhbGhvc3Q6ODAvZWpiY2EvcHVibGljd2ViL3dlYmRpc3Qv +Y2VydGRpc3Q/Y21kPWNybCZpc3N1ZXI9VUlEJTNEYy0wbnd3b3pxdmF3dmFvM2Vj +aCUyQ0NOJTNETWFuYWdlbWVudENBJTJDTyUzREVKQkNBK0NvbnRhaW5lcitRdWlj +a3N0YXJ0MB0GA1UdDgQWBBQxxPNv9OsTqHGG7zCMhrdoKH5PPDAOBgNVHQ8BAf8E +BAMCBaAwDQYJKoZIhvcNAQELBQADggGBAFexu1jD597UDagfEcWAE1d3lYQd1guQ +cagW0M6dpaDcnA0Zl/eX2uxFecXDyFL+Ngfk8y7v2p5/RSzjJKhIQPhec7BKJopF +APi+HLiFi11IcCFsBNiP6X4ByetSbTDMb961kB8qOmKc6SABtt6MRl865TeP6DX1 +s1jCGJXJeMYh8QtrzipzTsCHdwpN47CRNdLSJnQ/7UGF4e4ebnhEsj1GbajP4NkD +2qfrRBwK6YHwEHaRL0Y9b2VgKLXBCK/muaaKROelGhOGtvQ7KVUwNa1XIYSmJdoK +Q8YnkkIEK+tX1h180aCnHRWfjYKibsHfQsa125JehCg6Xn4r8M/RuG92+kfJTJfs +Jnk4D70lGDt+HPBrBdeNx1f1SjaAb6M+OEMi1H/tfl4h7lI3x2lwrV5gzqEOuDN3 +fYBq89txuSiQFAbm3G3HbuwIdw+c6r4nhoKGLfKsntZ7CFWAtSq2D9iqeY1gmnS7 +yMLtgdI+3gPi8DdjAVnZYXUU0jlZCAo1lA== +-----END CERTIFICATE----- diff --git a/revocation/crl/testdata/crlWithMultipleFreshestCRLs.crl b/revocation/crl/testdata/crlWithMultipleFreshestCRLs.crl new file mode 100644 index 00000000..49e5f5e0 Binary files /dev/null and b/revocation/crl/testdata/crlWithMultipleFreshestCRLs.crl differ diff --git a/revocation/crl/testdata/delta.crl b/revocation/crl/testdata/delta.crl new file mode 100644 index 00000000..8281417a Binary files /dev/null and b/revocation/crl/testdata/delta.crl differ diff --git a/revocation/internal/crl/const.go b/revocation/internal/crl/const.go new file mode 100644 index 00000000..a58ce967 --- /dev/null +++ b/revocation/internal/crl/const.go @@ -0,0 +1,32 @@ +// Copyright The Notary Project 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 crl + +const ( + // RFC 5280, 5.3.1 + // CRLReason ::= ENUMERATED { + // unspecified (0), + // keyCompromise (1), + // cACompromise (2), + // affiliationChanged (3), + // superseded (4), + // cessationOfOperation (5), + // certificateHold (6), + // -- value 7 is not used + // removeFromCRL (8), + // privilegeWithdrawn (9), + // aACompromise (10) } + reasonCodeCertificateHold = 6 + reasonCodeRemoveFromCRL = 8 +) diff --git a/revocation/internal/crl/crl.go b/revocation/internal/crl/crl.go index 50f2c085..c5684ad8 100644 --- a/revocation/internal/crl/crl.go +++ b/revocation/internal/crl/crl.go @@ -21,10 +21,12 @@ import ( "encoding/asn1" "errors" "fmt" + "math/big" "time" "github.com/notaryproject/notation-core-go/revocation/crl" "github.com/notaryproject/notation-core-go/revocation/result" + "golang.org/x/crypto/cryptobyte" ) var ( @@ -36,6 +38,10 @@ var ( // distribution point CRL extension. (See RFC 5280, Section 5.2.5) oidIssuingDistributionPoint = asn1.ObjectIdentifier{2, 5, 29, 28} + // oidDeltaCRLIndicator is the object identifier for the delta CRL indicator + // (See RFC 5280, Section 5.2.4) + oidDeltaCRLIndicator = asn1.ObjectIdentifier{2, 5, 29, 27} + // oidInvalidityDate is the object identifier for the invalidity date // CRL entry extension. (See RFC 5280, Section 5.3.2) oidInvalidityDate = asn1.ObjectIdentifier{2, 5, 29, 24} @@ -89,6 +95,7 @@ func CertCheckStatus(ctx context.Context, cert, issuer *x509.Certificate, opts C serverResults = make([]*result.ServerResult, 0, len(cert.CRLDistributionPoints)) lastErr error crlURL string + hasDeltaCRL = hasDeltaCRL(cert) ) // The CRLDistributionPoints contains the URIs of all the CRL distribution @@ -99,18 +106,35 @@ func CertCheckStatus(ctx context.Context, cert, issuer *x509.Certificate, opts C // point with one CRL URI, which will be cached, so checking all the URIs is // not a performance issue. for _, crlURL = range cert.CRLDistributionPoints { - bundle, err := opts.Fetcher.Fetch(ctx, crlURL) - if err != nil { - lastErr = fmt.Errorf("failed to download CRL from %s: %w", crlURL, err) - break + var ( + bundle *crl.Bundle + err error + ) + if !hasDeltaCRL { + bundle, err = opts.Fetcher.Fetch(ctx, crlURL) + if err != nil { + lastErr = fmt.Errorf("failed to download CRL from %s: %w", crlURL, err) + break + } + } else { + fetcher, ok := opts.Fetcher.(crl.FetcherWithCertificateExtensions) + if !ok { + lastErr = fmt.Errorf("fetcher does not support fetching delta CRL from certificate extension") + break + } + bundle, err = fetcher.FetchWithCertificateExtensions(ctx, crlURL, &cert.Extensions) + if err != nil { + lastErr = fmt.Errorf("failed to download CRL from %s: %w", crlURL, err) + break + } } - if err = validate(bundle.BaseCRL, issuer); err != nil { + if err = validate(bundle, issuer); err != nil { lastErr = fmt.Errorf("failed to validate CRL from %s: %w", crlURL, err) break } - crlResult, err := checkRevocation(cert, bundle.BaseCRL, opts.SigningTime, crlURL) + crlResult, err := checkRevocation(cert, bundle, opts.SigningTime, crlURL) if err != nil { lastErr = fmt.Errorf("failed to check revocation status from %s: %w", crlURL, err) break @@ -153,7 +177,56 @@ func Supported(cert *x509.Certificate) bool { return cert != nil && len(cert.CRLDistributionPoints) > 0 } -func validate(crl *x509.RevocationList, issuer *x509.Certificate) error { +func hasDeltaCRL(cert *x509.Certificate) bool { + for _, ext := range cert.Extensions { + if ext.Id.Equal(oidFreshestCRL) { + return true + } + } + return false +} + +func validate(bundle *crl.Bundle, issuer *x509.Certificate) error { + // validate base CRL + baseCRL := bundle.BaseCRL + if err := validateCRL(baseCRL, issuer); err != nil { + return fmt.Errorf("failed to validate base CRL: %w", err) + } + + if bundle.DeltaCRL != nil { + // validate delta CRL + // RFC 5280, Section 5.2.4 + deltaCRL := bundle.DeltaCRL + if err := validateCRL(deltaCRL, issuer); err != nil { + return fmt.Errorf("failed to validate delta CRL: %w", err) + } + + if deltaCRL.Number.Cmp(baseCRL.Number) <= 0 { + return fmt.Errorf("delta CRL number %d is not greater than the base CRL number %d", deltaCRL.Number, baseCRL.Number) + } + // check delta CRL indicator extension + var minimumBaseCRLNumber *big.Int + for _, ext := range deltaCRL.Extensions { + if ext.Id.Equal(oidDeltaCRLIndicator) { + minimumBaseCRLNumber = new(big.Int) + value := cryptobyte.String(ext.Value) + if !value.ReadASN1Integer(minimumBaseCRLNumber) { + return errors.New("failed to parse delta CRL indicator extension") + } + break + } + } + if minimumBaseCRLNumber == nil { + return errors.New("delta CRL indicator extension is not found") + } + if minimumBaseCRLNumber.Cmp(baseCRL.Number) > 0 { + return fmt.Errorf("delta CRL indicator %d is not less than or equal to the base CRL number %d", minimumBaseCRLNumber, baseCRL.Number) + } + } + return nil +} + +func validateCRL(crl *x509.RevocationList, issuer *x509.Certificate) error { // check signature if err := crl.CheckSignatureFrom(issuer); err != nil { return fmt.Errorf("CRL is not signed by CA %s: %w,", issuer.Subject, err) @@ -170,12 +243,12 @@ func validate(crl *x509.RevocationList, issuer *x509.Certificate) error { for _, ext := range crl.Extensions { switch { - case ext.Id.Equal(oidFreshestCRL): - return errors.New("delta CRL is not supported") case ext.Id.Equal(oidIssuingDistributionPoint): // IssuingDistributionPoint is a critical extension that identifies // the scope of the CRL. Since we will check all the CRL // distribution points, it is not necessary to check this extension. + case ext.Id.Equal(oidDeltaCRLIndicator): + // will be checked in validate() default: if ext.Critical { // unsupported critical extensions is not allowed. (See RFC 5280, Section 5.2) @@ -188,38 +261,73 @@ func validate(crl *x509.RevocationList, issuer *x509.Certificate) error { } // checkRevocation checks if the certificate is revoked or not -func checkRevocation(cert *x509.Certificate, baseCRL *x509.RevocationList, signingTime time.Time, crlURL string) (*result.ServerResult, error) { +func checkRevocation(cert *x509.Certificate, b *crl.Bundle, signingTime time.Time, crlURL string) (*result.ServerResult, error) { if cert == nil { return nil, errors.New("certificate cannot be nil") } - - if baseCRL == nil { + if b == nil { + return nil, errors.New("CRL bundle cannot be nil") + } + if b.BaseCRL == nil { return nil, errors.New("baseCRL cannot be nil") } - for _, revocationEntry := range baseCRL.RevokedCertificateEntries { - if revocationEntry.SerialNumber.Cmp(cert.SerialNumber) == 0 { - extensions, err := parseEntryExtensions(revocationEntry) - if err != nil { - return nil, err - } + entriesBundle := []*[]x509.RevocationListEntry{&b.BaseCRL.RevokedCertificateEntries} + if b.DeltaCRL != nil { + entriesBundle = append(entriesBundle, &b.DeltaCRL.RevokedCertificateEntries) + } - // validate signingTime and invalidityDate - if !signingTime.IsZero() && !extensions.invalidityDate.IsZero() && - signingTime.Before(extensions.invalidityDate) { - // signing time is before the invalidity date which means the - // certificate is not revoked at the time of signing. - break + // latestTempRevokedEntry contains the most recent revocation entry with + // reasons such as CertificateHold or RemoveFromCRL. + // + // If the certificate is revoked with CertificateHold, it is temporarily + // revoked. If the certificate is shown in the CRL with RemoveFromCRL, + // it is unrevoked. + var latestTempRevokedEntry *x509.RevocationListEntry + + // iterate over all the entries in the base and delta CRLs + for _, entries := range entriesBundle { + for i, revocationEntry := range *entries { + if revocationEntry.SerialNumber.Cmp(cert.SerialNumber) == 0 { + extensions, err := parseEntryExtensions(revocationEntry) + if err != nil { + return nil, err + } + + // validate signingTime and invalidityDate + if !signingTime.IsZero() && !extensions.invalidityDate.IsZero() && + signingTime.Before(extensions.invalidityDate) { + // signing time is before the invalidity date which means the + // certificate is not revoked at the time of signing. + break + } + + switch revocationEntry.ReasonCode { + case int(reasonCodeCertificateHold), int(reasonCodeRemoveFromCRL): + // temporarily revoked or unrevoked + if latestTempRevokedEntry == nil || latestTempRevokedEntry.RevocationTime.Before(revocationEntry.RevocationTime) { + // the revocation status depends on the most recent reason + latestTempRevokedEntry = &(*entries)[i] + } + default: + // permanently revoked + return &result.ServerResult{ + Result: result.ResultRevoked, + Server: crlURL, + RevocationMethod: result.RevocationMethodCRL, + }, nil + } } - - // revoked - return &result.ServerResult{ - Result: result.ResultRevoked, - Server: crlURL, - RevocationMethod: result.RevocationMethodCRL, - }, nil } } + if latestTempRevokedEntry != nil && latestTempRevokedEntry.ReasonCode == reasonCodeCertificateHold { + // revoked with CertificateHold + return &result.ServerResult{ + Result: result.ResultRevoked, + Server: crlURL, + RevocationMethod: result.RevocationMethodCRL, + }, nil + } return &result.ServerResult{ Result: result.ResultOK, diff --git a/revocation/internal/crl/crl_test.go b/revocation/internal/crl/crl_test.go index 27fa2475..53af4fc4 100644 --- a/revocation/internal/crl/crl_test.go +++ b/revocation/internal/crl/crl_test.go @@ -203,38 +203,6 @@ func TestCertCheckStatus(t *testing.T) { } }) - t.Run("CRL with delta CRL is not checked", func(t *testing.T) { - memoryCache := &memoryCache{} - - crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ - NextUpdate: time.Now().Add(time.Hour), - Number: big.NewInt(20240720), - ExtraExtensions: []pkix.Extension{ - { - Id: oidFreshestCRL, - Critical: false, - }, - }, - }, issuerCert, issuerKey) - if err != nil { - t.Fatal(err) - } - fetcher, err := crlutils.NewHTTPFetcher( - &http.Client{Transport: expectedRoundTripperMock{Body: crlBytes}}, - ) - if err != nil { - t.Fatal(err) - } - fetcher.Cache = memoryCache - fetcher.DiscardCacheError = true - r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{ - Fetcher: fetcher, - }) - if !strings.Contains(r.ServerResults[0].Error.Error(), "delta CRL is not supported") { - t.Fatalf("unexpected error, got %v, expected %v", r.ServerResults[0].Error, "delta CRL is not supported") - } - }) - memoryCache := &memoryCache{} // create a stale CRL @@ -328,6 +296,74 @@ func TestCertCheckStatus(t *testing.T) { t.Fatalf("expected OK, got %s", r.Result) } }) + + t.Run("certificate with invalid freshest crl extension", func(t *testing.T) { + chain[0].Cert.Extensions = []pkix.Extension{ + { + Id: oidFreshestCRL, + Value: []byte("invalid"), + }, + } + + crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + NextUpdate: time.Now().Add(time.Hour), + Number: big.NewInt(20240720), + }, issuerCert, issuerKey) + if err != nil { + t.Fatal(err) + } + + fetcher, err := crlutils.NewHTTPFetcher( + &http.Client{Transport: expectedRoundTripperMock{Body: crlBytes}}, + ) + if err != nil { + t.Fatal(err) + } + fetcher.DiscardCacheError = true + r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{ + Fetcher: fetcher, + }) + if r.Result != result.ResultUnknown { + t.Fatalf("expected Unknown, got %s", r.Result) + } + expectedErrorMsg := "failed to download CRL from http://localhost.test: failed to retrieve CRL: failed to parse Freshest CRL extension: x509: invalid CRL distribution points" + if r.ServerResults[0].Error == nil || r.ServerResults[0].Error.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, r.ServerResults[0].Error) + } + }) + + t.Run("fetcher doesn't support delta CRL", func(t *testing.T) { + chain[0].Cert.CRLDistributionPoints = []string{"http://localhost.test"} + chain[0].Cert.Extensions = []pkix.Extension{ + { + Id: oidFreshestCRL, + Value: []byte{0x00}, + }, + } + + fetcher := &fetcherMock{} + if err != nil { + t.Fatal(err) + } + r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{ + Fetcher: fetcher, + }) + if r.Result != result.ResultUnknown { + t.Fatalf("expected Unknown, got %s", r.Result) + } + + expectedErrorMsg := "fetcher does not support fetching delta CRL from certificate extension" + if r.ServerResults[0].Error == nil || r.ServerResults[0].Error.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, r.ServerResults[0].Error) + } + }) + +} + +type fetcherMock struct{} + +func (f *fetcherMock) Fetch(ctx context.Context, url string) (*crlutils.Bundle, error) { + return nil, fmt.Errorf("fetch error") } func TestValidate(t *testing.T) { @@ -349,7 +385,7 @@ func TestValidate(t *testing.T) { t.Fatal(err) } - if err := validate(crl, issuerCert); err == nil { + if err := validateCRL(crl, issuerCert); err == nil { t.Fatal("expected error") } }) @@ -359,7 +395,7 @@ func TestValidate(t *testing.T) { NextUpdate: time.Now().Add(time.Hour), } - if err := validate(crl, &x509.Certificate{}); err == nil { + if err := validateCRL(crl, &x509.Certificate{}); err == nil { t.Fatal("expected error") } }) @@ -390,7 +426,7 @@ func TestValidate(t *testing.T) { }, } - if err := validate(crl, issuerCert); err == nil { + if err := validateCRL(crl, issuerCert); err == nil { t.Fatal("expected error") } }) @@ -419,37 +455,215 @@ func TestValidate(t *testing.T) { t.Fatal(err) } - if err := validate(crl, issuerCert); err != nil { + if err := validateCRL(crl, issuerCert); err != nil { t.Fatal(err) } }) - t.Run("delta CRL is not supported", func(t *testing.T) { - chain := testhelper.GetRevokableRSAChainWithRevocations(1, false, true) - issuerCert := chain[0].Cert - issuerKey := chain[0].PrivateKey + chain := testhelper.GetRevokableRSAChainWithRevocations(1, false, true) + issuerCert := chain[0].Cert + issuerKey := chain[0].PrivateKey - crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + NextUpdate: time.Now().Add(time.Hour), + Number: big.NewInt(20240720), + }, issuerCert, issuerKey) + if err != nil { + t.Fatal(err) + } + + crl, err := x509.ParseRevocationList(crlBytes) + if err != nil { + t.Fatal(err) + } + + t.Run("valid crl and delta crl", func(t *testing.T) { + deltaCRLIndicator := big.NewInt(20240720) + deltaCRLIndicatorBytes, err := asn1.Marshal(deltaCRLIndicator) + if err != nil { + t.Fatal(err) + } + deltaCRLBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ NextUpdate: time.Now().Add(time.Hour), - Number: big.NewInt(20240720), + Number: big.NewInt(20240721), ExtraExtensions: []pkix.Extension{ { - Id: oidFreshestCRL, - Critical: false, + Id: oidDeltaCRLIndicator, + Critical: true, + Value: deltaCRLIndicatorBytes, }, }, }, issuerCert, issuerKey) if err != nil { t.Fatal(err) } + deltaCRL, err := x509.ParseRevocationList(deltaCRLBytes) + if err != nil { + t.Fatal(err) + } + bundle := &crlutils.Bundle{ + BaseCRL: crl, + DeltaCRL: deltaCRL, + } + if err := validate(bundle, issuerCert); err != nil { + t.Fatal(err) + } + }) - crl, err := x509.ParseRevocationList(crlBytes) + t.Run("invalid delta crl", func(t *testing.T) { + deltaCRLIndicator := big.NewInt(20240720) + deltaCRLIndicatorBytes, err := asn1.Marshal(deltaCRLIndicator) + if err != nil { + t.Fatal(err) + } + deltaCRLBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + Number: big.NewInt(20240721), + ExtraExtensions: []pkix.Extension{ + { + Id: oidDeltaCRLIndicator, + Critical: true, + Value: deltaCRLIndicatorBytes, + }, + }, + }, issuerCert, issuerKey) if err != nil { t.Fatal(err) } + deltaCRL, err := x509.ParseRevocationList(deltaCRLBytes) + if err != nil { + t.Fatal(err) + } + bundle := &crlutils.Bundle{ + BaseCRL: crl, + DeltaCRL: deltaCRL, + } + err = validate(bundle, issuerCert) + expectedErrorMsg := "failed to validate delta CRL: CRL NextUpdate is not set" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) + } + }) - if err := validate(crl, issuerCert); err.Error() != "delta CRL is not supported" { - t.Fatalf("got %v, expected delta CRL is not supported", err) + t.Run("invalid delta crl number", func(t *testing.T) { + deltaCRLIndicator := big.NewInt(20240720) + deltaCRLIndicatorBytes, err := asn1.Marshal(deltaCRLIndicator) + if err != nil { + t.Fatal(err) + } + deltaCRLBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + NextUpdate: time.Now().Add(time.Hour), + Number: big.NewInt(20240719), + ExtraExtensions: []pkix.Extension{ + { + Id: oidDeltaCRLIndicator, + Critical: true, + Value: deltaCRLIndicatorBytes, + }, + }, + }, issuerCert, issuerKey) + if err != nil { + t.Fatal(err) + } + deltaCRL, err := x509.ParseRevocationList(deltaCRLBytes) + if err != nil { + t.Fatal(err) + } + bundle := &crlutils.Bundle{ + BaseCRL: crl, + DeltaCRL: deltaCRL, + } + err = validate(bundle, issuerCert) + expectedErrorMsg := "delta CRL number 20240719 is not greater than the base CRL number 20240720" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) + } + }) + + t.Run("delta crl without delta crl indicator", func(t *testing.T) { + deltaCRLBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + NextUpdate: time.Now().Add(time.Hour), + Number: big.NewInt(20240721), + }, issuerCert, issuerKey) + if err != nil { + t.Fatal(err) + } + deltaCRL, err := x509.ParseRevocationList(deltaCRLBytes) + if err != nil { + t.Fatal(err) + } + bundle := &crlutils.Bundle{ + BaseCRL: crl, + DeltaCRL: deltaCRL, + } + err = validate(bundle, issuerCert) + expectedErrorMsg := "delta CRL indicator extension is not found" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) + } + }) + + t.Run("delta crl minimum base crl number is greater than base crl", func(t *testing.T) { + deltaCRLIndicator := big.NewInt(20240721) + deltaCRLIndicatorBytes, err := asn1.Marshal(deltaCRLIndicator) + if err != nil { + t.Fatal(err) + } + deltaCRLBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + NextUpdate: time.Now().Add(time.Hour), + Number: big.NewInt(20240722), + ExtraExtensions: []pkix.Extension{ + { + Id: oidDeltaCRLIndicator, + Critical: true, + Value: deltaCRLIndicatorBytes, + }, + }, + }, issuerCert, issuerKey) + if err != nil { + t.Fatal(err) + } + deltaCRL, err := x509.ParseRevocationList(deltaCRLBytes) + if err != nil { + t.Fatal(err) + } + bundle := &crlutils.Bundle{ + BaseCRL: crl, + DeltaCRL: deltaCRL, + } + err = validate(bundle, issuerCert) + expectedErrorMsg := "delta CRL indicator 20240721 is not less than or equal to the base CRL number 20240720" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) + } + }) + + t.Run("delta crl with invalid delta indicator extension", func(t *testing.T) { + deltaCRLBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + NextUpdate: time.Now().Add(time.Hour), + Number: big.NewInt(20240722), + ExtraExtensions: []pkix.Extension{ + { + Id: oidDeltaCRLIndicator, + Critical: true, + Value: []byte("invalid"), + }, + }, + }, issuerCert, issuerKey) + if err != nil { + t.Fatal(err) + } + deltaCRL, err := x509.ParseRevocationList(deltaCRLBytes) + if err != nil { + t.Fatal(err) + } + bundle := &crlutils.Bundle{ + BaseCRL: crl, + DeltaCRL: deltaCRL, + } + err = validate(bundle, issuerCert) + expectedErrorMsg := "failed to parse delta CRL indicator extension" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) } }) } @@ -461,14 +675,22 @@ func TestCheckRevocation(t *testing.T) { signingTime := time.Now() t.Run("certificate is nil", func(t *testing.T) { - _, err := checkRevocation(nil, &x509.RevocationList{}, signingTime, "") + _, err := checkRevocation(nil, &crlutils.Bundle{BaseCRL: &x509.RevocationList{}}, signingTime, "") if err == nil { t.Fatal("expected error") } }) - t.Run("CRL is nil", func(t *testing.T) { + t.Run("bundle is nil", func(t *testing.T) { _, err := checkRevocation(cert, nil, signingTime, "") + expectedErrorMsg := "CRL bundle cannot be nil" + if err == nil || err.Error() != expectedErrorMsg { + t.Fatalf("expected error %q, got %v", expectedErrorMsg, err) + } + }) + + t.Run("CRL is nil", func(t *testing.T) { + _, err := checkRevocation(cert, &crlutils.Bundle{}, signingTime, "") if err == nil { t.Fatal("expected error") } @@ -482,7 +704,7 @@ func TestCheckRevocation(t *testing.T) { }, }, } - r, err := checkRevocation(cert, baseCRL, signingTime, "") + r, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL}, signingTime, "") if err != nil { t.Fatal(err) } @@ -500,7 +722,7 @@ func TestCheckRevocation(t *testing.T) { }, }, } - r, err := checkRevocation(cert, baseCRL, signingTime, "") + r, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL}, signingTime, "") if err != nil { t.Fatal(err) } @@ -533,7 +755,7 @@ func TestCheckRevocation(t *testing.T) { }, }, } - r, err := checkRevocation(cert, baseCRL, signingTime, "") + r, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL}, signingTime, "") if err != nil { t.Fatal(err) } @@ -566,7 +788,7 @@ func TestCheckRevocation(t *testing.T) { }, }, } - r, err := checkRevocation(cert, baseCRL, signingTime, "") + r, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL}, signingTime, "") if err != nil { t.Fatal(err) } @@ -584,7 +806,7 @@ func TestCheckRevocation(t *testing.T) { }, }, } - r, err := checkRevocation(cert, baseCRL, time.Time{}, "") + r, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL}, time.Time{}, "") if err != nil { t.Fatal(err) } @@ -607,11 +829,91 @@ func TestCheckRevocation(t *testing.T) { }, }, } - _, err := checkRevocation(cert, baseCRL, signingTime, "") + _, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL}, signingTime, "") if err == nil { t.Fatal("expected error") } }) + + t.Run("delta crl with certificate hold", func(t *testing.T) { + baseCRL := &x509.RevocationList{} + deltaCRL := &x509.RevocationList{ + RevokedCertificateEntries: []x509.RevocationListEntry{ + { + SerialNumber: big.NewInt(1), + ReasonCode: reasonCodeCertificateHold, + }, + }, + } + r, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL, DeltaCRL: deltaCRL}, signingTime, "") + if err != nil { + t.Fatal(err) + } + if r.Result != result.ResultRevoked { + t.Fatalf("expected revoked, got %s", r.Result) + } + }) + + t.Run("certificate hold and remove hold", func(t *testing.T) { + baseCRL := &x509.RevocationList{ + RevokedCertificateEntries: []x509.RevocationListEntry{ + { + SerialNumber: big.NewInt(1), + ReasonCode: reasonCodeCertificateHold, + RevocationTime: time.Now().Add(-time.Hour), + }, + }, + } + deltaCRL := &x509.RevocationList{ + RevokedCertificateEntries: []x509.RevocationListEntry{ + { + SerialNumber: big.NewInt(1), + ReasonCode: reasonCodeRemoveFromCRL, + RevocationTime: time.Now(), + }, + }, + } + r, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL, DeltaCRL: deltaCRL}, signingTime, "") + if err != nil { + t.Fatal(err) + } + if r.Result != result.ResultOK { + t.Fatalf("expected OK, got %s", r.Result) + } + }) + + t.Run("certificate hold, remove hold and hold again", func(t *testing.T) { + baseCRL := &x509.RevocationList{ + RevokedCertificateEntries: []x509.RevocationListEntry{ + { + SerialNumber: big.NewInt(1), + ReasonCode: reasonCodeCertificateHold, + RevocationTime: time.Now().Add(-2 * time.Hour), + }, + }, + } + deltaCRL := &x509.RevocationList{ + RevokedCertificateEntries: []x509.RevocationListEntry{ + { + SerialNumber: big.NewInt(1), + ReasonCode: reasonCodeRemoveFromCRL, + RevocationTime: time.Now().Add(-time.Hour), + }, + { + SerialNumber: big.NewInt(1), + ReasonCode: reasonCodeCertificateHold, + RevocationTime: time.Now(), + }, + }, + } + r, err := checkRevocation(cert, &crlutils.Bundle{BaseCRL: baseCRL, DeltaCRL: deltaCRL}, signingTime, "") + if err != nil { + t.Fatal(err) + } + if r.Result != result.ResultRevoked { + t.Fatalf("expected revoked, got %s", r.Result) + } + }) } func TestParseEntryExtension(t *testing.T) { @@ -737,6 +1039,19 @@ func TestSupported(t *testing.T) { }) } +func TestHasDeltaCRL(t *testing.T) { + cert := &x509.Certificate{ + Extensions: []pkix.Extension{ + { + Id: oidFreshestCRL, + }, + }, + } + if !hasDeltaCRL(cert) { + t.Fatal("expected has delta CRL") + } +} + type errorRoundTripperMock struct{} func (rt errorRoundTripperMock) RoundTrip(req *http.Request) (*http.Response, error) {