diff --git a/pkg/cosign/kubernetes/webhook/validation.go b/pkg/cosign/kubernetes/webhook/validation.go index 55b379b8324..c5fe5a38aa8 100644 --- a/pkg/cosign/kubernetes/webhook/validation.go +++ b/pkg/cosign/kubernetes/webhook/validation.go @@ -36,17 +36,17 @@ import ( "github.com/sigstore/sigstore/pkg/signature" ) -func valid(ctx context.Context, ref name.Reference, keys []*ecdsa.PublicKey, opts ...ociremote.Option) error { +func valid(ctx context.Context, ref name.Reference, keys []*ecdsa.PublicKey, opts ...ociremote.Option) ([]oci.Signature, error) { if len(keys) == 0 { // If there are no keys, then verify against the fulcio root. sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroots.Get(), nil /* rekor */, opts...) if err != nil { - return err + return nil, err } if len(sps) > 0 { - return nil + return sps, nil } - return errors.New("no valid signatures were found") + return nil, errors.New("no valid signatures were found") } // We return nil if ANY key matches var lastErr error @@ -65,11 +65,11 @@ func valid(ctx context.Context, ref name.Reference, keys []*ecdsa.PublicKey, opt continue } if len(sps) > 0 { - return nil + return sps, nil } } logging.FromContext(ctx).Debug("No valid signatures were found.") - return lastErr + return nil, lastErr } // For testing diff --git a/pkg/cosign/kubernetes/webhook/validator.go b/pkg/cosign/kubernetes/webhook/validator.go index 3171ea215aa..c5f2fb1b668 100644 --- a/pkg/cosign/kubernetes/webhook/validator.go +++ b/pkg/cosign/kubernetes/webhook/validator.go @@ -27,6 +27,7 @@ import ( "github.com/pkg/errors" "github.com/sigstore/cosign/pkg/apis/config" "github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1" + "github.com/sigstore/cosign/pkg/oci" ociremote "github.com/sigstore/cosign/pkg/oci/remote" "github.com/sigstore/fulcio/pkg/api" rekor "github.com/sigstore/rekor/pkg/client" @@ -167,8 +168,8 @@ func (v *Validator) validatePodSpec(ctx context.Context, ps *corev1.PodSpec, opt // If there is at least one policy that matches, that means it // has to be satisfied. if len(policies) > 0 { - av, fieldErrors := validatePolicies(ctx, ref, kc, policies) - if av != len(policies) { + signatures, fieldErrors := validatePolicies(ctx, ref, kc, policies) + if len(signatures) != len(policies) { logging.FromContext(ctx).Warnf("Failed to validate at least one policy for %s", ref.Name()) // Do we really want to add all the error details here? // Seems like we can just say which policy failed, so @@ -190,7 +191,7 @@ func (v *Validator) validatePodSpec(ctx context.Context, ps *corev1.PodSpec, opt // Only say we passed (aka, we skip the traditidional check // below) if more than one authority was validated, which // means that there was a matching ClusterImagePolicy. - if av > 0 { + if len(signatures) > 0 { passedPolicyChecks = true } } @@ -202,7 +203,7 @@ func (v *Validator) validatePodSpec(ctx context.Context, ps *corev1.PodSpec, opt continue } - if err := valid(ctx, ref, containerKeys, ociremote.WithRemoteOptions(remote.WithAuthFromKeychain(kc))); err != nil { + if _, err := valid(ctx, ref, containerKeys, ociremote.WithRemoteOptions(remote.WithAuthFromKeychain(kc))); err != nil { errorField := apis.ErrGeneric(err.Error(), "image").ViaFieldIndex(field, i) errorField.Details = c.Image errs = errs.Also(errorField) @@ -218,14 +219,17 @@ func (v *Validator) validatePodSpec(ctx context.Context, ps *corev1.PodSpec, opt } // validatePolicies will go through all the matching Policies and their -// Authorities for a given image. Returns the number of Policies that -// had at least one successful validation against it. -// If there's a policy that did not match, it will be returned in the map +// Authorities for a given image. Returns the map of policy=>Validated +// signatures. From the map you can see the number of matched policies along +// with the signatures that were verified. +// If there's a policy that did not match, it will be returned in the errors map // along with all the errors that caused it to fail. // Note that if an image does not match any policies, it's perfectly // reasonable that the return value is 0, nil since there were no errors, but // the image was not validated against any matching policy and hence authority. -func validatePolicies(ctx context.Context, ref name.Reference, defaultKC authn.Keychain, policies map[string][]v1alpha1.Authority, _ ...ociremote.Option) (int, map[string][]error) { +func validatePolicies(ctx context.Context, ref name.Reference, kc authn.Keychain, policies map[string][]v1alpha1.Authority, remoteOpts ...ociremote.Option) (map[string][]oci.Signature, map[string][]error) { + // Gather all validated signatures here. + signatures := map[string][]oci.Signature{} // For a policy that does not pass at least one authority, gather errors // here so that we can give meaningful errors to the user. ret := map[string][]error{} @@ -234,46 +238,59 @@ func validatePolicies(ctx context.Context, ref name.Reference, defaultKC authn.K // From the Design document, the part about multiple Policies matching: // "If multiple policies match a particular image, then ALL of those // policies must be satisfied for the image to be admitted." - // So we keep a tally to make sure that all the policies matched. - policiesValidated := 0 // If none of the Authorities for a given policy pass the checks, gather // the errors here. If one passes, do not return the errors. authorityErrors := []error{} for p, authorities := range policies { logging.FromContext(ctx).Debugf("Checking Policy: %s", p) - // Now the part about having multiple Authority sections within a - // policy, any single match will do: - // "When multiple authorities are specified, any of them may be used - // to source the valid signature we are looking for to admit an image."" - authoritiesValidated := 0 - for _, authority := range authorities { - logging.FromContext(ctx).Debugf("Checking Authority: %+v", authority) - // TODO(vaikas): We currently only use the defaultKC, we have to look - // at authority.Sources to determine additional information for the - // WithRemoteOptions below, at least the 'TargetRepository' - // https://github.com/sigstore/cosign/issues/1651 - opts := ociremote.WithRemoteOptions(remote.WithAuthFromKeychain(defaultKC)) - - if authority.Key != nil { - // Get the key from authority data - if authorityKeys, fieldErr := parseAuthorityKeys(ctx, authority.Key.Data); fieldErr != nil { - authorityErrors = append(authorityErrors, errors.Wrap(fieldErr, "failed to parse Key values")) + sigs, errs := ValidatePolicy(ctx, ref, kc, authorities, remoteOpts...) + if len(errs) > 0 { + ret[p] = append(ret[p], authorityErrors...) + } else { + signatures[p] = append(signatures[p], sigs...) + } + } + return signatures, ret +} + +// ValidatePolicy will go through all the Authorities for a given image and +// return a success if at least one of the Authorities validated the signatures. +// Returns the validated signatures, or the errors encountered. +func ValidatePolicy(ctx context.Context, ref name.Reference, kc authn.Keychain, authorities []v1alpha1.Authority, remoteOpts ...ociremote.Option) ([]oci.Signature, []error) { + // If none of the Authorities for a given policy pass the checks, gather + // the errors here. If one passes, do not return the errors. + authorityErrors := []error{} + for _, authority := range authorities { + logging.FromContext(ctx).Debugf("Checking Authority: %+v", authority) + // TODO(vaikas): We currently only use the kc, we have to look + // at authority.Sources to determine additional information for the + // WithRemoteOptions below, at least the 'TargetRepository' + // https://github.com/sigstore/cosign/issues/1651 + + switch { + case authority.Key != nil: + // Get the key from authority data + if authorityKeys, fieldErr := parseAuthorityKeys(ctx, authority.Key.Data); fieldErr != nil { + authorityErrors = append(authorityErrors, errors.Wrap(fieldErr, "failed to parse Key values")) + } else { + // TODO(vaikas): What should happen if there are multiple keys + // Is it even allowed? 'valid' returns success if any key + // matches. + // https://github.com/sigstore/cosign/issues/1652 + sps, err := valid(ctx, ref, authorityKeys, remoteOpts...) + if err != nil { + authorityErrors = append(authorityErrors, errors.Wrap(err, "failed to validate keys")) + continue } else { - // TODO(vaikas): What should happen if there are multiple keys - // Is it even allowed? 'valid' returns success if any key - // matches. - // https://github.com/sigstore/cosign/issues/1652 - if err := valid(ctx, ref, authorityKeys, opts); err != nil { - authorityErrors = append(authorityErrors, errors.Wrap(err, "failed to validate keys")) - continue + if len(sps) > 0 { + logging.FromContext(ctx).Debugf("validated signature for %s, got %d signatures", len(sps)) + return sps, nil } - // This authority matched, so mark it as validated and - // continue through other policies, no need to look at more - // of the Authorities. - authoritiesValidated++ - break + logging.FromContext(ctx).Errorf("no validSignatures found for %s", ref.Name()) + authorityErrors = append(authorityErrors, fmt.Errorf("no valid signatures found for %s", ref.Name())) } } + case authority.Keyless != nil: if authority.Keyless != nil && authority.Keyless.URL != nil { logging.FromContext(ctx).Debugf("Fetching FulcioRoot for %s : From: %s ", ref.Name(), authority.Keyless.URL) fulcioroot, err := getFulcioCert(authority.Keyless.URL) @@ -291,32 +308,22 @@ func validatePolicies(ctx context.Context, ref name.Reference, defaultKC authn.K continue } } - sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroot, rekorClient, opts) + sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroot, rekorClient, remoteOpts...) if err != nil { logging.FromContext(ctx).Errorf("failed validSignatures with fulcio for %s: %v", ref.Name(), err) authorityErrors = append(authorityErrors, errors.Wrap(err, "validate signatures with fulcio")) } else { if len(sps) > 0 { logging.FromContext(ctx).Debugf("validated signature for %s, got %d signatures", len(sps)) - // This authority matched, so mark it as validated and - // continue through other policies, no need to look at - // more of the Authorities. - authoritiesValidated++ - break - } else { - logging.FromContext(ctx).Errorf("no validSignatures found for %s", ref.Name()) - authorityErrors = append(authorityErrors, fmt.Errorf("no valid signatures found for %s", ref.Name())) + return sps, nil } + logging.FromContext(ctx).Errorf("no validSignatures found for %s", ref.Name()) + authorityErrors = append(authorityErrors, fmt.Errorf("no valid signatures found for %s", ref.Name())) } } } - if authoritiesValidated > 0 { - policiesValidated++ - } else { - ret[p] = append(ret[p], authorityErrors...) - } } - return policiesValidated, ret + return nil, authorityErrors } // ResolvePodSpecable implements duckv1.PodSpecValidator