signer: add SigstoreSigner from_priv_key_uri and import_ methods#535
signer: add SigstoreSigner from_priv_key_uri and import_ methods#535lukpueh merged 5 commits intosecure-systems-lab:mainfrom
Conversation
Signed-off-by: Lukas Puehringer <[email protected]>
Supports token detection from ambient credentials (default), and token creation from OAuth2 + OpenID browser login (with '?ambient=false' URI param). Updates usage example and smoke test accordingly. Signed-off-by: Lukas Puehringer <[email protected]>
Creates SigstoreKey from passed identity and issuer, and signer URI based on a bool to toggle ambient credential usage. Other changes: - _get_uri() helper - _get_keyid() helper, the existing one would require many changes in the schema module - update usage example and move from module to SigstoreSigner class docstring - update smoke test Signed-off-by: Lukas Puehringer <[email protected]>
lukpueh
force-pushed
the
sigstore-signer-uri
branch
from
4356c1b to
5210a5e
Compare
|
A passing ci run is available in my fork: https://github.com/lukpueh/securesystemslib/actions/runs/4417013232/jobs/7742109165 |
lukpueh
force-pushed
the
sigstore-signer-uri
branch
from
5210a5e to
46f2623
Compare
jku
left a comment
jku
left a comment
There was a problem hiding this comment.
Looks great! Comments below should not block merging this IMO.
- figure out appropriate CI trigger conditions
yes, pull_request trigger has to go I think.
- use identity/issuer from public key, passed to from_priv_key_uri, in the OAuth2 + OpenID flow
- validate if public key and token passed to SigstoreSigner's from_priv_key_uri and constructor match
I've asked about this in a couple of channels, let's see what happens
- ... adding a SigstoreSigner.import_github_action()
no need to rush with this (since maybe there are alternatives like just providing some useful constants) but this might be useful -- producing the github action identity string is not completely trivial, and requiring the issuer string is just making the user do work we could avoid.
Co-authored-by: Jussi Kukkonen <[email protected]> Signed-off-by: Lukas Puehringer <[email protected]>
The sigstore workflow requires id-token: 'write' permission to sign with ambient credentials. This permission is not available in a PRs from forks. Signed-off-by: Lukas Puehringer <[email protected]>
lukpueh
force-pushed
the
sigstore-signer-uri
branch
from
92fa0fd to
2acf318
Compare
|
Thanks for the review. I accepted your inline suggestion, disabled "on pr", and created separate issues for the remaining TODOs. CI failure is unrelated (#520). Merging... |
2 participants
addresses most of #533
from_priv_key_urisupports token detection from ambient credentials (default), and token creation from OAuth2 + OpenID browser login (with '?ambient=false' URI param).import_creates sigstoreKey from passed identity and issuer, and signer URI based on passed bool to toggle ambient credential usage.TODOs (not necessarily in this PR)
from_priv_key_uri, in the OAuth2 + OpenID flowSigstoreSigner.import_github_action()