User/khangnguyen/mscrypto traits - #106
Conversation
|
Hey Sam, large PR so some notes that might help: This is the order I think you should look:
I will leave other notes inline at the call sites. I also made a small sample program that shows how the provider would be chosen etc. RE: the discussion on |
| // windows-sys exposes the SHA-2 algorithm pseudo-handles but not the SHA-3 ones, so | ||
| // define them the same way it defines its own (a BCRYPT_ALG_HANDLE built from the value | ||
| // in the bcrypt pseudo-handle table). SHA-3 pseudo-handles exist on Windows 11 24H2 and later. | ||
| #[cfg(feature = "sha3")] |
There was a problem hiding this comment.
will switch these once they land in windows.rs
| } | ||
|
|
||
| impl HashOps for BcryptHasher { | ||
| fn update(&mut self, data: &[u8]) { |
There was a problem hiding this comment.
Is this over-engineered? I dont think anyone is going to ever hash a big file like this in one shot
| BcryptHasher::new(alg, len) | ||
| } | ||
|
|
||
| fn digest(&self, algorithm: BaseHashAlgorithm, data: &[u8]) -> Digest { |
There was a problem hiding this comment.
Could one shot here with the bcrypt API, but see the other note about large hashes
|
|
||
| /// Panics on a non-success `NTSTATUS`. Used on the infallible SHA-2 path (and once | ||
| /// SHA-3 is running), where a failure is catastrophic and cannot be reported. | ||
| fn expect_success(call: &str, status: NTSTATUS) { |
There was a problem hiding this comment.
Thoughts on this panic idea. I assume that any error (aside from unavailable for sha3) would be catastrophic enough to panic anyways. In my mind there are few ways a hash can error out
|
|
||
| /// Probes SHA-3 availability, then verifies the require-list. SHA-2 needs no | ||
| /// setup (pseudo-handles), so it never fails here. | ||
| pub fn build(self) -> Result<BcryptProvider, ProviderBuildError> { |
There was a problem hiding this comment.
This is where we would error if SHA3 is not available
| } | ||
| } | ||
|
|
||
| fn backend_info() -> BackendInfo { |
There was a problem hiding this comment.
will try to find a way to use bcrypt / windows version number as a pseudo version
There was a problem hiding this comment.
Now looking at it, might make sense for the version to just be a string, then we can do checks per provider rather than forcing bcrypt into the symcrypt semver semantics.
| // first use, which aborts on a mismatch, so there is no recoverable failure to | ||
| // report today. A graceful module-init entry point will surface an incompatible | ||
| // module here as `ProviderBuildError::Backend { backend, operation }`. | ||
| fn initialize_module() -> Result<(), ProviderBuildError> { |
There was a problem hiding this comment.
will replace with SymCryptModuleInitEX so we can probe during build and graceful error on old versions of SymCrypt.
There was a problem hiding this comment.
We will only support new versions of symcrypt.dll for this crate
| @@ -0,0 +1,19 @@ | |||
| // Records how symcrypt-sys is linked so the provider can report it through | |||
| /// visibility. Ordinary callers receive a `Digest` from `Hash::digest` / | ||
| /// `HashOps::finalize` and read it via `as_bytes` / `as_ref`. | ||
| #[doc(hidden)] | ||
| pub mod provider { |
There was a problem hiding this comment.
This is separate since every API needs to be public for the trait to work. However, this is an implementation detail for the provider and should not be exposed to the caller. We can't physically stop them from using this. but hiding it like this from the docs is the best work around imo.
1 participant
Description of Changes:
Breaking Changes if any:
✅ Admin Checklist
cargo test --all-featuresonWindowsandWSL.Check the Developer Guidelines in DEVELOPER.md for more info.