-
Notifications
You must be signed in to change notification settings - Fork 210
feat!(sign): add package sign command #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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
✅ Deploy Preview for zarf-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
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.
Some comments for now, will need to do a more in-depth review at another point
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
| // this is a no-op as there may be many different ways to sign | ||
| // input validation should be performed in the calling function | ||
| if !opts.ShouldSign() { | ||
| l.Info("skipping package signing (no signing key material configured)") |
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.
Given that this line now prints on zarf package create for unsigned packages, I think there is a potential impact on user psychology. This will likely serve as a call to action for users who are not currently signing their package to look into what it takes to sign a package. I know there are improvements to signing packages in the pipeline. Keeping this as a debug log until we have newer, documented, signing methods we want to encourage, could be a boon on future signing adoption. User's will likely not look into signing twice.
Not formally requesting a change, but I do think this is an interesting topic.
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 agree with the sentiment - although I think things like signing, SBOMs, and any other attestations will continue to be reminded and required as regulation continues or begins to require as much. Not overpromising current capabilities is always a good reminder.
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
| if helpers.IsOCIURL(packageSource) { | ||
| // For OCI sources, use current working directory | ||
| wd, err := os.Getwd() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| outputDest = wd | ||
| } else { | ||
| // For file sources, use the same directory as the source | ||
| outputDest = filepath.Dir(packageSource) | ||
| } |
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.
As I'm trying out the UX of this command something that confused me was that when I ran the command on a local package it signed my package in place by default, however when I ran the command on a package in a registry, it gave me a copy of the package. I assumed that the package in the registry was signed and got confused when it wasn't. Thoughts on signing the package in the registry by default?
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.
That is good UX feedback. I'd say that is a reasonable and maybe optimal expectation.
| } | ||
|
|
||
| // Handle output - OCI or local file | ||
| if helpers.IsOCIURL(outputDest) { |
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.
This could be a future enhancement, but one option to eliminate duplication is using packagePublishOptions.Run()
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.
This is definitely out of scope, but I think it would be a good issue would be to move this file from utils to it's own cosign package. Ideally the package would be internal as well
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 am not against doing that now - it still provides surface area to support an embedded zarf tools sign-blob command if requested.
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Signed-off-by: Brandt Keller <brandt.keller@defenseunicorns.com>
Description
Adds a new zarf package sign command that enables signing existing Zarf packages with cryptographic signatures using Cosign. This allows packages to be signed independently from the build process.
Key Changes
New Command:
zarf package signFunctionality: Signs existing Zarf packages (local tarballs or OCI packages) with a private key
Key Features:
--overwriteflag to re-sign packages with a new keyCore Implementation Changes
Package Layout:
Build Metadata:
Cosign Utilities:
Assembly Process:
Testing & Documentation
E2E Test: New test case for package signing workflow
Unit Tests: Comprehensive coverage for signing logic (success, failure, rollback scenarios)
Examples
Related Issue
Fixes #3959
Checklist before merging