-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
Add SSlibKey Implementation #4
Conversation
A container class of public key required for verification of signature by DSSE. `SSlibKey` is a replication of the `Key` class of the python-tuf metadata API, providing more flexibility to it and intended to work with DSSE. Signed-off-by: Pradyumna Krishna <git@onpy.in>
Tests to check equalities, serialization and verify method of SSlibKey are added to ensure integritry of Key interfaces. Signed-off-by: Pradyumna Krishna <git@onpy.in>
db8106a
to
c74631a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! And we can pretty much merge as is, maybe after addressing my comments in the test module.
I am a bit worried about the looks of the {to, from}_securesystemslib_key
methods, which were crutches in python-tuf to deal with some securesystemslib key dict requirements in a black box manner, but should be handled now that we implement this in securesystemslib. We can ticketize it.
tests/test_key.py
Outdated
sslib_key = SSlibKey.from_securesystemslib_key(scheme_dict) | ||
|
||
with self.assertRaises( | ||
(CryptoError, UnsupportedAlgorithmError, FormatError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not know what Error is raised here? Or does it differ, depending on the key type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment applies below.
tests/test_key.py
Outdated
|
||
@classmethod | ||
def setUpClass(cls): | ||
cls.dicts = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: dicts
is not ver mnemonic, maybe call it key_pairs
?
tests/test_key.py
Outdated
def test_sslib_verify(self): | ||
"""Test to check verify method of key.""" | ||
|
||
for scheme_dict in self.dicts: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: scheme_dict
does not seem accurate. What about key_dict
, or key_pair
, or just key
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment applies below.
c74631a
to
22ef0c4
Compare
Thanks for addressing my changes! Let's merge and I'll write an issue about |
Part of secure-systems-lab#370
Description of the changes being introduced by the pull request:
This pull request is related to DSSE implementation and adds a
Key
andSSlibKey
interface to providepublicKey
class containers. These function will be used to addSign
andVerify
methods of DSSEEnvelope
.Tests are added to ensure the working of
SSlibKey
.Please verify and check that the pull request fulfils the following requirements: