Closed tetsuo-cpp closed 1 year ago
Now that we're including an intermediate key, this should also include that as well.
Hi, we are a small team (@wxjdsr, @vcharapa) that is working with professor Santiago (@SantiagoTorres) on fixing this issue.
Cool! Please let me or one of the other maintainers know if there's something we can do to help.
👋 python-tuf maintainer and sigstore/root-signing contributor here.
I posted a message on the #python Slack channel a few weeks back about some WIP changes I have to implement the certificate update workflow in python-sigstore. You can see my WIP branch here: https://github.com/joshuagl/sigstore-python/commits/joshuagl/tuf
Apologies for not posting about that work here, I didn't find this issue when I tried searching for something about the TUF workflow 🤦
It would be cool to collaborate on this, or have your work build on top of mine, @wxjdsr and @vcharapa?
👋 python-tuf maintainer and sigstore/root-signing contributor here.
I posted a message on the #python Slack channel a few weeks back about some WIP changes I have to implement the certificate update workflow in python-sigstore. You can see my WIP branch here: https://github.com/joshuagl/sigstore-python/commits/joshuagl/tuf
Apologies for not posting about that work here, I didn't find this issue when I tried searching for something about the TUF workflow 🤦
It would be cool to collaborate on this, or have your work build on top of mine, @wxjdsr and @vcharapa?
Cool. I think we can collaborate on this.
Great! Let me know how you want to proceed. Given timezone differences, if you want to build on my existing work (or even start afresh with that for inspiration) I'd be OK with that. Just want to make sure we aren't duplicating efforts, so please let me know how you would like to proceed :-)
Sorry for late reply. We would like to build on your existing work. Currently we are working on fetching data from RemoteRoot for TUF. And I hope this does not conflict with your current work. XD
Awesome, look forward to seeing your changes. I'd be more than willing to collaborate in any way that is useful for you and your team. Feel free to ping me on Sigstore Slack (I'm in the #python channel).
If you look at the branch I linked to above (https://github.com/joshuagl/sigstore-python/commits/joshuagl/tuf), you can see my WIP changes include initial retrieval of the TUF metadata and changes to access the certificates from a directory the TUF client fetches to, rather than accessing the bundled certificates using resourcelib (see https://github.com/joshuagl/sigstore-python/commit/e343366399bbe505b23c539cb2b94df2cb74c1c1).
What's next is adapting to the pending v5 root-signing changes in https://github.com/sigstore/root-signing/pull/430 where, instead of fetching a hard-coded set of targets, each of the certificates listed in the Targets metadata have additional metadata in the custom
field that describes their usage.
For CTFE certificates we can query the metadata to retrieve only the targets in the which have custom.sigstore.usage
== "CTFE". You can see how the Go Sigstore library is implementing this pattern in https://github.com/sigstore/sigstore/blob/fe89132b9369fc783254378ff8cfa68edfb64f72/pkg/fulcioroots/fulcioroots.go#L69 and https://github.com/sigstore/sigstore/blob/fe89132b9369fc783254378ff8cfa68edfb64f72/pkg/tuf/client.go#L373.
each of the certificates listed in the Targets metadata have additional metadata in the custom field that describes their usage.
While this will work currently for all top-level targets, we're also adding targets to folders named by their usage, so all targets under ctfe/**
will be for ctfe
: this pattern will be stable in the future, and secure for any future delegations we add, so I recommend that pattern!
Thanks for chiming in @asraa! To be clear, are we recommending targets are searched by delegation path not by the usage field in custom?
To be clear, are we recommending targets are searched by delegation path not by the usage field in custom?
Searched by path (not necessarily delegation path): for top-level targets you can do either, but for other targets, we can't necessarily trust their usage field; we only trust that they were designated the correct delegation paths. (e.g. should a fulcio delegation be allowed to write to fulcio/**
, but then adds a target there with custom usage of Rekor
: that would be a no-go).
Technically for this root it's moot and the same since there are only top-level targets, but supporting based on paths would be stable going forward forever.
Leaving a couple of notes:
Speaking of this: the best way forward is probably to keep Sigstore TUF root with only a top-level targets, where we CAN do path filtering because TUF clients can expose the top-level targets list.
Then when we add delegations, we can view that as a new feature using one of the suggestions in the doc: I particularly like the well-known path location listing the targets.
Just wanted to make sure I understand the conclusion reached here: when sigstore-python adds TUF support, should we discover the CT key(s) via their custom metadata, their filename, or some other mechanism?
I understand the conclusion reached here: when sigstore-python adds TUF support, should we discover the CT key(s) via their custom metadata, their filename, or some other mechanism?
Since Target Discovery/endpoint to retrieve by usage is still "in flux", I think the simplest (and most secure) would be to rely on their filenames: IF we do update the target names, it will be at a point in time where we have this discovery piece laid out, and you could sub for file in ctfe_filenames: get_target(file)
with for file in tuf.get_filenames(ctfe): get_target(file)
.
@joshuagl also has sigstore-python staged for our pre-submits when there's integration to make sure we won't break you before a TUF update.
My WIP patches have the known cert filenames hard coded and just retrieve those after refreshing TUF metadata.
I can pick up work on those patches after KubeCon next week, or hand-off to another interested party. Let's assign this issue so we know who's working on it?
@wxjdsr/@vcharapa/@SantiagoTorres should that be one of you? Haven't heard from you on this issue in a while.
@wxjdsr/@vcharapa/@SantiagoTorres should that be one of you? Haven't heard from you on this issue in a while.
Yes, we are working on it.
I've assigned this issue to @wxjdsr.
Let me or @tetsuo-cpp know if there's any help we can provide!
cc @wxjdsr @vcharapa any update here? We're getting close to a 1.0 release of sigstore-python
, and this will probably be a necessary component.
We (ToB) are happy to help in the development if it's stuck, or you have limited bandwidth. Just let us know!
For googleability, in older clients a missing key will manifest as a InvalidSctError
with a stacktrace like:
$ python -m sigstore sign <file>
Go to the following link in a browser:
https://oauth2.sigstore.dev/auth/auth?...
Enter verification code: <code>
Traceback (most recent call last):
File "/usr/lib/python3.8/site-packages/sigstore/_internal/sct.py", line 303, in verify_sct
ctfe_key.verify(
File "/usr/lib/python3.8/site-packages/cryptography/hazmat/backends/openssl/ec.py", line 315, in verify
_ecdsa_sig_verify(self._backend, self, signature, data)
File "/usr/lib/python3.8/site-packages/cryptography/hazmat/backends/openssl/ec.py", line 122, in _ecdsa_sig_verify
raise InvalidSignature
cryptography.exceptions.InvalidSignature
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/usr/lib/python3.8/site-packages/sigstore/__main__.py", line 22, in <module>
main()
File "/usr/lib/python3.8/site-packages/sigstore/_cli.py", line 257, in main
_sign(args)
File "/usr/lib/python3.8/site-packages/sigstore/_cli.py", line 382, in _sign
result = signer.sign(
File "/usr/lib/python3.8/site-packages/sigstore/_sign.py", line 101, in sign
verify_sct(
File "/usr/lib/python3.8/site-packages/sigstore/_internal/sct.py", line 314, in verify_sct
raise InvalidSctError from inval_sig
sigstore._internal.sct.InvalidSctError
and the solution is to upgrade to the latest version of the client with pip install -U sigstore
.
I plan to spend a bit of time on this this week: will update in a few days
Hi, I'll be picking up the code that @wxjdsr sketched out. Should we submit a draft PR with their code or should I just finalize it and send it off in, say this weekend?
@SantiagoTorres either works for us! A draft PR would help us make sure it meshes with our stabilization/1.0 plans, but whenever you feel it's ready.
(If you have limited bandwidth to finalize it, @jku or I could take it over from the draft as well.)
A draft PR would help us make sure it meshes with our stabilization/1.0 plans
A bit more detail on this: this is included as a prerequisite for our 1.0 release which we're hoping to make before EOY.
I'll leave some initial comments after just reading the code (I'm happy to work on this myself but I'm writing these down so A) it doesn't have to be me B) as reminder to myself):
$HOME/.local/share/sigstore-python/metadata/
on linux)I'll leave some initial comments after just reading the code (I'm happy to work on this myself but I'm writing these down so A) it doesn't have to be me B) as reminder to myself):
Where is the code 😅? I don't see it on a PR.
Your comments all sound reasonable (caveat being that I haven't reviewed it myself) -- in particular I agree completely that the _store
confusion needs to be resolved, and that we should absolutely use standard "runtime" state directories on relevant platforms rather than hardcoding ~/.sigstore
.
Re: _store
: we should probably put the TUF retrieval code under _tuf
. _store
should be limited to just static cryptographic materials, which after that PR should just be the TUF verification keys themselves.
I think Jussi reviewed @joshuagl's WIP here: https://github.com/sigstore/sigstore-python/compare/main...joshuagl:sigstore-python:joshuagl/tuf
Where is the code ? I don't see it on a PR.
fair point. I was looking at joshuas and wxjdsr's branches (latter is on top of former). Just to be clear: no-one asked for a review on those but I just needed to get a feel for the current situation...
https://github.com/wxjdsr/sigstore-python/commits/wxjdsr/tuf
FWIW, I agree with your assessment. I think we could split some if this into separate stuff. I was mostly going to do some cherry picking on @wxjdsr 's branch this weekend in hopes to getting some early progress in
Some notes on configuration:
--tuf-url
(it would make sense to have some .well-known scheme maybe?)--ctfe
and --rekor-root-pubkey
we should not trigger TUF traffic IMO: the tuf module/class should be lazy in this regard
- there does not seem to be a "tuf metadata location" that can be derived from any other url: this would mean there has to be yet another option
--tuf-url
(it would make sense to have some .well-known scheme maybe?)
This would be nice to have! I assume this is a standards topic, right?
- if user actually sets options like
--ctfe
and--rekor-root-pubkey
we should not trigger TUF traffic IMO: the tuf module/class should be lazy in this regard
Agreed.
I'll report some results tomorrow: I've been playing with some potential changes today and expect I'll have something reasonable running tomorrow.
Thanks a ton! Let me and @tetsuo-cpp know if we can help in any way.
Okay, status update for https://github.com/jku/sigstore-python/commits/tuf-refactor (diff):
on staging: As it really looks like staging keys are not available from any TUF repo, I've disabled TUF for staging now: so TUF gets used:
As it really looks like staging keys are not available from any TUF repo
I was completely incorrect: https://storage.googleapis.com/tuf-root-staging -- I am testing this right now
- other keys/certs are not yet used from TUF
If I'm understanding correctly, that leaves just the Fulcio certs, right? I see that they're at least hosted in the staging TUF repo, so we could do that as a follow-up 🙂
oh I'm on it already now that staging seems doable :) Fulcio should be supported now. I'm still getting a verify failure with --staging
so may still have missed something but the TUF parts seem to all work...
At the moment, we've decided to check in the CTFE public key and use it to verify SCTs. The way this should really work is that we should check in the root key (
root.json
) and use it to download the CTFE key via TUF.