Skip to content

Commit

Permalink
feat: adding OCSP revocation checks to Verify (notaryproject#295)
Browse files Browse the repository at this point in the history 
This PR adds OCSP revocation checking to the Verify function using the notation-core-go's revocation package.
This PR addresses part of the following issue: notaryproject/notation-core-go#124. It is dependent on notaryproject/notation-core-go#134

Signed-off-by: Kody Kimberl <[email protected]>
Signed-off-by: Kody Kimberl <[email protected]>
  • Loading branch information
@kody-kimberl
kody-kimberl authored Apr 20, 2023
1 parent 67a477f commit 3dd11cb
Show file tree
Hide file tree
Showing 5 changed files with 623 additions and 62 deletions.
2 changes: 1 addition & 1 deletion example_localVerify_test.go
View file
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
6 changes: 3 additions & 3 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ go 1.19

require (
github.com/go-ldap/ldap/v3 v3.4.4
github.com/notaryproject/notation-core-go v1.0.0-rc.2
github.com/notaryproject/notation-core-go v1.0.0-rc.2.0.20230420025358-cefe2efc755e
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.10.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
)
12 changes: 6 additions & 6 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ 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/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/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.0.20230420025358-cefe2efc755e h1:MSDmIlqyM9ii+vR+QqjAHsKAWVYJIPWeWFAubF0VqUg=
github.com/notaryproject/notation-core-go v1.0.0-rc.2.0.20230420025358-cefe2efc755e/go.mod h1:XWAlhOksW+c9AA/TyobkPv5Xoz8RWGwOAoDdybZLEiI=
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/opencontainers/image-spec v1.1.0-rc2 h1:2zx/Stx4Wc5pIPDvIxHXvXtQFW/7XWJGmnM7r3wg034=
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.10.0 h1:lFO9qtOdlre5W1jxS3r/4szv2/6iXxScdzjoBMXNhYk=
golang.org/x/mod v0.10.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
Expand Down
120 changes: 111 additions & 9 deletions verifier/verifier.go
View file
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"
revocationresult "github.com/notaryproject/notation-core-go/revocation/result"
"github.com/notaryproject/notation-core-go/signature"
"github.com/notaryproject/notation-go"
"github.com/notaryproject/notation-go/dir"
Expand All @@ -30,9 +33,18 @@ import (

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

// VerifierOptions specifies additional parameters that can be set when using
// the NewWithOptions constructor
type VerifierOptions struct {
// RevocationClient is an implementation of revocation.Revocation to use for
// verifying revocation
RevocationClient revocation.Revocation
}

// NewFromConfig returns a verifier based on local file system
Expand All @@ -50,16 +62,31 @@ 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) {
return NewWithOptions(trustPolicy, trustStore, pluginManager, VerifierOptions{})
}

// NewWithOptions creates a new verifier given trustPolicy, trustStore,
// pluginManager, and VerifierOptions
func NewWithOptions(trustPolicy *trustpolicy.Document, trustStore truststore.X509TrustStore, pluginManager plugin.Manager, opts VerifierOptions) (notation.Verifier, error) {
revocationClient := opts.RevocationClient
if revocationClient == nil {
var err error
revocationClient, err = revocation.New(&http.Client{Timeout: 5 * time.Second})
if err != nil {
return nil, err
}
}
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 +283,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")
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 +550,76 @@ 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"),
}
}

authenticSigningTime, err := outcome.EnvelopeContent.SignerInfo.AuthenticSigningTime()
if err != nil {
logger.Debugf("not using authentic signing time due to error retrieving AuthenticSigningTime, err: %v", err)
authenticSigningTime = time.Time{}
}
certResults, err := r.Validate(outcome.EnvelopeContent.SignerInfo.CertificateChain, authenticSigningTime)
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],
}
finalResult := revocationresult.ResultUnknown
numOKResults := 0
var problematicCertSubject string
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), outcome.EnvelopeContent.SignerInfo.CertificateChain[i].Subject.String(), certResults[i].ServerResults[0].Server, certResults[i].ServerResults[0].Error)
}

if certResults[i].Result == revocationresult.ResultOK || certResults[i].Result == revocationresult.ResultNonRevokable {
numOKResults++
} else {
finalResult = certResults[i].Result
problematicCertSubject = outcome.EnvelopeContent.SignerInfo.CertificateChain[i].Subject.String()
if certResults[i].Result == revocationresult.ResultRevoked {
revokedFound = true
revokedCertSubject = problematicCertSubject
}
}
}
if revokedFound {
problematicCertSubject = revokedCertSubject
finalResult = revocationresult.ResultRevoked
}
if numOKResults == len(certResults) {
finalResult = revocationresult.ResultOK
}

switch finalResult {
case revocationresult.ResultOK:
logger.Debug("no verification impacting errors encountered while checking revocation, status is OK")
case revocationresult.ResultRevoked:
result.Error = fmt.Errorf("signing certificate with subject %q is revoked", problematicCertSubject)
default:
// revocationresult.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

0 comments on commit 3dd11cb

Please sign in to comment.