Add Cosign signing/verification by mtrmac · Pull Request #1701 · containers/skopeo

mtrmac · 2022-07-06T05:25:14Z

⚠️ Warning: This is write-only code, as in I haven’t read it after myself, it has never been run, and it has no tests yet. Might be completely broken. It really needs ~~unit and~~ integration tests, and interoperability testing.

~~Depends on UNMERGED containers/image#1598 .~~

Intended to cover:

Storing Cosign signatures in registries, dir:, c/storage; and copying them around
Signing, using ordinary private keys (no Fulcio: non-trivial UI impact, much larger surface; no Rekor)
Verification, using ordinary public keys (no Fulcio/Rekor: tests for this verification first)

End-user visible UI surface:

Registries support requires opt-in via registries.d/*.yaml use-cosign-attachments = true in one or more of the existing sections.
Signing via the --sign-by-cosign-private-key options added here.
Verification using type: cosignSigned in policy.json.
No tooling for inspection — copy to dir:.

mtrmac · 2022-07-06T05:27:47Z

On macOS, this increases Skopeo’s binary size by ~10 MB (from ~28.8 to ~39.0 MB, or from ~26.7 to ~36.1 stripped). That’s rather surprising, the source code diff (even more than just additions) is a bit less than 2 MB.

cmd/skopeo/copy.go

mtrmac · 2022-07-08T17:29:09Z

cmd/skopeo/copy.go


-	passphrase, err := cli.ReadPassphraseFile(opts.signPassphraseFile)
-	if err != nil {
-		return err
+	// c/image.Copy does allow creating both simple signing and Cosign signatures simultaneously,
+	// with independent passphrases, but that would make the CLI probably too confusing.
+	// For now, use the passphrase with either, but only one of them.
+	if opts.signPassphraseFile != "" && opts.signByFingerprint != "" && opts.signByCosignPrivateKey != "" {
+		return fmt.Errorf("Only one of --sign-by and sign-by-cosign-private-key can be used with sign-passphrase-file")
 	}
+	var passphrase string
+	if opts.signPassphraseFile != "" {
+		p, err := cli.ReadPassphraseFile(opts.signPassphraseFile)
+		if err != nil {
+			return err
+		}
+		passphrase = p
+	} else if opts.signByCosignPrivateKey != "" {
+		p, err := promptForPassphrase(opts.signByCosignPrivateKey, os.Stdin, os.Stdout)
+		if err != nil {
+			return err
+		}
+		passphrase = p
+	} // opts.signByFingerprint triggers a GPG-agent passphrase prompt, possibly using a more secure channel, so we usually shouldn’t prompt ourselves if no passphrase was explicitly provided.



This should be moved to utils.go instead of copy&pasted.

Adding to https://github.com/containers/image/issues/1601 .

mtrmac · 2022-07-11T15:15:06Z

cmd/skopeo/copy.go

 	flags.BoolVar(&opts.removeSignatures, "remove-signatures", false, "Do not copy signatures from SOURCE-IMAGE")
 	flags.StringVar(&opts.signByFingerprint, "sign-by", "", "Sign the image using a GPG key with the specified `FINGERPRINT`")
-	flags.StringVar(&opts.signPassphraseFile, "sign-passphrase-file", "", "File that contains a passphrase for the --sign-by key")
+	flags.StringVar(&opts.signByCosignPrivateKey, "sign-by-cosign-private-key", "", "Sign the image using a Cosign private key at `PATH`")


FWIW I think it’s likely the only way to generate the key right now currently is /usr/bin/cosign. I don’t know if we should use a different file format or just add a “generate key” subcommand; either way that’s a to-do item for https://github.com/containers/image/issues/1601 .

mtrmac · 2022-07-11T20:40:40Z

docs/skopeo-copy.1.md

 **--sign-by** _key-id_

-Add a signature using that key ID for an image name corresponding to _destination-image_
+Add a “simple signing” signature using that key ID for an image name corresponding to _destination-image_
+
+**--sign-by-sigstore-private-key** _path_
+
+Add a sigstore signature using a private key at _path_ for an image name corresponding to _destination-image_

 **--sign-passphrase-file** _path_

-The passphare to use when signing with the key ID from `--sign-by`. Only the first line will be read. A passphrase stored in a file is of questionable security if other users can read this file. Do not use this option if at all avoidable.
+The passphare to use when signing with `--sign-by` or `--sign-by-sigstore-private-key`. Only the first line will be read. A passphrase stored in a file is of questionable security if other users can read this file. Do not use this option if at all avoidable.

 **--sign-identity** _reference_


@vrothberg @rhatdan PTAL.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

If a passphrase is not provided, prompt for one. Outstanding: - Should have integration tests. - The signing options shared between copy and sync should live in utils.go. Signed-off-by: Miloslav Trmač <mitr@redhat.com>

I left systemtest unmodified, to have _something_ that exercises the compatibility path. Signed-off-by: Miloslav Trmač <mitr@redhat.com>

mtrmac · 2022-07-12T11:49:52Z

Now ready for review/merging.

vrothberg

LGTM

@rhatdan PTAL

rhatdan · 2022-07-12T13:28:47Z

LGTM

mtrmac force-pushed the cosign-prototypes branch 3 times, most recently from 73f6862 to abc45e9 Compare July 6, 2022 16:27

mtrmac commented Jul 7, 2022

View reviewed changes

cmd/skopeo/copy.go Outdated Show resolved Hide resolved

mtrmac force-pushed the cosign-prototypes branch 2 times, most recently from ce87bf6 to 3946086 Compare July 8, 2022 17:25

mtrmac changed the title ~~DO NOT MERGE IRRESPONSIBLE: Cosign prototypes~~ DO NOT MERGE: Add Cosign signing/verification Jul 8, 2022

mtrmac force-pushed the cosign-prototypes branch from 3946086 to 457fe13 Compare July 8, 2022 17:28

mtrmac commented Jul 8, 2022

View reviewed changes

mtrmac commented Jul 11, 2022

View reviewed changes

mtrmac force-pushed the cosign-prototypes branch from 457fe13 to 916e291 Compare July 11, 2022 19:55

mtrmac mentioned this pull request Jul 11, 2022

s/cosign/sigstore/ containers/image#1605

Merged

mtrmac force-pushed the cosign-prototypes branch from 916e291 to 541b4df Compare July 11, 2022 20:39

mtrmac commented Jul 11, 2022

View reviewed changes

This was referenced Jul 11, 2022

s/sigstore/lookaside/ containers/image#1606

Merged

Implement OCI artifacts and revert #1672 #1673

Closed

mtrmac added 3 commits July 12, 2022 13:46

Vendor in c/image with sigstore support

06be7a1

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

Update for the renames of sigstore to lookaside

b000ada

I left systemtest unmodified, to have _something_ that exercises the compatibility path. Signed-off-by: Miloslav Trmač <mitr@redhat.com>

mtrmac force-pushed the cosign-prototypes branch from 541b4df to b000ada Compare July 12, 2022 11:49

mtrmac changed the title ~~DO NOT MERGE: Add Cosign signing/verification~~ Add Cosign signing/verification Jul 12, 2022

vrothberg approved these changes Jul 12, 2022

View reviewed changes

rhatdan merged commit 7ddc5ce into containers:main Jul 12, 2022

mtrmac deleted the cosign-prototypes branch July 12, 2022 13:35

mtrmac mentioned this pull request Jul 18, 2022

copy: consider support cosign: verify image while pulling from source & sign image while copying to destination #1533

Closed

cgwalters mentioned this pull request Sep 16, 2022

test that we support image signatures ostreedev/ostree-rs-ext#163

Closed

github-actions bot added the locked - please file new issue/PR label Sep 18, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Cosign signing/verification#1701

Add Cosign signing/verification#1701
rhatdan merged 3 commits intocontainers:mainfrom
mtrmac:cosign-prototypes

mtrmac commented Jul 6, 2022 •

edited

Loading

Uh oh!

mtrmac commented Jul 6, 2022

Uh oh!

Uh oh!

mtrmac Jul 8, 2022

Uh oh!

mtrmac Jul 11, 2022

Uh oh!

mtrmac Jul 11, 2022

Uh oh!

mtrmac commented Jul 12, 2022

Uh oh!

vrothberg left a comment

Uh oh!

rhatdan commented Jul 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mtrmac commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrmac commented Jul 6, 2022

Uh oh!

Uh oh!

mtrmac Jul 8, 2022

Choose a reason for hiding this comment

Uh oh!

mtrmac Jul 11, 2022

Choose a reason for hiding this comment

Uh oh!

mtrmac Jul 11, 2022

Choose a reason for hiding this comment

Uh oh!

mtrmac commented Jul 12, 2022

Uh oh!

vrothberg left a comment

Choose a reason for hiding this comment

Uh oh!

rhatdan commented Jul 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mtrmac commented Jul 6, 2022 •

edited

Loading