Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adding OCSP revocation checks to Verify #295

Merged
merged 32 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
45e4f33
Adding revocation to Verify using notation-core-go implementation
kody-kimberl Mar 29, 2023
1da5cdc
Refactoring verifyRevocation and changing timeout to failing
kody-kimberl Mar 29, 2023
5eb8697
updating to reflect new changes in notation-core-go PR #134
kody-kimberl Mar 29, 2023
e6275da
updating to reflect new changes in notation-core-go PR #134
kody-kimberl Mar 30, 2023
fcf4050
updating to reflect new naming in notation-core-go
kody-kimberl Mar 31, 2023
62276d1
refactoring constructor with revocation client
kody-kimberl Apr 4, 2023
b46ba1d
Updating to keep in-sync with changes to notation-core-go
kody-kimberl Apr 6, 2023
d0594e5
Updating again to keep in-sync with changes to notation-core-go
kody-kimberl Apr 7, 2023
0ac4a13
Addressing new PR comments
kody-kimberl Apr 10, 2023
516f187
Updating to reflect addition of the revocation/base package in notati…
kody-kimberl Apr 11, 2023
72490db
Incorporating NonRevokable Result and refactoring constructors
kody-kimberl Apr 11, 2023
83ec701
Updating to align with result name change in core-go
kody-kimberl Apr 12, 2023
fee006c
Updating error messages and return of Validate
kody-kimberl Apr 12, 2023
c3da558
Updating package name for revocation/base to revocation/result
kody-kimberl Apr 14, 2023
8769ae7
Adjusted logic for checking results and added server to debug logs
kody-kimberl Apr 14, 2023
b71d94f
Addressing new PR comments
kody-kimberl Apr 17, 2023
9b39737
Adding test and fix for bug with specific results
kody-kimberl Apr 17, 2023
2406b04
Added break and new test
kody-kimberl Apr 17, 2023
5eee868
Changed revocationClient type and loop logic
kody-kimberl Apr 17, 2023
e150ce1
Adjusting for invalidityDate change in notation-core-go
kody-kimberl Apr 18, 2023
206d21b
Addressing comments
kody-kimberl Apr 18, 2023
477691c
Adding VerifierOptions
kody-kimberl Apr 18, 2023
6556b33
Implementing GetAuthenticSigningTime
kody-kimberl Apr 18, 2023
92693c1
Updating tests
kody-kimberl Apr 18, 2023
62b753f
Merge branch 'main' into main
kody-kimberl Apr 19, 2023
7da3157
Updated naming
kody-kimberl Apr 19, 2023
72888b1
Updated to reflect change to GetAuthenticSigningTime error message
kody-kimberl Apr 19, 2023
d2a8f85
Updated to change wording of OK debug log
kody-kimberl Apr 19, 2023
3786bad
Changed GetAuthenticSigningTime to AuthenticSigningTime
kody-kimberl Apr 20, 2023
cc04997
Updated notation-core-go dependency now that #134 is merged
kody-kimberl Apr 20, 2023
dfccf1f
Addressing newest PR comments
kody-kimberl Apr 20, 2023
bbfdf3d
Merge branch 'main' into main
kody-kimberl Apr 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion example_localVerify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var examplePolicyDocument = trustpolicy.Document{
{
Name: "test-statement-name",
RegistryScopes: []string{"example/software"},
SignatureVerification: trustpolicy.SignatureVerification{VerificationLevel: trustpolicy.LevelStrict.Name},
SignatureVerification: trustpolicy.SignatureVerification{VerificationLevel: trustpolicy.LevelStrict.Name, Override: map[trustpolicy.ValidationType]trustpolicy.ValidationAction{trustpolicy.TypeRevocation: trustpolicy.ActionSkip}},
TrustStores: []string{"ca:valid-trust-store"},
TrustedIdentities: []string{"*"},
},
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/image-spec v1.1.0-rc2
github.com/veraison/go-cose v1.0.0
golang.org/x/crypto v0.7.0
golang.org/x/mod v0.9.0
oras.land/oras-go/v2 v2.0.2
)
Expand All @@ -16,8 +17,7 @@ require (
github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 // indirect
github.com/fxamacker/cbor/v2 v2.4.0 // indirect
github.com/go-asn1-ber/asn1-ber v1.5.4 // indirect
github.com/golang-jwt/jwt/v4 v4.4.3 // indirect
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
github.com/x448/float16 v0.8.4 // indirect
golang.org/x/crypto v0.5.0 // indirect
golang.org/x/sync v0.1.0 // indirect
)
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ github.com/go-asn1-ber/asn1-ber v1.5.4 h1:vXT6d/FNDiELJnLb6hGNa309LMsrCoYFvpwHDF
github.com/go-asn1-ber/asn1-ber v1.5.4/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0=
github.com/go-ldap/ldap/v3 v3.4.4 h1:qPjipEpt+qDa6SI/h1fzuGWoRUY+qqQ9sOZq67/PYUs=
github.com/go-ldap/ldap/v3 v3.4.4/go.mod h1:fe1MsuN5eJJ1FeLT/LEBVdWfNWKh459R7aXgXtJC+aI=
github.com/golang-jwt/jwt/v4 v4.4.3 h1:Hxl6lhQFj4AnOX6MLrsCb/+7tCj7DxP7VA+2rDIq5AU=
github.com/golang-jwt/jwt/v4 v4.4.3/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg=
github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/notaryproject/notation-core-go v1.0.0-rc.2 h1:nNJuXa12jVNSSETjGNJEcZgv1NwY5ToYPo+c0P9syCI=
github.com/notaryproject/notation-core-go v1.0.0-rc.2/go.mod h1:ASoc9KbJkSHLbKhO96lb0pIEWJRMZq9oprwBSZ0EAx0=
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
Expand All @@ -27,8 +27,8 @@ github.com/veraison/go-cose v1.0.0/go.mod h1:7ziE85vSq4ScFTg6wyoMXjucIGOf4JkFEZi
github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.5.0 h1:U/0M97KRkSFvyD/3FSmdP5W5swImpNgle/EHFhOsQPE=
golang.org/x/crypto v0.5.0/go.mod h1:NK/OQwhpMQP3MwtdjgLlYHnH9ebylxKWv3e0fK+mkQU=
golang.org/x/crypto v0.7.0 h1:AvwMYaRytfdeVt3u6mLaxYtErKYjxA2OXjJ1HHq6t3A=
golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU=
golang.org/x/mod v0.9.0 h1:KENHtAZL2y3NLMYZeHY9DW8HW8V+kQyJsY/V9JlKvCs=
golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
Expand Down
112 changes: 103 additions & 9 deletions verifier/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ import (
"encoding/json"
"errors"
"fmt"
"net/http"
"reflect"
"strings"
"time"

"github.com/notaryproject/notation-core-go/revocation"
revocation_result "github.com/notaryproject/notation-core-go/revocation/result"
kody-kimberl marked this conversation as resolved.
Show resolved Hide resolved
"github.com/notaryproject/notation-core-go/signature"
"github.com/notaryproject/notation-go"
"github.com/notaryproject/notation-go/dir"
Expand All @@ -30,9 +33,10 @@ import (

// verifier implements notation.Verifier and notation.skipVerifier
type verifier struct {
trustPolicyDoc *trustpolicy.Document
trustStore truststore.X509TrustStore
pluginManager plugin.Manager
trustPolicyDoc *trustpolicy.Document
trustStore truststore.X509TrustStore
pluginManager plugin.Manager
revocationClient revocation.Revocation
}

// NewFromConfig returns a verifier based on local file system
Expand All @@ -50,16 +54,29 @@ func NewFromConfig() (notation.Verifier, error) {

// New creates a new verifier given trustPolicy, trustStore and pluginManager
func New(trustPolicy *trustpolicy.Document, trustStore truststore.X509TrustStore, pluginManager plugin.Manager) (notation.Verifier, error) {
r, err := revocation.New(&http.Client{Timeout: 5 * time.Second})
if err != nil {
return nil, err
}
return NewWithOptions(trustPolicy, trustStore, pluginManager, r)
}

// NewWithOptions creates a new verifier given trustPolicy, trustStore, pluginManager, and revocationClient
kody-kimberl marked this conversation as resolved.
Show resolved Hide resolved
func NewWithOptions(trustPolicy *trustpolicy.Document, trustStore truststore.X509TrustStore, pluginManager plugin.Manager, revocationClient revocation.Revocation) (notation.Verifier, error) {
kody-kimberl marked this conversation as resolved.
Show resolved Hide resolved
if revocationClient == nil {
return nil, errors.New("revocationClient cannot be nil")
}
if trustPolicy == nil || trustStore == nil {
return nil, errors.New("trustPolicy or trustStore cannot be nil")
}
if err := trustPolicy.Validate(); err != nil {
return nil, err
}
return &verifier{
trustPolicyDoc: trustPolicy,
trustStore: trustStore,
pluginManager: pluginManager,
trustPolicyDoc: trustPolicy,
trustStore: trustStore,
pluginManager: pluginManager,
revocationClient: revocationClient,
}, nil
}

Expand Down Expand Up @@ -256,9 +273,14 @@ func (v *verifier) processSignature(ctx context.Context, sigBlob []byte, envelop
// skipped using a trust policy or a plugin may override the check
if outcome.VerificationLevel.Enforcement[trustpolicy.TypeRevocation] != trustpolicy.ActionSkip &&
!slices.Contains(pluginCapabilities, proto.CapabilityRevocationCheckVerifier) {
logger.Debugf("Validating revocation (not implemented)")
// TODO perform X509 revocation check (not in RC1)
// https://github.com/notaryproject/notation-go/issues/110

logger.Debug("Validating revocation")
kody-kimberl marked this conversation as resolved.
Show resolved Hide resolved
revocationResult := verifyRevocation(outcome, v.revocationClient, logger)
outcome.VerificationResults = append(outcome.VerificationResults, revocationResult)
logVerificationResult(logger, revocationResult)
if isCriticalFailure(revocationResult) {
return revocationResult.Error
}
}

// perform extended verification using verification plugin if present
Expand Down Expand Up @@ -518,6 +540,78 @@ func verifyAuthenticTimestamp(outcome *notation.VerificationOutcome) *notation.V
}
}

func verifyRevocation(outcome *notation.VerificationOutcome, r revocation.Revocation, logger log.Logger) *notation.ValidationResult {
if r == nil {
return &notation.ValidationResult{
Type: trustpolicy.TypeRevocation,
Action: outcome.VerificationLevel.Enforcement[trustpolicy.TypeRevocation],
Error: fmt.Errorf("unable to check revocation status, revocation client cannot be nil"),
}
}

signingTime := outcome.EnvelopeContent.SignerInfo.SignedAttributes.SigningTime
if signingTime.IsZero() {
signingTime = time.Now()
}
kody-kimberl marked this conversation as resolved.
Show resolved Hide resolved
revocationResult, err := r.Validate(outcome.EnvelopeContent.SignerInfo.CertificateChain, signingTime)
if err != nil {
logger.Debug("error while checking revocation status, err: %s", err.Error())
return &notation.ValidationResult{
Type: trustpolicy.TypeRevocation,
Action: outcome.VerificationLevel.Enforcement[trustpolicy.TypeRevocation],
Error: fmt.Errorf("unable to check revocation status, err: %s", err.Error()),
}
}

result := &notation.ValidationResult{
Type: trustpolicy.TypeRevocation,
Action: outcome.VerificationLevel.Enforcement[trustpolicy.TypeRevocation],
}

kody-kimberl marked this conversation as resolved.
Show resolved Hide resolved
finalResult := revocation_result.ResultUnknown
numOKResults := 0
kody-kimberl marked this conversation as resolved.
Show resolved Hide resolved
var problematicCertSubject string
revokedFound := false
var revokedCertSubject string
for i := len(revocationResult) - 1; i >= 0; i-- {
for _, serverResult := range revocationResult[i].ServerResults {
if serverResult != nil {
logger.Debugf("error for certificate #%d in chain with subject %v for server %q: %v", (i + 1), outcome.EnvelopeContent.SignerInfo.CertificateChain[i].Subject.String(), serverResult.Server, serverResult.Error)
}
}

if revocationResult[i].Result == revocation_result.ResultOK || revocationResult[i].Result == revocation_result.ResultNonRevokable {
numOKResults++
} else {
finalResult = revocationResult[i].Result
problematicCertSubject = outcome.EnvelopeContent.SignerInfo.CertificateChain[i].Subject.String()
if revocationResult[i].Result == revocation_result.ResultRevoked {
revokedFound = true
revokedCertSubject = problematicCertSubject
shizhMSFT marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
if revokedFound {
problematicCertSubject = revokedCertSubject
finalResult = revocation_result.ResultRevoked
}
patrickzheng200 marked this conversation as resolved.
Show resolved Hide resolved
if numOKResults == len(revocationResult) {
finalResult = revocation_result.ResultOK
}

switch finalResult {
case revocation_result.ResultOK:
logger.Debug("no important errors encountered while checking revocation, status is OK")
kody-kimberl marked this conversation as resolved.
Show resolved Hide resolved
case revocation_result.ResultRevoked:
result.Error = fmt.Errorf("signing certificate with subject %q is revoked", problematicCertSubject)
default:
// revocation_result.ResultUnknown
result.Error = fmt.Errorf("signing certificate with subject %q revocation status is unknown", problematicCertSubject)
}

return result
}

func executePlugin(ctx context.Context, installedPlugin plugin.Plugin, trustPolicy *trustpolicy.TrustPolicy, capabilitiesToVerify []proto.Capability, envelopeContent *signature.EnvelopeContent, pluginConfig map[string]string) (*proto.VerifySignatureResponse, error) {
logger := log.GetLogger(ctx)
// sanity check
Expand Down
Loading