fix: update

Signed-off-by: Junjie Gao <[email protected]>

revocation/crl/fetcher.go

@@ -25,6 +25,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"slices"
 	"time"
 
 	"golang.org/x/crypto/cryptobyte"
@@ -127,7 +128,7 @@ func (f *HTTPFetcher) fetch(ctx context.Context, url string) (*Bundle, error) {
 	}
 
 	// fetch delta CRL from base CRL extension
-	deltaCRL, err := f.fetchDeltaCRL(&base.Extensions)
+	deltaCRL, err := f.fetchDeltaCRL(ctx, base.Extensions)
 	if err != nil && !errors.Is(err, errDeltaCRLNotFound) {
 		return nil, err
 	}
@@ -141,39 +142,41 @@ func (f *HTTPFetcher) fetch(ctx context.Context, url string) (*Bundle, error) {
 // fetchDeltaCRL fetches the delta CRL from the given extensions of base CRL.
 //
 // It returns errDeltaCRLNotFound if the delta CRL is not found.
-func (f *HTTPFetcher) fetchDeltaCRL(extensions *[]pkix.Extension) (*x509.RevocationList, error) {
+func (f *HTTPFetcher) fetchDeltaCRL(ctx context.Context, extensions []pkix.Extension) (*x509.RevocationList, error) {
+
+	idx := slices.IndexFunc(extensions, func(ext pkix.Extension) bool {
+		return ext.Id.Equal(oidFreshestCRL)
+	})
+	if idx == -1 {
+		return nil, errDeltaCRLNotFound
+	}
+	// RFC 5280, 4.2.1.15
+	//    id-ce-freshestCRL OBJECT IDENTIFIER ::=  { id-ce 46 }
+	//
+	//    FreshestCRL ::= CRLDistributionPoints
+	urls, err := parseCRLDistributionPoint(extensions[idx].Value)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse Freshest CRL extension: %w", err)
+	}
+	if len(urls) == 0 {
+		return nil, errDeltaCRLNotFound
+	}
+
 	var (
 		lastError error
 		deltaCRL  *x509.RevocationList
 	)
-	for _, ext := range *extensions {
-		if ext.Id.Equal(oidFreshestCRL) {
-			// RFC 5280, 4.2.1.15
-			//    id-ce-freshestCRL OBJECT IDENTIFIER ::=  { id-ce 46 }
-			//
-			//    FreshestCRL ::= CRLDistributionPoints
-			urls, err := parseCRLDistributionPoint(ext.Value)
-			if err != nil {
-				return nil, fmt.Errorf("failed to parse Freshest CRL extension: %w", err)
-			}
-
-			for _, cdpURL := range urls {
-				// RFC 5280, 5.2.6
-				// Delta CRLs from the base CRL have the same scope as the base
-				// CRL, so the URLs are for redundancy and should be tried in
-				// order until one succeeds.
-				deltaCRL, lastError = fetchCRL(context.Background(), cdpURL, f.httpClient)
-				if lastError == nil {
-					return deltaCRL, nil
-				}
-			}
-			break
+	for _, cdpURL := range urls {
+		// RFC 5280, 5.2.6
+		// Delta CRLs from the base CRL have the same scope as the base
+		// CRL, so the URLs are for redundancy and should be tried in
+		// order until one succeeds.
+		deltaCRL, lastError = fetchCRL(ctx, cdpURL, f.httpClient)
+		if lastError == nil {
+			return deltaCRL, nil
 		}
 	}
-	if lastError != nil {
-		return nil, lastError
-	}
-	return nil, errDeltaCRLNotFound
+	return nil, lastError
 }
 
 // parseCRLDistributionPoint parses the CRL extension and returns the CRL URLs

revocation/crl/fetcher_test.go

@@ -381,7 +381,7 @@ func TestParseFreshestCRL(t *testing.T) {
 }
 
 func TestFetchDeltaCRL(t *testing.T) {
-	loadExtentsion := func(certPath string) *[]pkix.Extension {
+	loadExtentsion := func(certPath string) []pkix.Extension {
 		certData, err := os.ReadFile(certPath)
 		if err != nil {
 			t.Fatalf("failed to read certificate: %v", err)
@@ -397,7 +397,7 @@ func TestFetchDeltaCRL(t *testing.T) {
 			t.Fatalf("failed to parse certificate: %v", err)
 		}
 
-		return &cert.Extensions
+		return cert.Extensions
 	}
 
 	deltaCRL, err := os.ReadFile("testdata/delta.crl")
@@ -415,7 +415,7 @@ func TestFetchDeltaCRL(t *testing.T) {
 	t.Run("parse freshest CRL failed", func(t *testing.T) {
 		certPath := "testdata/certificateWithIncompleteFreshestCRL.cer"
 		extensions := loadExtentsion(certPath)
-		_, err := fetcher.fetchDeltaCRL(extensions)
+		_, err := fetcher.fetchDeltaCRL(context.Background(), 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)
@@ -425,7 +425,7 @@ func TestFetchDeltaCRL(t *testing.T) {
 	t.Run("zero freshest CRL URL", func(t *testing.T) {
 		certPath := "testdata/certificateWithZeroDeltaCRLURL.cer"
 		extensions := loadExtentsion(certPath)
-		_, err := fetcher.fetchDeltaCRL(extensions)
+		_, err := fetcher.fetchDeltaCRL(context.Background(), extensions)
 		expectedErr := errDeltaCRLNotFound
 		if err == nil || !errors.Is(err, expectedErr) {
 			t.Fatalf("expected error %v, got %v", expectedErr, err)
@@ -435,7 +435,7 @@ func TestFetchDeltaCRL(t *testing.T) {
 	t.Run("one freshest CRL URL", func(t *testing.T) {
 		certPath := "testdata/certificateWithDeltaCRL.cer"
 		extensions := loadExtentsion(certPath)
-		deltaCRL, err := fetcher.fetchDeltaCRL(extensions)
+		deltaCRL, err := fetcher.fetchDeltaCRL(context.Background(), extensions)
 		if err != nil {
 			t.Fatalf("failed to process delta CRL: %v", err)
 		}
@@ -453,7 +453,7 @@ func TestFetchDeltaCRL(t *testing.T) {
 		}
 		certPath := "testdata/certificateWith2DeltaCRL.cer"
 		extensions := loadExtentsion(certPath)
-		_, err = fetcherWithError.fetchDeltaCRL(extensions)
+		_, err = fetcherWithError.fetchDeltaCRL(context.Background(), extensions)
 		expectedErrorMsg := "request failed"
 		if err == nil || !strings.Contains(err.Error(), expectedErrorMsg) {
 			t.Fatalf("expected error %q, got %v", expectedErrorMsg, err)
@@ -463,7 +463,7 @@ func TestFetchDeltaCRL(t *testing.T) {
 	t.Run("process delta crl from certificate extension failed", func(t *testing.T) {
 		certPath := "testdata/certificateWithIncompleteFreshestCRL.cer"
 		extensions := loadExtentsion(certPath)
-		_, err := fetcher.fetchDeltaCRL(extensions)
+		_, err := fetcher.fetchDeltaCRL(context.Background(), 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)

revocation/internal/crl/crl.go

@@ -23,6 +23,7 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
+	"slices"
 	"time"
 
 	"github.com/notaryproject/notation-core-go/revocation/crl"
@@ -114,7 +115,7 @@ func CertCheckStatus(ctx context.Context, cert, issuer *x509.Certificate, opts C
 		serverResults               = make([]*result.ServerResult, 0, len(cert.CRLDistributionPoints))
 		lastErr                     error
 		crlURL                      string
-		hasFreshestCRLInCertificate = hasFreshestCRL(&cert.Extensions)
+		hasFreshestCRLInCertificate = hasFreshestCRL(cert.Extensions)
 	)
 
 	// The CRLDistributionPoints contains the URIs of all the CRL distribution
@@ -195,13 +196,10 @@ func Supported(cert *x509.Certificate) bool {
 	return cert != nil && len(cert.CRLDistributionPoints) > 0
 }
 
-func hasFreshestCRL(extensions *[]pkix.Extension) bool {
-	for _, ext := range *extensions {
-		if ext.Id.Equal(oidFreshestCRL) {
-			return true
-		}
-	}
-	return false
+func hasFreshestCRL(extensions []pkix.Extension) bool {
+	return slices.ContainsFunc(extensions, func(ext pkix.Extension) bool {
+		return ext.Id.Equal(oidFreshestCRL)
+	})
 }
 
 func validate(bundle *crl.Bundle, issuer *x509.Certificate) error {
@@ -211,35 +209,34 @@ func validate(bundle *crl.Bundle, issuer *x509.Certificate) error {
 		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 bundle.DeltaCRL == nil {
+		return nil
+	}
 
-		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)
-		}
+	// 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
+	idx := slices.IndexFunc(deltaCRL.Extensions, func(ext pkix.Extension) bool {
+		return ext.Id.Equal(oidDeltaCRLIndicator)
+	})
+	if idx == -1 {
+		return errors.New("delta CRL indicator extension is not found")
+	}
+	minimumBaseCRLNumber := new(big.Int)
+	value := cryptobyte.String(deltaCRL.Extensions[idx].Value)
+	if !value.ReadASN1Integer(minimumBaseCRLNumber) {
+		return errors.New("failed to parse delta CRL indicator extension")
+	}
+	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
 }
@@ -290,9 +287,9 @@ func checkRevocation(cert *x509.Certificate, b *crl.Bundle, signingTime time.Tim
 		return nil, errors.New("baseCRL cannot be nil")
 	}
 
-	revocationListBundle := []*[]x509.RevocationListEntry{&b.BaseCRL.RevokedCertificateEntries}
+	revocationListBundle := [][]x509.RevocationListEntry{b.BaseCRL.RevokedCertificateEntries}
 	if b.DeltaCRL != nil {
-		revocationListBundle = append(revocationListBundle, &b.DeltaCRL.RevokedCertificateEntries)
+		revocationListBundle = append(revocationListBundle, b.DeltaCRL.RevokedCertificateEntries)
 	}
 
 	// latestTempRevokedEntry contains the most recent revocation entry with
@@ -305,7 +302,7 @@ func checkRevocation(cert *x509.Certificate, b *crl.Bundle, signingTime time.Tim
 
 	// iterate over all the entries in the base and delta CRLs
 	for _, revocationList := range revocationListBundle {
-		for i, revocationEntry := range *revocationList {
+		for i, revocationEntry := range revocationList {
 			if revocationEntry.SerialNumber.Cmp(cert.SerialNumber) == 0 {
 				extensions, err := parseEntryExtensions(revocationEntry)
 				if err != nil {
@@ -321,11 +318,11 @@ func checkRevocation(cert *x509.Certificate, b *crl.Bundle, signingTime time.Tim
 				}
 
 				switch revocationEntry.ReasonCode {
-				case int(reasonCodeCertificateHold), int(reasonCodeRemoveFromCRL):
+				case reasonCodeCertificateHold, reasonCodeRemoveFromCRL:
 					// temporarily revoked or unrevoked
 					if latestTempRevokedEntry == nil || latestTempRevokedEntry.RevocationTime.Before(revocationEntry.RevocationTime) {
 						// the revocation status depends on the most recent reason
-						latestTempRevokedEntry = &(*revocationList)[i]
+						latestTempRevokedEntry = &revocationList[i]
 					}
 				default:
 					// permanently revoked

revocation/internal/crl/crl_test.go

@@ -1013,7 +1013,7 @@ func TestHasDeltaCRL(t *testing.T) {
 			},
 		},
 	}
-	if !hasFreshestCRL(&cert.Extensions) {
+	if !hasFreshestCRL(cert.Extensions) {
 		t.Fatal("expected has delta CRL")
 	}
 }