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

Add InsecureIgnoreSCT field to the keyless authorities #511

Merged
merged 4 commits into from Jan 19, 2023

Conversation

hectorj2f
Copy link
Collaborator

Summary

InsecureIgnoreSCT flag in cosign cli was not supported in the policy-controller. There wasn't a way to specific this type of behavior for the keyless authorities. Therefore, we decided to add a new API field to handle SCT.

closes to: #505

Release Note

Documentation

Signed-off-by: Hector Fernandez <hector@chainguard.dev>
Signed-off-by: Hector Fernandez <hector@chainguard.dev>
Signed-off-by: Hector Fernandez <hector@chainguard.dev>
@hectorj2f hectorj2f added the bug Something isn't working label Jan 18, 2023
@hectorj2f hectorj2f requested a review from vaikas January 18, 2023 15:13
@hectorj2f hectorj2f self-assigned this Jan 18, 2023
Signed-off-by: Hector Fernandez <hector@chainguard.dev>
@codecov-commenter
Copy link

Codecov Report

Merging #511 (ae4decf) into main (5449c8b) will decrease coverage by 0.17%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
- Coverage   56.38%   56.21%   -0.17%     
==========================================
  Files          42       42              
  Lines        4441     4454      +13     
==========================================
  Hits         2504     2504              
- Misses       1737     1747      +10     
- Partials      200      203       +3     
Impacted Files Coverage Δ
...g/apis/policy/v1alpha1/clusterimagepolicy_types.go 0.00% <ø> (ø)
pkg/apis/policy/v1alpha1/zz_generated.deepcopy.go 12.52% <0.00%> (-0.13%) ⬇️
...kg/apis/policy/v1beta1/clusterimagepolicy_types.go 0.00% <ø> (ø)
pkg/apis/policy/v1beta1/zz_generated.deepcopy.go 19.62% <0.00%> (-0.32%) ⬇️
pkg/webhook/validator.go 61.78% <0.00%> (-0.17%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -313,6 +313,36 @@ else
fi
echo '::endgroup::'

# Deploy a CIP that adds a keyless entry, that tests OR.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one is indeed a bit tricky to test since Fulcio will upload to CTLog unless a special flag is given, so I can't think of a good way to really test this here.
Ideally, we'd do:

  • Sign but skip the CTLog (can't do this with cosign today, you can ignore verifying, but Fulcio will upload)
  • Have the policy that does not specify this new flag, should fail
  • update policy to have insecureSkip and should pass.

Just jotting this down.

@hectorj2f hectorj2f marked this pull request as ready for review January 18, 2023 16:26
@hectorj2f hectorj2f merged commit eccca79 into sigstore:main Jan 19, 2023
@hectorj2f hectorj2f deleted the add_insecure_ignore_sct branch January 19, 2023 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants