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 5d2031c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 5 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(());
}

Err(Error::CryptoFail)
}

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
3 changes: 2 additions & 1 deletion quiche/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2367,7 +2367,8 @@ impl Connection {
match self.peer_transport_params.stateless_reset_token {
Some(token) => {
let token_len = 16;
ring::constant_time::verify_slices_are_equal(

crypto::verify_slices_are_equal(
&token.to_be_bytes(),
&buf[buf_len - token_len..buf_len],
)
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 5d2031c

Please sign in to comment.