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: fixes #724: add input for --provenance-repository while image verification #736

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
22 changes: 22 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,28 @@ The only requirement is that the provenance file covers all artifacts passed as

To verify a container image, you need to pass a container image name that is _immutable_ by providing its digest, in order to avoid [TOCTOU attacks](#toctou-attacks).

#### The verify-image command

```bash
$ slsa-verifier verify-image --help
Verifies SLSA provenance for an image

Usage:
slsa-verifier verify-image [flags] tarball

Flags:
--build-workflow-input map[] [optional] a workflow input provided by a user at trigger time in the format 'key=value'. (Only for 'workflow_dispatch' events on GitHub Actions). (default map[])
--builder-id string [optional] the unique builder ID who created the provenance
-h, --help help for verify-npm-package
--print-provenance [optional] print the verified provenance to stdout
--provenance-path string path to a provenance file
--provenance-repository string [optional] provenance repository when stored different from image repository. When set, overrides COSIGN_REPOSITORY environment variable
--source-branch string [optional] expected branch the binary was compiled from
--source-tag string [optional] expected tag the binary was compiled from
--source-uri string expected source repository that should have produced the binary, e.g. github.com/some/repo
--source-versioned-tag string [optional] expected version the binary was compiled from. Uses semantic version to match the tag
```

First set the image name:

```shell
Expand Down
5 changes: 4 additions & 1 deletion cli/slsa-verifier/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func verifyArtifactCmd() *cobra.Command {
}

func verifyImageCmd() *cobra.Command {
o := &verify.VerifyOptions{}
o := &verify.VerifyImageOptions{}

cmd := &cobra.Command{
Use: "verify-image [flags] image",
Expand All @@ -96,6 +96,9 @@ func verifyImageCmd() *cobra.Command {
if cmd.Flags().Changed("provenance-path") {
v.ProvenancePath = &o.ProvenancePath
}
if cmd.Flags().Changed("provenance-repository") {
v.ProvenanceRepository = &o.ProvenanceRepository
}
if cmd.Flags().Changed("source-branch") {
v.SourceBranch = &o.SourceBranch
}
Expand Down
18 changes: 18 additions & 0 deletions cli/slsa-verifier/verify/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,24 @@ func (o *VerifyOptions) AddFlags(cmd *cobra.Command) {
cmd.MarkFlagsMutuallyExclusive("source-versioned-tag", "source-tag")
}

// VerifyImageOptions is the top-level options for the `verifyImage` command

type VerifyImageOptions struct {
saisatishkarra marked this conversation as resolved.
Show resolved Hide resolved
VerifyOptions
/* Other */
ProvenanceRepository string
}

var _ Interface = (*VerifyImageOptions)(nil)

// AddFlags implements Interface.
func (o *VerifyImageOptions) AddFlags(cmd *cobra.Command) {
o.VerifyOptions.AddFlags(cmd)

cmd.Flags().StringVar(&o.ProvenanceRepository, "provenance-repository", "",
"image repository for provenance with format: <registry>/<repository>. When set, overrides COSIGN_REPOSITORY environment variable")
}

// VerifyNpmOptions is the top-level options for the `verifyNpmPackage` command.
type VerifyNpmOptions struct {
VerifyOptions
Expand Down
27 changes: 18 additions & 9 deletions cli/slsa-verifier/verify/verify_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ type ComputeDigestFn func(string) (string, error)
// Note: nil branch, tag, version-tag and builder-id means we ignore them during verification.
type VerifyImageCommand struct {
// May be nil if supplied alongside in the registry
ProvenancePath *string
BuilderID *string
SourceURI string
SourceBranch *string
SourceTag *string
SourceVersionTag *string
BuildWorkflowInputs map[string]string
PrintProvenance bool
ProvenancePath *string
ProvenanceRepository *string
BuilderID *string
SourceURI string
SourceBranch *string
SourceTag *string
SourceVersionTag *string
BuildWorkflowInputs map[string]string
PrintProvenance bool
}

func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (*utils.TrustedBuilderID, error) {
Expand Down Expand Up @@ -70,7 +71,15 @@ func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (*uti
}
}

verifiedProvenance, outBuilderID, err := verifiers.VerifyImage(ctx, artifacts[0], provenance, provenanceOpts, builderOpts)
var verifiedProvenance []byte
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can remove defining these two variables, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

var outBuilderID *utils.TrustedBuilderID

if c.ProvenanceRepository != nil {
verifiedProvenance, outBuilderID, err = verifiers.VerifyImageProvenanceRepo(ctx, artifacts[0], provenance, *c.ProvenanceRepository, provenanceOpts, builderOpts)
} else {
verifiedProvenance, outBuilderID, err = verifiers.VerifyImage(ctx, artifacts[0], provenance, provenanceOpts, builderOpts)
}

if err != nil {
return nil, err
}
Expand Down
10 changes: 8 additions & 2 deletions register/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,14 @@ type SLSAVerifier interface {

// VerifyImage verifies a provenance for a supplied OCI image.
VerifyImage(ctx context.Context,
provenance []byte, artifactImage string,
provenanceOpts *options.ProvenanceOpts,
provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
) ([]byte, *utils.TrustedBuilderID, error)

// VerifyImageProvenanceRepo verifies a provenance stored in a separate repository for a supplied OCI image.
VerifyImageProvenanceRepo(ctx context.Context,
provenance []byte, provenanceRepository string,
artifactImage string, provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
) ([]byte, *utils.TrustedBuilderID, error)

Expand Down
12 changes: 10 additions & 2 deletions verifiers/internal/gcb/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ func (v *GCBVerifier) VerifyNpmPackage(ctx context.Context,

// VerifyImage verifies provenance for an OCI image.
func (v *GCBVerifier) VerifyImage(ctx context.Context,
provenance []byte, artifactImage string,
provenanceOpts *options.ProvenanceOpts,
provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
) ([]byte, *utils.TrustedBuilderID, error) {
prov, err := ProvenanceFromBytes(provenance)
Expand Down Expand Up @@ -126,3 +125,12 @@ func (v *GCBVerifier) VerifyImage(ctx context.Context,
}
return content, builderID, nil
}

// VerifyImageProvenanceRepo verifies provenance for an OCI image.
func (v *GCBVerifier) VerifyImageProvenanceRepo(ctx context.Context,
Copy link
Contributor

@laurentsimon laurentsimon Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's re-use the VerifyImage(). Adding an additional parameter will be a breaking change, and that's why you're using a different unction, correct? Let' s keep it simple and just add a parameter to the options.ProvenanceOpts structure. This should make your changes simpler too.

I know retrofitting the provenance repo name in ProvenanceOpts is not as clean as having a dedicated imageOption, but that's a problem with the original API design. We're going to fix this eventually and redesign it with variadic options. @ramonpetgrave64

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I was also leaning toward adding ProvenanceOpts struct but it didn't seem to fit from underlying verification and expected params POV. Since refactoring this later is on the table, i will dump in there and submit new changes!! Thx @laurentsimon

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laurentsimon Added provenanceRepository to provenanceOpts struct and reused verifyImage

provenance []byte, provenanceRepository string,
artifactImage string, provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
) ([]byte, *utils.TrustedBuilderID, error) {
return v.VerifyImage(ctx, provenance, artifactImage, provenanceOpts, builderOpts)
}
101 changes: 68 additions & 33 deletions verifiers/internal/gha/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"strings"

"github.com/google/go-containerregistry/pkg/name"
"github.com/secure-systems-lab/go-securesystemslib/dsse"
"github.com/sigstore/cosign/v2/pkg/cosign"
"github.com/sigstore/rekor/pkg/client"
Expand Down Expand Up @@ -243,39 +244,9 @@ func (v *GHAVerifier) VerifyArtifact(ctx context.Context,
utils.MergeMaps(defaultArtifactTrustedReusableWorkflows, defaultBYOBReusableWorkflows))
}

// VerifyImage verifies provenance for an OCI image.
func (v *GHAVerifier) VerifyImage(ctx context.Context,
provenance []byte, artifactImage string,
provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
) ([]byte, *utils.TrustedBuilderID, error) {
/* Retrieve any valid signed attestations that chain up to Fulcio root CA. */
trustedRoot, err := TrustedRootSingleton(ctx)
if err != nil {
return nil, nil, err
}

// Parse any provenance target repository set using environment variable COSIGN_REPOSITORY
provenanceTargetRepository, err := ociremote.GetEnvTargetRepository()
if err != nil {
return nil, nil, err
}

registryClientOpts := []ociremote.Option{}

// Append target repository to OCI Registry opts
// Must be authenticated against the specified target repository externally
if provenanceTargetRepository.Name() != "" {
registryClientOpts = append(registryClientOpts, ociremote.WithTargetRepository(provenanceTargetRepository))
}

opts := &cosign.CheckOpts{
RegistryClientOpts: registryClientOpts,
RootCerts: trustedRoot.FulcioRoot,
IntermediateCerts: trustedRoot.FulcioIntermediates,
RekorPubKeys: trustedRoot.RekorPubKeys,
CTLogPubKeys: trustedRoot.CTPubKeys,
}
// verifyImageWithOptions abstracts the cosign options and returns verified provenance for an artifact.
func verifyImageWithOptions(ctx context.Context, artifactImage string, provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts, opts *cosign.CheckOpts) ([]byte, *utils.TrustedBuilderID, error) {
atts, _, err := container.RunCosignImageVerification(ctx,
artifactImage, opts)
if err != nil {
Expand Down Expand Up @@ -322,6 +293,70 @@ func (v *GHAVerifier) VerifyImage(ctx context.Context,
return nil, nil, fmt.Errorf("%w", serrors.ErrorNoValidSignature)
}

// VerifyImage verifies provenance for an OCI image.
func (v *GHAVerifier) VerifyImage(ctx context.Context,
provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
) ([]byte, *utils.TrustedBuilderID, error) {
/* Retrieve any valid signed attestations that chain up to Fulcio root CA. */
trustedRoot, err := TrustedRootSingleton(ctx)
if err != nil {
return nil, nil, err
}
opts := &cosign.CheckOpts{
RootCerts: trustedRoot.FulcioRoot,
IntermediateCerts: trustedRoot.FulcioIntermediates,
RekorPubKeys: trustedRoot.RekorPubKeys,
CTLogPubKeys: trustedRoot.CTPubKeys,
}
return verifyImageWithOptions(ctx, artifactImage, provenanceOpts, builderOpts, opts)
}

// VerifyImageProvenanceRepo verifies provenance from a separate store for an OCI image.
func (v *GHAVerifier) VerifyImageProvenanceRepo(ctx context.Context,
provenance []byte, provenanceRepository string,
artifactImage string, provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
) ([]byte, *utils.TrustedBuilderID, error) {
/* Retrieve any valid signed attestations that chain up to Fulcio root CA. */
trustedRoot, err := TrustedRootSingleton(ctx)
if err != nil {
return nil, nil, err
}

var provenanceTargetRepository name.Repository
// Consume input for --provenance-repository when set
if provenanceRepository != "" {
provenanceTargetRepository, err = name.NewRepository(provenanceRepository)
if err != nil {
return nil, nil, err
}
} else {
// If user input --provenance-repository is empty, look for COSIGN_REPOSITORY environment
provenanceTargetRepository, err = ociremote.GetEnvTargetRepository()
if err != nil {
return nil, nil, err
}
}

registryClientOpts := []ociremote.Option{}

// Append target repository to OCI Registry opts
// Must be authenticated against the specified target repository externally
if provenanceTargetRepository.Name() != "" {
registryClientOpts = append(registryClientOpts, ociremote.WithTargetRepository(provenanceTargetRepository))
}

opts := &cosign.CheckOpts{
RegistryClientOpts: registryClientOpts,
RootCerts: trustedRoot.FulcioRoot,
IntermediateCerts: trustedRoot.FulcioIntermediates,
RekorPubKeys: trustedRoot.RekorPubKeys,
CTLogPubKeys: trustedRoot.CTPubKeys,
}
return verifyImageWithOptions(ctx, artifactImage, provenanceOpts, builderOpts, opts)
}

// VerifyNpmPackage verifies an npm package tarball.
func (v *GHAVerifier) VerifyNpmPackage(ctx context.Context,
attestations []byte, tarballHash string,
Expand Down
13 changes: 13 additions & 0 deletions verifiers/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ func VerifyImage(ctx context.Context, artifactImage string,
return verifier.VerifyImage(ctx, provenance, artifactImage, provenanceOpts, builderOpts)
}

func VerifyImageProvenanceRepo(ctx context.Context, artifactImage string,
provenance []byte,
provenanceRepository string,
provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
) ([]byte, *utils.TrustedBuilderID, error) {
verifier, err := getVerifier(builderOpts)
if err != nil {
return nil, nil, err
}
return verifier.VerifyImageProvenanceRepo(ctx, provenance, provenanceRepository, artifactImage, provenanceOpts, builderOpts)
}

func VerifyArtifact(ctx context.Context,
provenance []byte, artifactHash string,
provenanceOpts *options.ProvenanceOpts,
Expand Down
Loading