Support verifying Sigstore bundles #478

tetsuo-cpp · 2023-01-30T09:26:16Z

No description provided.

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

tetsuo-cpp · 2023-01-30T09:33:08Z

sigstore/transparency.py

@@ -76,6 +76,13 @@ class LogEntry:
    The base64-encoded Signed Entry Timestamp (SET) for this log entry.
    """

+    # NOTE: After Rekor bundles (provided by `--rekor-bundle`) are removed, this will no longer be
+    # necessary.
+    _from_rekor_bundle: bool


This isn't great, but we'll be able to remove it soon when we get rid of --rekor-bundle so I'm not too worried about it.

The only problem I see is that even though this field is marked as internal, you still need to provide a value for it if you construct one yourself. But I don't think users of the API have a good reason to be doing that anyway.

This also outputs a RuntimeWarning from Pydantic at runtime:

RuntimeWarning: fields may not start with an underscore, ignoring "_from_rekor_bundle"

Yeah, I think we can do a little better than this API (both the private field and the manual initialization). I'll look into it.

Hmm, maybe we can just remove --rekor-bundle outright? We never "actually" stabilized it (we've always emitted a warning said that it's temporary and will be removed in an upcoming release), and we never integrated it into other tooling (like the GH Action) or other expectations (e.g. CPython doesn't list it).

Thoughts @di?

Fine with me!

Cool! I'll just yank all of that out, then, and save us from even having to think about this 🙂

di

LGTM aside from the RuntimeWarning, we should find a workaround for that.

Vestigial now that we have Sigstore bundle support. Signed-off-by: William Woodruff <william@trailofbits.com>

Signed-off-by: William Woodruff <william@trailofbits.com>

woodruffw · 2023-01-30T23:15:18Z

sigstore/verify/models.py

+    offline: bool
+    """
+    Whether to do offline Rekor entry verification.
+
+    This is a slightly weaker verification verification mode, as it demonstrates
+    that an entry has been signed by the log but not necessarily included in it.
    """


Flagging: I'm not super happy with this setting being here, since it's conceptually a verification option rather than a material.

The other possibility I can think of is a separate VerificationOptions model, but that might be overkill given that this is currently the only option we support. But maybe there will be more in the future?

Personally, I think it's ok.

If we go for a separate VerificationOptions model, we're never going to collapse that back into VerificationMaterials even if it remains as one option for a very long time because there's always going to be the possibility of more.

Yeah, that's a good point. I think what I'll do is make it "private" like _rekor_entry so that it isn't part of our public API commitment. It'll still end up being stabilized as part of the methods on VerificationMaterials, but that'll at least avoid the need to change this model's public fields if we ever do decide to refactor it.

Signed-off-by: William Woodruff <william@trailofbits.com>

woodruffw · 2023-01-30T23:19:43Z

@tetsuo-cpp I'll kick this back over to you, but to summarize the changes I've made:

I removed Rekor bundle support entirely, both in the internal APIs and in the CLI
I added VerificationMaterials.offline and VerificationMaterials.from_bundle(...), the latter being the expected API for converting a Sigstore bundle into its equivalent materials.
I fixed up the unit tests for my changes and added some new assets/updated the fixtures to reflect them.

I think this is in a pretty good state overall, but IMO we should bring the test coverage up a bit -- it'd be good to get some coverage for the new InvalidMaterials exception and its raise-sites.

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

tetsuo-cpp · 2023-01-31T01:18:08Z

I think this is in a pretty good state overall, but IMO we should bring the test coverage up a bit -- it'd be good to get some coverage for the new InvalidMaterials exception and its raise-sites.

Done in dc1cb3b.

woodruffw · 2023-01-31T15:14:35Z

sigstore/_cli.py

-        "--require-rekor-offline",
+        "--offline",
        action="store_true",
-        default=_boolify_env("SIGSTORE_REQUIRE_REKOR_OFFLINE"),
-        help="Require offline Rekor verification with a bundle; implied by --rekor-bundle",
+        default=_boolify_env("SIGSTORE_OFFLINE"),
+        help="Perform offline verification; requires a Sigstore bundle",


I'm now realizing that this is slightly misleading, since --offline implies "fully" offline, whereas we still do some network requests for TUF. I think we should keep this flag, though, and work on removing/suppressing those requests when the user requests offline verification.

Signed-off-by: William Woodruff <william@trailofbits.com>

woodruffw

LGTM!

tetsuo-cpp added 7 commits January 30, 2023 17:29

_cli: Add support for verifying Sigstore bundles

638d632

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

treewide: Verify inclusion proof for Sigstore bundles

ce00d3c

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

_cli: Clean up comments

8e37dc7

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

CHANGELOG: Add changelog entry

40a65c7

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

_cli: Comment formatting

fcd5a7d

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

_cli: Rename to a private field

a74fd8a

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

CHANGELOG: Add PR link

a63c64d

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

tetsuo-cpp commented Jan 30, 2023

View reviewed changes

tetsuo-cpp requested review from woodruffw, di and jleightcap January 30, 2023 09:33

di reviewed Jan 30, 2023

View reviewed changes

Merge branch 'main' into alex/verify-bundle

8546c1f

woodruffw assigned tetsuo-cpp Jan 30, 2023

woodruffw added component:cli CLI components component:verification Core verification functionality labels Jan 30, 2023

woodruffw added 7 commits January 30, 2023 14:28

Merge branch 'main' into alex/verify-bundle

cbb1e53

sigstore: remove "rekor bundle" support

03f7321

Vestigial now that we have Sigstore bundle support. Signed-off-by: William Woodruff <william@trailofbits.com>

docs, sigstore: more pruning

1b32fc1

Signed-off-by: William Woodruff <william@trailofbits.com>

sigstore, test: devolve bundle handling, fix tests

c397f8d

Signed-off-by: William Woodruff <william@trailofbits.com>

sigstore, test: lintage

24f291b

Signed-off-by: William Woodruff <william@trailofbits.com>

verify/models: docstring

6d7adc1

Signed-off-by: William Woodruff <william@trailofbits.com>

test: fix offline test

f13d6b6

Signed-off-by: William Woodruff <william@trailofbits.com>

woodruffw reviewed Jan 30, 2023

View reviewed changes

assets: remove old rekor bundle

36392d5

Signed-off-by: William Woodruff <william@trailofbits.com>

test: Add more test coverage for InvalidMaterials error cases

dc1cb3b

Signed-off-by: Alex Cameron <asc@tetsuo.sh>

tetsuo-cpp requested review from di and woodruffw January 31, 2023 01:21

woodruffw reviewed Jan 31, 2023

View reviewed changes

sigstore, test: docs, API cleanup

4d8cb62

Signed-off-by: William Woodruff <william@trailofbits.com>

woodruffw mentioned this pull request Jan 31, 2023

CLI: --offline should mean fully offline #483

Closed

woodruffw approved these changes Jan 31, 2023

View reviewed changes

woodruffw merged commit fc4e9b3 into main Jan 31, 2023

woodruffw deleted the alex/verify-bundle branch January 31, 2023 16:01

woodruffw mentioned this pull request Jan 31, 2023

Support generating and verifying with "Sigstore" bundles #251

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support verifying Sigstore bundles #478

Support verifying Sigstore bundles #478

tetsuo-cpp commented Jan 30, 2023

tetsuo-cpp Jan 30, 2023

di Jan 30, 2023

woodruffw Jan 30, 2023 •

edited

Loading

woodruffw Jan 30, 2023

di Jan 30, 2023

woodruffw Jan 30, 2023

di left a comment

woodruffw Jan 30, 2023

tetsuo-cpp Jan 31, 2023

woodruffw Jan 31, 2023 •

edited

Loading

woodruffw commented Jan 30, 2023

tetsuo-cpp commented Jan 31, 2023

woodruffw Jan 31, 2023 •

edited

Loading

woodruffw left a comment

Support verifying Sigstore bundles #478

Support verifying Sigstore bundles #478

Conversation

tetsuo-cpp commented Jan 30, 2023

tetsuo-cpp Jan 30, 2023

Choose a reason for hiding this comment

di Jan 30, 2023

Choose a reason for hiding this comment

woodruffw Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

woodruffw Jan 30, 2023

Choose a reason for hiding this comment

di Jan 30, 2023

Choose a reason for hiding this comment

woodruffw Jan 30, 2023

Choose a reason for hiding this comment

di left a comment

Choose a reason for hiding this comment

woodruffw Jan 30, 2023

Choose a reason for hiding this comment

tetsuo-cpp Jan 31, 2023

Choose a reason for hiding this comment

woodruffw Jan 31, 2023 • edited Loading

Choose a reason for hiding this comment

woodruffw commented Jan 30, 2023

tetsuo-cpp commented Jan 31, 2023

woodruffw Jan 31, 2023 • edited Loading

Choose a reason for hiding this comment

woodruffw left a comment

Choose a reason for hiding this comment

woodruffw Jan 30, 2023 •

edited

Loading

woodruffw Jan 31, 2023 •

edited

Loading

woodruffw Jan 31, 2023 •

edited

Loading