API for allowing signature validation given a SPKI/RPK

[holodorum]

This PR creates a new public API which allows for signature validation given a SPKI or a Raw Public Key. This aids the implementation of RFC 7250 (Raw Public Key support) in rustls.

Public API Changes

• We have added a RawPublicKeyEntity, which creates a SubjectPublicKeyInfo from a `SubjectPublicKeyInfoDer.
• RawPublicKeyEntity only has a verify_signature function, which verifies the signature with the spki.

API Changes

• In the SubjectPublicKeyInfo type we've added the method subject_public_key_info. This method returns the DER of SubjectPublicKeyInfo.

Open Questions

• To transform the SubjectPublicKeyInfo back into properly encoded ASN.1 we need to prepend the key_value and algorithm_value with the correct ASN.1 tag. For this we are using der::asn1_wrap, this function works as expected for the sequence algorithm_value, but not for the bitstring key_value. I assume the reason being that bit strings also need a byte for the number of unused bits. My current workaround is to manually add a 0 byte to the key_value and only then use asn1_wrap(). If this is not the expected behaviour for asn1_wrap() I can also open a PR for to update this.
• The signed_data::verify_signature function expects a SPKI, but without the ASN.1 tag wrapping the algorithm value AND the key value.
  That's why I added a boolean for_verification to subject_public_key_info(). If for_verification is true the output DER can be used in signed_data::verify_signature, else it's a properly ASN.1 encoded SPKI. Is this a good way to go about this difference?
  EDIT: Instead of using the subject_public_key_info() function to get a key that can be used for verification, it might be better to add a der field to the SubjectPublicKeyInfo struct. In that way we don't need the subject_public_key_info() function anymore and are we not dependent on the alloc` feature.

Contributors:

This PR is made in collaboration with @aochagavia.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.33%. Comparing base (c2ff93b) to head (c3d60bb).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #275      +/-   ##
==========================================
+ Coverage   97.30%   97.33%   +0.03%     
==========================================
  Files          19       20       +1     
  Lines        4234     4283      +49     
==========================================
+ Hits         4120     4169      +49     
  Misses        114      114              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cpu]

Hi there. Thanks for getting the ball rolling :-)

I think this would also benefit from the equivalent of a tests/signatures.rs integration test using the generate.py script to output tests/testdata for doing a signature verification using a raw public key entity across all the supported algorithms (e.g. ecdsa p256, p384, p521, ed25519, rsa 2048, 3072 and 4096). Right now there's just one ed25519 test being used from a unit test, and not an integration test.

[cpu]

The way the generate.py script works isn't great for incremental updates. I think your CI failure will fix if you rm the existing signature test files and then run your updated generator script again. (we should make this less painful 😵‍💫)

Nevermind, looks like a different root cause.

[holodorum]

The way the generate.py script works isn't great for incremental updates. I think your CI failure will fix if you rm the existing signature test files and then run your updated generator script again.

@ctz seems you were right, this did seem to be the root cause. After removing the existing signature test files and rerunning generate.py the tests do pass!

[djc]

The deny failure is addressed in #277.

[holodorum]

@djc that solved the deny failure indeed. I've also added rpk_entity.rs to the cargo include so all tests should pass after #277 is merged!
Shall I bump the version so we can use the RawPublicKeyEntity in rustls/rustls#2062?

[djc]

Shall I bump the version so we can use the RawPublicKeyEntity in rustls/rustls#2062?

Yes, in a separate commit please!

[ctz]

personal terminology bugbear of mine :)

[cpu]

ci / Cargo Deny (pull_request) Failing after 5s

Needs a rebase? 🤔

[cpu]

Thanks for applying our feedback!

[cpu]

nit: I think these comments would benefit from a space between the // and the text (surprised cargo fmt isn't doing that automagically tbh...)

[djc]

Thanks!