Add Signer trait - #340
Conversation
| /// A signature of a [Biscuit](crate::Biscuit) block. | ||
| /// | ||
| /// May be constructed via [Into] from [ed25519_dalek::Signature] or | ||
| /// from [ecdsa::Signature] with the [NistP256](::p256::NistP256) curve. | ||
| pub struct Signature(pub(crate) Vec<u8>); |
There was a problem hiding this comment.
Previously this was implicitly public as a return type; now it's explicit (hence the added comment) and addressable.
There may be some merit with regards to changing this into an enum type, although this may technically be a breaking change.
| pub fn from_bytes(data: &[u8]) -> Result<Self, error::Format> { | ||
| Ok(Signature(data.to_owned())) | ||
| } | ||
|
|
There was a problem hiding this comment.
This was never usable before as Signature wasn't exposed publicly. I've opted to remove it since it's fairly unsafe. Happy to add it back.
| pub trait Signer { | ||
| /// The algorithm used. Must match the signature returned via [sign](Self::sign). | ||
| fn algorithm(&self) -> Algorithm; | ||
| /// Sign a series of bytes, returning a signature. This signature must match | ||
| /// what [self.algorithm()](Self::algorithm) returns. Any incorrect values | ||
| /// will likely result in invalid tokens. | ||
| fn sign(&self, data: &[u8]) -> Result<Signature, error::Format>; | ||
| } |
There was a problem hiding this comment.
I don't super like this API, but it should do fine. The other option would be to have sign (or the Signer trait itself) take an algorithm type as an argument, where that type must impl Algorithm or similar.
Also, this may be better named as try_sign to follow the convention used in the ed25519_dalek and ecdsa crates.
Could also add an associated Error type, although then we'd have to pass it back.
There was a problem hiding this comment.
Notably we want this separate from a potential later PrivateKey trait, as root keys don't actually need to provide public keys. At most they may optionally have an ID.
| match (block_keypair.algorithm(), next_keypair.algorithm()) { | ||
| (Algorithm::Ed25519, SchemaAlgorithm::Ed25519) => {} |
There was a problem hiding this comment.
Kinda gross here (the SchemaAlgorithm) but I think it's a little neater than relying on the underlying enum structure.
1 participant
Another attempt at allowing for private keys which are not in-memory (e.g. for HSMs, TPMs, KMS). This is a minimal implementation to add an interface for arbitrary signers. Currently only supports root keys.
In the future, an async version of this trait could be added (which would be great for KMS and similar). For now this is considered out of scope, as async versions of the rest of the code would need to be added.
Closes
The closest other PR to this is #334, but it's adding a lot more at once and seems to be incomplete. This PR aims to be as simple as possible and to be non-breaking.
#320 may be closed by this too, depending on whether first party AWS support is desired or not.