fix: resolve comments for Two-Hearts

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>

Author: JeyJeyGao

revocation/crl/fetcher.go

@@ -25,11 +25,11 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"slices"
 	"time"
 
+	"github.com/notaryproject/notation-core-go/revocation/internal/x509util"
 	"golang.org/x/crypto/cryptobyte"
-	cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1"
+	cbasn1 "golang.org/x/crypto/cryptobyte/asn1"
 )
 
 // oidFreshestCRL is the object identifier for the distribution point
@@ -142,17 +142,16 @@ func (f *HTTPFetcher) fetch(ctx context.Context, url string) (*Bundle, error) {
 //
 // It returns errDeltaCRLNotFound if the delta CRL is not found.
 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 < 0 {
+	extension := x509util.FindExtensionByOID(oidFreshestCRL, extensions)
+	if extension == nil {
 		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)
+	urls, err := parseCRLDistributionPoint(extension.Value)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse Freshest CRL extension: %w", err)
 	}
@@ -197,31 +196,31 @@ func parseCRLDistributionPoint(value []byte) ([]string, error) {
 	//     fullName                [0]     GeneralNames,
 	//     nameRelativeToCRLIssuer [1]     RelativeDistinguishedName }
 	val := cryptobyte.String(value)
-	if !val.ReadASN1(&val, cryptobyte_asn1.SEQUENCE) {
+	if !val.ReadASN1(&val, cbasn1.SEQUENCE) {
 		return nil, errors.New("x509: invalid CRL distribution points")
 	}
 	for !val.Empty() {
 		var dpDER cryptobyte.String
-		if !val.ReadASN1(&dpDER, cryptobyte_asn1.SEQUENCE) {
+		if !val.ReadASN1(&dpDER, cbasn1.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()) {
+		if !dpDER.ReadOptionalASN1(&dpNameDER, &dpNamePresent, cbasn1.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()) {
+		if !dpNameDER.ReadASN1(&dpNameDER, cbasn1.Tag(0).Constructed().ContextSpecific()) {
 			return nil, errors.New("x509: invalid CRL distribution point")
 		}
 		for !dpNameDER.Empty() {
-			if !dpNameDER.PeekASN1Tag(cryptobyte_asn1.Tag(6).ContextSpecific()) {
+			if !dpNameDER.PeekASN1Tag(cbasn1.Tag(6).ContextSpecific()) {
 				break
 			}
 			var uri cryptobyte.String
-			if !dpNameDER.ReadASN1(&uri, cryptobyte_asn1.Tag(6).ContextSpecific()) {
+			if !dpNameDER.ReadASN1(&uri, cbasn1.Tag(6).ContextSpecific()) {
 				return nil, errors.New("x509: invalid CRL distribution point")
 			}
 			urls = append(urls, string(uri))

revocation/internal/crl/crl.go

@@ -23,30 +23,34 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
-	"slices"
 	"time"
 
 	"github.com/notaryproject/notation-core-go/revocation/crl"
+	"github.com/notaryproject/notation-core-go/revocation/internal/x509util"
 	"github.com/notaryproject/notation-core-go/revocation/result"
 	"golang.org/x/crypto/cryptobyte"
 )
 
+// 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) }
 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) }
+	// certificateHold
 	reasonCodeCertificateHold = 6
-	reasonCodeRemoveFromCRL   = 8
+
+	// removeFromCRL
+	reasonCodeRemoveFromCRL = 8
 )
 
 var (
@@ -197,9 +201,7 @@ func Supported(cert *x509.Certificate) bool {
 }
 
 func hasFreshestCRL(extensions []pkix.Extension) bool {
-	return slices.ContainsFunc(extensions, func(ext pkix.Extension) bool {
-		return ext.Id.Equal(oidFreshestCRL)
-	})
+	return x509util.FindExtensionByOID(oidFreshestCRL, extensions) != nil
 }
 
 func validate(bundle *crl.Bundle, issuer *x509.Certificate) error {
@@ -224,14 +226,12 @@ func validate(bundle *crl.Bundle, issuer *x509.Certificate) error {
 	}
 
 	// check delta CRL indicator extension
-	idx := slices.IndexFunc(deltaCRL.Extensions, func(ext pkix.Extension) bool {
-		return ext.Id.Equal(oidDeltaCRLIndicator)
-	})
-	if idx < 0 {
+	extension := x509util.FindExtensionByOID(oidDeltaCRLIndicator, deltaCRL.Extensions)
+	if extension == nil {
 		return errors.New("delta CRL indicator extension is not found")
 	}
 	minimumBaseCRLNumber := new(big.Int)
-	value := cryptobyte.String(deltaCRL.Extensions[idx].Value)
+	value := cryptobyte.String(extension.Value)
 	if !value.ReadASN1Integer(minimumBaseCRLNumber) {
 		return errors.New("failed to parse delta CRL indicator extension")
 	}

revocation/internal/x509util/extension.go

@@ -0,0 +1,31 @@
+// 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 x509util
+
+import (
+	"crypto/x509/pkix"
+	"encoding/asn1"
+	"slices"
+)
+
+// FindExtensionByOID finds the extension by the given OID.
+func FindExtensionByOID(oid asn1.ObjectIdentifier, extensions []pkix.Extension) *pkix.Extension {
+	idx := slices.IndexFunc(extensions, func(ext pkix.Extension) bool {
+		return ext.Id.Equal(oid)
+	})
+	if idx < 0 {
+		return nil
+	}
+	return &extensions[idx]
+}

revocation/internal/x509util/extension_test.go

@@ -0,0 +1,45 @@
+package x509util
+
+import (
+	"crypto/x509/pkix"
+	"encoding/asn1"
+	"testing"
+)
+
+func TestFindExtensionByOID(t *testing.T) {
+	oid1 := asn1.ObjectIdentifier{1, 2, 3, 4}
+	oid2 := asn1.ObjectIdentifier{1, 2, 3, 5}
+	extensions := []pkix.Extension{
+		{Id: oid1, Value: []byte("value1")},
+		{Id: oid2, Value: []byte("value2")},
+	}
+
+	tests := []struct {
+		name       string
+		oid        asn1.ObjectIdentifier
+		extensions []pkix.Extension
+		expected   *pkix.Extension
+	}{
+		{
+			name:       "Extension found",
+			oid:        oid1,
+			extensions: extensions,
+			expected:   &extensions[0],
+		},
+		{
+			name:       "Extension not found",
+			oid:        asn1.ObjectIdentifier{1, 2, 3, 6},
+			extensions: extensions,
+			expected:   nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := FindExtensionByOID(tt.oid, tt.extensions)
+			if result != tt.expected {
+				t.Errorf("expected %v, got %v", tt.expected, result)
+			}
+		})
+	}
+}

revocation/internal/x509util/validate.go

@@ -12,7 +12,8 @@
 // limitations under the License.
 
 // Package x509util provides the method to validate the certificate chain for a
-// specific purpose, including code signing and timestamping.
+// specific purpose, including code signing and timestamping. It also provides
+// the method to find the extension by the given OID.
 package x509util
 
 import (