fixture removal + generate signing key #819
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
yuroitaki
commented
Apr 30, 2025
yuroitaki
commented
- generate signing key if not provided
- migrate fixture to test crate
- removes redundant code from tee module
- derive pub key from provided signing key
sinui0
requested changes
Apr 30, 2025
crates/notary/server/src/signing.rs
Outdated
Comment on lines
106
to
174
| impl SigningKey { | ||
| fn verifying_key(&self) -> VerifyingKey { | ||
| match self { | ||
| SigningKey::Secp256k1(key) => VerifyingKey::K256(*key.verifying_key()), | ||
| SigningKey::Secp256r1(key) => VerifyingKey::P256(*key.verifying_key()), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Corresponding public key of the attestation key. | ||
| pub struct PublicKey { | ||
| #[allow(dead_code)] | ||
| alg_id: KeyAlgId, | ||
| key: VerifyingKey, | ||
| } | ||
|
|
||
| impl PublicKey { | ||
| fn new(alg_id: KeyAlgId, key: VerifyingKey) -> Self { | ||
| Self { alg_id, key } | ||
| } | ||
|
|
||
| /// Converts the public key into PEM encoding in compressed form. | ||
| pub fn to_pem(&self) -> Result<String, pkcs8::Error> { | ||
| Ok(self.key.to_public_key_pem(LineEnding::LF)?) | ||
| } | ||
|
|
||
| #[cfg(feature = "tee_quote")] | ||
| /// Coverts the public key into bytes in compressed form. | ||
| pub fn to_compressed_bytes(&self) -> Vec<u8> { | ||
| self.key.to_compressed_bytes() | ||
| } | ||
| } | ||
|
|
||
| enum VerifyingKey { | ||
| K256(k256::ecdsa::VerifyingKey), | ||
| P256(p256::ecdsa::VerifyingKey), | ||
| } | ||
|
|
||
| impl VerifyingKey { | ||
| fn to_compressed_bytes(&self) -> Vec<u8> { | ||
| let encoded_point = match self { | ||
| VerifyingKey::K256(key) => key.to_encoded_point(true), | ||
| VerifyingKey::P256(key) => key.to_encoded_point(true), | ||
| }; | ||
| encoded_point.as_bytes().to_vec() | ||
| } | ||
| } | ||
| // The default `EncodePublicKey` impl for both `k256::ecdsa::VerifyingKey` and | ||
| // `p256::ecdsa::VerifyingKey` are serializing the public key in uncompressed | ||
| // format. This overrides that to obtain the compressed format. | ||
| // | ||
| // Reference: https://github.com/RustCrypto/traits/blob/f44963a897af10d125efe3af89b20930ebe4a999/elliptic-curve/src/public_key.rs#L476-L493 | ||
| impl EncodePublicKey for VerifyingKey { | ||
| fn to_public_key_der(&self) -> Result<der::Document, pkcs8::spki::Error> { | ||
| let algorithm = match self { | ||
| VerifyingKey::K256(key) => key.algorithm_identifier()?, | ||
| VerifyingKey::P256(key) => key.algorithm_identifier()?, | ||
| }; | ||
|
|
||
| let public_key_bytes = self.to_compressed_bytes(); | ||
| let subject_public_key = der::asn1::BitStringRef::new(0, &public_key_bytes)?; | ||
|
|
||
| pkcs8::SubjectPublicKeyInfo { | ||
| algorithm, | ||
| subject_public_key, | ||
| } | ||
| .try_into() | ||
| } | ||
| } |
There was a problem hiding this comment.
This feels like functionality that should be in tlsn-core.
Perhaps the Signer trait should also have a required method for providing the verifying key in PEM format.
There was a problem hiding this comment.
Agreed, didn't realise technically all of PublicKey (and VerifyingKey) functionalities are already present in Signer except for to_pem; so if that's added, then we don't need Publickey anymore since in notary server crate I can just do
signer.verifying_key()signer.to_pem()
right?
heeckhau
reviewed
May 2, 2025
|
@yuroitaki The TEE config needs an update too. Or should we drop it?
|
added 6 commits
May 2, 2025 19:18
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.