-
Notifications
You must be signed in to change notification settings - Fork 792
Require new bundle format for signing and verification #4959
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?
Changes from 2 commits
28ccd1f
d6d0a2f
a2efab0
2a70df6
e3775b5
baa6db0
4f33850
4537ba6
de506c6
d435b3b
5ac6312
6c0cc72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,17 +47,7 @@ func TestSignBlobCmd(t *testing.T) { | |
| keyOpts := options.KeyOpts{KeyRef: keyRef, BundlePath: bundlePath} | ||
|
|
||
| // Test happy path | ||
| _, err := SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, "", "", true, "", "", false) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error %v", err) | ||
| } | ||
|
|
||
| // Test file outputs | ||
| keyOpts.NewBundleFormat = true | ||
| sigPath := filepath.Join(td, "output.sig") | ||
| certPath := filepath.Join(td, "output.pem") | ||
| _, err = SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, "", "", false, sigPath, certPath, false) | ||
| if err != nil { | ||
| if err := SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, "", ""); err != nil { | ||
| t.Fatalf("unexpected error %v", err) | ||
| } | ||
|
|
||
|
|
@@ -81,8 +71,7 @@ func TestSignBlobCmd(t *testing.T) { | |
| certPrivKeyRef := writeFile(t, td, string(pemBytes), "certkey.pem") | ||
| keyOpts.KeyRef = certPrivKeyRef | ||
|
|
||
| _, err = SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, signCertPath, "", false, "", "", false) | ||
| if err != nil { | ||
| if err := SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, signCertPath, ""); err != nil { | ||
| t.Fatalf("unexpected error %v", err) | ||
| } | ||
|
|
||
|
|
@@ -119,14 +108,17 @@ func TestSignBlobCmd(t *testing.T) { | |
| t.Fatal(err) | ||
| } | ||
| keyOpts.SigningConfig = sc | ||
| sigBytes, err := SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, "", "", true, "", "", true) | ||
| if err != nil { | ||
| if err := SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, "", ""); err != nil { | ||
| t.Fatalf("unexpected error %v", err) | ||
| } | ||
| decodedSig, err := base64.StdEncoding.DecodeString(string(sigBytes)) | ||
| if err != nil { | ||
| t.Fatalf("failed to decode base64 signature: %v", err) | ||
| var b1 struct { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, a better var name would help readability, even if this is just an inlined struct |
||
| MessageSignature struct { | ||
| Signature string `json:"signature"` | ||
| } `json:"messageSignature"` | ||
| } | ||
| bytes1, _ := os.ReadFile(bundlePath) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. even though it's unnecessary, i'd check the second return val and fail fatally if not nil |
||
| json.Unmarshal(bytes1, &b1) | ||
| decodedSig, _ := base64.StdEncoding.DecodeString(b1.MessageSignature.Signature) | ||
| if !ed25519.Verify(pub, blob, decodedSig) { | ||
| errString := "expected ed25519 signature" | ||
| if ed25519.VerifyWithOptions(pub, blob, decodedSig, &ed25519.Options{Hash: crypto.SHA512}) == nil { | ||
|
|
@@ -137,14 +129,17 @@ func TestSignBlobCmd(t *testing.T) { | |
|
|
||
| // Test signing using Ed25519 key with default signing config and no transparency log upload | ||
| keyOpts = options.KeyOpts{KeyRef: edKeyRef, BundlePath: bundlePath} | ||
| sigBytes, err = SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, "", "", true, "", "", false) | ||
| if err != nil { | ||
| if err := SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, "", ""); err != nil { | ||
| t.Fatalf("unexpected error %v", err) | ||
| } | ||
| decodedSig, err = base64.StdEncoding.DecodeString(string(sigBytes)) | ||
| if err != nil { | ||
| t.Fatalf("failed to decode base64 signature: %v", err) | ||
| var b2 struct { | ||
| MessageSignature struct { | ||
| Signature string `json:"signature"` | ||
| } `json:"messageSignature"` | ||
| } | ||
| bytes2, _ := os.ReadFile(bundlePath) | ||
| json.Unmarshal(bytes2, &b2) | ||
| decodedSig, _ = base64.StdEncoding.DecodeString(b2.MessageSignature.Signature) | ||
| if !ed25519.Verify(pub, blob, decodedSig) { | ||
| errString := "expected ed25519 signature" | ||
| if ed25519.VerifyWithOptions(pub, blob, decodedSig, &ed25519.Options{Hash: crypto.SHA512}) == nil { | ||
|
|
@@ -162,43 +157,3 @@ func writeFile(t *testing.T, td string, blob string, name string) string { | |
| } | ||
| return blobPath | ||
| } | ||
|
|
||
| func TestSignBlobCmd_LegacyBundleNoCert(t *testing.T) { | ||
| td := t.TempDir() | ||
| bundlePath := filepath.Join(td, "legacy-bundle.json") | ||
|
|
||
| keys, _ := cosign.GenerateKeyPair(nil) | ||
| keyRef := writeFile(t, td, string(keys.PrivateBytes), "key.pem") | ||
|
|
||
| blob := []byte("foo") | ||
| blobPath := writeFile(t, td, string(blob), "foo.txt") | ||
|
|
||
| rootOpts := &options.RootOptions{} | ||
| keyOpts := options.KeyOpts{ | ||
| KeyRef: keyRef, | ||
| BundlePath: bundlePath, | ||
| NewBundleFormat: false, | ||
| } | ||
|
|
||
| _, err := SignBlobCmd(t.Context(), rootOpts, keyOpts, blobPath, "", "", true, "", "", false) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| bundleBytes, err := os.ReadFile(bundlePath) | ||
| if err != nil { | ||
| t.Fatalf("failed to read written bundle file: %v", err) | ||
| } | ||
|
|
||
| var payload cosign.LocalSignedPayload | ||
| if err := json.Unmarshal(bundleBytes, &payload); err != nil { | ||
| t.Fatalf("failed to unmarshal legacy bundle: %v", err) | ||
| } | ||
|
|
||
| if payload.Cert != "" { | ||
| t.Fatalf("expected empty cert field in legacy bundle when signing with a key without certificate, got: %q", payload.Cert) | ||
| } | ||
| if payload.Base64Signature == "" { | ||
| t.Fatal("expected non-empty Base64Signature in legacy bundle") | ||
| } | ||
| } | ||
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.
In what case will this be nil? Currently, you can either provide a signing config, or specify --use-signing-config to fetch it from TUF. If we're removing that latter flag, then the signing config would always populated, either by TUF or with one explicitly provided (which could be empty).
(this might be answered in a later commit, feel free to point to that if so)