From 82ddf9becfd15b1a25a8ee28b9509a47e9129c7e Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Wed, 18 Sep 2024 03:29:48 +0000 Subject: [PATCH] Squashed commit of the following: commit a69fd622ae62af8170147a177e628b62d8829069 Author: Junjie Gao Date: Wed Sep 18 01:36:05 2024 +0000 fix: update Signed-off-by: Junjie Gao commit c26dabbc94e758ff578d0e5a71833829bb315db5 Author: Junjie Gao Date: Sat Sep 14 06:22:15 2024 +0000 fix: update Signed-off-by: Junjie Gao commit 00d464f8786460803b1c34ccda9a334063d0bb2e Author: Junjie Gao Date: Sat Sep 14 06:17:25 2024 +0000 fix: update Signed-off-by: Junjie Gao commit b7bbef4898ea10b3f28761189486fb5a805ab5a2 Author: Junjie Gao Date: Sat Sep 14 00:40:10 2024 +0000 fix: update Signed-off-by: Junjie Gao commit c00b9567ea72b042860ce330fa9a26ba1a3b9e3e Author: Junjie Gao Date: Tue Sep 10 08:18:39 2024 +0000 fix: update Signed-off-by: Junjie Gao commit 8437ba19d65f4cfc2b40c0dc5246baa0bd3d7342 Author: Junjie Gao Date: Tue Sep 10 08:16:39 2024 +0000 feat: add CRL support Signed-off-by: Junjie Gao commit cdfdd68ff0e6247a660bc9d47e862dcd4330d3d6 Merge: 6e816b5 694e3a0 Author: Junjie Gao Date: Tue Sep 10 06:25:59 2024 +0000 Merge remote-tracking branch 'upstream/main' into feat/crl_no_cache commit 6e816b5fc9571337b79749bf273e632a1b4a61d8 Author: Junjie Gao Date: Tue Aug 27 06:05:11 2024 +0000 fix: update Signed-off-by: Junjie Gao commit 640b3bbec307d01dd90a5de520f6d44c0e387c9b Author: Junjie Gao Date: Tue Aug 20 07:20:27 2024 +0000 feat: support CRL Signed-off-by: Junjie Gao Signed-off-by: Junjie Gao --- go.mod | 2 +- go.sum | 4 +- verifier/verifier.go | 21 +++++-- verifier/verifier_test.go | 118 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 73b520a0..0d63313d 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.22.0 require ( github.com/go-ldap/ldap/v3 v3.4.8 - github.com/notaryproject/notation-core-go v1.1.0 + github.com/notaryproject/notation-core-go v1.1.1-0.20240918011623-695ea0c1ad1f github.com/notaryproject/notation-plugin-framework-go v1.0.0 github.com/notaryproject/tspclient-go v0.2.0 github.com/opencontainers/go-digest v1.0.0 diff --git a/go.sum b/go.sum index 342b3c3d..bb03b145 100644 --- a/go.sum +++ b/go.sum @@ -32,8 +32,8 @@ github.com/jcmturner/gokrb5/v8 v8.4.4 h1:x1Sv4HaTpepFkXbt2IkL29DXRf8sOfZXo8eRKh6 github.com/jcmturner/gokrb5/v8 v8.4.4/go.mod h1:1btQEpgT6k+unzCwX1KdWMEwPPkkgBtP+F6aCACiMrs= github.com/jcmturner/rpc/v2 v2.0.3 h1:7FXXj8Ti1IaVFpSAziCZWNzbNuZmnvw/i6CqLNdWfZY= github.com/jcmturner/rpc/v2 v2.0.3/go.mod h1:VUJYCIDm3PVOEHw8sgt091/20OJjskO/YJki3ELg/Hc= -github.com/notaryproject/notation-core-go v1.1.0 h1:xCybcONOKcCyPNihJUSa+jRNsyQFNkrk0eJVVs1kWeg= -github.com/notaryproject/notation-core-go v1.1.0/go.mod h1:+6AOh41JPrnVLbW/19SJqdhVHwKgIINBO/np0e7nXJA= +github.com/notaryproject/notation-core-go v1.1.1-0.20240918011623-695ea0c1ad1f h1:TmwJtM3AZ7iQ1LJEbHRPAMRw4hA52/AbVrllSVjCNP0= +github.com/notaryproject/notation-core-go v1.1.1-0.20240918011623-695ea0c1ad1f/go.mod h1:+6AOh41JPrnVLbW/19SJqdhVHwKgIINBO/np0e7nXJA= github.com/notaryproject/notation-plugin-framework-go v1.0.0 h1:6Qzr7DGXoCgXEQN+1gTZWuJAZvxh3p8Lryjn5FaLzi4= github.com/notaryproject/notation-plugin-framework-go v1.0.0/go.mod h1:RqWSrTOtEASCrGOEffq0n8pSg2KOgKYiWqFWczRSics= github.com/notaryproject/tspclient-go v0.2.0 h1:g/KpQGmyk/h7j60irIRG1mfWnibNOzJ8WhLqAzuiQAQ= diff --git a/verifier/verifier.go b/verifier/verifier.go index 90d00c95..0b62904e 100644 --- a/verifier/verifier.go +++ b/verifier/verifier.go @@ -808,16 +808,25 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult, revokedFound := false var revokedCertSubject string for i := len(certResults) - 1; i >= 0; i-- { - if len(certResults[i].ServerResults) > 0 && certResults[i].ServerResults[0].Error != nil { - logger.Debugf("Error for certificate #%d in chain with subject %v for server %q: %v", (i + 1), certChain[i].Subject.String(), certResults[i].ServerResults[0].Server, certResults[i].ServerResults[0].Error) + cert := certChain[i] + certResult := certResults[i] + if certResult.RevocationMethod == revocationresult.RevocationMethodOCSPFallbackCRL { + // log the 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 { + // log the revocation error + logger.Errorf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject.String(), serverResult.RevocationMethod, serverResult.Server, serverResult.Error) + } } - if certResults[i].Result == revocationresult.ResultOK || certResults[i].Result == revocationresult.ResultNonRevokable { + if certResult.Result == revocationresult.ResultOK || certResult.Result == revocationresult.ResultNonRevokable { numOKResults++ } else { - finalResult = certResults[i].Result - problematicCertSubject = certChain[i].Subject.String() - if certResults[i].Result == revocationresult.ResultRevoked { + finalResult = certResult.Result + problematicCertSubject = cert.Subject.String() + if certResult.Result == revocationresult.ResultRevoked { revokedFound = true revokedCertSubject = problematicCertSubject } diff --git a/verifier/verifier_test.go b/verifier/verifier_test.go index 1038e260..90f9cb51 100644 --- a/verifier/verifier_test.go +++ b/verifier/verifier_test.go @@ -16,6 +16,7 @@ package verifier import ( "context" "crypto/x509" + "crypto/x509/pkix" "encoding/pem" "errors" "fmt" @@ -30,6 +31,8 @@ import ( "github.com/notaryproject/notation-core-go/revocation" "github.com/notaryproject/notation-core-go/revocation/purpose" + "github.com/notaryproject/notation-core-go/revocation/result" + revocationresult "github.com/notaryproject/notation-core-go/revocation/result" "github.com/notaryproject/notation-core-go/signature" _ "github.com/notaryproject/notation-core-go/signature/cose" "github.com/notaryproject/notation-core-go/signature/jws" @@ -39,6 +42,7 @@ import ( "github.com/notaryproject/notation-go/dir" "github.com/notaryproject/notation-go/internal/envelope" "github.com/notaryproject/notation-go/internal/mock" + "github.com/notaryproject/notation-go/log" "github.com/notaryproject/notation-go/plugin/proto" "github.com/notaryproject/notation-go/signer" "github.com/notaryproject/notation-go/verifier/trustpolicy" @@ -1368,6 +1372,120 @@ func TestIsRequiredVerificationPluginVer(t *testing.T) { } } +func TestRevocationFinalResult(t *testing.T) { + certResult := []*revocationresult.CertRevocationResult{ + { + // update leaf cert result in each sub-test + }, + { + Result: revocationresult.ResultNonRevokable, + ServerResults: []*revocationresult.ServerResult{ + { + Result: revocationresult.ResultNonRevokable, + }, + }, + }, + } + certChain := []*x509.Certificate{ + { + Subject: pkix.Name{ + CommonName: "leafCert", + }, + }, + { + Subject: pkix.Name{ + CommonName: "rootCert", + }, + }, + } + t.Run("OCSP error without fallback", func(t *testing.T) { + certResult[0] = &revocationresult.CertRevocationResult{ + Result: revocationresult.ResultUnknown, + ServerResults: []*revocationresult.ServerResult{ + { + Server: "http://ocsp.example.com", + Result: revocationresult.ResultUnknown, + Error: errors.New("ocsp error"), + RevocationMethod: result.RevocationMethodOCSP, + }, + }, + } + + 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("OCSP error with fallback", func(t *testing.T) { + certResult[0] = &revocationresult.CertRevocationResult{ + Result: revocationresult.ResultOK, + ServerResults: []*revocationresult.ServerResult{ + { + Server: "http://ocsp.example.com", + Result: revocationresult.ResultUnknown, + Error: errors.New("ocsp error"), + RevocationMethod: result.RevocationMethodOCSP, + }, + { + Result: revocationresult.ResultOK, + Server: "http://crl.example.com", + RevocationMethod: result.RevocationMethodCRL, + }, + }, + RevocationMethod: result.RevocationMethodOCSPFallbackCRL, + } + + finalResult, problematicCertSubject := revocationFinalResult(certResult, certChain, log.Discard) + if finalResult != revocationresult.ResultOK || problematicCertSubject != "" { + t.Fatalf("unexpected final result: %v, problematic cert subject: %s", finalResult, problematicCertSubject) + } + }) + + t.Run("OCSP error with fallback and CRL error", func(t *testing.T) { + certResult[0] = &revocationresult.CertRevocationResult{ + Result: revocationresult.ResultUnknown, + ServerResults: []*revocationresult.ServerResult{ + { + Server: "http://ocsp.example.com", + Result: revocationresult.ResultUnknown, + Error: errors.New("ocsp error"), + RevocationMethod: result.RevocationMethodOCSP, + }, + { + Result: revocationresult.ResultUnknown, + Error: errors.New("crl error"), + RevocationMethod: result.RevocationMethodCRL, + }, + }, + RevocationMethod: result.RevocationMethodOCSPFallbackCRL, + } + + 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("unknown error"), + RevocationMethod: result.RevocationMethodUnknown, + }, + }, + } + + 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) + } + }) +} + func verifyResult(outcome *notation.VerificationOutcome, expectedResult notation.ValidationResult, expectedErr error, t *testing.T) { var actualResult *notation.ValidationResult for _, r := range outcome.VerificationResults {