-
Notifications
You must be signed in to change notification settings - Fork 183
cmd-sign: add support for signing OCI images #4301
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
Conversation
Prep for adding functionality that doesn't require it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new cosa sign --oci
command for signing OCI container images, which is a significant step towards a container-native build flow. The implementation is comprehensive, covering payload creation, interaction with Robosignatory, and signature verification. My review focuses on improving the robustness and correctness of the new robosign_oci
function. I've identified a few potential issues related to deterministic file naming, S3 object handling, and image reference parsing, and have provided specific suggestions to address them. I've also included a minor suggestion to improve code style.
28d8b77
to
eff6542
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial questions. Thanks for working on this!
5c23931
to
584d63c
Compare
"public keys to use for signature verification", | ||
default="/etc/pki/rpm-gpg") | ||
robosig.add_argument("--s3-sigstore", help="bucket and prefix to S3 sigstore") | ||
robosig.add_argument("--manifest-list-digest", metavar="ALGO:DIGEST", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit interested to see the update to coreos/fedora-coreos-pipeline#1211 for this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, will update soon!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This adds a new `cosa sign --oci` command to sign OCI container images. This is part of the effort to move FCOS to a container-native build flow, where we now produce non-encapsulated container images. The new command works by sending a request to Robosignatory to sign the image manifest digest. Robosignatory returns a detached signature, which we then merge with the original payload to create a cleartext signed message that can be understood by containers/image. This is a short-term solution until we can move to Sigstore. Part of coreos/fedora-coreos-tracker#1969.
584d63c
to
bc3b733
Compare
I just fixed the typo. Merging since CI was green. |
This adds a new
cosa sign --oci
command to sign OCI container images. This is part of the effort to move FCOS to a container-native build flow, where we now produce non-encapsulated container images.The new command works by sending a request to Robosignatory to sign the image manifest digest. Robosignatory returns a detached signature, which we then merge with the original payload to create a cleartext signed message that can be understood by containers/image.
This is a short-term solution until we can move to Sigstore.
Part of coreos/fedora-coreos-tracker#1969.