Document assumptions made by bssl-crypto's unboxed HMAC_CTX

I believe it is currently fine, but we probably should either box it, or
get to the point that the assumptions are less precarious. Rust FFI is
anything but safe.

Bug: 682
Change-Id: I4b45dd3c3f58fb0ce7c0b8b80b1e6d7d2f7f119f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65627
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

2 files changed, 12 insertions, 1 deletions

diff --git a/rust/bssl-crypto/README.md b/rust/bssl-crypto/README.md
index 9518862..678f45e 100644
--- a/rust/bssl-crypto/README.md
+++ b/rust/bssl-crypto/README.md
@@ -10,5 +10,5 @@ cd rust/bssl-crypto && cargo clippy && cargo deny check && cargo test
 
 Unlike BoringSSL itself, this crate does not attempt to handle allocation failures. If an allocation fails, functions in this crate will panic.
 
-WARNING - This crate is experimental and does *NOT* have a stable API. We expect to iterate on the API as it develops. If you use this crate you must be prepared to adapt your code to future changes as they occur.
+WARNING - This crate is experimental and does *NOT* have a stable API. We expect to iterate on the API as it develops. If you use this crate you must be prepared to adapt your code to future changes as they occur. Additionally, this crate must be updated atomically with BoringSSL. The crate, internally, may depend on implementation details of the library.
 
diff --git a/rust/bssl-crypto/src/hmac.rs b/rust/bssl-crypto/src/hmac.rs
index bf482f7..5924fa8 100644
--- a/rust/bssl-crypto/src/hmac.rs
+++ b/rust/bssl-crypto/src/hmac.rs
@@ -234,6 +234,10 @@ fn hmac<const N: usize, MD: digest::Algorithm>(key: &[u8], data: &[u8]) -> [u8;
 /// until the Rust language can support the `min_const_generics` feature. Until then we will have to
 /// pass both separately: https://github.com/rust-lang/rust/issues/60551
 struct Hmac<const N: usize, MD: digest::Algorithm> {
+    // Safety: this relies on HMAC_CTX being relocatable via `memcpy`, which is
+    // not generally true of BoringSSL types. This is fine to rely on only
+    // because we do not allow any version skew between bssl-crypto and
+    // BoringSSL. It is *not* safe to copy this code in any other project.
     ctx: bssl_sys::HMAC_CTX,
     _marker: PhantomData<MD>,
 }
@@ -366,6 +370,7 @@ impl<const N: usize, MD: digest::Algorithm> Drop for Hmac<N, MD> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use alloc::boxed::Box;
 
     #[test]
     fn hmac_sha256() {
@@ -401,10 +406,13 @@ mod tests {
         let mut hmac = HmacSha256::new_from_slice(&key);
         hmac.update(&data[..1]);
         let mut hmac2 = hmac.clone();
+        let mut hmac3 = Box::new(hmac2.clone());
         hmac.update(&data[1..]);
         hmac2.update(&data[1..]);
+        hmac3.update(&data[1..]);
         assert_eq!(hmac.digest(), expected);
         assert_eq!(hmac2.digest(), expected);
+        assert_eq!(hmac3.digest(), expected);
     }
 
     #[test]
@@ -458,9 +466,12 @@ mod tests {
         let mut hmac = HmacSha512::new_from_slice(&key);
         hmac.update(&data[..1]);
         let mut hmac2 = hmac.clone();
+        let mut hmac3 = Box::new(hmac.clone());
         hmac.update(&data[1..]);
         hmac2.update(&data[1..]);
+        hmac3.update(&data[1..]);
         assert_eq!(hmac.digest(), expected);
         assert_eq!(hmac2.digest(), expected);
+        assert_eq!(hmac3.digest(), expected);
     }
 }