Conversation
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
sigstore/transparency.py
Outdated
|
|
||
| # NOTE: After Rekor bundles (provided by `--rekor-bundle`) are removed, this will no longer be | ||
| # necessary. | ||
| _from_rekor_bundle: bool |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This also outputs a RuntimeWarning from Pydantic at runtime:
RuntimeWarning: fields may not start with an underscore, ignoring "_from_rekor_bundle"
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Cool! I'll just yank all of that out, then, and save us from even having to think about this 🙂
di
left a comment
There was a problem hiding this comment.
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>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
sigstore/verify/models.py
Outdated
| 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. | ||
| """ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
|
@tetsuo-cpp I'll kick this back over to you, but to summarize the changes I've made:
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 |
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Done in dc1cb3b. |
| "--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", |
There was a problem hiding this comment.
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>
No description provided.