Skip to content

Commit

Permalink
crypto: replace ring constant-time comparison API
Browse files Browse the repository at this point in the history
Both BoringCrypto and OpenSSL provide `CRYPTO_memcmp()` for
constant-time comparisons, so there's no point in pulling a whole
dependency just for that.
  • Loading branch information
ghedo committed Jan 14, 2025
1 parent 122780a commit cd3c94c
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 4 deletions.
18 changes: 18 additions & 0 deletions quiche/src/crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

use ring::aead;

use libc::c_int;
use libc::c_void;

use crate::Error;
Expand Down Expand Up @@ -481,10 +482,27 @@ fn make_nonce(iv: &[u8], counter: u64) -> [u8; aead::NONCE_LEN] {
nonce
}

pub fn verify_slices_are_equal(a: &[u8], b: &[u8]) -> Result<()> {
if a.len() != b.len() {
return Err(Error::CryptoFail);
}

let rc = unsafe { CRYPTO_memcmp(a.as_ptr(), b.as_ptr(), a.len()) };

if rc == 0 {
return Ok(());
}

return Err(Error::CryptoFail);

Check failure on line 496 in quiche/src/crypto/mod.rs

View workflow job for this annotation

GitHub Actions / quiche

unneeded `return` statement
}

extern {
fn EVP_sha256() -> *const EVP_MD;

fn EVP_sha384() -> *const EVP_MD;

// CRYPTO
fn CRYPTO_memcmp(a: *const u8, b: *const u8, len: usize) -> c_int;
}

#[cfg(test)]
Expand Down
5 changes: 1 addition & 4 deletions quiche/src/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,13 +778,10 @@ pub fn verify_retry_integrity(
) -> Result<()> {
let tag = compute_retry_integrity_tag(b, odcid, version)?;

ring::constant_time::verify_slices_are_equal(
crypto::verify_slices_are_equal(
&b.as_ref()[..aead::AES_128_GCM.tag_len()],
tag.as_ref(),
)
.map_err(|_| Error::CryptoFail)?;

Ok(())
}

fn compute_retry_integrity_tag(
Expand Down

0 comments on commit cd3c94c

Please sign in to comment.