Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support priv_key_uri as CLI argument #533

Closed
lukpueh opened this issue Jan 17, 2023 · 1 comment · Fixed by #651
Closed

Support priv_key_uri as CLI argument #533

lukpueh opened this issue Jan 17, 2023 · 1 comment · Fixed by #651

Comments

@lukpueh
Copy link
Member

lukpueh commented Jan 17, 2023

Description of issue or feature request:
The new securesystemslib v0.26.0 signer API allows loading arbitrary signing providers from private key URIs. Supporting a private key URI argument in relevant in-toto CLI allows adding and removing support for arbitrary signing providers in the future, without changing the CLI. Relevant CLI are: in-toto-run, in-toto-record.

IMPORTANT: Coordinate with #532!

Current behavior:
Relevant CLI don't support private key URI argument.

Expected behavior:
Relevant CLI support private key URI argument.

Optional expected behavior:
Deprecate existing signing arguments in relevant CLI (e.g. --key, --gpg,--gpg-home).
This was referenced Jan 17, 2023
Closed
Merged
This was referenced Mar 22, 2023
Closed
Open
lukpueh added a commit to lukpueh/in-toto that referenced this issue Jul 4, 2023
@lukpueh
runlib: add signer kwarg to run and record methods (WIP)
a9b87b0 
IMPORTANT: Requires unlreleased securesystemslib code from
secure-systems-lab/securesystemslib#590

Adds optional signer (Signer) arg to runlib's run/record functions, as
alternative way of signing resulting link metadata, instead of using
signing_key, gpg_keyid, or use_default_gpg.

Closes in-toto#532

**Addtional notes:**

In in_toto_record_stop, the public_key (Key) field of the signer is used
to verify the preliminary link metadata file. The field is not yet part
of the interface, although all signer implementations in secureystemslib
have it: secure-systems-lab/securesystemslib#605

The patch aims to be minimally invasive, and thus barely refactors any
of the existing signing argument handling in the relevant functions.
Although, it was tempting to simplify the code, it turned out harder
than thought, and therefor not worth the effort, given that these
arguments are bound to
deprecation.

This is patch is part of a series of patches to prepare for the planned
removal of securesystemslib legacy interfaces, see
secure-systems-lab/securesystemslib#604 for details.

**TODO (follow-up)**
- add deprecation warning for legacy key args and legacy gpg formats
  (only show on API use, not on CLI use!)

- add Metadata.verify(public_key: Key), if necessary (maybe we can just
  do this in runlib/verifylib), and use in in_toto_record_stop instead
  of passing `signer.public_key.to_dict()` to `Metadata.verify_signature(...)`

- prepare for ssslib legacy interface removal in other parts of in_toto:
  - in-toto-run, in-toto-record (in-toto#533)
  - verifylib / in-toto-verify
  - in-toto-sign
  - Metadata API / docs
  - ...

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh lukpueh mentioned this issue Jul 4, 2023
Merged
@lukpueh
Copy link
Member Author

lukpueh commented Oct 24, 2023

Turns out, "priv_key_uri" does not do what I had in mind when writing this issue. That is, it does not encode all necessary information to load a signer.

Instead, the API, which takes the URI, also requires a secrets handler and a related public key

We could work around this by taking addtional options, which, together with the URI, would allow us to construct the required public key and secrets handler first. For all currently available signers, these options should work:

--uri <uri>  # private key uri, can also be (ab)used to infer pubkey
[--secret [<secret>]  # takes None, password, or toggles prompt
[--keyid <keyid>]  # optional non-default keyid
[--scheme <scheme>]  # optional non-default signing scheme

But it does not seem so desirable anymore, for various reasons:

  • We can't use the 'Signer.from_priv_key_uri' interface as is, and it probably does not make sense to change this upstream, because its usage pattern just does not fit (see securesystemslib#664).

  • We don't know for sure if new signers, won't need yet additional options. So the original flexibility argument becomes less strong.

  • The UX of manually creating and passing signer URIs is questionable to start with. If the user has to pass addtional options, it becomes even more complex for no reason.

Taking all this into account I suggest we just add new per-signer options and ask upstream to provide the corresponding API to load signer + pubkey (also see securesystemslib#664)

lukpueh added a commit to lukpueh/in-toto that referenced this issue Oct 25, 2023
@lukpueh
runlib: add signer kwarg to run and record methods
4bf538e 
Adds optional signer (Signer) arg to runlib's run/record functions, as
alternative way of signing resulting link metadata, instead of using
signing_key, gpg_keyid, or use_default_gpg.

Closes in-toto#532

**Addtional notes:**

In in_toto_record_stop, the public_key (Key) field of the signer is used
to verify the preliminary link metadata file. The field is not yet part
of the interface, although all signer implementations in secureystemslib
have it: secure-systems-lab/securesystemslib#605

The patch aims to be minimally invasive, and thus barely refactors any
of the existing signing argument handling in the relevant functions.
Although, it was tempting to simplify the code, it turned out harder
than thought, and therefor not worth the effort, given that these
arguments are bound to
deprecation.

This patch is part of a series of patches to prepare for the planned
removal of securesystemslib legacy interfaces, see
secure-systems-lab/securesystemslib#604 for details.

**TODO (follow-up)**
- add deprecation warning for legacy key args and legacy gpg formats
  (only show on API use, not on CLI use!)

- add Metadata.verify(public_key: Key), if necessary (maybe we can just
  do this in runlib/verifylib), and use in in_toto_record_stop instead
  of passing `signer.public_key.to_dict()` to `Metadata.verify_signature(...)`

- prepare for ssslib legacy interface removal in other parts of in_toto:
  - in-toto-run, in-toto-record (in-toto#533)
  - verifylib / in-toto-verify
  - in-toto-sign
  - Metadata API / docs
  - ...

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/in-toto that referenced this issue Oct 25, 2023
@lukpueh
runlib: add signer kwarg to run and record methods
1c23378 
Adds optional signer (Signer) arg to runlib's run/record functions, as
alternative way of signing resulting link metadata, instead of using
signing_key, gpg_keyid, or use_default_gpg.

Closes in-toto#532

**Addtional notes:**

In in_toto_record_stop, the public_key (Key) field of the signer is used
to verify the preliminary link metadata file. The field is not yet part
of the interface, although all signer implementations in secureystemslib
have it: secure-systems-lab/securesystemslib#605

The patch aims to be minimally invasive, and thus barely refactors any
of the existing signing argument handling in the relevant functions.
Although, it was tempting to simplify the code, it turned out harder
than thought, and therefor not worth the effort, given that these
arguments are bound to
deprecation.

This patch is part of a series of patches to prepare for the planned
removal of securesystemslib legacy interfaces, see
secure-systems-lab/securesystemslib#604 for details.

**TODO (follow-up)**
- add deprecation warning for legacy key args and legacy gpg formats
  (only show on API use, not on CLI use!)

- add Metadata.verify(public_key: Key), if necessary (maybe we can just
  do this in runlib/verifylib), and use in in_toto_record_stop instead
  of passing `signer.public_key.to_dict()` to `Metadata.verify_signature(...)`

- prepare for ssslib legacy interface removal in other parts of in_toto:
  - in-toto-run, in-toto-record (in-toto#533)
  - verifylib / in-toto-verify
  - in-toto-sign
  - Metadata API / docs
  - ...

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh lukpueh mentioned this issue Nov 20, 2023
Merged
lukpueh added a commit to lukpueh/in-toto that referenced this issue Nov 21, 2023
@lukpueh
Add --pkcs8 signing arg to in-toto-record
fc7293e 
See in-toto#649, which does the same thing for in-toto.

Blocks on in-toto#649
Fixes in-toto#533 together with in-toto#649.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh lukpueh mentioned this issue Nov 21, 2023
Merged
lukpueh added a commit to lukpueh/in-toto that referenced this issue Nov 21, 2023
@lukpueh
Change details in in-toto#649, which adds the same feature to in-toto…
99312a1 
…-run.

Blocks on in-toto#649
Fixes in-toto#533 together with in-toto#649.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/in-toto that referenced this issue Nov 24, 2023
@lukpueh
Change details in in-toto#649, which adds the same feature to in-toto…
d56c424 
…-run.

Blocks on in-toto#649
Fixes in-toto#533 together with in-toto#649.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/in-toto that referenced this issue Dec 4, 2023
@lukpueh
Change details in in-toto#649, which adds the same feature to in-toto…
f64b69d 
…-run.

Blocks on in-toto#649
Fixes in-toto#533 together with in-toto#649.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/in-toto that referenced this issue Dec 4, 2023
@lukpueh
Change details in in-toto#649, which adds the same feature to in-toto…
5fe3be7 
…-run.

Blocks on in-toto#649
Fixes in-toto#533 together with in-toto#649.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/in-toto that referenced this issue Dec 5, 2023
@lukpueh
Change details in in-toto#649, which adds the same feature to in-toto…
9b7ae21 
…-run.

Blocks on in-toto#649
Fixes in-toto#533 together with in-toto#649.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add --signing-key (pkcs8) arg to in-toto-record lukpueh/in-toto
1 participant
@lukpueh