feat: add HKDF mechanisms #227
Conversation
|
Thank you!! As a prerequisite to this PR maybe we should agree and go on with #156 so that we use |
That's a good idea. This PR unnecessarily couples the HKDF and the 3.0 change that maybe should be there in the first place. Does #156 also solve the "unofficial OASIS header" issue reported at #66? If so maybe we need some acks from other maintainers that the licensing looks good in #156 and then rebase this on top of master (when #156 is done). @hug-dev would you ask in |
That will be nice, thank you! |
Yes! Also pinged on the related PR to use another free source of headers. |
There was a problem hiding this comment.
In general this looks good, just a couple of nits.
Btw it seems softhsm does not implement HKDF which is sad :(
| use super::MechanismType; | ||
|
|
||
| #[derive(Debug, Clone, Copy)] | ||
| /// The salt for the extract stage. |
There was a problem hiding this comment.
Could you move the doc comment above the #[derive] for consistency?
cryptoki/src/mechanism/hkdf.rs
Outdated
| expand: bool, | ||
| prf_hash_mechanism: MechanismType, | ||
| salt: HkdfSalt, | ||
| info: &'a [u8], |
There was a problem hiding this comment.
Hmm... info is required even when expand is set to false? Is info always set when expand is true and empty otherwise?
If so the two could be collapsed into an expand_info: Option<&[u8]> param 🤔
(in general bool parameters are annoying since it's not visible from the call-site which is which)
cryptoki/src/mechanism/hkdf.rs
Outdated
| .expect("salt length does not fit in CK_ULONG"), | ||
| _ => 0, | ||
| }, | ||
| hSaltKey: match salt { |
There was a problem hiding this comment.
I'd convert all these two-arm matches to if lets... but this is purely a stylistic choice 😅
| @@ -0,0 +1,116 @@ | |||
| //! Mechanisms of hash-based key derive function (HKDF) | |||
There was a problem hiding this comment.
It'd be good to insert a copyright header here, just like so: https://github.com/parallaxsecond/rust-cryptoki/blob/main/cryptoki/src/types.rs#L1-L2
There was a problem hiding this comment.
@wiktor-k do we use to put 2021 everywhere or the year the file is added instead?
There was a problem hiding this comment.
Let's use 2024 here instead then, until we find a better solution :)
| @@ -0,0 +1,116 @@ | |||
| //! Mechanisms of hash-based key derive function (HKDF) | |||
There was a problem hiding this comment.
@wiktor-k do we use to put 2021 everywhere or the year the file is added instead?
|
@Direktor799 You might need to rebase! |
I think the date of the file addition was used in previous instances. The entire copyright years issues is a minefield: https://daniel.haxx.se/blog/2023/01/08/copyright-without-years/ In my other project we've migrated to reuse which automatically checks licensing info. Previously we used headers, just like here, but then we aggregated everything in one file and it's good. (I guess this is a question if we want to improve copyright headers in the long run and if so how). |
Signed-off-by: Direktor799 <[email protected]>
Signed-off-by: Direktor799 <[email protected]>
Signed-off-by: Direktor799 <[email protected]>
cryptoki/src/mechanism/hkdf.rs
Outdated
| @@ -0,0 +1,121 @@ | |||
| // Copyright 2021 Contributors to the Parsec project. | |||
There was a problem hiding this comment.
| // Copyright 2021 Contributors to the Parsec project. | |
| // Copyright 2024 Contributors to the Parsec project. |
Signed-off-by: Direktor799 <[email protected]>
Add HKDF mechanisms to
cryptokiandcryptoki-sys. The mechanisms were defined in PKCS#11 v3.0.I found that SoftHSM doesn't support such mechanisms so I couldn't write any unit test. But I did run some
CKM_HKDF_DERIVEtests against our own HSM and it works.