Add a sigstore signing parameter file format, and CLI utility by mtrmac · Pull Request #1787 · containers/image

mtrmac · 2023-01-11T20:25:30Z

This defines a new file format (containers-sigstore-signing-params.yaml(5)), and provides an API to integrate it into callers.

That way, skopeo and podman can add a single --sigstore-sign-by=param-file option, instead of 9 options; and users can maintain their site-specific configuration in one file, instead of manually copy&pasting 6 options in a typical case.

The parameter file format definition is a separate package that does not drag in the Rekor and Fulcio dependency tree; only actually using the file has that cost.

I’m not too happy about the “containers sigstore signing parameters” name, nor about the sigstore/params subpackage name; I’ll certainly welcome suggestions.

(One possibility would be to make this a “containers signing parameters file”, not sigstore-specific, and to move all options in that file into a sigstore sub-object; then we could add a “simple signing” option to that same file as well.)

I also didn’t bother with using “functional options” for the NewSignerFromParameterFile function, nor with hiding the actual file format and only providing a constructor or an edit API. This is a non-essential utility and we can always provide a cli/fulciov2 without impacting any existing callers, so keeping the API opaque and stable over time is not that extremely important in this case.

Depends on #1785 .

@vrothberg PTAL. @rhatdan FYI.

mtrmac · 2023-01-11T21:08:49Z

Tested manually in Skopeo containers/skopeo#1849 , basically with the provided example configs.

Adding unit tests is tracked in containers/container-libs#235.

mtrmac · 2023-01-11T23:56:53Z

Podman WIP: containers/common#1288 https://github.com/containers/podman/pull/17088/files

vrothberg

LGTM

mtrmac · 2023-01-12T17:04:00Z

Rebased, ready for merging. (Or wait to see what the Podman PR ends up like.)

mtrmac · 2023-01-12T18:43:58Z

… and I do want to encourage changing the name of the file. I’m not happy at all with the current one.

mtrmac · 2023-01-12T19:00:39Z

Updated to quote YAML strings, motivated by https://ruudvanasseldonk.com/2023/01/11/the-yaml-document-from-hell .

(I chose to avoid TOML because of #1054 , and YAML’s expression of nesting is much nicer. But I’m fine with any other format, to be consistent with the rest of the ecosystem.)

vrothberg · 2023-01-13T08:04:58Z

Updated to quote YAML strings, motivated by https://ruudvanasseldonk.com/2023/01/11/the-yaml-document-from-hell .

(I chose to avoid TOML because of #1054 , and YAML’s expression of nesting is much nicer. But I’m fine with any other format, to be consistent with the rest of the ecosystem.)

LGTM

I don't have an opinion on the file format. Made me think of Mutt's slog: "All mail clients suck. This one just sucks less."

TomSweeneyRedHat · 2023-01-13T16:05:50Z

docs/containers-sigstore-signing-params.yaml.5.md

+# NAME
+containers-sigstore-signing-params.yaml - syntax for the sigstore signing parameter file
+
+# DESCRIPTINO


Suggested change

# DESCRIPTINO

# DESCRIPTION

docs/containers-sigstore-signing-params.yaml.5.md

TomSweeneyRedHat · 2023-01-13T16:10:51Z

docs/containers-sigstore-signing-params.yaml.5.md

+# DESCRIPTINO
+
+Sigstore signing parameter files are used to store options that may be required to create sigstore signatures.
+There is no standard location for these files; they are user-managed, and used as inputs to a container image signing operation,


On the fence, your call. "standard location" or "default location"?

(I’m not very sure that sentence even needs to exist… I just wanted to give some context for users who stumble on the page directly, without starting at a --sign-by-sigstore option documentation.)

Sure, “default location” works.

TomSweeneyRedHat · 2023-01-13T16:19:58Z

docs/containers-sigstore-signing-params.yaml.5.md

+  `deviceGrant` uses a device authorization grant flow (RFC 8628).
+  It requires the user to be able to read text printed by the signing process, and to act on it reasonably promptly.
+
+  `staticToken` provides a pre-existing Open ID Connect ID token which must have been obtained separately.


Based solely on grokking Google, it looks like it should be "OpenID Connect". (https://openid.net/connect/). Please change here and elsewhere. I've added quotes to set it off from the rest of the text and to let the reader know that's a thing that could be looked up if they're unfamiliar as I was. That said, I'm still on the fence about having the quotes, so no foul if you want to drop them.

Suggested change

`staticToken` provides a pre-existing Open ID Connect ID token which must have been obtained separately.

`staticToken` provides a pre-existing "OpenID Connect" ID token which must have been obtained separately.

I'll hop off the fence, drop the quotes. I see you have a number of instances and it would look odd all over.

Thanks! The quotes are a great idea for separating the two terms.

(I didn’t find any other instance of “Open ID” with a space.)

How about quoting the “ID token” part instead? That would be less useful as a stand-alone search term, but assuming the readers figure out OpenID Connect is a term to search from other parts of the page, making it clear “ID token” is a single phrase might be worthwhile.

Works for me!

Thanks, changed to quote “ID token”.

TomSweeneyRedHat · 2023-01-13T16:26:39Z

Couple doc nits which I think @mtrmac has tended to already, otherwise, LGTM.

mtrmac · 2023-01-13T16:28:20Z

@TomSweeneyRedHat Thanks a lot!

Introduce a "sigstore signing parameter file" that can carry all the required configuration, so that we don't need to add 9 different CLI options. Signed-off-by: Miloslav Trmač <mitr@redhat.com>

TomSweeneyRedHat

LGTM

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

We drop a patch that is now part of the release. Bumping skopeo to version v1.11.0-39-g95680f3c, which comprises the following commits: 643a2359 Update c/image after containers/image#1816 2c6e15b5 Run codespell on codebase df708d16 [CI:DOCS] Disable dependabot 2acac8a6 Update module golang.org/x/term to v0.5.0 f9e2c676 Update golang.org/x/exp digest to 46f607a 47c7902e Remove unnecessary blank lines c1a57ca1 Pre-allocate an array 2a7b1327 Simplify a condition e7ab33e6 Rename a variable to avoid an underscore e90c381a Add missing comment punctuation 70c06b4a Fix, or remove, comments using lint syntax 9137ac56 Simplify an increment efc6e837 Reformat import statements a8b9e4e3 Use %w when wrapping errors 99215e40 Remove a duplicate word afa031e8 Use net/netip.Addr instead of net.IP 891ba3d4 s/interface{}/any/g f2b3a9c0 Use golang.org/x/exp f1a6d427 Use strings.Cut 22955d05 go mod tidy -go=1.18 007f01c6 [CI:BUILD] enable debuginfo for el8 copr builds 036bf598 [CI:BUILD] copr: fix el8 build and enable debuginfo f9406bb0 Cirrus: Use human-readable CI VM Images b41b85ab Update module gopkg.in/yaml.v2 to v3 d2fbec35 Add unit tests for tlsVerifyConfig's yaml.Unmarshaler 9e24a195 [CI:DOCS] Fix up language in README cc958d3e Move to v1.11.1-dev 9d036f30 Bump to v1.11.0 83bcd136 [CI:DOCS] Format manual page documents afbdaf8e Update module github.com/containers/common to v0.51.0 c9114248 Update module github.com/containers/image/v5 to v5.24.0 0fad1193 Add (skopeo generate-sigstore-key) 48b9d94c Update c/image after containers/image#1810 80e3fd10 Touch up conscious language issues 9f04dfde Partially fix removal of temporary data in (make test-system) 36c480f6 Don't affect $XDG_RUNTIME_DIR of Podman starting the registry 850bc49d Update module github.com/containers/storage to v1.45.3 a98c1372 Fix storage.conf setup in test-system 19815502 Fix (test-integration), in a container without CI 67a8bef6 Cirrus: Fix c/image CI testing 63da8390 Bump github.com/containers/ocicrypt from 1.1.6 to 1.1.7 1fac61ef Cirrus: Add a common intra-test reset function 292962d3 Fix unnecessary use of podman in CI test e239f32a Cirrus: Update to F37 CI VM Images ee804858 Cirrus: Remove redundant package install attempt 0698e82b fix(deps): update module github.com/containers/storage to v1.45.1 bb1ac893 Add support for Fulcio and Rekor, and --sign-by-sigstore=param-file 03b5bdec Update c/image after containers/image#1787 1133a2a3 fix(deps): update module github.com/containers/storage to v1.45.0 d0cf39d8 Cirrus: Skip OSX CI on release-branches f17eafe8 Correctly use the stdout parameter in some places 58bccf38 fix(deps): update module golang.org/x/term to v0.4.0 Signed-off-by: Bruce Ashfield <bruce.ashfield@gmail.com>

mtrmac mentioned this pull request Aug 27, 2025

Sigstore technical debt containers/container-libs#235

Open

16 tasks

mtrmac force-pushed the signing-cli branch from 22d6dfc to 52a330b Compare January 11, 2023 20:29

mtrmac mentioned this pull request Jan 11, 2023

Add support for Fulcio and Rekor, and --sign-by-sigstore=param-file containers/skopeo#1849

Merged

mtrmac added the kind/feature A request for, or a PR adding, new functionality label Jan 11, 2023

mtrmac force-pushed the signing-cli branch from 52a330b to 5126f0f Compare January 11, 2023 23:01

mtrmac mentioned this pull request Jan 11, 2023

Add … push --sign-by-sigstore containers/podman#17088

Merged

vrothberg approved these changes Jan 12, 2023

View reviewed changes

mtrmac force-pushed the signing-cli branch from 5126f0f to 66c7087 Compare January 12, 2023 17:03

mtrmac marked this pull request as ready for review January 12, 2023 17:04

mtrmac force-pushed the signing-cli branch from 66c7087 to bc233ac Compare January 12, 2023 18:58

mtrmac force-pushed the signing-cli branch 2 times, most recently from 00cafd6 to 508d364 Compare January 13, 2023 12:35

TomSweeneyRedHat reviewed Jan 13, 2023

View reviewed changes

docs/containers-sigstore-signing-params.yaml.5.md Show resolved Hide resolved

TomSweeneyRedHat reviewed Jan 13, 2023

View reviewed changes

mtrmac force-pushed the signing-cli branch 2 times, most recently from fa660ed to 996e9b2 Compare January 13, 2023 16:19

TomSweeneyRedHat reviewed Jan 13, 2023

View reviewed changes

mtrmac force-pushed the signing-cli branch from 996e9b2 to 2cfd9b1 Compare January 13, 2023 16:24

Add pkg/cli/sigstore

9e3177f

Introduce a "sigstore signing parameter file" that can carry all the required configuration, so that we don't need to add 9 different CLI options. Signed-off-by: Miloslav Trmač <mitr@redhat.com>

mtrmac force-pushed the signing-cli branch from 2cfd9b1 to 9e3177f Compare January 13, 2023 16:41

TomSweeneyRedHat reviewed Jan 13, 2023

View reviewed changes

rhatdan merged commit cf9ccfb into containers:main Jan 13, 2023

mtrmac deleted the signing-cli branch January 14, 2023 12:27

mtrmac added a commit to mtrmac/skopeo that referenced this pull request Jan 14, 2023

Update c/image after containers/image#1787

03b5bde

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

mtrmac added a commit to mtrmac/common that referenced this pull request Jan 14, 2023

Update c/image after containers/image#1787

1bd7178

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

This was referenced Jan 26, 2023

[v4.4] Add … push --sign-by-sigstore containers/podman#17240

Closed

[v4.4] Add … push --sign-by-sigstore containers/podman#17241

Merged

	`staticToken` provides a pre-existing Open ID Connect ID token which must have been obtained separately.
	`staticToken` provides a pre-existing "OpenID Connect" ID token which must have been obtained separately.

Conversation

mtrmac commented Jan 11, 2023

Uh oh!

mtrmac commented Jan 11, 2023

Uh oh!

mtrmac commented Jan 11, 2023

Uh oh!

vrothberg left a comment

Choose a reason for hiding this comment

Uh oh!

mtrmac commented Jan 12, 2023

Uh oh!

mtrmac commented Jan 12, 2023

Uh oh!

mtrmac commented Jan 12, 2023

Uh oh!

vrothberg commented Jan 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomSweeneyRedHat commented Jan 13, 2023

Uh oh!

mtrmac commented Jan 13, 2023

Uh oh!

TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants