Updates #5
Updates #5
Conversation
| @@ -9,7 +9,7 @@ | |||
| //! [p256]: https://docs.rs/p256/ | |||
|
|
|||
| #![no_std] | |||
| #![cfg_attr(docsrs, feature(doc_cfg))] | |||
| #![cfg_attr(docsrs, feature(doc_cfg), feature(doc_auto_cfg))] | |||
There was a problem hiding this comment.
It does the condition documentation attribute automatically: rust-lang/rust#43781
| use p256_cortex_m4::{PublicKey, SecretKey, Signature}; | ||
|
|
||
| // message hash | ||
| const HASH: [u8; 32] = [ |
There was a problem hiding this comment.
Generally, I prefer a dev-dependency on hex-literal for these kind of constants (for compactness + readability).
But no worries.
| @@ -170,7 +170,7 @@ impl SecretKey { | |||
| /// Internally, draws 256-bit `k` repeatedly, until signing succeeds. | |||
| pub fn sign_prehashed( | |||
| &self, | |||
| prehashed_message: &[u8], | |||
| prehashed_message: [u8; 32], | |||
There was a problem hiding this comment.
Do you have strong opinions on [u8; 32] vs &[u8; 32] here?
There was a problem hiding this comment.
I changed that one to match Scalar in p256, the API changed and it now takes ownership instead of a reference: https://docs.rs/p256/latest/p256/struct.Scalar.html#method.from_be_bytes_reduced
I have no opinion, but if it is &[u8; 32] then a copy has to be made internally.
| der = { version = "0.4", features = ["bigint", "derive"], optional = true } | ||
| ecdsa = { version = "0.12", package = "ecdsa", default-features = false, optional = true } | ||
| elliptic-curve = { version = "0.10", default-features = false, optional = true } | ||
| der = { version = "0.6", features = ["derive"], optional = true } |
There was a problem hiding this comment.
Thoughts on keeping these dependencies up to going forward?
For -sys, I released a 0.1.0 based on your changes, which I expect to be relatively stable. But here, the core interest is in the Cortex M4 implementation, not so much keeping up with the "fallback".
On the other hand, if we add implementations for the signature crates (e.g. in particular DigestSigner with its more abstract sign_digest on top of our raw sign_prehashed - which I think we should keep for its ease of use in embedded contexts. And for instance RandomizedSigner has arguments in a different order), we might at least want to keep releasing breaking changes at least when signature or other trait crates have breaking changes.
There was a problem hiding this comment.
I would like to see this (eventually) implement RustCrypto traits so that it can be swapped out at compile time with generics by the consumer of this crate. Then it would be easy to add embedded-optimized implementations for other architectures in the future (at least, that's my hope).
Various updates, see the individual commits for more details.