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

Add Policy WithKey() for verifing content you know is signed with a key #235

Merged
merged 2 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 5 additions & 1 deletion docs/verification.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,18 @@ Next, we'll create a verifier with some options, which will enable SCT verificat
}
```

Then, we need to prepare the expected artifact digest and certificate identity. Note that these options may be omitted, but only if the options `WithoutIdentitiesUnsafe`/`WithoutArtifactUnsafe` are provided. This is a failsafe to ensure that the caller is aware that simply verifying the bundle is not enough, you must also verify the contents of the bundle against a specific identity and artifact.
Then, we need to prepare the expected artifact digest. Note that this option has an alternative option `WithoutArtifactUnsafe`. This is a failsafe to ensure that the caller is aware that simply verifying the bundle is not enough, you must also verify the contents of the bundle against a specific artifact.

```go
digest, err := hex.DecodeString("76176ffa33808b54602c7c35de5c6e9a4deb96066dba6533f50ac234f4f1f4c6b3527515dc17c06fbe2860030f410eee69ea20079bd3a2c6f3dcf3b329b10751")
if err != nil {
panic(err)
}
```

In this case, we also need to prepare the expected certificate identity. Note that this option has an alternative option `WithoutIdentitiesUnsafe`. This is a failsafe to ensure that the caller is aware that simply verifying the bundle is not enough, you must also verify the contents of the bundle against a specific identity. If your bundle was signed with a key, and thus does not have a certificate identity, a better choice is to use the `WithKey` option.

```go
certID, err := verify.NewShortCertificateIdentity("https://token.actions.githubusercontent.com", "", "", "^https://github.com/sigstore/sigstore-js/")
if err != nil {
panic(err)
Expand Down
6 changes: 1 addition & 5 deletions pkg/verify/certificate_identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,5 @@ func (c CertificateIdentity) Verify(actualCert certificate.Summary) error {
if err = c.SubjectAlternativeName.Verify(actualCert); err != nil {
return err
}
if err = certificate.CompareExtensions(c.Extensions, actualCert.Extensions); err != nil {
return err
}

return nil
return certificate.CompareExtensions(c.Extensions, actualCert.Extensions)
}
28 changes: 28 additions & 0 deletions pkg/verify/signed_entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ func (pc PolicyBuilder) BuildConfig() (*PolicyConfig, error) {
type PolicyConfig struct {
weDoNotExpectAnArtifact bool
weDoNotExpectIdentities bool
weExpectSigningKey bool
certificateIdentities CertificateIdentities
verifyArtifact bool
artifact io.Reader
Expand Down Expand Up @@ -317,6 +318,12 @@ func (p *PolicyConfig) WeExpectIdentities() bool {
return !p.weDoNotExpectIdentities
}

// WeExpectSigningKey returns true if we expect the SignedEntity to be signed
// with a key and not a certificate.
func (p *PolicyConfig) WeExpectSigningKey() bool {
return p.weExpectSigningKey
}

func NewPolicy(artifactOpt ArtifactPolicyOption, options ...PolicyOption) PolicyBuilder {
return PolicyBuilder{artifactPolicy: artifactOpt, policyOptions: options}
}
Expand Down Expand Up @@ -365,12 +372,29 @@ func WithCertificateIdentity(identity CertificateIdentity) PolicyOption {
if p.weDoNotExpectIdentities {
return errors.New("can't use WithCertificateIdentity while using WithoutIdentitiesUnsafe")
}
if p.weExpectSigningKey {
return errors.New("can't use WithCertificateIdentity while using WithKey")
}

p.certificateIdentities = append(p.certificateIdentities, identity)
return nil
}
}

// WithKey allows the caller of Verify to require the SignedEntity being
// verified was signed with a key and not a certificate.
func WithKey() PolicyOption {
return func(p *PolicyConfig) error {
if len(p.certificateIdentities) > 0 {
return errors.New("can't use WithKey while using WithCertificateIdentity")
}

p.weExpectSigningKey = true
p.weDoNotExpectIdentities = true
return nil
}
}

// WithoutArtifactUnsafe allows the caller of Verify to skip checking whether
// the SignedEntity was created from, or references, an artifact.
//
Expand Down Expand Up @@ -495,6 +519,10 @@ func (v *SignedEntityVerifier) Verify(entity SignedEntity, pb PolicyBuilder) (*V
// If the bundle was signed with a long-lived key, and does not have a Fulcio certificate,
// then skip the certificate verification steps
if leafCert := verificationContent.GetCertificate(); leafCert != nil {
if policy.WeExpectSigningKey() {
return nil, errors.New("expected key signature, not certificate")
}

signedWithCertificate = true

// From spec:
Expand Down
15 changes: 15 additions & 0 deletions pkg/verify/signed_entity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,16 @@ func TestVerifyPolicyOptionErors(t *testing.T) {
assert.True(t, p.WeExpectAnArtifact())
assert.False(t, p.WeExpectIdentities())

// ---

noArtifactKeyHappyPath := verify.NewPolicy(verify.WithoutArtifactUnsafe(), verify.WithKey())
p, err = noArtifactKeyHappyPath.BuildConfig()
assert.Nil(t, err)
assert.NotNil(t, p)

assert.True(t, p.WeExpectSigningKey())
assert.False(t, p.WeExpectIdentities())

// let's exercise the different error cases!
// 1. can't combine WithoutArtifactUnsafe with other Artifact options
// technically a hack that requires casting but better safe than sorry:
Expand Down Expand Up @@ -279,6 +289,11 @@ func TestVerifyPolicyOptionErors(t *testing.T) {

_, err = verifier.Verify(entity, badIdentityPolicyCombo)
assert.NotNil(t, err)

// 5. can't expect certificate and key signature
badIdentityPolicyCombo2 := verify.NewPolicy(verify.WithoutArtifactUnsafe(), verify.WithCertificateIdentity(goodCertID), verify.WithKey())
_, err = badIdentityPolicyCombo2.BuildConfig()
assert.NotNil(t, err)
}

func TestEntitySignedByPublicGoodWithCertificateIdentityVerifiesSuccessfully(t *testing.T) {
Expand Down
Loading