Export Base64 signatures in Envelope #565
Conversation
As per DSSE spec, Signatures in Envelope should be base64 encoded. While securesystemslib implementation uses hex signatures for signing and verifying them. Best way to achieve this by using base64 encoded signatures externally and not changing any internal representation of signature. This commit adds two helper method that encodes and converts signature into base64 dict and vice versa. Also, testcases are added and updated as well. Signed-off-by: Pradyumna Krishna <[email protected]>
PradyumnaKrishna
force-pushed
the
fix-dsse-signature
branch
from
3e68a9f to
801addd
Compare
There was a problem hiding this comment.
Thanks for the great patch, @PradyumnaKrishna!
What do you think about adding the conversion logic as internal helpers to Envelope? We don't need it elsewhere, so why not keep the API smaller.
Also, apart from the docstring, which says "hex", the Signature class itself has been agnostic to the encoding of the sig field so far. I suggest we keep it that way.
Just bike shedding here but I suppose you could have a DSSESignature that is just a Signature with overridden |
I considered that too, or even to just allow to also write base64 into a regular Signature. Technically, nothing stops you from doing that already. The problem is that the signature objects are returned from the signers, so you'd need dsse-flavored signers to produce dsse-flavored signature objects, right? |
Yes, simplest would be to encode signature into base64 on dict conversion. |
Moves signature base64 encoding from helper functions to Envelope, as it will make the API smaller and base64 encoded signature aren't rqeuired anywhere else. Now, DSSE Envelope create base64 encoded signature on dict conversion. Signed-off-by: Pradyumna Krishna <[email protected]>
securesystemslib/dsse.py
Outdated
| signatures = [] | ||
| for signature in self.signatures: | ||
| sig_dict = signature.to_dict() | ||
| sig_dict["sig"] = b64enc(sig_dict["sig"].encode("utf-8")) |
There was a problem hiding this comment.
I think this is incompatible with other DSSE envelope signing libraries. If I'm reading this right, we're writing the b64 encoding of the hex representation of the signatures. I think we should first decode the hex value before passing the decoded bytes to b64 encoding.
There was a problem hiding this comment.
Good point, Aditya! DSSE spec doesn't go into details about the format, but I would also expect other implementations to base64-encode the "raw" signature bytes.
@adityasaky found that we are encoding hex representation of signatures into base64, that would be incompatible with other DSSE envelope signing libraries. This commit changes the direct base64 encoding in Envelope to decoding hex signature and then encode the bytes into base64. Signed-off-by: Pradyumna Krishna <[email protected]>
securesystemslib/dsse.py
Outdated
| @@ -71,7 +74,7 @@ def to_dict(self) -> dict: | |||
| signatures = [] | |||
| for signature in self.signatures: | |||
| sig_dict = signature.to_dict() | |||
| sig_dict["sig"] = b64enc(sig_dict["sig"].encode("utf-8")) | |||
| sig_dict["sig"] = b64enc(binascii.unhexlify(sig_dict["sig"])) | |||
There was a problem hiding this comment.
| sig_dict["sig"] = b64enc(binascii.unhexlify(sig_dict["sig"])) | |
| sig_dict["sig"] = b64enc(bytes.fromhex(sig_dict["sig"])) |
securesystemslib/dsse.py
Outdated
| @@ -1,6 +1,7 @@ | |||
| """Dead Simple Signing Envelope | |||
| """ | |||
|
|
|||
| import binascii | |||
There was a problem hiding this comment.
| import binascii |
securesystemslib/dsse.py
Outdated
| signature["sig"] = binascii.hexlify( | ||
| b64dec(signature["sig"]) | ||
| ).decode("utf-8") |
There was a problem hiding this comment.
| signature["sig"] = binascii.hexlify( | |
| b64dec(signature["sig"]) | |
| ).decode("utf-8") | |
| signature["sig"] = b64dec(signature["sig"]).hex() |
Since Python3.5 binascii functions are built into bytes type. Hence, removing binascii and using bytes for hex encoding/decoding. Signed-off-by: Pradyumna Krishna <[email protected]>
|
Thanks @PradyumnaKrishna! @lukpueh, can we cut a release with this PR included? I'd like to release in-toto-py with |
Sure, let's release next Monday! |
Description of the changes being introduced by the pull request:
As per DSSE spec, Signatures in Envelope should be base64 encoded. While securesystemslib implementation uses hex signatures for signing and verifying them. Best way to achieve this by using base64 encoded signatures externally and not changing any internal representation of signature.
This PR adds two helper method that encodes and converts signature into base64 dict and vice versa. Testcases are added and updated as well.
Reference: in-toto/in-toto#563 (comment)
Please verify and check that the pull request fulfils the following requirements: