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

Require embedded SCT on verification by default #2382

Closed
haydentherapper opened this issue Oct 25, 2022 · 0 comments · Fixed by #2400
Closed

Require embedded SCT on verification by default #2382

haydentherapper opened this issue Oct 25, 2022 · 0 comments · Fixed by #2400
Assignees
Labels
enhancement New feature or request

Comments

@haydentherapper
Copy link
Contributor

Description

Currently, SCT verification happens on:

  • Signing, either by checking an embedded SCT (public good Fulcio) or a detached SCT returned in a header. It's skipped if an insecure flag is provided
  • Verification, if either the enforce-sct flag is set or if an embedded SCT is present.

Signing is good - Always check, unless you explicitly provide an insecure flag. However, for verification, you must present a flag to require the check, which is not ideal - This was added because previously issued certificates did not include an embedded SCT and the Cosign metadata did not include a way to persist the detached SCT.

I propose:

  1. We require that an SCT be embedded or explicitly provided for the detached case. For the detached case, we could either add new annotation metadata, or simply require it be passed by flag. Given the detached case is only for private deployments, I would probably go with flag for now.
  2. We remove the enforce-sct flag and leverage an insecure flag to skip the check on verification.

This would be a part of Cosign 2.0, since it's a breaking change.

Code:

  • cosign/pkg/cosign/verify.go

    Lines 204 to 215 in d795dcb

    if co.EnforceSCT && !contains {
    return nil, &VerificationError{"certificate does not include required embedded SCT"}
    }
    if contains {
    // handle if chains has more than one chain - grab first and print message
    if len(chains) > 1 {
    fmt.Fprintf(os.Stderr, "**Info** Multiple valid certificate chains found. Selecting the first to verify the SCT.\n")
    }
    if err := ctl.VerifyEmbeddedSCT(context.Background(), chains[0]); err != nil {
    return nil, err
    }
    }
  • cmd.Flags().BoolVar(&o.EnforceSCT, "enforce-sct", false,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant