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.

Sign up for GitHub

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 support for tsa signing and verification of images #2460

Merged
merged 5 commits into from
Nov 15, 2022
Merged

add support for tsa signing and verification of images #2460

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
45db380
add support for tsa signing and verification of images
hectorj2f Nov 15, 2022
7b9db3f
add tsa flag to cli docs
hectorj2f Nov 15, 2022
30403e7
read tsa bundle from signatures
hectorj2f Nov 15, 2022
54543bd
address reviewer comments
hectorj2f Nov 15, 2022
6f2a95b
add TODOs to the code
hectorj2f Nov 15, 2022
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
1 change: 1 addition & 0 deletions cmd/cosign/cli/options/key.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type KeyOpts struct {
OIDCProvider string // Specify which OIDC credential provider to use for keyless signer
BundlePath string
SkipConfirmation bool
TSAServerURL string

// FulcioAuthFlow is the auth flow to use when authenticating against
// Fulcio. See https://pkg.go.dev/github.com/sigstore/cosign/cmd/cosign/cli/fulcio#pkg-constants
Expand Down
4 changes: 4 additions & 0 deletions cmd/cosign/cli/options/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type SignOptions struct {
Attachment string
SkipConfirmation bool
TlogUpload bool
TSAServerURL string

Rekor RekorOptions
Fulcio FulcioOptions
Expand Down Expand Up @@ -94,4 +95,7 @@ func (o *SignOptions) AddFlags(cmd *cobra.Command) {

cmd.Flags().BoolVar(&o.TlogUpload, "tlog-upload", false,
"whether or not to upload to the tlog")

cmd.Flags().StringVar(&o.TSAServerURL, "timestamp-server-url", "",
"url to the Timestamp RFC3161 server, default none")
}
11 changes: 10 additions & 1 deletion cmd/cosign/cli/options/verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,20 @@ import (
)

type CommonVerifyOptions struct {
Offline bool // Force offline verification
Offline bool // Force offline verification
TSACertChainPath string
TSAServerURL string
}

func (o *CommonVerifyOptions) AddFlags(cmd *cobra.Command) {
cmd.Flags().BoolVar(&o.Offline, "offline", false,
"only allow offline verification")

cmd.Flags().StringVar(&o.TSAServerURL, "timestamp-server-url", "",
"url to a timestamp RFC3161 server, default none")

cmd.Flags().StringVar(&o.TSACertChainPath, "timestamp-cert-chain", "",
"path to certificate chain PEM file for the Timestamp Authority")
}

// VerifyOptions is the top level wrapper for the `verify` command.
Expand All @@ -43,6 +51,7 @@ type VerifyOptions struct {
Rekor RekorOptions
Registry RegistryOptions
SignatureDigest SignatureDigestOptions

AnnotationOptions
}

Expand Down
1 change: 1 addition & 0 deletions cmd/cosign/cli/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ race conditions or (worse) malicious tampering.
OIDCDisableProviders: o.OIDC.DisableAmbientProviders,
OIDCProvider: o.OIDC.Provider,
SkipConfirmation: o.SkipConfirmation,
TSAServerURL: o.TSAServerURL,
}
if err := sign.SignCmd(ro, ko, *o, args); err != nil {
if o.Attachment == "" {
Expand Down
14 changes: 13 additions & 1 deletion cmd/cosign/cli/sign/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
ifulcio "github.com/sigstore/cosign/internal/pkg/cosign/fulcio"
ipayload "github.com/sigstore/cosign/internal/pkg/cosign/payload"
irekor "github.com/sigstore/cosign/internal/pkg/cosign/rekor"
"github.com/sigstore/cosign/internal/pkg/cosign/tsa"
"github.com/sigstore/cosign/pkg/cosign"
"github.com/sigstore/cosign/pkg/cosign/pivkey"
"github.com/sigstore/cosign/pkg/cosign/pkcs11key"
Expand All @@ -54,6 +55,8 @@ import (
signatureoptions "github.com/sigstore/sigstore/pkg/signature/options"
sigPayload "github.com/sigstore/sigstore/pkg/signature/payload"

tsaclient "github.com/sigstore/timestamp-authority/pkg/client"

// Loads OIDC providers
_ "github.com/sigstore/cosign/pkg/providers/all"
)
Expand Down Expand Up @@ -231,7 +234,16 @@ func signDigest(ctx context.Context, digest name.Digest, payload []byte, ko opti
if sv.Cert != nil {
s = ifulcio.NewSigner(s, sv.Cert, sv.Chain)
}
if ShouldUploadToTlog(ctx, ko, digest, ko.SkipConfirmation, tlogUpload) {

// TODO: For the moment you can only use the timestamped service OR the transparency log.
if ko.TSAServerURL != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to support both and not have this as a TODO. Timestamps are simply a way to have a third-party provide time, but don't provide transparency. Having this as a TODO risks changing the security model of Sigstore.

Following the existing code, can we have the tsa.NewSigner be wrapped by irekor.NewSigner?

cc @asraa

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@haydentherapper @asraa Could we add support for both as part of a follow-up PR ? Likewise I will advocate for supporting the TSA as an alternative to the tlog. Some users may not want transparency.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as the follow up is before we cut a new release, sure. Is it a large change to support both? I would have thought we could support both by just wrapping the tsa signer in the rekor signer struct.

If a user doesn't want to use the transparency log, that should be controlled via an explicit flag (tlog-upload) rather than the usage of timestamping. I just want to make sure we're clear on the messaging and security model for Sigstore, that timestamping provides an alternative to how to verify expiration of a certificate. Rekor currently plays two roles, transparency and timestamping, so using a TSA only changes the second.

There should be a limited number of reasons to not use Rekor - private artifacts are the main one. Otherwise, you really should be uploading entries to Rekor.

Copy link
Contributor

Choose a reason for hiding this comment

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

There should be a limited number of reasons to not use Rekor - private artifacts are the main one. Otherwise, you really should be uploading entries to Rekor.

+1. We should still support this use case though. We can default to using the TSA+Rekor together, and provide an opt-in flag for folks who want to sign private artifacts and verify against the TSA but not Rekor. I think because of this use case it's more logical to have a separate TSA signer/verifier than to wrap it into Rekor.

Could we add support for both as part of a follow-up PR ?

sounds good! as a reviewer I definitely prefer lots of smaller PRs :) there's no plan to cut any releases at the moment so should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the code to support both should be pretty straightforward, since irekor.NewSigner takes a cosign.Signer and tsa.NewSigner implements that already.

Copy link
Contributor

Choose a reason for hiding this comment

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

This all sounds good to me: I think everyone laid it out right. We can support TSA+Rekor, with a control knob on removing hte Rekor upload.

For verification, we can start verifying TSA's on conditional requirements, soon always validating them in the long tail. The verification flow will look the same, we still will use a Rekor promise of inclusion offline, but instead use the verified TSA as the time check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I extracted the following list of action items as result of this thread:

  • Add support for TSA and Rekor together.
  • Users could rely on TSA-only whenever they set --tlog-upload=false. We'll add a warning message and documentation about the limitations of using the timestamp verification instead of the tlog verification.
  • Add a --skip-tlog-verify flag to our cosign verify* commands to skip the tlog verification done by default.

clientTSA, err := tsaclient.GetTimestampClient(ko.TSAServerURL)
if err != nil {
return fmt.Errorf("failed to create TSA client: %w", err)
}

s = tsa.NewSigner(s, clientTSA)
} else if ShouldUploadToTlog(ctx, ko, digest, ko.SkipConfirmation, tlogUpload) {
rClient, err := rekor.NewClient(ko.RekorURL)
if err != nil {
return err
Expand Down
2 changes: 2 additions & 0 deletions cmd/cosign/cli/verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ against the transparency log.`,
SignatureRef: o.SignatureRef,
LocalImage: o.LocalImage,
Offline: o.CommonVerifyOptions.Offline,
TSAServerURL: o.CommonVerifyOptions.TSAServerURL,
TSACertChainPath: o.CommonVerifyOptions.TSACertChainPath,
}

if o.Registry.AllowInsecure {
Expand Down
23 changes: 23 additions & 0 deletions cmd/cosign/cli/verify/verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"github.com/sigstore/sigstore/pkg/cryptoutils"
"github.com/sigstore/sigstore/pkg/signature"
"github.com/sigstore/sigstore/pkg/signature/payload"
tsaclient "github.com/sigstore/timestamp-authority/pkg/client"
)

// VerifyCommand verifies a signature on a supplied container image
Expand Down Expand Up @@ -75,6 +76,8 @@ type VerifyCommand struct {
LocalImage bool
NameOptions []name.Option
Offline bool
TSAServerURL string
TSACertChainPath string
}

// Exec runs the verification command
Expand Down Expand Up @@ -113,11 +116,25 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) {
IgnoreSCT: c.IgnoreSCT,
SignatureRef: c.SignatureRef,
Offline: c.Offline,
TSACertChainPath: c.TSACertChainPath,
}
if c.CheckClaims {
co.ClaimVerifier = cosign.SimpleClaimVerifier
}

if c.TSAServerURL != "" {
co.TSAClient, err = tsaclient.GetTimestampClient(c.TSAServerURL)
if err != nil {
return fmt.Errorf("failed to create TSA client: %w", err)
}
if c.TSACertChainPath != "" {
_, err := os.Stat(c.TSACertChainPath)
if err != nil {
return fmt.Errorf("unable to open timestamp certificate chain file: %w", err)
}
}
}

if keylessVerification(c.KeyRef, c.Sk) {
if c.RekorURL != "" {
rekorClient, err := rekor.NewClient(c.RekorURL)
Expand Down Expand Up @@ -356,6 +373,12 @@ func PrintVerification(imgRef string, verified []oci.Signature, output string) {
}
ss.Optional["Bundle"] = bundle
}
if tsaBundle, err := sig.TSABundle(); err == nil && tsaBundle != nil {
if ss.Optional == nil {
ss.Optional = make(map[string]interface{})
}
ss.Optional["TSABundle"] = tsaBundle
}

outputKeys = append(outputKeys, ss)
}
Expand Down
2 changes: 2 additions & 0 deletions doc/cosign_dockerfile_verify.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions doc/cosign_manifest_verify.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions doc/cosign_sign.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions doc/cosign_verify-attestation.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions doc/cosign_verify-blob.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions doc/cosign_verify.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 19 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/chrismellard/docker-credential-acr-env v0.0.0-20220119192733-fe33c00cee21
github.com/cyberphone/json-canonicalization v0.0.0-20210823021906-dc406ceaf94b
github.com/depcheck-test/depcheck-test v0.0.0-20220607135614-199033aaa936
github.com/digitorus/timestamp v0.0.0-20221019182153-ef3b63b79b31
github.com/go-openapi/runtime v0.24.2
github.com/go-openapi/strfmt v0.21.3
github.com/go-openapi/swag v0.22.3
Expand All @@ -28,6 +29,7 @@ require (
github.com/sigstore/fulcio v1.0.0
github.com/sigstore/rekor v1.0.1
github.com/sigstore/sigstore v1.4.5
github.com/sigstore/timestamp-authority v0.1.3-0.20221114113831-cf271cea5d83
github.com/spf13/cobra v1.6.1
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.14.0
Expand All @@ -53,7 +55,8 @@ require (
cloud.google.com/go/compute v1.12.1 // indirect
cloud.google.com/go/compute/metadata v0.2.1 // indirect
cloud.google.com/go/iam v0.6.0 // indirect
cloud.google.com/go/kms v1.5.0 // indirect
cloud.google.com/go/kms v1.6.0 // indirect
filippo.io/edwards25519 v1.0.0 // indirect
github.com/AliyunContainerService/ack-ram-tool/pkg/credentials/alibabacloudsdkgo/helper v0.2.0 // indirect
github.com/Azure/azure-sdk-for-go v67.0.0+incompatible // indirect
github.com/Azure/go-autorest v14.2.0+incompatible // indirect
Expand Down Expand Up @@ -83,6 +86,7 @@ require (
github.com/armon/go-metrics v0.4.1 // indirect
github.com/armon/go-radix v1.0.0 // indirect
github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d // indirect
github.com/aws/aws-sdk-go v1.44.132 // indirect
github.com/aws/aws-sdk-go-v2 v1.16.16 // indirect
github.com/aws/aws-sdk-go-v2/config v1.17.8 // indirect
github.com/aws/aws-sdk-go-v2/credentials v1.12.21 // indirect
Expand All @@ -98,8 +102,10 @@ require (
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.13.6 // indirect
github.com/aws/aws-sdk-go-v2/service/sts v1.16.19 // indirect
github.com/aws/smithy-go v1.13.3 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/blang/semver v3.5.1+incompatible // indirect
github.com/cenkalti/backoff/v3 v3.2.2 // indirect
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect
github.com/clbanning/mxj/v2 v2.5.6 // indirect
github.com/cockroachdb/apd/v2 v2.0.1 // indirect
Expand All @@ -108,11 +114,13 @@ require (
github.com/coreos/go-oidc/v3 v3.4.0 // indirect
github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/digitorus/pkcs7 v0.0.0-20221019075359-21b8b40e6bb4 // indirect
github.com/dimchansky/utfbom v1.1.1 // indirect
github.com/docker/cli v20.10.20+incompatible // indirect
github.com/docker/distribution v2.8.1+incompatible // indirect
github.com/docker/docker v20.10.20+incompatible // indirect
github.com/docker/docker-credential-helpers v0.7.0 // indirect
github.com/docker/go-units v0.5.0 // indirect
github.com/emicklei/proto v1.6.15 // indirect
github.com/fatih/color v1.13.0 // indirect
github.com/fsnotify/fsnotify v1.6.0 // indirect
Expand All @@ -138,6 +146,7 @@ require (
github.com/golang/snappy v0.0.4 // indirect
github.com/google/go-querystring v1.1.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/tink/go v1.7.0 // indirect
github.com/google/trillian v1.5.1-0.20220819043421-0a389c4bb8d9 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.2.0 // indirect
Expand Down Expand Up @@ -176,6 +185,7 @@ require (
github.com/mailru/easyjson v0.7.7 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.16 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/go-testing-interface v1.14.1 // indirect
Expand All @@ -193,8 +203,13 @@ require (
github.com/pelletier/go-toml/v2 v2.0.5 // indirect
github.com/pierrec/lz4 v2.6.1+incompatible // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_golang v1.14.0 // indirect
github.com/prometheus/client_model v0.3.0 // indirect
github.com/prometheus/common v0.37.0 // indirect
github.com/prometheus/procfs v0.8.0 // indirect
github.com/protocolbuffers/txtpbfmt v0.0.0-20201118171849-f6a6b3f636fc // indirect
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect
github.com/rs/cors v1.8.2 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/ryanuber/go-glob v1.0.0 // indirect
github.com/sassoftware/relic v0.0.0-20210427151427-dfb082b79b74 // indirect
Expand All @@ -212,13 +227,15 @@ require (
github.com/thales-e-security/pool v0.0.2 // indirect
github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 // indirect
github.com/tjfoc/gmsm v1.3.2 // indirect
github.com/urfave/negroni v1.0.0 // indirect
github.com/vbatts/tar-split v0.11.2 // indirect
github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
github.com/yashtewari/glob-intersection v0.1.0 // indirect
github.com/zeebo/errs v1.2.2 // indirect
go.mongodb.org/mongo-driver v1.10.0 // indirect
go.mongodb.org/mongo-driver v1.10.1 // indirect
go.opencensus.io v0.24.0 // indirect
go.step.sm/crypto v0.23.1 // indirect
go.uber.org/atomic v1.10.0 // indirect
go.uber.org/multierr v1.8.0 // indirect
go.uber.org/zap v1.23.0 // indirect
Expand Down
Loading