Add the Signer interface #319

MVrachev · 2021-02-03T16:45:08Z

Fixes: theupdateframework/python-tuf#1263

Description of the changes being introduced by the pull request:

Create a Signer interface with the purpose of supporting multiple signing implementations.
Read more on the issue above.

Please verify and check that the pull request fulfils the following requirements:

The code follows the Code Style Guidelines
Tests have been added for the bug fix or new feature
Docs have been added for the bug fix or new feature

Signed-off-by: Martin Vrachev [email protected]

coveralls · 2021-02-03T16:47:27Z

Coverage increased (+0.01%) to 98.544% when pulling 4fe7f9b on MVrachev:signer-interface into dff4425 on secure-systems-lab:master.

trishankatdatadog · 2021-02-04T16:52:05Z

tests/test_signer.py

+  def test_sslib_sign(self):
+    # Test generation of RSA signatures.
+    sslib_signer = SSlibSigner(self.rsakey_dict)
+    rsa_sig_obj = sslib_signer.sign(self.DATA)


I like how simple it looks to use, judging by the code so far! 👏🏽

securesystemslib/signer.py

MVrachev · 2021-02-09T16:27:46Z

This interface is ready for a review @lukpueh!
I will add another pr for the GPGSigner.

lukpueh

Nice work, @MVrachev. I only pointed out some minor issues, most of which are not really your fault but seem to stem from inaccurate code documentation in this repo.

securesystemslib/signer.py

tests/test_signer.py

securesystemslib/signer.py

MVrachev · 2021-02-10T21:42:36Z

Updated the pr.

I added a new commit fixing the documentation for secureystemslib.keys.create_signature().
Added a Signature.from_dict() and a unit test for it.
Fixed the wording in the documentation
Rewrote the test and made it simpler.

@lukpueh sorry for the obvious mistakes in the tests. I copied a lot from tests/test_keys.py and it seems I did it badly. :D

lukpueh

Thanks, @MVrachev! This is getting better and better. Here are a few more suggestions.

securesystemslib/keys.py

securesystemslib/signer.py

tests/test_signer.py

MVrachev · 2021-02-11T19:50:40Z

I rebased and squashed the commits where I address @lukpueh comments into the appropriate commits.
I added the ed25519key_dict into the tests and fixed another group of docstring mistakes.

lukpueh

I think we should use the same strategy to move away from legacy code style as we do in TUF (see ADR 5). So given that you are adding completely new modules I suggest you also follow the new style guideline, especially in regards to code documentation. What do you think?

securesystemslib/keys.py

MVrachev · 2021-02-12T17:47:56Z

I think we should use the same strategy to move away from legacy code style as we do in TUF (see ADR 5). So given that you are adding completely new modules I suggest you also follow the new style guideline, especially in regards to code documentation. What do you think?

I thought I should follow the style guidelines in the other parts in securesytemslib.
My reasoning was that in tuf where we write the new API from scratch we should use the new style guidelines, but for files that are in the old folders, we should use the older style guidelines.

It seems I am wrong. I will fix it on Monday.

lukpueh · 2021-02-15T12:10:56Z

Your reasoning makes perfect sense, @MVrachev, especially if you read ADR 5 together with ADR 3. However, ADR 5 alone even prescribes the new style for "new files" and not only for "new files in a certain new directory".

But regardless of ADR 5, I think it makes a lot of sense to use the new docstring format everywhere especially for classes / functions that should be public API (I even adopted it in an existing module recently, see #288).

I am less passionate about most of the other style recommendations, but to make things easier I would just follow the "new style for new files" rule of thumb.

MVrachev · 2021-02-16T14:38:10Z

I updated the pr with the new style guidelines.

MVrachev · 2021-02-17T11:49:56Z

I had to force push to resolve conflicts.
I updated the Update doc for ecdsa-sha2-nistp384 schema commit to using a keys_types_set as Trishank suggested.

lukpueh

Hey Martin, this looks great, I only left a few comments regarding docstrings (format), and I'd like to kindly ask you to remove the changes you made in securesystemslib/keys.py from this PR. :)

securesystemslib/signer.py

securesystemslib/keys.py

Signed-off-by: Martin Vrachev <[email protected]>

MVrachev · 2021-02-23T12:50:48Z

Updated the pr:

replaced ' with " in both signer.py and test_signer.py.
fixed some comments
addressed all other Lukas comments.

I will need time to get used to the new code style guidelines.
Sorry for creating so much work in reviewing for you @lukpueh.

lukpueh · 2021-02-23T14:31:47Z

I will need time to get used to the new code style guidelines.
Sorry for creating so much work in reviewing for you @lukpueh.

No worries, I need to look things up in the new guidelines too. ;)

In the securesystemslib pr secure-systems-lab/securesystemslib#319 I added a new Signer interface with the purpose of supporting multiple signing implementations. Additionally, I added the SSlibSigner implementation of that interface which implements the signing operation for rsa, ed25519 and ecdsa schemes. With this commit, I integrate the SSlibSigner into the new API in tuf. Signed-off-by: Martin Vrachev <[email protected]>

The GPGSigner is an implementation of the new "Signer" interface that was merged in secure-systems-lab#319 and could be found in securesystemslib.signer.py This is the next logical step given that securesystemslib supports GPG and we want this interface to have implementations for all signature types which are already supported by securesystemslib. While implementing the GPGSigner, I wanted to make sure that one can easly sign a portion of data, receive a Signature object and use the information stored in that object to verify the signature. To verifty a GPG signature, one have to use securesystemslib/gpg/functions.verifty_signature(). There, the signature_object argument should be in the securesystemslib.formats.GPG_SIGNATURE_SCHEMA format. I searched for a way to easly retrieve the additional fields in the GPG_SIGNATURE_SCHEMA -"other_headers" and "info" from the keyid stored in the "Signature" object returned by the "sign" operation. Unfortunatly, right now there is no function that I can use for that purpose. The only option I was left with, was to create a new class: "GPGSignature" where we can store those additional fields returned from securesystemslib.gpg.functions.create_signature() which we call during the "sign" process. Signed-off-by: Martin Vrachev <[email protected]>

The GPGSigner is an implementation of the new "Signer" interface that was merged in secure-systems-lab#319 and could be found in securesystemslib.signer.py This is the next logical step given that securesystemslib supports GPG and we want this interface to have implementations for all signature types which are already supported by securesystemslib. While implementing the GPGSigner, I wanted to make sure that one can easily sign a portion of data, receive a Signature object and use the information stored in that object to verify the signature. To verifty a GPG signature, one have to use securesystemslib/gpg/functions.verifty_signature(). There, the signature_object argument should be in the securesystemslib.formats.GPG_SIGNATURE_SCHEMA format. I searched for a way to easily retrieve the additional fields in the GPG_SIGNATURE_SCHEMA -"other_headers" and "info" from the keyid stored in the "Signature" object returned by the "sign" operation. Unfortunately, right now there is no function that I can use for that purpose. The only option I was left with, was to create a new class: "GPGSignature" where we can store those additional fields returned from securesystemslib.gpg.functions.create_signature() which we call during the "sign" process. Signed-off-by: Martin Vrachev <[email protected]>

In the securesystemslib pr secure-systems-lab/securesystemslib#319 I added a new Signer interface with the purpose of supporting multiple signing implementations. Additionally, I added the SSlibSigner implementation of that interface which implements the signing operation for rsa, ed25519 and ecdsa schemes. With this commit, I integrate the SSlibSigner into the new API in tuf. Signed-off-by: Martin Vrachev <[email protected]>

The GPGSigner is an implementation of the new "Signer" interface that was merged in secure-systems-lab#319 and could be found in securesystemslib.signer.py This is the next logical step given that securesystemslib supports GPG and we want this interface to have implementations for all signature types which are already supported by securesystemslib. While implementing the GPGSigner, I wanted to make sure that one can easily sign a portion of data, receive a Signature object and use the information stored in that object to verify the signature. To verifty a GPG signature, one have to use securesystemslib/gpg/functions.verifty_signature(). There, the signature_object argument should be in the securesystemslib.formats.GPG_SIGNATURE_SCHEMA format. I searched for a way to easily retrieve the additional fields in the GPG_SIGNATURE_SCHEMA -"other_headers" and "info" from the keyid stored in the "Signature" object returned by the "sign" operation. Unfortunately, right now there is no function that I can use for that purpose. The only option I was left with, was to create a new class: "GPGSignature" where we can store those additional fields returned from securesystemslib.gpg.functions.create_signature() which we call during the "sign" process. Signed-off-by: Martin Vrachev <[email protected]>

MVrachev mentioned this pull request Feb 3, 2021

Make new api compatible with the Signing interface theupdateframework/python-tuf#1272

Merged

MVrachev changed the title ~~Signer interface: initial version~~ Signer interface Feb 3, 2021

MVrachev changed the title ~~Signer interface~~ Add the Signer interface Feb 3, 2021

trishankatdatadog reviewed Feb 4, 2021

View reviewed changes

securesystemslib/signer.py Outdated Show resolved Hide resolved

MVrachev force-pushed the signer-interface branch 3 times, most recently from 384a053 to 0285e39 Compare February 9, 2021 16:24

MVrachev marked this pull request as ready for review February 9, 2021 16:24

lukpueh requested changes Feb 10, 2021

View reviewed changes

lukpueh reviewed Feb 10, 2021

View reviewed changes

securesystemslib/signer.py Outdated Show resolved Hide resolved

MVrachev force-pushed the signer-interface branch from d18d92e to 4fe7f9b Compare February 10, 2021 21:38

lukpueh requested changes Feb 11, 2021

View reviewed changes

MVrachev force-pushed the signer-interface branch from 4fe7f9b to 3fd74d1 Compare February 11, 2021 19:46

lukpueh requested changes Feb 12, 2021

View reviewed changes

securesystemslib/keys.py Outdated Show resolved Hide resolved

MVrachev force-pushed the signer-interface branch 2 times, most recently from 645ee74 to 2c403f8 Compare February 16, 2021 14:37

MVrachev force-pushed the signer-interface branch 3 times, most recently from da9148b to af0451d Compare February 17, 2021 11:48

lukpueh requested changes Feb 18, 2021

View reviewed changes

MVrachev force-pushed the signer-interface branch from af0451d to 0978f5e Compare February 19, 2021 17:59

MVrachev added 2 commits February 23, 2021 14:48

Add tests for SSlib Signer

1e2185c

Signed-off-by: Martin Vrachev <[email protected]>

Skip test_signer on Python2.7

2bc36c9

Signed-off-by: Martin Vrachev <[email protected]>

MVrachev force-pushed the signer-interface branch from 62c9817 to 2bc36c9 Compare February 23, 2021 12:48

lukpueh merged commit 760523c into secure-systems-lab:master Feb 23, 2021

woodruffw mentioned this pull request Feb 24, 2021

Roadmap update for TUF support pypi/warehouse#5247

Closed

MVrachev mentioned this pull request Feb 25, 2021

Add GPGSigner implementation #341

Merged

3 tasks

lukpueh mentioned this pull request Feb 26, 2021

Prepare 0.20.0 release #342

Merged

dependabot bot mentioned this pull request Mar 9, 2021

Bump securesystemslib[crypto,pynacl] from 0.19.0 to 0.20.0 CrossStream/tuf#1

Closed

joshuagl mentioned this pull request Apr 22, 2021

Enable signing and verifying using keys stored in Hashicorp Vault #307

Closed

di mentioned this pull request Feb 1, 2022

Roadmap for PEP 458 pypi/warehouse#10672

Open

52 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the Signer interface #319

Add the Signer interface #319

MVrachev commented Feb 3, 2021 •

edited

Loading

coveralls commented Feb 3, 2021 •

edited

Loading

trishankatdatadog Feb 4, 2021

MVrachev commented Feb 9, 2021 •

edited

Loading

lukpueh left a comment

MVrachev commented Feb 10, 2021 •

edited

Loading

lukpueh left a comment

MVrachev commented Feb 11, 2021

lukpueh left a comment

MVrachev commented Feb 12, 2021

lukpueh commented Feb 15, 2021

MVrachev commented Feb 16, 2021

MVrachev commented Feb 17, 2021

lukpueh left a comment

MVrachev commented Feb 23, 2021 •

edited

Loading

lukpueh commented Feb 23, 2021

Add the Signer interface #319

Add the Signer interface #319

Conversation

MVrachev commented Feb 3, 2021 • edited Loading

Description of the changes being introduced by the pull request:

Please verify and check that the pull request fulfils the following requirements:

coveralls commented Feb 3, 2021 • edited Loading

trishankatdatadog Feb 4, 2021

Choose a reason for hiding this comment

MVrachev commented Feb 9, 2021 • edited Loading

lukpueh left a comment

Choose a reason for hiding this comment

MVrachev commented Feb 10, 2021 • edited Loading

lukpueh left a comment

Choose a reason for hiding this comment

MVrachev commented Feb 11, 2021

lukpueh left a comment

Choose a reason for hiding this comment

MVrachev commented Feb 12, 2021

lukpueh commented Feb 15, 2021

MVrachev commented Feb 16, 2021

MVrachev commented Feb 17, 2021

lukpueh left a comment

Choose a reason for hiding this comment

MVrachev commented Feb 23, 2021 • edited Loading

lukpueh commented Feb 23, 2021

MVrachev commented Feb 3, 2021 •

edited

Loading

coveralls commented Feb 3, 2021 •

edited

Loading

MVrachev commented Feb 9, 2021 •

edited

Loading

MVrachev commented Feb 10, 2021 •

edited

Loading

MVrachev commented Feb 23, 2021 •

edited

Loading