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 {