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 Signer argument in metadata generation functions #532

Closed
lukpueh opened this issue Jan 17, 2023 · 6 comments · Fixed by #612
Closed

Support Signer argument in metadata generation functions #532

lukpueh opened this issue Jan 17, 2023 · 6 comments · Fixed by #612

Comments

@lukpueh
Copy link
Member

lukpueh commented Jan 17, 2023

Description of issue or feature request:
securesystemslib v0.26.0 released an improved signer interface. Supporting Signer objects as arguments in in-toto metadata generation function allows using arbitrary signing providers (e.g. Google Cloud KMS and Yubikey signers are already available in securesystemslib). Relevant functions are: in_toto_run, in_toto_record_start and in_toto_record_stop.

IMPORTANT: Coordinate with #503, which refactors relevant functions to use the new Signer API internally!

Current behavior:
Relevant function don't support Signer argument.

Expected behavior:
Relevant functions support Signer argument. (in_toto_record_stop should also support a Key argument, because unlike existing signing arguments, Signer objects don't necessarily give access to the public key needed for verification.)

Optional expected behavior:
Deprecate existing signing arguments in relevant functions (e.g. signing_key gpg_keyid, gpg_use_default, gpg_home).
This was referenced Jan 17, 2023
Closed
Closed
Merged
@lukpueh lukpueh mentioned this issue Feb 22, 2023
Merged
3 tasks
This was referenced Mar 22, 2023
Closed
Open
@chasen-bettinger
Copy link
Contributor

@lukpueh This ticket ok to pick up?

@adityasaky
Copy link
Member

I think @ckilcoin may also be working on this.

@lukpueh
Copy link
Member Author

lukpueh commented Apr 26, 2023

I think @ckilcoin may also be working on this.

@ckilcoin, can you confirm?

@ckilcoin
Copy link

Yep, currently working on it.

lukpueh added a commit to lukpueh/securesystemslib that referenced this issue May 23, 2023
@lukpueh
Drop legacy signature verification code (WIP)
3059d83 
IMPORTANT NOTE:

- This is a massive breaking change, which I suggest to hold off for
  1.0.0

- The commit is mostly to visualize how much more lightweight the new
  implementation is.
- It might be worth to skim the removed tests, if we are removing any
  interesting cases that we should port to test_signer.
- python-tuf may not be affected, because it already uses the signer API
- in-toto definitely is (see in-toto/in-toto#532)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/securesystemslib that referenced this issue May 24, 2023
@lukpueh
Drop legacy signature verification code (DO NOT MERGE!)
11e5c7c 
IMPORTANT NOTE:

- This is a massive breaking change, which I suggest to hold off for
  1.0.0, after the keys refactor is finished.
- The commit is mostly to visualize how much more lightweight the new
  implementation is.
- It might be worth to skim the removed tests, if we are removing any
  interesting cases that we should port to test_signer.
- python-tuf should not be affected, because it already uses the signer API
- in-toto definitely is (see in-toto/in-toto#532)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/securesystemslib that referenced this issue May 24, 2023
@lukpueh
Drop legacy signature verification code (DO NOT MERGE!)
caf31db 
IMPORTANT NOTE:

- This is a massive breaking change, which I suggest to hold off for
  1.0.0, after the keys refactor is finished.
- The commit is mostly to visualize how much more lightweight the new
  implementation is.
- It might be worth to skim the removed tests, if we are removing any
  interesting cases that we should port to test_signer.
- python-tuf should not be affected, because it already uses the signer API
- in-toto definitely is (see in-toto/in-toto#532)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/securesystemslib that referenced this issue May 24, 2023
@lukpueh
Drop legacy signature verification code (DO NOT MERGE!)
92c3809 
IMPORTANT NOTE:

- This is a massive breaking change, which I suggest to hold off for
  1.0.0, after the keys refactor is finished.
- The commit is mostly to visualize how much more lightweight the new
  implementation is.
- It might be worth to skim the removed tests, if we are removing any
  interesting cases that we should port to test_signer.
- python-tuf should not be affected, because it already uses the signer API
- in-toto definitely is (see in-toto/in-toto#532)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/securesystemslib that referenced this issue May 24, 2023
@lukpueh
Drop legacy signature verification code (DO NOT MERGE!)
cb09097 
IMPORTANT NOTE:

- This is a massive breaking change, which I suggest to hold off for
  1.0.0, after the keys refactor is finished.
- The commit is mostly to visualize how much more lightweight the new
  implementation is.
- It might be worth to skim the removed tests, if we are removing any
  interesting cases that we should port to test_signer.
- python-tuf should not be affected, because it already uses the signer API
- in-toto definitely is (see in-toto/in-toto#532)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/securesystemslib that referenced this issue May 30, 2023
@lukpueh
Drop legacy signature verification code (DO NOT MERGE!)
6472cc0 
IMPORTANT NOTE:

- This is a massive breaking change, which I suggest to hold off for
  1.0.0, after the keys refactor is finished.
- The commit is mostly to visualize how much more lightweight the new
  implementation is.
- It might be worth to skim the removed tests, if we are removing any
  interesting cases that we should port to test_signer.
- python-tuf should not be affected, because it already uses the signer API
- in-toto definitely is (see in-toto/in-toto#532)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/securesystemslib that referenced this issue May 30, 2023
@lukpueh
Remove legacy signature verification code
692b6ac 
IMPORTANT NOTE:

- This is a massive breaking change, which I suggest to hold off for
  1.0.0, after the keys refactor is finished.
- The commit is mostly to visualize how much more lightweight the new
  implementation is.
- It might be worth to skim the removed tests, if we are removing any
  interesting cases that we should port to test_signer.
- python-tuf should not be affected, because it already uses the signer API
- in-toto definitely is (see in-toto/in-toto#532)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh
Copy link
Member Author

lukpueh commented Jun 6, 2023

@ckilcoin, are you still working on this? If not, @chasen-bettinger, are you still interested in taking a stab at it? I'm happy to assist either way. 💪

@chasen-bettinger
Copy link
Contributor

@lukpueh I am!

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 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 added a commit to lukpueh/in-toto that referenced this issue Nov 20, 2023
@lukpueh
runlib: add signer kwarg to run and record methods
fb96ed0 
Adds optional signer (`securesystemslib.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.

A deprecation warning is added for `signing_key` only, because `signer`
can be used as backwards compatible replacement, and the patch is part
of a series of patches to prepare for the planned removal of
securesystemslib legacy modules `interface` and `keys`, needed by
`signing_key`.

gpg related arguments are not yet deprecated, as the related
implementation in the securesystemslib Signer API is not compatible
(it uses a different public key metadata format).

Note: 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.

Closes in-toto#532

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/in-toto that referenced this issue Nov 20, 2023
@lukpueh
runlib: add signer kwarg to run and record methods
ce3e964 
Adds optional signer (`securesystemslib.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.

A deprecation warning is added for `signing_key` only, because `signer`
can be used as backwards compatible replacement, and the patch is part
of a series of patches to prepare for the planned removal of
securesystemslib legacy modules `interface` and `keys`, needed by
`signing_key`.

gpg related arguments are not yet deprecated, as the related
implementation in the securesystemslib Signer API is not compatible
(it uses a different public key metadata format).

Note: 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.

Closes in-toto#532

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.

runlib: add signer kwarg to run and record methods lukpueh/in-toto
4 participants
@lukpueh @adityasaky @chasen-bettinger @ckilcoin