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

Jump to bottom

Accept a slice of remote.Option for cosign verification #916

Merged
merged 2 commits into from
Sep 29, 2022
Merged

Accept a slice of remote.Option for cosign verification #916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2f4e5e
Accept a slice of remote.Option for cosign verification
souleb Sep 26, 2022
f51c98e
Fail when verifying with insecure
souleb Sep 29, 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
104 changes: 73 additions & 31 deletions controllers/ocirepository_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ func (r *OCIRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.O
// reconcileSource fetches the upstream OCI artifact metadata and content.
// If this fails, it records v1beta2.FetchFailedCondition=True on the object and returns early.
func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sourcev1.OCIRepository, metadata *sourcev1.Artifact, dir string) (sreconcile.Result, error) {
var auth authn.Authenticator

ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
defer cancel()

Expand All @@ -310,8 +312,6 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
conditions.Delete(obj, sourcev1.SourceVerifiedCondition)
}

options := r.craneOptions(ctxTimeout, obj.Spec.Insecure)

// Generate the registry credential keychain either from static credentials or using cloud OIDC
keychain, err := r.keychain(ctx, obj)
if err != nil {
Expand All @@ -322,10 +322,10 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}
options = append(options, crane.WithAuthFromKeychain(keychain))

if _, ok := keychain.(soci.Anonymous); obj.Spec.Provider != sourcev1.GenericOCIProvider && ok {
auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
var authErr error
auth, authErr = oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
e := serror.NewGeneric(
fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr),
Expand All @@ -334,9 +334,6 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}
if auth != nil {
options = append(options, crane.WithAuth(auth))
}
}

// Generate the transport for remote operations
Expand All @@ -349,12 +346,11 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}
if transport != nil {
options = append(options, crane.WithTransport(transport))
}

opts := makeRemoteOptions(ctx, obj, transport, keychain, auth)

// Determine which artifact revision to pull
url, err := r.getArtifactURL(obj, options)
url, err := r.getArtifactURL(obj, opts.craneOpts)
if err != nil {
if _, ok := err.(invalidOCIURLError); ok {
e := serror.NewStalling(
Expand All @@ -372,7 +368,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
}

// Get the upstream revision from the artifact digest
revision, err := r.getRevision(url, options)
revision, err := r.getRevision(url, opts.craneOpts)
if err != nil {
e := serror.NewGeneric(
fmt.Errorf("failed to determine artifact digest: %w", err),
Expand Down Expand Up @@ -403,7 +399,18 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
} else if !obj.GetArtifact().HasRevision(revision) ||
conditions.GetObservedGeneration(obj, sourcev1.SourceVerifiedCondition) != obj.Generation ||
conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) {
err := r.verifySignature(ctx, obj, url, keychain)

// Insecure is not supported for verification
if obj.Spec.Insecure {
e := serror.NewGeneric(
fmt.Errorf("cosign does not support insecure registries"),
sourcev1.VerificationError,
)
conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}

err := r.verifySignature(ctx, obj, url, opts.verifyOpts...)
if err != nil {
provider := obj.Spec.Verify.Provider
if obj.Spec.Verify.SecretRef == nil {
Expand All @@ -429,7 +436,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
}

// Pull artifact from the remote container registry
img, err := crane.Pull(url, options...)
img, err := crane.Pull(url, opts.craneOpts...)
if err != nil {
e := serror.NewGeneric(
fmt.Errorf("failed to pull artifact from '%s': %w", obj.Spec.URL, err),
Expand Down Expand Up @@ -589,15 +596,15 @@ func (r *OCIRepositoryReconciler) digestFromRevision(revision string) string {

// verifySignature verifies the authenticity of the given image reference url. First, it tries using a key
// if a secret with a valid public key is provided. If not, it falls back to a keyless approach for verification.
func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *sourcev1.OCIRepository, url string, keychain authn.Keychain) error {
func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *sourcev1.OCIRepository, url string, opt ...remote.Option) error {
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
defer cancel()

provider := obj.Spec.Verify.Provider
switch provider {
case "cosign":
defaultCosignOciOpts := []soci.Options{
soci.WithAuthnKeychain(keychain),
soci.WithRemoteOptions(opt...),
}

ref, err := name.ParseReference(url)
Expand Down Expand Up @@ -857,21 +864,6 @@ func oidcAuth(ctx context.Context, url, provider string) (authn.Authenticator, e
return login.NewManager().Login(ctx, u, ref, opts)
}

// craneOptions sets the auth headers, timeout and user agent
// for all operations against remote container registries.
func (r *OCIRepositoryReconciler) craneOptions(ctx context.Context, insecure bool) []crane.Option {
options := []crane.Option{
crane.WithContext(ctx),
crane.WithUserAgent(oci.UserAgent),
}

if insecure {
options = append(options, crane.Insecure)
}

return options
}

// reconcileStorage ensures the current state of the storage matches the
// desired and previously observed state.
//
Expand Down Expand Up @@ -1166,3 +1158,53 @@ func (r *OCIRepositoryReconciler) calculateContentConfigChecksum(obj *sourcev1.O

return fmt.Sprintf("sha256:%x", sha256.Sum256(c))
}

// craneOptions sets the auth headers, timeout and user agent
// for all operations against remote container registries.
func craneOptions(ctx context.Context, insecure bool) []crane.Option {
options := []crane.Option{
crane.WithContext(ctx),
crane.WithUserAgent(oci.UserAgent),
}

if insecure {
options = append(options, crane.Insecure)
}

return options
}

// makeRemoteOptions returns a remoteOptions struct with the authentication and transport options set.
// The returned struct can be used to interact with a remote registry using go-containerregistry based libraries.
func makeRemoteOptions(ctxTimeout context.Context, obj *sourcev1.OCIRepository, transport http.RoundTripper,
keychain authn.Keychain, auth authn.Authenticator) remoteOptions {
o := remoteOptions{
craneOpts: craneOptions(ctxTimeout, obj.Spec.Insecure),
verifyOpts: []remote.Option{},
}

if transport != nil {
o.craneOpts = append(o.craneOpts, crane.WithTransport(transport))
o.verifyOpts = append(o.verifyOpts, remote.WithTransport(transport))
}

if auth != nil {
// auth take precedence over keychain here as we expect the caller to set
// the auth only if it is required.
o.verifyOpts = append(o.verifyOpts, remote.WithAuth(auth))
o.craneOpts = append(o.craneOpts, crane.WithAuth(auth))
return o
}

o.verifyOpts = append(o.verifyOpts, remote.WithAuthFromKeychain(keychain))
o.craneOpts = append(o.craneOpts, crane.WithAuthFromKeychain(keychain))

return o
}

// remoteOptions contains the options to interact with a remote registry.
// It can be used to pass options to go-containerregistry based libraries.
type remoteOptions struct {
craneOpts []crane.Option
verifyOpts []remote.Option
}
27 changes: 24 additions & 3 deletions controllers/ocirepository_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
Storage: testStorage,
}

opts := r.craneOptions(ctx, true)
opts := craneOptions(ctx, true)
opts = append(opts, crane.WithAuthFromKeychain(authn.DefaultKeychain))
repoURL, err := r.getArtifactURL(obj, opts)
g.Expect(err).To(BeNil())
Expand Down Expand Up @@ -1036,6 +1036,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
tests := []struct {
name string
reference *sourcev1.OCIRepositoryRef
insecure bool
digest string
want sreconcile.Result
wantErr bool
Expand Down Expand Up @@ -1132,6 +1133,22 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, "Verified", "verified"),
},
},
{
name: "insecure registries are not supported",
reference: &sourcev1.OCIRepositoryRef{
Tag: "6.1.4",
},
digest: img4.digest.Hex,
shouldSign: true,
insecure: true,
wantErr: true,
want: sreconcile.ResultEmpty,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "cosign does not support insecure registries"),
},
},
}

builder := fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme())
Expand Down Expand Up @@ -1181,6 +1198,10 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
},
}

if tt.insecure {
obj.Spec.Insecure = true
}

if !tt.keyless {
obj.Spec.Verify.SecretRef = &meta.LocalObjectReference{Name: "cosign-key"}
}
Expand All @@ -1194,7 +1215,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
g.Expect(err).ToNot(HaveOccurred())
}

opts := r.craneOptions(ctx, true)
opts := craneOptions(ctx, true)
opts = append(opts, crane.WithAuthFromKeychain(keychain))
artifactURL, err := r.getArtifactURL(obj, opts)
g.Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -1677,7 +1698,7 @@ func TestOCIRepository_getArtifactURL(t *testing.T) {
obj.Spec.Reference = tt.reference
}

opts := r.craneOptions(ctx, true)
opts := craneOptions(ctx, true)
opts = append(opts, crane.WithAuthFromKeychain(authn.DefaultKeychain))
got, err := r.getArtifactURL(obj, opts)
if tt.wantErr {
Expand Down
16 changes: 9 additions & 7 deletions internal/oci/verifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"context"
"crypto"
"fmt"
"github.com/google/go-containerregistry/pkg/authn"

"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/sigstore/cosign/cmd/cosign/cli/fulcio"
"github.com/sigstore/cosign/cmd/cosign/cli/rekor"
Expand All @@ -37,7 +37,7 @@ import (
// options is a struct that holds options for verifier.
type options struct {
PublicKey []byte
Keychain authn.Keychain
ROpt []remote.Option
}

// Options is a function that configures the options applied to a Verifier.
Expand All @@ -50,9 +50,11 @@ func WithPublicKey(publicKey []byte) Options {
}
}

func WithAuthnKeychain(keychain authn.Keychain) Options {
return func(opts *options) {
opts.Keychain = keychain
// WithRemoteOptions is a functional option for overriding the default
// remote options used by the verifier.
func WithRemoteOptions(opts ...remote.Option) Options {
return func(o *options) {
o.ROpt = opts
}
}

Expand All @@ -76,8 +78,8 @@ func NewVerifier(ctx context.Context, opts ...Options) (*Verifier, error) {
return nil, err
}

if o.Keychain != nil {
co = append(co, ociremote.WithRemoteOptions(remote.WithAuthFromKeychain(o.Keychain)))
if o.ROpt != nil {
co = append(co, ociremote.WithRemoteOptions(o.ROpt...))
}

checkOpts.RegistryClientOpts = co
Expand Down
Loading