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

Change the verify attestation implementation #1012

Closed
dlorenc opened this issue Nov 8, 2021 · 4 comments · Fixed by #1035
Closed

Change the verify attestation implementation #1012

dlorenc opened this issue Nov 8, 2021 · 4 comments · Fixed by #1035
Labels
enhancement New feature or request

Comments

@dlorenc
Copy link
Member

dlorenc commented Nov 8, 2021

Right now when using the verify-attestation command, only the predicate is passed to the verification policy. This is nice, but has a few issues:

  • It loses the predicate type itself. This is in the attestation header, but could be useful in policy.
  • It makes the commands hard to use separately cosign verify-attestation with just a key will output the full attestation to stdout. This should be usable directly with "cue vet" and the same policy.
@dlorenc dlorenc added the enhancement New feature or request label Nov 8, 2021
@dlorenc
Copy link
Member Author

dlorenc commented Nov 8, 2021

Cc @developer-guy @Dentrax what do you think?

@Dentrax
Copy link
Member

Dentrax commented Nov 8, 2021

IIUC, we are currently unmarshal ing the predicate body to pass into validation steps, which we do not check the attestation header in current implementation. What we want to here is to validate the entire attestation in policy validation step?

@developer-guy
Copy link
Member

Yes, @dlorenc, it makes sense. Actually, in the beginning, we (w/@Dentrax) were aware of that, but we didn't think that this header might be a valuable thing to validate, so we didn't care about it much. Okay, then, we can pass the Attestation as a whole (body + header) to the validation steps and revise the logic according to that. Is it okay for you, too, @dlorenc? 😋

@dlorenc
Copy link
Member Author

dlorenc commented Nov 9, 2021

Yeah! I think that would be good. I went back and forth on this one too but I think being able to pass the entire thing directly into "cue vet" is the big win.

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.

3 participants