Add ContainerImageSignature type to verifier client #521
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/gcbrun |
2 similar comments
|
/gcbrun |
|
/gcbrun |
jessieqliu
changed the title
jessieqliu
requested review from
jkl73 and
yawangwang
and removed request for
jkl73
yawangwang
reviewed
Jan 6, 2025
launcher/agent/agent.go
Outdated
Comment on lines
200
to
229
| verifierSigs, err := convertToContainerSignatures(signatures) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error converting container signatures: %v", err) | ||
| } |
There was a problem hiding this comment.
The current behavior of converting sigs seems to return on error. However, this violates the previous behavior which is logging on error and sending any valid signature found to verifier. We don't want a single failure of conversion to break the entire signature verification process.
There was a problem hiding this comment.
Done. Changed the conversion function to be for one signature (not the full list) and log/continue on error instead of returning.
|
/gcbrun |
yawangwang
approved these changes
Jan 6, 2025
jkl73
approved these changes
Jan 16, 2025
jessieqliu
force-pushed
the
sig_fields
branch
from
448fccc to
77f20c7
Compare
|
/gcbrun |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
OCI signatures are converted to a different structure in the Rest client for GCA. With the addition of ITA, we want to send a similar structure, so the conversion should be done at a higher level - the agent - rather than being verifier-specific.
The verifier client package will have its own
ContainerSignaturetype - which contains only the fields that will be sent to verifier services - rather than using the fulloci.Signatureobject.fakesignatureis also updated to remove implementations forPublicKeyandSigningAlgorthmsince the correspondingfakeverifierwill use the newContainerSignaturetype that lacks these methods. Instead,fakeverifierwill parse the pubkey and sigalg from the payload - similar to how this is done in the actual verification process.verifier/client,VerifyAttestationRequest.ContainerImageSignaturestype is changed from[]ociSignatureto[]*ContainerSignatureverifier/fake,verifyContainerImageSignature()is changed toextractClaims()verifier/oci/cosign/fakesignature,fakeSig.PublicKey()andfakeSig.SigningAlgorithmnow return "not implemented" errors. Additionally,fakeSig.Payload()returns "f.data,f.sigAlg" rather than the data alone.verifier/rest,convertOCISignatureToRESTis removed