fixing issue 3426

[Mukuls77]

Closes #3426

Summary

The PR fixes the issue mentioned in issue #3426
The code changes done in this PR allow user to do cosign verification for a BYO PKI use case.
The use case is that it is a restricted environment, i.e. no access to TUF root CDN.
Signer has a BYO PKI which generates signatures, and attach those to registry.
Verifier uses Trusted root certificate shared by signer to to verification in a Air gap environment with no access to TUF root CDN.
The user passes the flags
cosign verify harbor.demo-ncd.services.te0014-demo-ncd.dyn.nesc.net/ncd-orb/orbs/ncd-ncd_fp6_generic-799@sha256:c08f847db8877aeefa3852ae9ee471fa7c421be4089b855fd0e545d521e2d87c --certificate-identity-regexp='.' --certificate-oidc-issuer-regexp='.' --insecure-ignore-sct=true --insecure-ignore-tlog=true --cert-chain=rootCA.crt --verbose=true

so user has provided --insecure-ignore-sct=true but still cosign verification was failing. due to code changes done in PR
#3415

which has introduced the code change
https://github.com/sigstore/cosign/blob/main/cmd/cosign/cli/verify/verify.go (209):
// Ignore Signed Certificate Timestamp if the flag is set or a key is provided
if !c.IgnoreSCT || keylessVerification(c.KeyRef, c.Sk) {
co.CTLogPubKeys, err = cosign.GetCTLogPubs(ctx)
if err != nil {
return fmt.Errorf("getting ctlog public keys: %w", err)
}
}

With this code change the passing of flag IgnoreSCT was not of much use in keylessverification as it was still going to get CTLogPub key from CND TUF, so this impact the BYO PKI use case where we are using keyless verification by passing Trusted Root certificate via --cert-chain argument or using SIGSTORE_ROOT_FILE env variable.
The code changes done in this PR introduces following logic

1. if user provide key we dont need to pass --insecure-ignore-sct=true
2. If user is using keyless verification using cert-chain or SIGSTORE_ROOT_FILE option and passing --insecure-ignore-sct=true than cosign will not go to get CTLog Public key.

Release Note

Fixes Issue #3426
File changed:
/cosign/cmd/cosign/cli/verify/verify.go
/cosign/cmd/cosign/cli/verify/verify_attestation.go
/cosign/cmd/cosign/cli/verify/verify_blob.go
/cosign/cmd/cosign/cli/verify/verify_blob_attestation.go

Introduced a new function to check if it is keylessverification using SCT option.
func keylessVerificationWithSCTEnabled(IgnoreSCT bool, keyRef string, sk bool, CertChain string) bool {
rootEnv := env.Getenv(env.VariableSigstoreRootFile)
if keyRef != "" {
return false
}
if sk {
return false
}
if IgnoreSCT && (CertChain != "" || rootEnv != "") {
return false
}
if IgnoreSCT {
return false
}
return true
}

and used this function to check if we need to get CTLogs Public key or not during Verify signature procedure.

Documentation

No change is needed in Documentation

[codecov]

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (585397b) 30.38% compared to head (ee8800d) 30.38%.
Report is 4 commits behind head on main.

Files	Patch %	Lines
cmd/cosign/cli/verify/verify.go	58.33%	4 Missing and 1 partial ⚠️
cmd/cosign/cli/verify/verify_attestation.go	0.00%	1 Missing ⚠️
cmd/cosign/cli/verify/verify_blob.go	0.00%	0 Missing and 1 partial ⚠️
cmd/cosign/cli/verify/verify_blob_attestation.go	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3427      +/-   ##
==========================================
- Coverage   30.38%   30.38%   -0.01%     
==========================================
  Files         155      155              
  Lines        9971     9982      +11     
==========================================
+ Hits         3030     3033       +3     
- Misses       6490     6497       +7     
- Partials      451      452       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Mukuls77]

@haydentherapper i have done the requested changes pls review

[hectorj2f]

keylessVerification doesn't seem to be used anywhere else. So we could remove this function now.

[Mukuls77]

keylessVerification doesn't seem to be used anywhere else. So we could remove this function now.

@hectorj2f keylessVerification function is still used in other places
/c/Users/musharma/cosign>grep -R "keylessVerification"
cmd/cosign/cli/verify/verify.go: if keylessVerification(c.KeyRef, c.Sk) {
cmd/cosign/cli/verify/verify.go:func keylessVerification(keyRef string, sk bool) bool {
cmd/cosign/cli/verify/verify_attestation.go: if keylessVerification(c.KeyRef, c.Sk) {
cmd/cosign/cli/verify/verify_blob.go: if keylessVerification(c.KeyRef, c.Sk) {
cmd/cosign/cli/verify/verify_blob_attestation.go: if keylessVerification(c.KeyRef, c.Sk) {

so removing that will require changes in all these files.

sgtm