Add offline signing flag - #5059
Conversation
Signed-off-by: Aaron Lew <64337293+aaronlew02@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5059 +/- ##
==========================================
- Coverage 40.10% 39.64% -0.47%
==========================================
Files 155 207 +52
Lines 10044 13067 +3023
==========================================
+ Hits 4028 5180 +1152
- Misses 5530 7153 +1623
- Partials 486 734 +248 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Aaron Lew <64337293+aaronlew02@users.noreply.github.com>
2b55a6a to
df61d1c
Compare
Hayden-IO
left a comment
There was a problem hiding this comment.
Looks good, just a few small suggestions! Can you also update the README to document this? While we need to do a lot of updates for the main README, I think this is worth calling out.
| return fmt.Errorf("cannot specify service URLs when using a signing config") | ||
| } | ||
| if !tlogUpload { | ||
| return fmt.Errorf("--tlog-upload=false is not supported with a signing config. Provide a signing config with --signing-config without a transparency log service, which can be created with `cosign signing-config create` or `curl https://raw.githubusercontent.com/sigstore/root-signing/refs/heads/main/targets/signing_config.v0.2.json | jq 'del(.rekorTlogUrls)'` for the public instance") |
There was a problem hiding this comment.
Can this error message be updated to mention the offline flag as an alternative now? Or as the only alternative, rather than mentioning an empty signing config? I think we can remove the mention of deleting just the Rekor URLs because it still logs the certificate issuance, and I think those who would want to skip the tlog upload would also want to skip Fulcio/CT logging too.
| return fmt.Errorf("loading trusted root: %w", err) | ||
| } | ||
| } else { | ||
| ko.TrustedMaterial, err = cosign.TrustedRoot() |
There was a problem hiding this comment.
It'd be cool if we could also have an e2e test that demonstrates signing is offline. Something like, clear the TUF repo directory, call sign, verify the directory is still empty?
| "whether to use a TUF-provided signing config for the service URLs") | ||
| _ = cmd.Flags().MarkDeprecated("use-signing-config", "an offline signing flag will be added in the future; TUF will continue to provide a signing config by default if one is not provided manually") | ||
| cmd.Flags().BoolVar(&o.Offline, "offline", false, | ||
| "only allow offline signing with a local key without contacting network services") |
There was a problem hiding this comment.
Can the flag's documentation call out that the key usage isn't logged and therefore isn't auditable? I want to convey that the signer is acknowledging that using this flag means they are not mitigating some of the threats outlined in https://docs.sigstore.dev/about/threat-model/.
| if serviceURLsSpecified { | ||
| return fmt.Errorf("cannot specify service URLs when signing offline") | ||
| } | ||
| ko.SigningConfig = NewEmptySigningConfig() |
There was a problem hiding this comment.
nit, can you add a comment documenting this line? I think it's easy to miss that this is what prevents any network calls.
Closes #5013
Summary
This change adds an
--offlineflag to signing and attestation commands to sign without connectivity to any online services.This change removes the deprecated
--use-signing-configflag.Release Note
--signing-configor--trusted-root.--offlinewill:--signing-config,--trusted-root,--issue-certificate, and custom service URLs.--key).