Skip to content

Commit

Permalink
fix: working on delta CRL
Browse files Browse the repository at this point in the history
Signed-off-by: Junjie Gao <[email protected]>
  • Loading branch information
JeyJeyGao committed Nov 27, 2024
1 parent 2049193 commit 33d5590
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 12 deletions.
48 changes: 48 additions & 0 deletions revocation/crl/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ import (
"context"
"crypto/rand"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"errors"
"fmt"
"io"
"math/big"
"net/http"
"os"
"strings"
"sync"
"testing"
Expand Down Expand Up @@ -257,6 +260,51 @@ func TestFetch(t *testing.T) {
})
}

func TestParseFreshestCRL(t *testing.T) {
certPath := "testdata/certificateWithDeltaCRL.cer"
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)
}

var freshestCRLExtension pkix.Extension
found := false
for _, ext := range cert.Extensions {
if ext.Id.Equal([]int{2, 5, 29, 46}) { // id-ce-freshestCRL
freshestCRLExtension = ext
found = true
break
}
}

if !found {
t.Fatalf("freshest CRL extension not found in certificate")
}

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])
}
}

func TestDownload(t *testing.T) {
t.Run("parse url error", func(t *testing.T) {
_, err := fetchCRL(context.Background(), ":", http.DefaultClient)
Expand Down
28 changes: 28 additions & 0 deletions revocation/crl/testdata/certificateWithDeltaCRL.cer
Original file line number Diff line number Diff line change
@@ -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-----
55 changes: 47 additions & 8 deletions revocation/internal/crl/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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}
Expand Down Expand Up @@ -123,16 +129,10 @@ func CertCheckStatus(ctx context.Context, cert, issuer *x509.Certificate, opts C
}
}

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
}
if bundle.DeltaCRL != nil {
if err = validate(bundle.DeltaCRL, issuer); err != nil {
lastErr = fmt.Errorf("failed to validate delta CRL from %s: %w", crlURL, err)
break
}
}

crlResult, err := checkRevocation(cert, bundle, opts.SigningTime, crlURL)
if err != nil {
Expand Down Expand Up @@ -186,7 +186,46 @@ func hasDeltaCRL(cert *x509.Certificate) bool {
return false
}

func validate(crl *x509.RevocationList, issuer *x509.Certificate) error {
func validate(bundle *crl.Bundle, issuer *x509.Certificate) error {
// validate base CRL
baseCRL := bundle.BaseCRL
if err := validateCRL(baseCRL, issuer); err != nil {
return err
}

if bundle.DeltaCRL != nil {
// validate delta CRL
// RFC 5280, Section 5.2.4
deltaCRL := bundle.DeltaCRL
if err := validateCRL(bundle.DeltaCRL, issuer); err != nil {
return err
}

Check warning on line 202 in revocation/internal/crl/crl.go

View check run for this annotation

Codecov / codecov/patch

revocation/internal/crl/crl.go#L197-L202

Added lines #L197 - L202 were not covered by tests

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 warning on line 206 in revocation/internal/crl/crl.go

View check run for this annotation

Codecov / codecov/patch

revocation/internal/crl/crl.go#L204-L206

Added lines #L204 - L206 were not covered by tests
// 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")
}

Check warning on line 215 in revocation/internal/crl/crl.go

View check run for this annotation

Codecov / codecov/patch

revocation/internal/crl/crl.go#L208-L215

Added lines #L208 - L215 were not covered by tests
}
}
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)
}

Check warning on line 223 in revocation/internal/crl/crl.go

View check run for this annotation

Codecov / codecov/patch

revocation/internal/crl/crl.go#L218-L223

Added lines #L218 - L223 were not covered by tests
}
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)
Expand Down
8 changes: 4 additions & 4 deletions revocation/internal/crl/crl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,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")
}
})
Expand All @@ -327,7 +327,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")
}
})
Expand Down Expand Up @@ -358,7 +358,7 @@ func TestValidate(t *testing.T) {
},
}

if err := validate(crl, issuerCert); err == nil {
if err := validateCRL(crl, issuerCert); err == nil {
t.Fatal("expected error")
}
})
Expand Down Expand Up @@ -387,7 +387,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(err)
}
})
Expand Down

0 comments on commit 33d5590

Please sign in to comment.