fix: update

Signed-off-by: Junjie Gao <[email protected]>
JeyJeyGao · Sep 14, 2024 · b7bbef4 · b7bbef4
1 parent c00b956
commit b7bbef4
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 34 deletions.
diff --git a/go.mod b/go.mod
@@ -25,4 +25,4 @@ require (
 	golang.org/x/sync v0.6.0 // indirect
 )
 
-replace github.com/notaryproject/notation-core-go => github.com/JeyJeyGao/notation-core-go v0.0.0-20240820065505-3bb154fda9f6
+replace github.com/notaryproject/notation-core-go => github.com/JeyJeyGao/notation-core-go v0.0.0-20240913022242-982561e18bc8
diff --git a/go.sum b/go.sum
@@ -1,7 +1,7 @@
 github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 h1:mFRzDkZVAjdal+s7s0MwaRv9igoPqLRdzOLzw/8Xvq8=
 github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358/go.mod h1:chxPXzSsl7ZWRAuOIE23GDNzjWuZquvFlgA8xmpunjU=
-github.com/JeyJeyGao/notation-core-go v0.0.0-20240820065505-3bb154fda9f6 h1:ssy8DajeEYdS4hzGFEadvOk1frLWreBZPv2J3MmS83o=
-github.com/JeyJeyGao/notation-core-go v0.0.0-20240820065505-3bb154fda9f6/go.mod h1:v6QLd2jSJwhPHQZr7VeWPtWidAcgzp3e0Ra/uerw0bw=
+github.com/JeyJeyGao/notation-core-go v0.0.0-20240913022242-982561e18bc8 h1:PkucC/S776INq9SAFaSvzpDvffrPFCN8w0VgYXpcaWw=
+github.com/JeyJeyGao/notation-core-go v0.0.0-20240913022242-982561e18bc8/go.mod h1:+6AOh41JPrnVLbW/19SJqdhVHwKgIINBO/np0e7nXJA=
 github.com/alexbrainman/sspi v0.0.0-20231016080023-1a75b4708caa h1:LHTHcTQiSGT7VVbI0o4wBRNQIgn917usHWOd6VAffYI=
 github.com/alexbrainman/sspi v0.0.0-20231016080023-1a75b4708caa/go.mod h1:cEWa1LVoE5KvSD9ONXsZrj0z6KqySlCCNKHlLzbqAt4=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

diff --git a/verifier/verifier.go b/verifier/verifier.go
@@ -810,19 +810,25 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult,
 	for i := len(certResults) - 1; i >= 0; i-- {
 		cert := certChain[i]
 		certResult := certResults[i]
-		if len(certResult.ServerResults) > 0 && certResult.ServerResults[0].Error != nil {
-			// OCSP error
-			logger.Debugf("Certificate #%d in chain with subject %v for server %q encountered an OCSP error: %v", (i + 1), cert.Subject.String(), certResult.ServerResults[0].Server, certResult.ServerResults[0].Error)
+		if certResult.RevocationMethod == revocation.MethodOCSPFallbackCRL {
+			// fallback warning
+			logger.Warnf("OCSP check failed with unknown error and fallback to CRL check for certificate #%d in chain with subject %v", (i + 1), cert.Subject.String())
+		}
+		for _, serverResult := range certResult.ServerResults {
+			if serverResult.Error == nil {
+				continue
+			}
 
-			if len(certResult.CRLResults) > 0 {
-				// fallback to CRL check
-				logger.Warnf("OCSP check failed with unknown error and fallback to CRL check for certificate #%d in chain with subject %v", (i + 1), cert.Subject.String())
+			// log the errors
+			switch serverResult.RevocationMethod {
+			case revocation.MethodOCSP:
+				logger.Debugf("Certificate #%d in chain with subject %v encountered an OCSP error: %v", (i + 1), cert.Subject.String(), serverResult.Error)
+			case revocation.MethodCRL:
+				logger.Debugf("Certificate #%d in chain with subject %v encountered a CRL error: %v", (i + 1), cert.Subject.String(), serverResult.Error)
+			default:
+				logger.Debugf("Certificate #%d in chain with subject %v encountered a revocation error: %v", (i + 1), cert.Subject.String(), serverResult.Error)
 			}
 		}
-		if len(certResult.CRLResults) > 0 && certResult.CRLResults[0].Error != nil {
-			// CRL error
-			logger.Debugf("Certificate #%d in chain with subject %v for CRL %q encountered an CRL error: %v", (i + 1), cert.Subject.String(), certResult.CRLResults[0].URI, certResult.CRLResults[0].Error)
-		}
 
 		if certResult.Result == revocationresult.ResultOK || certResult.Result == revocationresult.ResultNonRevokable {
 			numOKResults++

diff --git a/verifier/verifier_test.go b/verifier/verifier_test.go
@@ -1383,11 +1383,6 @@ func TestRevocationFinalResult(t *testing.T) {
 					Result: revocationresult.ResultNonRevokable,
 				},
 			},
-			CRLResults: []*revocationresult.CRLResult{
-				{
-					Result: revocationresult.ResultNonRevokable,
-				},
-			},
 		},
 	}
 	certChain := []*x509.Certificate{
@@ -1407,9 +1402,10 @@ func TestRevocationFinalResult(t *testing.T) {
 			Result: revocationresult.ResultUnknown,
 			ServerResults: []*revocationresult.ServerResult{
 				{
-					Server: "http://ocsp.example.com",
-					Result: revocationresult.ResultUnknown,
-					Error:  errors.New("ocsp error"),
+					Server:           "http://ocsp.example.com",
+					Result:           revocationresult.ResultUnknown,
+					Error:            errors.New("ocsp error"),
+					RevocationMethod: revocation.MethodOCSP,
 				},
 			},
 		}
@@ -1425,17 +1421,18 @@ func TestRevocationFinalResult(t *testing.T) {
 			Result: revocationresult.ResultOK,
 			ServerResults: []*revocationresult.ServerResult{
 				{
-					Server: "http://ocsp.example.com",
-					Result: revocationresult.ResultUnknown,
-					Error:  errors.New("ocsp error"),
+					Server:           "http://ocsp.example.com",
+					Result:           revocationresult.ResultUnknown,
+					Error:            errors.New("ocsp error"),
+					RevocationMethod: revocation.MethodOCSP,
 				},
-			},
-			CRLResults: []*revocationresult.CRLResult{
 				{
-					Result: revocationresult.ResultOK,
-					URI:    "http://crl.example.com",
+					Result:           revocationresult.ResultOK,
+					Server:           "http://crl.example.com",
+					RevocationMethod: revocation.MethodCRL,
 				},
 			},
+			RevocationMethod: revocation.MethodOCSPFallbackCRL,
 		}
 
 		finalResult, problematicCertSubject := revocationFinalResult(certResult, certChain, log.Discard)
@@ -1449,15 +1446,34 @@ func TestRevocationFinalResult(t *testing.T) {
 			Result: revocationresult.ResultUnknown,
 			ServerResults: []*revocationresult.ServerResult{
 				{
-					Server: "http://ocsp.example.com",
-					Result: revocationresult.ResultUnknown,
-					Error:  errors.New("ocsp error"),
+					Server:           "http://ocsp.example.com",
+					Result:           revocationresult.ResultUnknown,
+					Error:            errors.New("ocsp error"),
+					RevocationMethod: revocation.MethodOCSP,
+				},
+				{
+					Result:           revocationresult.ResultUnknown,
+					Error:            errors.New("crl error"),
+					RevocationMethod: revocation.MethodCRL,
 				},
 			},
-			CRLResults: []*revocationresult.CRLResult{
+			RevocationMethod: revocation.MethodOCSPFallbackCRL,
+		}
+
+		finalResult, problematicCertSubject := revocationFinalResult(certResult, certChain, log.Discard)
+		if finalResult != revocationresult.ResultUnknown || problematicCertSubject != "CN=leafCert" {
+			t.Fatalf("unexpected final result: %v, problematic cert subject: %s", finalResult, problematicCertSubject)
+		}
+	})
+
+	t.Run("revocation method unknown error(should never reach here)", func(t *testing.T) {
+		certResult[0] = &revocationresult.CertRevocationResult{
+			Result: revocationresult.ResultUnknown,
+			ServerResults: []*revocationresult.ServerResult{
 				{
-					Result: revocationresult.ResultUnknown,
-					Error:  errors.New("crl error"),
+					Result:           revocationresult.ResultUnknown,
+					Error:            errors.New("unknown error"),
+					RevocationMethod: revocation.MethodUnknown,
 				},
 			},
 		}