From 81ed2b3f6a135449772c46980067b8d4f71f5c82 Mon Sep 17 00:00:00 2001 From: Maurice Lam Date: Fri, 8 Sep 2023 16:34:43 -0700 Subject: [PATCH 01/11] Implement bssl-crypto wrappers for AES-CBC - Create an internal `BlockCipher` trait similar to the existing `StreamCipher` trait for AES-CBC. - Create wrappers in the internal `Cipher` struct for one-shot allocating encryption and decryption operations. Change-Id: I17f667b3b92f907bc14c3454ee49b88cb91c49f3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63125 Commit-Queue: Bob Beck Reviewed-by: Bob Beck --- rust/bssl-crypto/src/cipher/aes_cbc.rs | 194 +++++++++++++++++ rust/bssl-crypto/src/cipher/aes_ctr.rs | 8 +- rust/bssl-crypto/src/cipher/mod.rs | 278 +++++++++++++++++++++++-- 3 files changed, 462 insertions(+), 18 deletions(-) create mode 100644 rust/bssl-crypto/src/cipher/aes_cbc.rs diff --git a/rust/bssl-crypto/src/cipher/aes_cbc.rs b/rust/bssl-crypto/src/cipher/aes_cbc.rs new file mode 100644 index 0000000000..6d22a18020 --- /dev/null +++ b/rust/bssl-crypto/src/cipher/aes_cbc.rs @@ -0,0 +1,194 @@ +/* Copyright (c) 2023, Google Inc. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +extern crate alloc; + +use crate::cipher::{ + BlockCipher, Cipher, CipherError, CipherInitPurpose, EvpAes128Cbc, EvpAes256Cbc, +}; +use alloc::vec::Vec; + +/// AES-CBC-128 Cipher implementation. +pub struct Aes128Cbc(Cipher); + +impl BlockCipher for Aes128Cbc { + type Key = [u8; 16]; + type Nonce = [u8; 16]; + + fn new_encrypt(key: &Self::Key, nonce: &Self::Nonce) -> Self { + Self(Cipher::new(key, nonce, CipherInitPurpose::Encrypt)) + } + + fn new_decrypt(key: &Self::Key, nonce: &Self::Nonce) -> Self { + Self(Cipher::new(key, nonce, CipherInitPurpose::Decrypt)) + } + + fn encrypt_padded(self, buffer: &[u8]) -> Result, CipherError> { + // Note: Padding is enabled because we did not disable it with `EVP_CIPHER_CTX_set_padding` + self.0.encrypt(buffer) + } + + fn decrypt_padded(self, buffer: &[u8]) -> Result, CipherError> { + // Note: Padding is enabled because we did not disable it with `EVP_CIPHER_CTX_set_padding` + self.0.decrypt(buffer) + } +} + +/// AES-CBC-256 Cipher implementation. +pub struct Aes256Cbc(Cipher); + +impl BlockCipher for Aes256Cbc { + type Key = [u8; 32]; + type Nonce = [u8; 16]; + + fn new_encrypt(key: &Self::Key, nonce: &Self::Nonce) -> Self { + Self(Cipher::new(key, nonce, CipherInitPurpose::Encrypt)) + } + + fn new_decrypt(key: &Self::Key, nonce: &Self::Nonce) -> Self { + Self(Cipher::new(key, nonce, CipherInitPurpose::Decrypt)) + } + + fn encrypt_padded(self, buffer: &[u8]) -> Result, CipherError> { + // Note: Padding is enabled because we did not disable it with `EVP_CIPHER_CTX_set_padding` + self.0.encrypt(buffer) + } + + fn decrypt_padded(self, buffer: &[u8]) -> Result, CipherError> { + // Note: Padding is enabled because we did not disable it with `EVP_CIPHER_CTX_set_padding` + self.0.decrypt(buffer) + } +} + +#[allow(clippy::expect_used)] +#[cfg(test)] +mod test { + use super::*; + use crate::test_helpers::decode_hex; + + #[test] + fn aes_128_cbc_test_encrypt() { + // https://github.com/google/wycheproof/blob/master/testvectors/aes_cbc_pkcs5_test.json#L30 + // tcId: 2 + let iv = decode_hex("c9ee3cd746bf208c65ca9e72a266d54f"); + let key = decode_hex("e09eaa5a3f5e56d279d5e7a03373f6ea"); + + let cipher = Aes128Cbc::new_encrypt(&key, &iv); + let msg: [u8; 16] = decode_hex("ef4eab37181f98423e53e947e7050fd0"); + + let output = cipher.encrypt_padded(&msg).expect("Failed to encrypt"); + + let expected_ciphertext: [u8; 32] = + decode_hex("d1fa697f3e2e04d64f1a0da203813ca5bc226a0b1d42287b2a5b994a66eaf14a"); + assert_eq!(expected_ciphertext, &output[..]); + } + + #[test] + fn aes_128_cbc_test_encrypt_more_than_one_block() { + // https://github.com/google/wycheproof/blob/master/testvectors/aes_cbc_pkcs5_test.json#L210 + // tcId: 20 + let iv = decode_hex("54f2459e40e002763144f4752cde2fb5"); + let key = decode_hex("831e664c9e3f0c3094c0b27b9d908eb2"); + + let cipher = Aes128Cbc::new_encrypt(&key, &iv); + let msg: [u8; 17] = decode_hex("26603bb76dd0a0180791c4ed4d3b058807"); + + let output = cipher.encrypt_padded(&msg).expect("Failed to encrypt"); + + let expected_ciphertext: [u8; 32] = + decode_hex("8d55dc10584e243f55d2bdbb5758b7fabcd58c8d3785f01c7e3640b2a1dadcd9"); + assert_eq!(expected_ciphertext, &output[..]); + } + + #[test] + fn aes_128_cbc_test_decrypt() { + // https://github.com/google/wycheproof/blob/master/testvectors/aes_cbc_pkcs5_test.json#L30 + // tcId: 2 + let key = decode_hex("e09eaa5a3f5e56d279d5e7a03373f6ea"); + let iv = decode_hex("c9ee3cd746bf208c65ca9e72a266d54f"); + let cipher = Aes128Cbc::new_decrypt(&key, &iv); + let ciphertext: [u8; 32] = + decode_hex("d1fa697f3e2e04d64f1a0da203813ca5bc226a0b1d42287b2a5b994a66eaf14a"); + let decrypted = cipher + .decrypt_padded(&ciphertext) + .expect("Failed to decrypt"); + let expected_plaintext: [u8; 16] = decode_hex("ef4eab37181f98423e53e947e7050fd0"); + assert_eq!(expected_plaintext, &decrypted[..]); + } + + #[test] + fn aes_128_cbc_test_decrypt_empty_message() { + // https://github.com/google/wycheproof/blob/master/testvectors/aes_cbc_pkcs5_test.json#L20 + // tcId: 1 + let key = decode_hex("e34f15c7bd819930fe9d66e0c166e61c"); + let iv = decode_hex("da9520f7d3520277035173299388bee2"); + let cipher = Aes128Cbc::new_decrypt(&key, &iv); + let ciphertext: [u8; 16] = decode_hex("b10ab60153276941361000414aed0a9d"); + let decrypted = cipher + .decrypt_padded(&ciphertext) + .expect("Failed to decrypt"); + let expected_plaintext: [u8; 0] = decode_hex(""); + assert_eq!(expected_plaintext, &decrypted[..]); + } + + #[test] + pub fn aes_256_cbc_test_encrypt() { + // https://github.com/google/wycheproof/blob/master/testvectors/aes_cbc_pkcs5_test.json#L1412 + // tcId: 124 + let iv = decode_hex("9ec7b863ac845cad5e4673da21f5b6a9"); + let key = decode_hex("612e837843ceae7f61d49625faa7e7494f9253e20cb3adcea686512b043936cd"); + + let cipher = Aes256Cbc::new_encrypt(&key, &iv); + let msg: [u8; 16] = decode_hex("cc37fae15f745a2f40e2c8b192f2b38d"); + + let output = cipher.encrypt_padded(&msg).expect("Failed to encrypt"); + + let expected_ciphertext: [u8; 32] = + decode_hex("299295be47e9f5441fe83a7a811c4aeb2650333e681e69fa6b767d28a6ccf282"); + assert_eq!(expected_ciphertext, &output[..]); + } + + #[test] + pub fn aes_256_cbc_test_encrypt_more_than_one_block() { + // https://github.com/google/wycheproof/blob/master/testvectors/aes_cbc_pkcs5_test.json#L1582C24-L1582C24 + // tcId: 141 + let iv = decode_hex("4b74bd981ea9d074757c3e2ef515e5fb"); + let key = decode_hex("73216fafd0022d0d6ee27198b2272578fa8f04dd9f44467fbb6437aa45641bf7"); + + let cipher = Aes256Cbc::new_encrypt(&key, &iv); + let msg: [u8; 17] = decode_hex("d5247b8f6c3edcbfb1d591d13ece23d2f5"); + + let output = cipher.encrypt_padded(&msg).expect("Failed to encrypt"); + + let expected_ciphertext: [u8; 32] = + decode_hex("fbea776fb1653635f88e2937ed2450ba4e9063e96d7cdba04928f01cb85492fe"); + assert_eq!(expected_ciphertext, &output[..]); + } + + #[test] + fn aes_256_cbc_test_decrypt() { + // https://github.com/google/wycheproof/blob/master/testvectors/aes_cbc_pkcs5_test.json#L1452 + // tcId: 128 + let key = decode_hex("ea3b016bdd387dd64d837c71683808f335dbdc53598a4ea8c5f952473fafaf5f"); + let iv = decode_hex("fae3e2054113f6b3b904aadbfe59655c"); + let cipher = Aes256Cbc::new_decrypt(&key, &iv); + let ciphertext: [u8; 16] = decode_hex("b90c326b72eb222ddb4dae47f2bc223c"); + let decrypted = cipher + .decrypt_padded(&ciphertext) + .expect("Failed to decrypt"); + let expected_plaintext: [u8; 2] = decode_hex("6601"); + assert_eq!(expected_plaintext, &decrypted[..]); + } +} diff --git a/rust/bssl-crypto/src/cipher/aes_ctr.rs b/rust/bssl-crypto/src/cipher/aes_ctr.rs index 1375d3e8a7..c9a122f014 100644 --- a/rust/bssl-crypto/src/cipher/aes_ctr.rs +++ b/rust/bssl-crypto/src/cipher/aes_ctr.rs @@ -13,7 +13,9 @@ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -use crate::cipher::{Cipher, CipherError, EvpAes128Ctr, EvpAes256Ctr, StreamCipher}; +use crate::cipher::{ + Cipher, CipherError, CipherInitPurpose, EvpAes128Ctr, EvpAes256Ctr, StreamCipher, +}; /// AES-CTR-128 Cipher implementation. pub struct Aes128Ctr(Cipher); @@ -24,7 +26,7 @@ impl StreamCipher for Aes128Ctr { /// Creates a new AES-128-CTR cipher instance from key material. fn new(key: &Self::Key, nonce: &Self::Nonce) -> Self { - Self(Cipher::new(key, nonce)) + Self(Cipher::new(key, nonce, CipherInitPurpose::Encrypt)) } /// Applies the keystream in-place, advancing the counter state appropriately. @@ -42,7 +44,7 @@ impl StreamCipher for Aes256Ctr { /// Creates a new AES-256-CTR cipher instance from key material. fn new(key: &Self::Key, nonce: &Self::Nonce) -> Self { - Self(Cipher::new(key, nonce)) + Self(Cipher::new(key, nonce, CipherInitPurpose::Encrypt)) } /// Applies the keystream in-place, advancing the counter state appropriately. diff --git a/rust/bssl-crypto/src/cipher/mod.rs b/rust/bssl-crypto/src/cipher/mod.rs index 2ff6b3ab30..16def56bdf 100644 --- a/rust/bssl-crypto/src/cipher/mod.rs +++ b/rust/bssl-crypto/src/cipher/mod.rs @@ -13,7 +13,11 @@ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +extern crate alloc; + use crate::{CSlice, CSliceMut}; +use alloc::vec; +use alloc::vec::Vec; use bssl_sys::EVP_CIPHER; use core::ffi::c_int; use core::marker::PhantomData; @@ -21,6 +25,9 @@ use core::marker::PhantomData; /// AES-CTR stream cipher operations. pub mod aes_ctr; +/// AES-CBC stream cipher operations. +pub mod aes_cbc; + /// Error returned in the event of an unsuccessful cipher operation. #[derive(Debug)] pub struct CipherError; @@ -42,6 +49,33 @@ pub trait StreamCipher { fn apply_keystream(&mut self, buffer: &mut [u8]) -> Result<(), CipherError>; } +/// Synchronous block cipher trait. +pub trait BlockCipher { + /// The byte array key type which specifies the size of the key used to instantiate the cipher. + type Key: AsRef<[u8]>; + + /// The byte array nonce type which specifies the size of the nonce used in the cipher + /// operations. + type Nonce: AsRef<[u8]>; + + /// Instantiate a new instance of a block cipher for encryption from a `key` and `iv`. + fn new_encrypt(key: &Self::Key, iv: &Self::Nonce) -> Self; + + /// Instantiate a new instance of a block cipher for decryption from a `key` and `iv`. + fn new_decrypt(key: &Self::Key, iv: &Self::Nonce) -> Self; + + /// Encrypts the given data in `buffer`, and returns the result (with padding) in a newly + /// allocated vector, or a [`CipherError`] if the operation was unsuccessful. + fn encrypt_padded(self, buffer: &[u8]) -> Result, CipherError>; + + /// Decrypts the given data in a `buffer`, and returns the result (with padding removed) in a + /// newly allocated vector, or a [`CipherError`] if the operation was unsuccessful. + fn decrypt_padded(self, buffer: &[u8]) -> Result, CipherError>; +} + +/// A cipher type, where `Key` is the size of the Key and `Nonce` is the size of the nonce or IV. +/// This must only be exposed publicly by types who ensure that `Key` is the correct size for the +/// given CipherType. This can be checked via `bssl_sys::EVP_CIPHER_key_length`. trait EvpCipherType { type Key: AsRef<[u8]>; type Nonce: AsRef<[u8]>; @@ -70,19 +104,41 @@ impl EvpCipherType for EvpAes256Ctr { } } -// Internal cipher implementation which wraps EVP_CIPHER_*, where K is the size of the Key and I is -// the size of the IV. This must only be exposed publicly by types who ensure that K is the correct -// size for the given CipherType. This can be checked via bssl_sys::EVP_CIPHER_key_length. -// -// WARNING: This is not safe to re-use for the CBC mode of operation since it is applying the -// key stream in-place. +struct EvpAes128Cbc; +impl EvpCipherType for EvpAes128Cbc { + type Key = [u8; 16]; + type Nonce = [u8; 16]; + fn evp_cipher() -> *const EVP_CIPHER { + // Safety: + // - this just returns a constant value + unsafe { bssl_sys::EVP_aes_128_cbc() } + } +} + +struct EvpAes256Cbc; +impl EvpCipherType for EvpAes256Cbc { + type Key = [u8; 32]; + type Nonce = [u8; 16]; + fn evp_cipher() -> *const EVP_CIPHER { + // Safety: + // - this just returns a constant value + unsafe { bssl_sys::EVP_aes_256_cbc() } + } +} + +enum CipherInitPurpose { + Encrypt, + Decrypt, +} + +/// Internal cipher implementation which wraps `EVP_CIPHER_*` struct Cipher { ctx: *mut bssl_sys::EVP_CIPHER_CTX, _marker: PhantomData, } impl Cipher { - fn new(key: &C::Key, iv: &C::Nonce) -> Self { + fn new(key: &C::Key, iv: &C::Nonce, purpose: CipherInitPurpose) -> Self { // Safety: // - Panics on allocation failure. let ctx = unsafe { bssl_sys::EVP_CIPHER_CTX_new() }; @@ -94,14 +150,25 @@ impl Cipher { // Safety: // - Key size and iv size must be properly set by the higher level wrapper types. // - Panics on allocation failure. - let result = unsafe { - bssl_sys::EVP_EncryptInit_ex( - ctx, - C::evp_cipher(), - core::ptr::null_mut(), - key_cslice.as_ptr(), - iv_cslice.as_ptr(), - ) + let result = match purpose { + CipherInitPurpose::Encrypt => unsafe { + bssl_sys::EVP_EncryptInit_ex( + ctx, + C::evp_cipher(), + core::ptr::null_mut(), + key_cslice.as_ptr(), + iv_cslice.as_ptr(), + ) + }, + CipherInitPurpose::Decrypt => unsafe { + bssl_sys::EVP_DecryptInit_ex( + ctx, + C::evp_cipher(), + core::ptr::null_mut(), + key_cslice.as_ptr(), + iv_cslice.as_ptr(), + ) + }, }; assert_eq!(result, 1); @@ -111,7 +178,20 @@ impl Cipher { } } + fn cipher_mode(&self) -> u32 { + // Safety: + // - The cipher context is initialized with EVP_EncryptInit_ex in `new` + unsafe { bssl_sys::EVP_CIPHER_CTX_mode(self.ctx) } + } + fn apply_keystream_in_place(&mut self, buffer: &mut [u8]) -> Result<(), CipherError> { + // WARNING: This is not safe to re-use for the CBC mode of operation since it is applying + // the key stream in-place. + assert_eq!( + self.cipher_mode(), + bssl_sys::EVP_CIPH_CTR_MODE as u32, + "Cannot use apply_keystraem_in_place for non-CTR modes" + ); let mut cslice_buf_mut = CSliceMut::from(buffer); let mut out_len = 0; @@ -135,6 +215,143 @@ impl Cipher { Err(CipherError) } } + + #[allow(clippy::expect_used)] + fn encrypt(self, buffer: &[u8]) -> Result, CipherError> { + // Safety: self.ctx is initialized with a cipher in `new()`. + let block_size_u32 = unsafe { bssl_sys::EVP_CIPHER_CTX_block_size(self.ctx) }; + let block_size: usize = block_size_u32 + .try_into() + .expect("Block size should always fit in usize"); + // Allocate an output vec that is large enough for both EncryptUpdate and EncryptFinal + // operations + let max_encrypt_update_output_size = buffer.len() + block_size - 1; + let max_encrypt_final_output_size = block_size; + let mut output_vec = + vec![0_u8; max_encrypt_update_output_size + max_encrypt_final_output_size]; + // EncryptUpdate block + let update_out_len_usize = { + let mut cslice_out_buf_mut = CSliceMut::from(&mut output_vec[..]); + let mut update_out_len = 0; + + let cslice_in_buf = CSlice::from(buffer); + let in_buff_len_int = c_int::try_from(cslice_in_buf.len()).map_err(|_| CipherError)?; + + // Safety: + // - `EVP_EncryptUpdate` requires that "The number of output bytes may be up to `in_len` + // plus the block length minus one and `out` must have sufficient space". This is the + // `max_encrypt_update_output_size` part of the output_vec's capacity. + let update_result = unsafe { + bssl_sys::EVP_EncryptUpdate( + self.ctx, + cslice_out_buf_mut.as_mut_ptr(), + &mut update_out_len, + cslice_in_buf.as_ptr(), + in_buff_len_int, + ) + }; + if update_result != 1 { + return Err(CipherError); + } + update_out_len + .try_into() + .expect("Output length should always fit in usize") + }; + + // EncryptFinal block + { + // Slice indexing here will not panic because we ensured `output_vec` is larger than + // what `EncryptUpdate` will write. + #[allow(clippy::indexing_slicing)] + let mut cslice_finalize_buf_mut = + CSliceMut::from(&mut output_vec[update_out_len_usize..]); + let mut final_out_len = 0; + let final_result = unsafe { + bssl_sys::EVP_EncryptFinal_ex( + self.ctx, + cslice_finalize_buf_mut.as_mut_ptr(), + &mut final_out_len, + ) + }; + let final_put_len_usize = + ::try_from(final_out_len).expect("Output length should always fit in usize"); + if final_result == 1 { + output_vec.truncate(update_out_len_usize + final_put_len_usize) + } else { + return Err(CipherError); + } + } + Ok(output_vec) + } + + #[allow(clippy::expect_used)] + fn decrypt(self, in_buffer: &[u8]) -> Result, CipherError> { + // Safety: self.ctx is initialized with a cipher in `new()`. + let block_size_u32 = unsafe { bssl_sys::EVP_CIPHER_CTX_block_size(self.ctx) }; + let block_size: usize = block_size_u32 + .try_into() + .expect("Block size should always fit in usize"); + // Allocate an output vec that is large enough for both DecryptUpdate and DecryptFinal + // operations + let max_decrypt_update_output_size = in_buffer.len() + block_size - 1; + let max_decrypt_final_output_size = block_size; + let mut output_vec = + vec![0_u8; max_decrypt_update_output_size + max_decrypt_final_output_size]; + + // DecryptUpdate block + let update_out_len_usize = { + let mut cslice_out_buf_mut = CSliceMut::from(&mut output_vec[..]); + let mut update_out_len = 0; + + let cslice_in_buf = CSlice::from(in_buffer); + let in_buff_len_int = c_int::try_from(cslice_in_buf.len()).map_err(|_| CipherError)?; + + // Safety: + // - `EVP_DecryptUpdate` requires that "The number of output bytes may be up to `in_len` + // plus the block length minus one and `out` must have sufficient space". This is the + // `max_decrypt_update_output_size` part of the output_vec's capacity. + let update_result = unsafe { + bssl_sys::EVP_DecryptUpdate( + self.ctx, + cslice_out_buf_mut.as_mut_ptr(), + &mut update_out_len, + cslice_in_buf.as_ptr(), + in_buff_len_int, + ) + }; + if update_result != 1 { + return Err(CipherError); + } + update_out_len + .try_into() + .expect("Output length should always fit in usize") + }; + + // DecryptFinal block + { + // Slice indexing here will not panic because we ensured `output_vec` is larger than + // what `DecryptUpdate` will write. + #[allow(clippy::indexing_slicing)] + let mut cslice_final_buf_mut = CSliceMut::from(&mut output_vec[update_out_len_usize..]); + let mut final_out_len = 0; + let final_result = unsafe { + bssl_sys::EVP_DecryptFinal_ex( + self.ctx, + cslice_final_buf_mut.as_mut_ptr(), + &mut final_out_len, + ) + }; + let final_put_len_usize = + ::try_from(final_out_len).expect("Output length should always fit in usize"); + + if final_result == 1 { + output_vec.truncate(update_out_len_usize + final_put_len_usize) + } else { + return Err(CipherError); + } + } + Ok(output_vec) + } } impl Drop for Cipher { @@ -144,3 +361,34 @@ impl Drop for Cipher { unsafe { bssl_sys::EVP_CIPHER_CTX_free(self.ctx) } } } + +#[cfg(test)] +mod test { + use crate::cipher::{CipherInitPurpose, EvpAes128Cbc, EvpAes128Ctr}; + + use super::Cipher; + + #[test] + fn test_cipher_mode() { + assert_eq!( + Cipher::::new(&[0; 16], &[0; 16], CipherInitPurpose::Encrypt) + .cipher_mode(), + bssl_sys::EVP_CIPH_CTR_MODE as u32 + ); + + assert_eq!( + Cipher::::new(&[0; 16], &[0; 16], CipherInitPurpose::Encrypt) + .cipher_mode(), + bssl_sys::EVP_CIPH_CBC_MODE as u32 + ); + } + + #[should_panic] + #[test] + fn test_apply_keystream_on_cbc() { + let mut cipher = + Cipher::::new(&[0; 16], &[0; 16], CipherInitPurpose::Encrypt); + let mut buf = [0; 16]; + let _ = cipher.apply_keystream_in_place(&mut buf); // This should panic + } +} From 20c9406971b39d214d4d6997f3a6e3ec772c440a Mon Sep 17 00:00:00 2001 From: Andres Erbsen Date: Mon, 25 Sep 2023 19:28:44 +0000 Subject: [PATCH 02/11] Add table-independent x86+adx asm for P-256 With -march=haswell -DOPENSSL_SMALL=1 on cascadelake: Did 9999 ECDH P-256 operations in 1062469us (9411.1 ops/sec) [+63.5%] Did 25000 ECDSA P-256 signing operations in 1028302us (24311.9 ops/sec) [+48.9%] Did 11004 ECDSA P-256 verify operations in 1072646us (10258.7 ops/sec) [+58.8%] Same configuration measured no performance difference on haswell. The added assembly code occupies 1352 bytes. Change-Id: I42635b7a9bf24d942817976a5d4ce269f642251c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63185 Reviewed-by: David Benjamin Commit-Queue: David Benjamin --- crypto/CMakeLists.txt | 2 + crypto/fipsmodule/ec/p256_test.cc | 47 ++++++ sources.cmake | 1 + third_party/fiat/asm/fiat_p256_adx_mul.S | 178 +++++++++++++++++++++++ third_party/fiat/asm/fiat_p256_adx_sqr.S | 167 +++++++++++++++++++++ third_party/fiat/p256_64.h | 20 +++ 6 files changed, 415 insertions(+) create mode 100644 crypto/fipsmodule/ec/p256_test.cc create mode 100644 third_party/fiat/asm/fiat_p256_adx_mul.S create mode 100644 third_party/fiat/asm/fiat_p256_adx_sqr.S diff --git a/crypto/CMakeLists.txt b/crypto/CMakeLists.txt index 68fb65b30e..56f9b908b4 100644 --- a/crypto/CMakeLists.txt +++ b/crypto/CMakeLists.txt @@ -18,6 +18,8 @@ set( poly1305/poly1305_arm_asm.S ../third_party/fiat/asm/fiat_curve25519_adx_mul.S ../third_party/fiat/asm/fiat_curve25519_adx_square.S + ../third_party/fiat/asm/fiat_p256_adx_mul.S + ../third_party/fiat/asm/fiat_p256_adx_sqr.S ) perlasm(CRYPTO_SOURCES aarch64 chacha/chacha-armv8 chacha/asm/chacha-armv8.pl) perlasm(CRYPTO_SOURCES aarch64 cipher_extra/chacha20_poly1305_armv8 cipher_extra/asm/chacha20_poly1305_armv8.pl) diff --git a/crypto/fipsmodule/ec/p256_test.cc b/crypto/fipsmodule/ec/p256_test.cc new file mode 100644 index 0000000000..2af9319b5a --- /dev/null +++ b/crypto/fipsmodule/ec/p256_test.cc @@ -0,0 +1,47 @@ +/* Copyright (c) 2023, Google Inc. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ + +#include +#include "../../internal.h" +#include "../../test/abi_test.h" + +#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) && \ + defined(SUPPORTS_ABI_TEST) +extern "C" { +#include "../../../third_party/fiat/p256_64.h" +} + +TEST(P256Test, AdxMulABI) { + static const uint64_t in1[4] = {0}, in2[4] = {0}; + uint64_t out[4]; + if (CRYPTO_is_BMI1_capable() && CRYPTO_is_BMI2_capable() && + CRYPTO_is_ADX_capable()) { + CHECK_ABI(fiat_p256_adx_mul, out, in1, in2); + } else { + GTEST_SKIP() << "Can't test ABI of ADX code without ADX"; + } +} + +#include +TEST(P256Test, AdxSquareABI) { + static const uint64_t in[4] = {0}; + uint64_t out[4]; + if (CRYPTO_is_BMI1_capable() && CRYPTO_is_BMI2_capable() && + CRYPTO_is_ADX_capable()) { + CHECK_ABI(fiat_p256_adx_sqr, out, in); + } else { + GTEST_SKIP() << "Can't test ABI of ADX code without ADX"; + } +} +#endif diff --git a/sources.cmake b/sources.cmake index d2e15c737f..2e7923f7c2 100644 --- a/sources.cmake +++ b/sources.cmake @@ -38,6 +38,7 @@ set( crypto/fipsmodule/cmac/cmac_test.cc crypto/fipsmodule/ec/ec_test.cc crypto/fipsmodule/ec/p256-nistz_test.cc + crypto/fipsmodule/ec/p256_test.cc crypto/fipsmodule/ecdsa/ecdsa_test.cc crypto/fipsmodule/hkdf/hkdf_test.cc crypto/fipsmodule/md5/md5_test.cc diff --git a/third_party/fiat/asm/fiat_p256_adx_mul.S b/third_party/fiat/asm/fiat_p256_adx_mul.S new file mode 100644 index 0000000000..d7ebd21719 --- /dev/null +++ b/third_party/fiat/asm/fiat_p256_adx_mul.S @@ -0,0 +1,178 @@ +#include + +#if !defined(OPENSSL_NO_ASM) && defined(OPENSSL_X86_64) && \ + (defined(__APPLE__) || defined(__ELF__)) + +.intel_syntax noprefix +.text +#if defined(__APPLE__) +.private_extern _fiat_p256_adx_mul +.global _fiat_p256_adx_mul +_fiat_p256_adx_mul: +#else +.type fiat_p256_adx_mul, @function +.hidden fiat_p256_adx_mul +.global fiat_p256_adx_mul +fiat_p256_adx_mul: +#endif + +.cfi_startproc +_CET_ENDBR +push rbp +.cfi_adjust_cfa_offset 8 +.cfi_offset rbp, -16 +mov rbp, rsp +mov rax, rdx +mov rdx, [ rsi + 0x0 ] +test al, al +mulx r8, rcx, [ rax + 0x0 ] +mov [ rsp - 0x80 ], rbx +.cfi_offset rbx, -16-0x80 +mulx rbx, r9, [ rax + 0x8 ] +mov [ rsp - 0x68 ], r14 +.cfi_offset r14, -16-0x68 +adc r9, r8 +mov [ rsp - 0x60 ], r15 +.cfi_offset r15, -16-0x60 +mulx r15, r14, [ rax + 0x10 ] +mov [ rsp - 0x78 ], r12 +.cfi_offset r12, -16-0x78 +adc r14, rbx +mulx r11, r10, [ rax + 0x18 ] +mov [ rsp - 0x70 ], r13 +.cfi_offset r13, -16-0x70 +adc r10, r15 +mov rdx, [ rsi + 0x8 ] +mulx rbx, r8, [ rax + 0x0 ] +adc r11, 0x0 +xor r15, r15 +adcx r8, r9 +adox rbx, r14 +mov [ rsp - 0x58 ], rdi +mulx rdi, r9, [ rax + 0x8 ] +adcx r9, rbx +adox rdi, r10 +mulx rbx, r14, [ rax + 0x10 ] +adcx r14, rdi +adox rbx, r11 +mulx r13, r12, [ rax + 0x18 ] +adcx r12, rbx +mov rdx, 0x100000000 +mulx r11, r10, rcx +adox r13, r15 +adcx r13, r15 +xor rdi, rdi +adox r10, r8 +mulx r8, rbx, r10 +adox r11, r9 +adcx rbx, r11 +adox r8, r14 +mov rdx, 0xffffffff00000001 +mulx r9, r15, rcx +adcx r15, r8 +adox r9, r12 +mulx r14, rcx, r10 +mov rdx, [ rsi + 0x10 ] +mulx r10, r12, [ rax + 0x8 ] +adcx rcx, r9 +adox r14, r13 +mulx r11, r13, [ rax + 0x0 ] +mov r9, rdi +adcx r14, r9 +adox rdi, rdi +adc rdi, 0x0 +xor r9, r9 +adcx r13, rbx +adox r11, r15 +mov rdx, [ rsi + 0x10 ] +mulx r15, r8, [ rax + 0x10 ] +adox r10, rcx +mulx rcx, rbx, [ rax + 0x18 ] +mov rdx, [ rsi + 0x18 ] +adcx r12, r11 +mulx rsi, r11, [ rax + 0x8 ] +adcx r8, r10 +adox r15, r14 +adcx rbx, r15 +adox rcx, r9 +adcx rcx, r9 +mulx r15, r10, [ rax + 0x0 ] +add rcx, rdi +mov r14, r9 +adc r14, 0 +xor r9, r9 +adcx r10, r12 +adox r15, r8 +adcx r11, r15 +adox rsi, rbx +mulx r8, r12, [ rax + 0x10 ] +adox r8, rcx +mulx rcx, rbx, [ rax + 0x18 ] +adcx r12, rsi +adox rcx, r9 +mov rdx, 0x100000000 +adcx rbx, r8 +adc rcx, 0 +mulx rdi, r15, r13 +xor rax, rax +adcx rcx, r14 +adc rax, 0 +xor r9, r9 +adox r15, r10 +mulx r14, r10, r15 +adox rdi, r11 +mov rdx, 0xffffffff00000001 +adox r14, r12 +adcx r10, rdi +mulx r12, r11, r13 +adcx r11, r14 +adox r12, rbx +mulx rbx, r13, r15 +adcx r13, r12 +adox rbx, rcx +mov r8, r9 +adox rax, r9 +adcx r8, rbx +adc rax, 0x0 +mov rcx, rax +mov r15, 0xffffffffffffffff +mov rdi, r10 +sub rdi, r15 +mov r14, 0xffffffff +mov r12, r11 +sbb r12, r14 +mov rbx, r13 +sbb rbx, r9 +mov rax, rax +mov rax, r8 +sbb rax, rdx +sbb rcx, r9 +cmovc rdi, r10 +mov r10, [ rsp - 0x58 ] +cmovc rbx, r13 +mov r13, [ rsp - 0x70 ] +.cfi_restore r13 +cmovc r12, r11 +cmovc rax, r8 +mov [ r10 + 0x10 ], rbx +mov rbx, [ rsp - 0x80 ] +.cfi_restore rbx +mov [ r10 + 0x0 ], rdi +mov [ r10 + 0x8 ], r12 +mov [ r10 + 0x18 ], rax +mov r12, [ rsp - 0x78 ] +.cfi_restore r12 +mov r14, [ rsp - 0x68 ] +.cfi_restore r14 +mov r15, [ rsp - 0x60 ] +.cfi_restore r15 +pop rbp +.cfi_restore rbp +.cfi_adjust_cfa_offset -8 +ret +.cfi_endproc +#if defined(__ELF__) +.size fiat_p256_adx_mul, .-fiat_p256_adx_mul +#endif + +#endif diff --git a/third_party/fiat/asm/fiat_p256_adx_sqr.S b/third_party/fiat/asm/fiat_p256_adx_sqr.S new file mode 100644 index 0000000000..cca269f52f --- /dev/null +++ b/third_party/fiat/asm/fiat_p256_adx_sqr.S @@ -0,0 +1,167 @@ +#include + +#if !defined(OPENSSL_NO_ASM) && defined(OPENSSL_X86_64) && \ + (defined(__APPLE__) || defined(__ELF__)) + +.intel_syntax noprefix +.text +#if defined(__APPLE__) +.private_extern _fiat_p256_adx_sqr +.global _fiat_p256_adx_sqr +_fiat_p256_adx_sqr: +#else +.type fiat_p256_adx_sqr, @function +.hidden fiat_p256_adx_sqr +.global fiat_p256_adx_sqr +fiat_p256_adx_sqr: +#endif + +.cfi_startproc +_CET_ENDBR +push rbp +.cfi_adjust_cfa_offset 8 +.cfi_offset rbp, -16 +mov rbp, rsp +mov rdx, [ rsi + 0x0 ] +mulx r10, rax, [ rsi + 0x18 ] +mulx rcx, r11, rdx +mulx r9, r8, [ rsi + 0x8 ] +mov [ rsp - 0x80 ], rbx +.cfi_offset rbx, -16-0x80 +xor rbx, rbx +adox r8, r8 +mov [ rsp - 0x78 ], r12 +.cfi_offset r12, -16-0x78 +mulx r12, rbx, [ rsi + 0x10 ] +mov rdx, [ rsi + 0x8 ] +mov [ rsp - 0x70 ], r13 +.cfi_offset r13, -16-0x70 +mov [ rsp - 0x68 ], r14 +.cfi_offset r14, -16-0x68 +mulx r14, r13, rdx +mov [ rsp - 0x60 ], r15 +.cfi_offset r15, -16-0x60 +mov [ rsp - 0x58 ], rdi +mulx rdi, r15, [ rsi + 0x10 ] +adcx r12, r15 +mov [ rsp - 0x50 ], r11 +mulx r11, r15, [ rsi + 0x18 ] +adcx r10, rdi +mov rdi, 0x0 +adcx r11, rdi +clc +adcx rbx, r9 +adox rbx, rbx +adcx rax, r12 +adox rax, rax +adcx r15, r10 +adox r15, r15 +mov rdx, [ rsi + 0x10 ] +mulx r12, r9, [ rsi + 0x18 ] +adcx r9, r11 +adcx r12, rdi +mulx r11, r10, rdx +clc +adcx rcx, r8 +adcx r13, rbx +adcx r14, rax +adox r9, r9 +adcx r10, r15 +mov rdx, [ rsi + 0x18 ] +mulx rbx, r8, rdx +adox r12, r12 +adcx r11, r9 +mov rsi, [ rsp - 0x50 ] +adcx r8, r12 +mov rax, 0x100000000 +mov rdx, rax +mulx r15, rax, rsi +adcx rbx, rdi +adox rbx, rdi +xor r9, r9 +adox rax, rcx +adox r15, r13 +mulx rcx, rdi, rax +adcx rdi, r15 +adox rcx, r14 +mov rdx, 0xffffffff00000001 +mulx r14, r13, rsi +adox r14, r10 +adcx r13, rcx +mulx r12, r10, rax +adox r12, r11 +mov r11, r9 +adox r11, r8 +adcx r10, r14 +mov r8, r9 +adcx r8, r12 +mov rax, r9 +adcx rax, r11 +mov r15, r9 +adox r15, rbx +mov rdx, 0x100000000 +mulx rcx, rbx, rdi +mov r14, r9 +adcx r14, r15 +mov r12, r9 +adox r12, r12 +adcx r12, r9 +adox rbx, r13 +mulx r11, r13, rbx +mov r15, 0xffffffff00000001 +mov rdx, r15 +mulx rsi, r15, rbx +adox rcx, r10 +adox r11, r8 +mulx r8, r10, rdi +adcx r13, rcx +adox r8, rax +adcx r10, r11 +adox rsi, r14 +mov rdi, r12 +mov rax, r9 +adox rdi, rax +adcx r15, r8 +mov r14, rax +adcx r14, rsi +adcx rdi, r9 +dec r9 +mov rbx, r13 +sub rbx, r9 +mov rcx, 0xffffffff +mov r11, r10 +sbb r11, rcx +mov r8, r15 +sbb r8, rax +mov rsi, r14 +sbb rsi, rdx +sbb rdi, rax +cmovc rbx, r13 +cmovc r8, r15 +cmovc r11, r10 +cmovc rsi, r14 +mov rdi, [ rsp - 0x58 ] +mov [ rdi + 0x18 ], rsi +mov [ rdi + 0x0 ], rbx +mov [ rdi + 0x8 ], r11 +mov [ rdi + 0x10 ], r8 +mov rbx, [ rsp - 0x80 ] +.cfi_restore rbx +mov r12, [ rsp - 0x78 ] +.cfi_restore r12 +mov r13, [ rsp - 0x70 ] +.cfi_restore r13 +mov r14, [ rsp - 0x68 ] +.cfi_restore r14 +mov r15, [ rsp - 0x60 ] +.cfi_restore r15 +pop rbp +.cfi_restore rbp +.cfi_adjust_cfa_offset -8 +ret +.cfi_endproc +#if defined(__ELF__) +.size fiat_p256_adx_sqr, .-fiat_p256_adx_sqr +#endif + +#endif diff --git a/third_party/fiat/p256_64.h b/third_party/fiat/p256_64.h index c77263843a..a691407b60 100644 --- a/third_party/fiat/p256_64.h +++ b/third_party/fiat/p256_64.h @@ -1,3 +1,9 @@ +#include "../../crypto/internal.h" +#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) +void fiat_p256_adx_mul(uint64_t*, const uint64_t*, const uint64_t*); +void fiat_p256_adx_sqr(uint64_t*, const uint64_t*); +#endif + /* Autogenerated: 'src/ExtractionOCaml/word_by_word_montgomery' --inline --static --use-value-barrier p256 64 '2^256 - 2^224 + 2^192 + 2^96 - 1' mul square add sub opp from_montgomery to_montgomery nonzero selectznz to_bytes from_bytes one msat divstep divstep_precomp */ /* curve description: p256 */ /* machine_wordsize = 64 (from "64") */ @@ -165,6 +171,13 @@ static FIAT_P256_FIAT_INLINE void fiat_p256_cmovznz_u64(uint64_t* out1, fiat_p25 * */ static FIAT_P256_FIAT_INLINE void fiat_p256_mul(fiat_p256_montgomery_domain_field_element out1, const fiat_p256_montgomery_domain_field_element arg1, const fiat_p256_montgomery_domain_field_element arg2) { +#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) + if (CRYPTO_is_BMI1_capable() && CRYPTO_is_BMI2_capable() && + CRYPTO_is_ADX_capable()) { + fiat_p256_adx_mul(out1, arg1, arg2); + return; + } +#endif uint64_t x1; uint64_t x2; uint64_t x3; @@ -472,6 +485,13 @@ static FIAT_P256_FIAT_INLINE void fiat_p256_mul(fiat_p256_montgomery_domain_fiel * */ static FIAT_P256_FIAT_INLINE void fiat_p256_square(fiat_p256_montgomery_domain_field_element out1, const fiat_p256_montgomery_domain_field_element arg1) { +#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) + if (CRYPTO_is_BMI1_capable() && CRYPTO_is_BMI2_capable() && + CRYPTO_is_ADX_capable()) { + fiat_p256_adx_sqr(out1, arg1); + return; + } +#endif uint64_t x1; uint64_t x2; uint64_t x3; From dd68e4bb4d63b74b0996b714d0bc8c7f51af334b Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 2 Oct 2023 23:13:13 -0400 Subject: [PATCH 03/11] Add OPENSSL_zalloc OpenSSL added a similar helper function. It's very, very common for us to malloc something an then zero it. This saves some effort. Also replace some more malloc + memcpy pairs with memdup. Change-Id: I1e765c8774a0d15742827c39a1f16df9748ef247 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63345 Reviewed-by: Bob Beck Commit-Queue: David Benjamin Auto-Submit: David Benjamin --- crypto/asn1/tasn_new.c | 6 ++---- crypto/base64/base64.c | 7 +------ crypto/bio/bio.c | 6 ++---- crypto/bio/connect.c | 5 +---- crypto/bio/pair.c | 5 +---- crypto/buf/buf.c | 12 +----------- crypto/conf/conf.c | 9 +-------- crypto/curve25519/spake25519.c | 3 +-- crypto/dsa/dsa.c | 17 ++--------------- crypto/engine/engine.c | 10 +--------- crypto/evp/evp.c | 6 +----- crypto/evp/evp_ctx.c | 7 ++----- crypto/evp/p_ec.c | 5 +---- crypto/evp/p_hkdf.c | 3 +-- crypto/evp/p_rsa.c | 4 +--- crypto/fipsmodule/bn/prime.c | 9 +-------- crypto/fipsmodule/cipher/cipher.c | 3 +-- crypto/fipsmodule/dh/dh.c | 6 +----- crypto/fipsmodule/ec/ec.c | 3 +-- crypto/fipsmodule/ec/ec_key.c | 7 ++----- crypto/fipsmodule/rsa/blinding.c | 3 +-- crypto/fipsmodule/rsa/rsa.c | 4 +--- crypto/fipsmodule/self_check/fips.c | 3 +-- crypto/lhash/lhash.c | 9 +++------ crypto/mem.c | 8 ++++++++ crypto/obj/obj.c | 10 +++------- crypto/pkcs7/pkcs7_x509.c | 6 ++---- crypto/pkcs8/pkcs8_x509.c | 11 +++-------- crypto/pool/pool.c | 9 +++------ crypto/stack/stack.c | 13 ++++--------- crypto/trust_token/trust_token.c | 9 +++------ crypto/x509/policy.c | 6 ++---- crypto/x509/x509_lu.c | 3 +-- crypto/x509/x509_vpm.c | 4 +--- crypto/x509/x_pkey.c | 3 +-- crypto/x509/x_x509.c | 3 +-- decrepit/bio/base64_bio.c | 6 +----- include/openssl/mem.h | 4 ++++ ssl/d1_both.cc | 3 +-- ssl/ssl_test.cc | 16 ++++------------ ssl/test/async_bio.cc | 3 +-- 41 files changed, 74 insertions(+), 195 deletions(-) diff --git a/crypto/asn1/tasn_new.c b/crypto/asn1/tasn_new.c index 8a90b436d1..e896ead147 100644 --- a/crypto/asn1/tasn_new.c +++ b/crypto/asn1/tasn_new.c @@ -127,11 +127,10 @@ int ASN1_item_ex_new(ASN1_VALUE **pval, const ASN1_ITEM *it) { return 1; } } - *pval = OPENSSL_malloc(it->size); + *pval = OPENSSL_zalloc(it->size); if (!*pval) { goto memerr; } - OPENSSL_memset(*pval, 0, it->size); asn1_set_choice_selector(pval, -1, it); if (asn1_cb && !asn1_cb(ASN1_OP_NEW_POST, pval, it, NULL)) { goto auxerr2; @@ -151,11 +150,10 @@ int ASN1_item_ex_new(ASN1_VALUE **pval, const ASN1_ITEM *it) { return 1; } } - *pval = OPENSSL_malloc(it->size); + *pval = OPENSSL_zalloc(it->size); if (!*pval) { goto memerr; } - OPENSSL_memset(*pval, 0, it->size); asn1_refcount_set_one(pval, it); asn1_enc_init(pval, it); for (i = 0, tt = it->templates; i < it->tcount; tt++, i++) { diff --git a/crypto/base64/base64.c b/crypto/base64/base64.c index d2b1e58406..666f832692 100644 --- a/crypto/base64/base64.c +++ b/crypto/base64/base64.c @@ -121,12 +121,7 @@ int EVP_EncodedLength(size_t *out_len, size_t len) { } EVP_ENCODE_CTX *EVP_ENCODE_CTX_new(void) { - EVP_ENCODE_CTX *ret = OPENSSL_malloc(sizeof(EVP_ENCODE_CTX)); - if (ret == NULL) { - return NULL; - } - OPENSSL_memset(ret, 0, sizeof(EVP_ENCODE_CTX)); - return ret; + return OPENSSL_zalloc(sizeof(EVP_ENCODE_CTX)); } void EVP_ENCODE_CTX_free(EVP_ENCODE_CTX *ctx) { diff --git a/crypto/bio/bio.c b/crypto/bio/bio.c index b2d9563873..ed2456071a 100644 --- a/crypto/bio/bio.c +++ b/crypto/bio/bio.c @@ -70,12 +70,11 @@ BIO *BIO_new(const BIO_METHOD *method) { - BIO *ret = OPENSSL_malloc(sizeof(BIO)); + BIO *ret = OPENSSL_zalloc(sizeof(BIO)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(BIO)); ret->method = method; ret->shutdown = 1; ret->references = 1; @@ -640,11 +639,10 @@ int BIO_get_new_index(void) { } BIO_METHOD *BIO_meth_new(int type, const char *name) { - BIO_METHOD *method = OPENSSL_malloc(sizeof(BIO_METHOD)); + BIO_METHOD *method = OPENSSL_zalloc(sizeof(BIO_METHOD)); if (method == NULL) { return NULL; } - OPENSSL_memset(method, 0, sizeof(BIO_METHOD)); method->type = type; method->name = name; return method; diff --git a/crypto/bio/connect.c b/crypto/bio/connect.c index d48d14e91d..900e659bd9 100644 --- a/crypto/bio/connect.c +++ b/crypto/bio/connect.c @@ -296,13 +296,10 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { } static BIO_CONNECT *BIO_CONNECT_new(void) { - BIO_CONNECT *ret = OPENSSL_malloc(sizeof(BIO_CONNECT)); - + BIO_CONNECT *ret = OPENSSL_zalloc(sizeof(BIO_CONNECT)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(BIO_CONNECT)); - ret->state = BIO_CONN_S_BEFORE; return ret; } diff --git a/crypto/bio/pair.c b/crypto/bio/pair.c index 40711cdf95..988b4cea0f 100644 --- a/crypto/bio/pair.c +++ b/crypto/bio/pair.c @@ -81,13 +81,10 @@ struct bio_bio_st { }; static int bio_new(BIO *bio) { - struct bio_bio_st *b; - - b = OPENSSL_malloc(sizeof *b); + struct bio_bio_st *b = OPENSSL_zalloc(sizeof *b); if (b == NULL) { return 0; } - OPENSSL_memset(b, 0, sizeof(struct bio_bio_st)); b->size = 17 * 1024; // enough for one TLS record (just a default) bio->ptr = b; diff --git a/crypto/buf/buf.c b/crypto/buf/buf.c index 57bf34d4b0..1fe8fe6126 100644 --- a/crypto/buf/buf.c +++ b/crypto/buf/buf.c @@ -64,17 +64,7 @@ #include "../internal.h" -BUF_MEM *BUF_MEM_new(void) { - BUF_MEM *ret; - - ret = OPENSSL_malloc(sizeof(BUF_MEM)); - if (ret == NULL) { - return NULL; - } - - OPENSSL_memset(ret, 0, sizeof(BUF_MEM)); - return ret; -} +BUF_MEM *BUF_MEM_new(void) { return OPENSSL_zalloc(sizeof(BUF_MEM)); } void BUF_MEM_free(BUF_MEM *buf) { if (buf == NULL) { diff --git a/crypto/conf/conf.c b/crypto/conf/conf.c index ca950d6284..024fa74486 100644 --- a/crypto/conf/conf.c +++ b/crypto/conf/conf.c @@ -118,14 +118,7 @@ CONF *NCONF_new(void *method) { return conf; } -CONF_VALUE *CONF_VALUE_new(void) { - CONF_VALUE *v = OPENSSL_malloc(sizeof(CONF_VALUE)); - if (!v) { - return NULL; - } - OPENSSL_memset(v, 0, sizeof(CONF_VALUE)); - return v; -} +CONF_VALUE *CONF_VALUE_new(void) { return OPENSSL_zalloc(sizeof(CONF_VALUE)); } static void value_free_contents(CONF_VALUE *value) { OPENSSL_free(value->section); diff --git a/crypto/curve25519/spake25519.c b/crypto/curve25519/spake25519.c index c45d15a58d..adbf60d581 100644 --- a/crypto/curve25519/spake25519.c +++ b/crypto/curve25519/spake25519.c @@ -272,12 +272,11 @@ static const uint8_t kSpakeMSmallPrecomp[15 * 2 * 32] = { SPAKE2_CTX *SPAKE2_CTX_new(enum spake2_role_t my_role, const uint8_t *my_name, size_t my_name_len, const uint8_t *their_name, size_t their_name_len) { - SPAKE2_CTX *ctx = OPENSSL_malloc(sizeof(SPAKE2_CTX)); + SPAKE2_CTX *ctx = OPENSSL_zalloc(sizeof(SPAKE2_CTX)); if (ctx == NULL) { return NULL; } - OPENSSL_memset(ctx, 0, sizeof(SPAKE2_CTX)); ctx->my_role = my_role; CBS my_name_cbs, their_name_cbs; diff --git a/crypto/dsa/dsa.c b/crypto/dsa/dsa.c index 5eb7894882..4583dc6f45 100644 --- a/crypto/dsa/dsa.c +++ b/crypto/dsa/dsa.c @@ -88,18 +88,14 @@ static int dsa_sign_setup(const DSA *dsa, BN_CTX *ctx_in, BIGNUM **out_kinv, static CRYPTO_EX_DATA_CLASS g_ex_data_class = CRYPTO_EX_DATA_CLASS_INIT; DSA *DSA_new(void) { - DSA *dsa = OPENSSL_malloc(sizeof(DSA)); + DSA *dsa = OPENSSL_zalloc(sizeof(DSA)); if (dsa == NULL) { return NULL; } - OPENSSL_memset(dsa, 0, sizeof(DSA)); - dsa->references = 1; - CRYPTO_MUTEX_init(&dsa->method_mont_lock); CRYPTO_new_ex_data(&dsa->ex_data); - return dsa; } @@ -533,16 +529,7 @@ int DSA_generate_key(DSA *dsa) { return ok; } -DSA_SIG *DSA_SIG_new(void) { - DSA_SIG *sig; - sig = OPENSSL_malloc(sizeof(DSA_SIG)); - if (!sig) { - return NULL; - } - sig->r = NULL; - sig->s = NULL; - return sig; -} +DSA_SIG *DSA_SIG_new(void) { return OPENSSL_zalloc(sizeof(DSA_SIG)); } void DSA_SIG_free(DSA_SIG *sig) { if (!sig) { diff --git a/crypto/engine/engine.c b/crypto/engine/engine.c index 973a57c809..831d468933 100644 --- a/crypto/engine/engine.c +++ b/crypto/engine/engine.c @@ -31,15 +31,7 @@ struct engine_st { ECDSA_METHOD *ecdsa_method; }; -ENGINE *ENGINE_new(void) { - ENGINE *engine = OPENSSL_malloc(sizeof(ENGINE)); - if (engine == NULL) { - return NULL; - } - - OPENSSL_memset(engine, 0, sizeof(ENGINE)); - return engine; -} +ENGINE *ENGINE_new(void) { return OPENSSL_zalloc(sizeof(ENGINE)); } int ENGINE_free(ENGINE *engine) { // Methods are currently required to be static so are not unref'ed. diff --git a/crypto/evp/evp.c b/crypto/evp/evp.c index 37b3631db1..f3f3d7e55c 100644 --- a/crypto/evp/evp.c +++ b/crypto/evp/evp.c @@ -81,17 +81,13 @@ OPENSSL_DECLARE_ERROR_REASON(EVP, NOT_XOF_OR_INVALID_LENGTH) OPENSSL_DECLARE_ERROR_REASON(EVP, EMPTY_PSK) EVP_PKEY *EVP_PKEY_new(void) { - EVP_PKEY *ret; - - ret = OPENSSL_malloc(sizeof(EVP_PKEY)); + EVP_PKEY *ret = OPENSSL_zalloc(sizeof(EVP_PKEY)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(EVP_PKEY)); ret->type = EVP_PKEY_NONE; ret->references = 1; - return ret; } diff --git a/crypto/evp/evp_ctx.c b/crypto/evp/evp_ctx.c index 771f13f003..ea2781f0ac 100644 --- a/crypto/evp/evp_ctx.c +++ b/crypto/evp/evp_ctx.c @@ -86,11 +86,10 @@ static const EVP_PKEY_METHOD *evp_pkey_meth_find(int type) { static EVP_PKEY_CTX *evp_pkey_ctx_new(EVP_PKEY *pkey, ENGINE *e, const EVP_PKEY_METHOD *pmeth) { - EVP_PKEY_CTX *ret = OPENSSL_malloc(sizeof(EVP_PKEY_CTX)); + EVP_PKEY_CTX *ret = OPENSSL_zalloc(sizeof(EVP_PKEY_CTX)); if (!ret) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(EVP_PKEY_CTX)); ret->engine = e; ret->pmeth = pmeth; @@ -156,13 +155,11 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(EVP_PKEY_CTX *ctx) { return NULL; } - EVP_PKEY_CTX *ret = OPENSSL_malloc(sizeof(EVP_PKEY_CTX)); + EVP_PKEY_CTX *ret = OPENSSL_zalloc(sizeof(EVP_PKEY_CTX)); if (!ret) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(EVP_PKEY_CTX)); - ret->pmeth = ctx->pmeth; ret->engine = ctx->engine; ret->operation = ctx->operation; diff --git a/crypto/evp/p_ec.c b/crypto/evp/p_ec.c index ed89cc3800..0e4349f05c 100644 --- a/crypto/evp/p_ec.c +++ b/crypto/evp/p_ec.c @@ -80,15 +80,12 @@ typedef struct { static int pkey_ec_init(EVP_PKEY_CTX *ctx) { - EC_PKEY_CTX *dctx; - dctx = OPENSSL_malloc(sizeof(EC_PKEY_CTX)); + EC_PKEY_CTX *dctx = OPENSSL_zalloc(sizeof(EC_PKEY_CTX)); if (!dctx) { return 0; } - OPENSSL_memset(dctx, 0, sizeof(EC_PKEY_CTX)); ctx->data = dctx; - return 1; } diff --git a/crypto/evp/p_hkdf.c b/crypto/evp/p_hkdf.c index 0d7ede82c2..d9cbfc7cf2 100644 --- a/crypto/evp/p_hkdf.c +++ b/crypto/evp/p_hkdf.c @@ -35,12 +35,11 @@ typedef struct { } HKDF_PKEY_CTX; static int pkey_hkdf_init(EVP_PKEY_CTX *ctx) { - HKDF_PKEY_CTX *hctx = OPENSSL_malloc(sizeof(HKDF_PKEY_CTX)); + HKDF_PKEY_CTX *hctx = OPENSSL_zalloc(sizeof(HKDF_PKEY_CTX)); if (hctx == NULL) { return 0; } - OPENSSL_memset(hctx, 0, sizeof(HKDF_PKEY_CTX)); if (!CBB_init(&hctx->info, 0)) { OPENSSL_free(hctx); return 0; diff --git a/crypto/evp/p_rsa.c b/crypto/evp/p_rsa.c index 15eb1efbd4..3bdd85d687 100644 --- a/crypto/evp/p_rsa.c +++ b/crypto/evp/p_rsa.c @@ -97,12 +97,10 @@ typedef struct { } RSA_OAEP_LABEL_PARAMS; static int pkey_rsa_init(EVP_PKEY_CTX *ctx) { - RSA_PKEY_CTX *rctx; - rctx = OPENSSL_malloc(sizeof(RSA_PKEY_CTX)); + RSA_PKEY_CTX *rctx = OPENSSL_zalloc(sizeof(RSA_PKEY_CTX)); if (!rctx) { return 0; } - OPENSSL_memset(rctx, 0, sizeof(RSA_PKEY_CTX)); rctx->nbits = 2048; rctx->pad_mode = RSA_PKCS1_PADDING; diff --git a/crypto/fipsmodule/bn/prime.c b/crypto/fipsmodule/bn/prime.c index 2d2ab6937f..fb30768310 100644 --- a/crypto/fipsmodule/bn/prime.c +++ b/crypto/fipsmodule/bn/prime.c @@ -359,14 +359,7 @@ static int probable_prime_dh(BIGNUM *rnd, int bits, const BIGNUM *add, static int probable_prime_dh_safe(BIGNUM *rnd, int bits, const BIGNUM *add, const BIGNUM *rem, BN_CTX *ctx); -BN_GENCB *BN_GENCB_new(void) { - BN_GENCB *callback = OPENSSL_malloc(sizeof(BN_GENCB)); - if (callback == NULL) { - return NULL; - } - OPENSSL_memset(callback, 0, sizeof(BN_GENCB)); - return callback; -} +BN_GENCB *BN_GENCB_new(void) { return OPENSSL_zalloc(sizeof(BN_GENCB)); } void BN_GENCB_free(BN_GENCB *callback) { OPENSSL_free(callback); } diff --git a/crypto/fipsmodule/cipher/cipher.c b/crypto/fipsmodule/cipher/cipher.c index bff7996a5d..7ce3c20c20 100644 --- a/crypto/fipsmodule/cipher/cipher.c +++ b/crypto/fipsmodule/cipher/cipher.c @@ -113,12 +113,11 @@ int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in) { OPENSSL_memcpy(out, in, sizeof(EVP_CIPHER_CTX)); if (in->cipher_data && in->cipher->ctx_size) { - out->cipher_data = OPENSSL_malloc(in->cipher->ctx_size); + out->cipher_data = OPENSSL_memdup(in->cipher_data, in->cipher->ctx_size); if (!out->cipher_data) { out->cipher = NULL; return 0; } - OPENSSL_memcpy(out->cipher_data, in->cipher_data, in->cipher->ctx_size); } if (in->cipher->flags & EVP_CIPH_CUSTOM_COPY) { diff --git a/crypto/fipsmodule/dh/dh.c b/crypto/fipsmodule/dh/dh.c index a20b6d11df..d57b0935f2 100644 --- a/crypto/fipsmodule/dh/dh.c +++ b/crypto/fipsmodule/dh/dh.c @@ -71,17 +71,13 @@ DH *DH_new(void) { - DH *dh = OPENSSL_malloc(sizeof(DH)); + DH *dh = OPENSSL_zalloc(sizeof(DH)); if (dh == NULL) { return NULL; } - OPENSSL_memset(dh, 0, sizeof(DH)); - CRYPTO_MUTEX_init(&dh->method_mont_p_lock); - dh->references = 1; - return dh; } diff --git a/crypto/fipsmodule/ec/ec.c b/crypto/fipsmodule/ec/ec.c index 00587a1f2e..0ae566a9a0 100644 --- a/crypto/fipsmodule/ec/ec.c +++ b/crypto/fipsmodule/ec/ec.c @@ -250,11 +250,10 @@ EC_GROUP *EC_GROUP_new_curve_GFp(const BIGNUM *p, const BIGNUM *a, goto err; } - ret = OPENSSL_malloc(sizeof(EC_GROUP)); + ret = OPENSSL_zalloc(sizeof(EC_GROUP)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(EC_GROUP)); ret->references = 1; ret->meth = EC_GFp_mont_method(); bn_mont_ctx_init(&ret->field); diff --git a/crypto/fipsmodule/ec/ec_key.c b/crypto/fipsmodule/ec/ec_key.c index 90a4404cd8..a48671a2b5 100644 --- a/crypto/fipsmodule/ec/ec_key.c +++ b/crypto/fipsmodule/ec/ec_key.c @@ -86,12 +86,11 @@ DEFINE_STATIC_EX_DATA_CLASS(g_ec_ex_data_class) static EC_WRAPPED_SCALAR *ec_wrapped_scalar_new(const EC_GROUP *group) { - EC_WRAPPED_SCALAR *wrapped = OPENSSL_malloc(sizeof(EC_WRAPPED_SCALAR)); + EC_WRAPPED_SCALAR *wrapped = OPENSSL_zalloc(sizeof(EC_WRAPPED_SCALAR)); if (wrapped == NULL) { return NULL; } - OPENSSL_memset(wrapped, 0, sizeof(EC_WRAPPED_SCALAR)); wrapped->bignum.d = wrapped->scalar.words; wrapped->bignum.width = group->order.N.width; wrapped->bignum.dmax = group->order.N.width; @@ -106,13 +105,11 @@ static void ec_wrapped_scalar_free(EC_WRAPPED_SCALAR *scalar) { EC_KEY *EC_KEY_new(void) { return EC_KEY_new_method(NULL); } EC_KEY *EC_KEY_new_method(const ENGINE *engine) { - EC_KEY *ret = OPENSSL_malloc(sizeof(EC_KEY)); + EC_KEY *ret = OPENSSL_zalloc(sizeof(EC_KEY)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(EC_KEY)); - if (engine) { ret->ecdsa_meth = ENGINE_get_ECDSA_method(engine); } diff --git a/crypto/fipsmodule/rsa/blinding.c b/crypto/fipsmodule/rsa/blinding.c index c4cfcc2313..8838ad8fa1 100644 --- a/crypto/fipsmodule/rsa/blinding.c +++ b/crypto/fipsmodule/rsa/blinding.c @@ -130,11 +130,10 @@ static int bn_blinding_create_param(BN_BLINDING *b, const BIGNUM *e, const BN_MONT_CTX *mont, BN_CTX *ctx); BN_BLINDING *BN_BLINDING_new(void) { - BN_BLINDING *ret = OPENSSL_malloc(sizeof(BN_BLINDING)); + BN_BLINDING *ret = OPENSSL_zalloc(sizeof(BN_BLINDING)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(BN_BLINDING)); ret->A = BN_new(); if (ret->A == NULL) { diff --git a/crypto/fipsmodule/rsa/rsa.c b/crypto/fipsmodule/rsa/rsa.c index 77ab6c6e70..8babba1829 100644 --- a/crypto/fipsmodule/rsa/rsa.c +++ b/crypto/fipsmodule/rsa/rsa.c @@ -206,13 +206,11 @@ RSA *RSA_new_private_key_large_e(const BIGNUM *n, const BIGNUM *e, RSA *RSA_new(void) { return RSA_new_method(NULL); } RSA *RSA_new_method(const ENGINE *engine) { - RSA *rsa = OPENSSL_malloc(sizeof(RSA)); + RSA *rsa = OPENSSL_zalloc(sizeof(RSA)); if (rsa == NULL) { return NULL; } - OPENSSL_memset(rsa, 0, sizeof(RSA)); - if (engine) { rsa->meth = ENGINE_get_RSA_method(engine); } diff --git a/crypto/fipsmodule/self_check/fips.c b/crypto/fipsmodule/self_check/fips.c index ce03957641..c3515ea9e3 100644 --- a/crypto/fipsmodule/self_check/fips.c +++ b/crypto/fipsmodule/self_check/fips.c @@ -94,12 +94,11 @@ void boringssl_fips_inc_counter(enum fips_counter_t counter) { CRYPTO_get_thread_local(OPENSSL_THREAD_LOCAL_FIPS_COUNTERS); if (!array) { const size_t num_bytes = sizeof(size_t) * (fips_counter_max + 1); - array = OPENSSL_malloc(num_bytes); + array = OPENSSL_zalloc(num_bytes); if (!array) { return; } - OPENSSL_memset(array, 0, num_bytes); if (!CRYPTO_set_thread_local(OPENSSL_THREAD_LOCAL_FIPS_COUNTERS, array, OPENSSL_free)) { // |OPENSSL_free| has already been called by |CRYPTO_set_thread_local|. diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c index 4a95a2e67d..8e20c88d46 100644 --- a/crypto/lhash/lhash.c +++ b/crypto/lhash/lhash.c @@ -104,19 +104,17 @@ struct lhash_st { }; _LHASH *OPENSSL_lh_new(lhash_hash_func hash, lhash_cmp_func comp) { - _LHASH *ret = OPENSSL_malloc(sizeof(_LHASH)); + _LHASH *ret = OPENSSL_zalloc(sizeof(_LHASH)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(_LHASH)); ret->num_buckets = kMinNumBuckets; - ret->buckets = OPENSSL_malloc(sizeof(LHASH_ITEM *) * ret->num_buckets); + ret->buckets = OPENSSL_zalloc(sizeof(LHASH_ITEM *) * ret->num_buckets); if (ret->buckets == NULL) { OPENSSL_free(ret); return NULL; } - OPENSSL_memset(ret->buckets, 0, sizeof(LHASH_ITEM *) * ret->num_buckets); ret->comp = comp; ret->hash = hash; @@ -214,11 +212,10 @@ static void lh_rebucket(_LHASH *lh, const size_t new_num_buckets) { return; } - new_buckets = OPENSSL_malloc(alloc_size); + new_buckets = OPENSSL_zalloc(alloc_size); if (new_buckets == NULL) { return; } - OPENSSL_memset(new_buckets, 0, alloc_size); for (i = 0; i < lh->num_buckets; i++) { for (cur = lh->buckets[i]; cur != NULL; cur = next) { diff --git a/crypto/mem.c b/crypto/mem.c index 89832fce50..b17267fc33 100644 --- a/crypto/mem.c +++ b/crypto/mem.c @@ -267,6 +267,14 @@ void *OPENSSL_malloc(size_t size) { return NULL; } +void *OPENSSL_zalloc(size_t size) { + void *ret = OPENSSL_malloc(size); + if (ret != NULL) { + OPENSSL_memset(ret, 0, size); + } + return ret; +} + void OPENSSL_free(void *orig_ptr) { if (orig_ptr == NULL) { return; diff --git a/crypto/obj/obj.c b/crypto/obj/obj.c index 9be3730576..6519933653 100644 --- a/crypto/obj/obj.c +++ b/crypto/obj/obj.c @@ -115,16 +115,12 @@ ASN1_OBJECT *OBJ_dup(const ASN1_OBJECT *o) { } r->ln = r->sn = NULL; - data = OPENSSL_malloc(o->length); - if (data == NULL) { + // once data is attached to an object, it remains const + r->data = OPENSSL_memdup(o->data, o->length); + if (o->length != 0 && r->data == NULL) { goto err; } - if (o->data != NULL) { - OPENSSL_memcpy(data, o->data, o->length); - } - // once data is attached to an object, it remains const - r->data = data; r->length = o->length; r->nid = o->nid; diff --git a/crypto/pkcs7/pkcs7_x509.c b/crypto/pkcs7/pkcs7_x509.c index fd71bd7b37..7b10f6f239 100644 --- a/crypto/pkcs7/pkcs7_x509.c +++ b/crypto/pkcs7/pkcs7_x509.c @@ -237,11 +237,10 @@ int PKCS7_bundle_CRLs(CBB *out, const STACK_OF(X509_CRL) *crls) { } static PKCS7 *pkcs7_new(CBS *cbs) { - PKCS7 *ret = OPENSSL_malloc(sizeof(PKCS7)); + PKCS7 *ret = OPENSSL_zalloc(sizeof(PKCS7)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(PKCS7)); ret->type = OBJ_nid2obj(NID_pkcs7_signed); ret->d.sign = OPENSSL_malloc(sizeof(PKCS7_SIGNED)); if (ret->d.sign == NULL) { @@ -326,11 +325,10 @@ int i2d_PKCS7(const PKCS7 *p7, uint8_t **out) { } if (*out == NULL) { - *out = OPENSSL_malloc(p7->ber_len); + *out = OPENSSL_memdup(p7->ber_bytes, p7->ber_len); if (*out == NULL) { return -1; } - OPENSSL_memcpy(*out, p7->ber_bytes, p7->ber_len); } else { OPENSSL_memcpy(*out, p7->ber_bytes, p7->ber_len); *out += p7->ber_len; diff --git a/crypto/pkcs8/pkcs8_x509.c b/crypto/pkcs8/pkcs8_x509.c index 92bdb9d17c..2d0bf088a4 100644 --- a/crypto/pkcs8/pkcs8_x509.c +++ b/crypto/pkcs8/pkcs8_x509.c @@ -741,26 +741,22 @@ struct pkcs12_st { PKCS12 *d2i_PKCS12(PKCS12 **out_p12, const uint8_t **ber_bytes, size_t ber_len) { - PKCS12 *p12; - - p12 = OPENSSL_malloc(sizeof(PKCS12)); + PKCS12 *p12 = OPENSSL_malloc(sizeof(PKCS12)); if (!p12) { return NULL; } - p12->ber_bytes = OPENSSL_malloc(ber_len); + p12->ber_bytes = OPENSSL_memdup(*ber_bytes, ber_len); if (!p12->ber_bytes) { OPENSSL_free(p12); return NULL; } - OPENSSL_memcpy(p12->ber_bytes, *ber_bytes, ber_len); p12->ber_len = ber_len; *ber_bytes += ber_len; if (out_p12) { PKCS12_free(*out_p12); - *out_p12 = p12; } @@ -843,11 +839,10 @@ int i2d_PKCS12(const PKCS12 *p12, uint8_t **out) { } if (*out == NULL) { - *out = OPENSSL_malloc(p12->ber_len); + *out = OPENSSL_memdup(p12->ber_bytes, p12->ber_len); if (*out == NULL) { return -1; } - OPENSSL_memcpy(*out, p12->ber_bytes, p12->ber_len); } else { OPENSSL_memcpy(*out, p12->ber_bytes, p12->ber_len); *out += p12->ber_len; diff --git a/crypto/pool/pool.c b/crypto/pool/pool.c index e889f521da..fc048409e4 100644 --- a/crypto/pool/pool.c +++ b/crypto/pool/pool.c @@ -42,12 +42,11 @@ static int CRYPTO_BUFFER_cmp(const CRYPTO_BUFFER *a, const CRYPTO_BUFFER *b) { } CRYPTO_BUFFER_POOL* CRYPTO_BUFFER_POOL_new(void) { - CRYPTO_BUFFER_POOL *pool = OPENSSL_malloc(sizeof(CRYPTO_BUFFER_POOL)); + CRYPTO_BUFFER_POOL *pool = OPENSSL_zalloc(sizeof(CRYPTO_BUFFER_POOL)); if (pool == NULL) { return NULL; } - OPENSSL_memset(pool, 0, sizeof(CRYPTO_BUFFER_POOL)); pool->bufs = lh_CRYPTO_BUFFER_new(CRYPTO_BUFFER_hash, CRYPTO_BUFFER_cmp); if (pool->bufs == NULL) { OPENSSL_free(pool); @@ -109,11 +108,10 @@ static CRYPTO_BUFFER *crypto_buffer_new(const uint8_t *data, size_t len, } } - CRYPTO_BUFFER *const buf = OPENSSL_malloc(sizeof(CRYPTO_BUFFER)); + CRYPTO_BUFFER *const buf = OPENSSL_zalloc(sizeof(CRYPTO_BUFFER)); if (buf == NULL) { return NULL; } - OPENSSL_memset(buf, 0, sizeof(CRYPTO_BUFFER)); if (data_is_static) { buf->data = (uint8_t *)data; @@ -170,11 +168,10 @@ CRYPTO_BUFFER *CRYPTO_BUFFER_new(const uint8_t *data, size_t len, } CRYPTO_BUFFER *CRYPTO_BUFFER_alloc(uint8_t **out_data, size_t len) { - CRYPTO_BUFFER *const buf = OPENSSL_malloc(sizeof(CRYPTO_BUFFER)); + CRYPTO_BUFFER *const buf = OPENSSL_zalloc(sizeof(CRYPTO_BUFFER)); if (buf == NULL) { return NULL; } - OPENSSL_memset(buf, 0, sizeof(CRYPTO_BUFFER)); buf->data = OPENSSL_malloc(len); if (len != 0 && buf->data == NULL) { diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c index a326eb785e..269959e865 100644 --- a/crypto/stack/stack.c +++ b/crypto/stack/stack.c @@ -84,19 +84,16 @@ struct stack_st { static const size_t kMinSize = 4; OPENSSL_STACK *OPENSSL_sk_new(OPENSSL_sk_cmp_func comp) { - OPENSSL_STACK *ret = OPENSSL_malloc(sizeof(OPENSSL_STACK)); + OPENSSL_STACK *ret = OPENSSL_zalloc(sizeof(OPENSSL_STACK)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(OPENSSL_STACK)); - ret->data = OPENSSL_malloc(sizeof(void *) * kMinSize); + ret->data = OPENSSL_zalloc(sizeof(void *) * kMinSize); if (ret->data == NULL) { goto err; } - OPENSSL_memset(ret->data, 0, sizeof(void *) * kMinSize); - ret->comp = comp; ret->num_alloc = kMinSize; @@ -370,19 +367,17 @@ OPENSSL_STACK *OPENSSL_sk_dup(const OPENSSL_STACK *sk) { return NULL; } - OPENSSL_STACK *ret = OPENSSL_malloc(sizeof(OPENSSL_STACK)); + OPENSSL_STACK *ret = OPENSSL_zalloc(sizeof(OPENSSL_STACK)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(OPENSSL_STACK)); - ret->data = OPENSSL_malloc(sizeof(void *) * sk->num_alloc); + ret->data = OPENSSL_memdup(sk->data, sizeof(void *) * sk->num_alloc); if (ret->data == NULL) { goto err; } ret->num = sk->num; - OPENSSL_memcpy(ret->data, sk->data, sizeof(void *) * sk->num); ret->sorted = sk->sorted; ret->num_alloc = sk->num_alloc; ret->comp = sk->comp; diff --git a/crypto/trust_token/trust_token.c b/crypto/trust_token/trust_token.c index 93172c37c2..521e7adc06 100644 --- a/crypto/trust_token/trust_token.c +++ b/crypto/trust_token/trust_token.c @@ -118,11 +118,10 @@ void TRUST_TOKEN_PRETOKEN_free(TRUST_TOKEN_PRETOKEN *pretoken) { } TRUST_TOKEN *TRUST_TOKEN_new(const uint8_t *data, size_t len) { - TRUST_TOKEN *ret = OPENSSL_malloc(sizeof(TRUST_TOKEN)); + TRUST_TOKEN *ret = OPENSSL_zalloc(sizeof(TRUST_TOKEN)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(TRUST_TOKEN)); ret->data = OPENSSL_memdup(data, len); if (len != 0 && ret->data == NULL) { OPENSSL_free(ret); @@ -205,11 +204,10 @@ TRUST_TOKEN_CLIENT *TRUST_TOKEN_CLIENT_new(const TRUST_TOKEN_METHOD *method, return NULL; } - TRUST_TOKEN_CLIENT *ret = OPENSSL_malloc(sizeof(TRUST_TOKEN_CLIENT)); + TRUST_TOKEN_CLIENT *ret = OPENSSL_zalloc(sizeof(TRUST_TOKEN_CLIENT)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(TRUST_TOKEN_CLIENT)); ret->method = method; ret->max_batchsize = (uint16_t)max_batchsize; return ret; @@ -446,11 +444,10 @@ TRUST_TOKEN_ISSUER *TRUST_TOKEN_ISSUER_new(const TRUST_TOKEN_METHOD *method, return NULL; } - TRUST_TOKEN_ISSUER *ret = OPENSSL_malloc(sizeof(TRUST_TOKEN_ISSUER)); + TRUST_TOKEN_ISSUER *ret = OPENSSL_zalloc(sizeof(TRUST_TOKEN_ISSUER)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(TRUST_TOKEN_ISSUER)); ret->method = method; ret->max_batchsize = (uint16_t)max_batchsize; return ret; diff --git a/crypto/x509/policy.c b/crypto/x509/policy.c index b0c27126c4..ce99599971 100644 --- a/crypto/x509/policy.c +++ b/crypto/x509/policy.c @@ -107,11 +107,10 @@ static void x509_policy_node_free(X509_POLICY_NODE *node) { static X509_POLICY_NODE *x509_policy_node_new(const ASN1_OBJECT *policy) { assert(!is_any_policy(policy)); - X509_POLICY_NODE *node = OPENSSL_malloc(sizeof(X509_POLICY_NODE)); + X509_POLICY_NODE *node = OPENSSL_zalloc(sizeof(X509_POLICY_NODE)); if (node == NULL) { return NULL; } - OPENSSL_memset(node, 0, sizeof(X509_POLICY_NODE)); node->policy = OBJ_dup(policy); node->parent_policies = sk_ASN1_OBJECT_new_null(); if (node->policy == NULL || node->parent_policies == NULL) { @@ -134,11 +133,10 @@ static void x509_policy_level_free(X509_POLICY_LEVEL *level) { } static X509_POLICY_LEVEL *x509_policy_level_new(void) { - X509_POLICY_LEVEL *level = OPENSSL_malloc(sizeof(X509_POLICY_LEVEL)); + X509_POLICY_LEVEL *level = OPENSSL_zalloc(sizeof(X509_POLICY_LEVEL)); if (level == NULL) { return NULL; } - OPENSSL_memset(level, 0, sizeof(X509_POLICY_LEVEL)); level->nodes = sk_X509_POLICY_NODE_new(x509_policy_node_cmp); if (level->nodes == NULL) { x509_policy_level_free(level); diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index c1389cc2c1..814f0ce9bb 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -164,10 +164,9 @@ static int x509_object_cmp_sk(const X509_OBJECT *const *a, X509_STORE *X509_STORE_new(void) { X509_STORE *ret; - if ((ret = (X509_STORE *)OPENSSL_malloc(sizeof(X509_STORE))) == NULL) { + if ((ret = (X509_STORE *)OPENSSL_zalloc(sizeof(X509_STORE))) == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(*ret)); CRYPTO_MUTEX_init(&ret->objs_lock); ret->objs = sk_X509_OBJECT_new(x509_object_cmp_sk); if (ret->objs == NULL) { diff --git a/crypto/x509/x509_vpm.c b/crypto/x509/x509_vpm.c index 583b4a050b..c13437d0cc 100644 --- a/crypto/x509/x509_vpm.c +++ b/crypto/x509/x509_vpm.c @@ -156,12 +156,10 @@ static void x509_verify_param_zero(X509_VERIFY_PARAM *param) { } X509_VERIFY_PARAM *X509_VERIFY_PARAM_new(void) { - X509_VERIFY_PARAM *param; - param = OPENSSL_malloc(sizeof(X509_VERIFY_PARAM)); + X509_VERIFY_PARAM *param = OPENSSL_zalloc(sizeof(X509_VERIFY_PARAM)); if (!param) { return NULL; } - OPENSSL_memset(param, 0, sizeof(X509_VERIFY_PARAM)); x509_verify_param_zero(param); return param; } diff --git a/crypto/x509/x_pkey.c b/crypto/x509/x_pkey.c index d48ecd111f..33a9aa91d2 100644 --- a/crypto/x509/x_pkey.c +++ b/crypto/x509/x_pkey.c @@ -67,11 +67,10 @@ X509_PKEY *X509_PKEY_new(void) { - X509_PKEY *ret = OPENSSL_malloc(sizeof(X509_PKEY)); + X509_PKEY *ret = OPENSSL_zalloc(sizeof(X509_PKEY)); if (ret == NULL) { goto err; } - OPENSSL_memset(ret, 0, sizeof(X509_PKEY)); ret->enc_algor = X509_ALGOR_new(); if (ret->enc_algor == NULL) { diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c index 31dbebe159..37a11c6d9c 100644 --- a/crypto/x509/x_x509.c +++ b/crypto/x509/x_x509.c @@ -92,11 +92,10 @@ IMPLEMENT_ASN1_FUNCTIONS(X509_CINF) // x509_new_null returns a new |X509| object where the |cert_info|, |sig_alg|, // and |signature| fields are not yet filled in. static X509 *x509_new_null(void) { - X509 *ret = OPENSSL_malloc(sizeof(X509)); + X509 *ret = OPENSSL_zalloc(sizeof(X509)); if (ret == NULL) { return NULL; } - OPENSSL_memset(ret, 0, sizeof(X509)); ret->references = 1; ret->ex_pathlen = -1; diff --git a/decrepit/bio/base64_bio.c b/decrepit/bio/base64_bio.c index eb87186faa..35218971e3 100644 --- a/decrepit/bio/base64_bio.c +++ b/decrepit/bio/base64_bio.c @@ -89,15 +89,11 @@ typedef struct b64_struct { } BIO_B64_CTX; static int b64_new(BIO *bio) { - BIO_B64_CTX *ctx; - - ctx = OPENSSL_malloc(sizeof(*ctx)); + BIO_B64_CTX *ctx = OPENSSL_zalloc(sizeof(*ctx)); if (ctx == NULL) { return 0; } - OPENSSL_memset(ctx, 0, sizeof(*ctx)); - ctx->cont = 1; ctx->start = 1; diff --git a/include/openssl/mem.h b/include/openssl/mem.h index 8da1dd67ed..cbbb11f01e 100644 --- a/include/openssl/mem.h +++ b/include/openssl/mem.h @@ -81,6 +81,10 @@ extern "C" { // the case of a malloc failure, prior to returning NULL |OPENSSL_malloc| will // push |ERR_R_MALLOC_FAILURE| onto the openssl error stack. OPENSSL_EXPORT void *OPENSSL_malloc(size_t size); + +// OPENSSL_zalloc behaves like |OPENSSL_malloc| except it also initializes the +// resulting memory to zero. +OPENSSL_EXPORT void *OPENSSL_zalloc(size_t size); #endif // !_BORINGSSL_PROHIBIT_OPENSSL_MALLOC // OPENSSL_free does nothing if |ptr| is NULL. Otherwise it zeros out the diff --git a/ssl/d1_both.cc b/ssl/d1_both.cc index 55c92fad8a..b910b96d9c 100644 --- a/ssl/d1_both.cc +++ b/ssl/d1_both.cc @@ -184,11 +184,10 @@ static UniquePtr dtls1_hm_fragment_new( return nullptr; } size_t bitmask_len = (msg_hdr->msg_len + 7) / 8; - frag->reassembly = (uint8_t *)OPENSSL_malloc(bitmask_len); + frag->reassembly = (uint8_t *)OPENSSL_zalloc(bitmask_len); if (frag->reassembly == NULL) { return nullptr; } - OPENSSL_memset(frag->reassembly, 0, bitmask_len); } return frag; diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index b97680d1fe..fc3d4c3f39 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -4725,8 +4725,8 @@ enum ssl_test_ticket_aead_failure_mode { }; struct ssl_test_ticket_aead_state { - unsigned retry_count; - ssl_test_ticket_aead_failure_mode failure_mode; + unsigned retry_count = 0; + ssl_test_ticket_aead_failure_mode failure_mode = ssl_test_ticket_aead_ok; }; static int ssl_test_ticket_aead_ex_index_dup(CRYPTO_EX_DATA *to, @@ -4739,12 +4739,7 @@ static int ssl_test_ticket_aead_ex_index_dup(CRYPTO_EX_DATA *to, static void ssl_test_ticket_aead_ex_index_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad, int index, long argl, void *argp) { - auto state = reinterpret_cast(ptr); - if (state == nullptr) { - return; - } - - OPENSSL_free(state); + delete reinterpret_cast(ptr); } static CRYPTO_once_t g_ssl_test_ticket_aead_ex_index_once = CRYPTO_ONCE_INIT; @@ -4835,10 +4830,7 @@ static void ConnectClientAndServerWithTicketMethod( SSL_set_connect_state(client.get()); SSL_set_accept_state(server.get()); - auto state = reinterpret_cast( - OPENSSL_malloc(sizeof(ssl_test_ticket_aead_state))); - ASSERT_TRUE(state); - OPENSSL_memset(state, 0, sizeof(ssl_test_ticket_aead_state)); + auto state = new ssl_test_ticket_aead_state; state->retry_count = retry_count; state->failure_mode = failure_mode; diff --git a/ssl/test/async_bio.cc b/ssl/test/async_bio.cc index 9eae290f1a..1c9859afee 100644 --- a/ssl/test/async_bio.cc +++ b/ssl/test/async_bio.cc @@ -108,11 +108,10 @@ static long AsyncCtrl(BIO *bio, int cmd, long num, void *ptr) { } static int AsyncNew(BIO *bio) { - AsyncBio *a = (AsyncBio *)OPENSSL_malloc(sizeof(*a)); + AsyncBio *a = (AsyncBio *)OPENSSL_zalloc(sizeof(*a)); if (a == NULL) { return 0; } - OPENSSL_memset(a, 0, sizeof(*a)); a->enforce_write_quota = true; bio->init = 1; bio->ptr = (char *)a; From 216db679a787e39e5a69a47a7440791d0da8b759 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 5 Oct 2023 10:42:15 -0400 Subject: [PATCH 04/11] Also add OPENSSL_calloc This is not in upstream OpenSSL but saves a bunch of manual overflow checks. Note it does also introduce some zeroing of buffers, but I think this should be fine here. Change-Id: I0c3e65ce2d21ee9d206ccbe3075ce5291c3acb30 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63365 Reviewed-by: Bob Beck Commit-Queue: David Benjamin --- crypto/asn1/tasn_enc.c | 7 +---- crypto/bytestring/cbb.c | 5 +--- crypto/evp/scrypt.c | 2 +- crypto/fipsmodule/bn/bn.c | 2 +- crypto/fipsmodule/ec/wnaf.c | 9 ++---- crypto/fipsmodule/rsa/rsa_impl.c | 2 +- crypto/lhash/lhash.c | 2 +- crypto/mem.c | 9 ++++++ crypto/stack/stack.c | 2 +- crypto/trust_token/pmbtoken.c | 32 +++++++-------------- crypto/trust_token/voprf.c | 48 ++++++++------------------------ include/openssl/mem.h | 15 ++++++---- 12 files changed, 49 insertions(+), 86 deletions(-) diff --git a/crypto/asn1/tasn_enc.c b/crypto/asn1/tasn_enc.c index e85400b2c8..bffc3cc498 100644 --- a/crypto/asn1/tasn_enc.c +++ b/crypto/asn1/tasn_enc.c @@ -452,14 +452,9 @@ static int asn1_set_seq_out(STACK_OF(ASN1_VALUE) *sk, unsigned char **out, return 1; } - if (sk_ASN1_VALUE_num(sk) > ((size_t)-1) / sizeof(DER_ENC)) { - OPENSSL_PUT_ERROR(ASN1, ERR_R_OVERFLOW); - return 0; - } - int ret = 0; unsigned char *const buf = OPENSSL_malloc(skcontlen); - DER_ENC *encoded = OPENSSL_malloc(sk_ASN1_VALUE_num(sk) * sizeof(*encoded)); + DER_ENC *encoded = OPENSSL_calloc(sk_ASN1_VALUE_num(sk), sizeof(*encoded)); if (encoded == NULL || buf == NULL) { goto err; } diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c index 5280dc8f9c..ac936c8156 100644 --- a/crypto/bytestring/cbb.c +++ b/crypto/bytestring/cbb.c @@ -649,16 +649,13 @@ int CBB_flush_asn1_set_of(CBB *cbb) { if (num_children < 2) { return 1; // Nothing to do. This is the common case for X.509. } - if (num_children > ((size_t)-1) / sizeof(CBS)) { - return 0; // Overflow. - } // Parse out the children and sort. We alias them into a copy of so they // remain valid as we rewrite |cbb|. int ret = 0; size_t buf_len = CBB_len(cbb); uint8_t *buf = OPENSSL_memdup(CBB_data(cbb), buf_len); - CBS *children = OPENSSL_malloc(num_children * sizeof(CBS)); + CBS *children = OPENSSL_calloc(num_children, sizeof(CBS)); if (buf == NULL || children == NULL) { goto err; } diff --git a/crypto/evp/scrypt.c b/crypto/evp/scrypt.c index 8212cd15ee..7d35597378 100644 --- a/crypto/evp/scrypt.c +++ b/crypto/evp/scrypt.c @@ -175,7 +175,7 @@ int EVP_PBE_scrypt(const char *password, size_t password_len, size_t B_bytes = B_blocks * sizeof(block_t); size_t T_blocks = 2 * r; size_t V_blocks = N * 2 * r; - block_t *B = OPENSSL_malloc((B_blocks + T_blocks + V_blocks) * sizeof(block_t)); + block_t *B = OPENSSL_calloc(B_blocks + T_blocks + V_blocks, sizeof(block_t)); if (B == NULL) { return 0; } diff --git a/crypto/fipsmodule/bn/bn.c b/crypto/fipsmodule/bn/bn.c index d7d86263d2..ecebcca500 100644 --- a/crypto/fipsmodule/bn/bn.c +++ b/crypto/fipsmodule/bn/bn.c @@ -361,7 +361,7 @@ int bn_wexpand(BIGNUM *bn, size_t words) { return 0; } - a = OPENSSL_malloc(sizeof(BN_ULONG) * words); + a = OPENSSL_calloc(words, sizeof(BN_ULONG)); if (a == NULL) { return 0; } diff --git a/crypto/fipsmodule/ec/wnaf.c b/crypto/fipsmodule/ec/wnaf.c index f5214b2472..225cdfe1d7 100644 --- a/crypto/fipsmodule/ec/wnaf.c +++ b/crypto/fipsmodule/ec/wnaf.c @@ -197,13 +197,8 @@ int ec_GFp_mont_mul_public_batch(const EC_GROUP *group, EC_JACOBIAN *r, wNAF = wNAF_stack; precomp = precomp_stack; } else { - if (num >= ((size_t)-1) / sizeof(wNAF_alloc[0]) || - num >= ((size_t)-1) / sizeof(precomp_alloc[0])) { - OPENSSL_PUT_ERROR(EC, ERR_R_OVERFLOW); - goto err; - } - wNAF_alloc = OPENSSL_malloc(num * sizeof(wNAF_alloc[0])); - precomp_alloc = OPENSSL_malloc(num * sizeof(precomp_alloc[0])); + wNAF_alloc = OPENSSL_calloc(num, sizeof(wNAF_alloc[0])); + precomp_alloc = OPENSSL_calloc(num, sizeof(precomp_alloc[0])); if (wNAF_alloc == NULL || precomp_alloc == NULL) { goto err; } diff --git a/crypto/fipsmodule/rsa/rsa_impl.c b/crypto/fipsmodule/rsa/rsa_impl.c index 6cdc290903..e847f9355c 100644 --- a/crypto/fipsmodule/rsa/rsa_impl.c +++ b/crypto/fipsmodule/rsa/rsa_impl.c @@ -376,7 +376,7 @@ static BN_BLINDING *rsa_blinding_get(RSA *rsa, size_t *index_used, assert(new_num_blindings > rsa->num_blindings); BN_BLINDING **new_blindings = - OPENSSL_malloc(sizeof(BN_BLINDING *) * new_num_blindings); + OPENSSL_calloc(new_num_blindings, sizeof(BN_BLINDING *)); uint8_t *new_blindings_inuse = OPENSSL_malloc(new_num_blindings); if (new_blindings == NULL || new_blindings_inuse == NULL) { goto err; diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c index 8e20c88d46..fbab430ad3 100644 --- a/crypto/lhash/lhash.c +++ b/crypto/lhash/lhash.c @@ -110,7 +110,7 @@ _LHASH *OPENSSL_lh_new(lhash_hash_func hash, lhash_cmp_func comp) { } ret->num_buckets = kMinNumBuckets; - ret->buckets = OPENSSL_zalloc(sizeof(LHASH_ITEM *) * ret->num_buckets); + ret->buckets = OPENSSL_calloc(ret->num_buckets, sizeof(LHASH_ITEM *)); if (ret->buckets == NULL) { OPENSSL_free(ret); return NULL; diff --git a/crypto/mem.c b/crypto/mem.c index b17267fc33..560922440b 100644 --- a/crypto/mem.c +++ b/crypto/mem.c @@ -275,6 +275,15 @@ void *OPENSSL_zalloc(size_t size) { return ret; } +void *OPENSSL_calloc(size_t num, size_t size) { + if (size != 0 && num > SIZE_MAX / size) { + OPENSSL_PUT_ERROR(CRYPTO, ERR_R_OVERFLOW); + return NULL; + } + + return OPENSSL_zalloc(num * size); +} + void OPENSSL_free(void *orig_ptr) { if (orig_ptr == NULL) { return; diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c index 269959e865..97fae1b9a3 100644 --- a/crypto/stack/stack.c +++ b/crypto/stack/stack.c @@ -89,7 +89,7 @@ OPENSSL_STACK *OPENSSL_sk_new(OPENSSL_sk_cmp_func comp) { return NULL; } - ret->data = OPENSSL_zalloc(sizeof(void *) * kMinSize); + ret->data = OPENSSL_calloc(kMinSize, sizeof(void *)); if (ret->data == NULL) { goto err; } diff --git a/crypto/trust_token/pmbtoken.c b/crypto/trust_token/pmbtoken.c index 5334a0c6f2..0aa4d0992a 100644 --- a/crypto/trust_token/pmbtoken.c +++ b/crypto/trust_token/pmbtoken.c @@ -799,18 +799,12 @@ static int pmbtoken_sign(const PMBTOKEN_METHOD *method, return 0; } - if (num_to_issue > ((size_t)-1) / sizeof(EC_JACOBIAN) || - num_to_issue > ((size_t)-1) / sizeof(EC_SCALAR)) { - OPENSSL_PUT_ERROR(TRUST_TOKEN, ERR_R_OVERFLOW); - return 0; - } - int ret = 0; - EC_JACOBIAN *Tps = OPENSSL_malloc(num_to_issue * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Sps = OPENSSL_malloc(num_to_issue * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Wps = OPENSSL_malloc(num_to_issue * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Wsps = OPENSSL_malloc(num_to_issue * sizeof(EC_JACOBIAN)); - EC_SCALAR *es = OPENSSL_malloc(num_to_issue * sizeof(EC_SCALAR)); + EC_JACOBIAN *Tps = OPENSSL_calloc(num_to_issue, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Sps = OPENSSL_calloc(num_to_issue, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Wps = OPENSSL_calloc(num_to_issue, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Wsps = OPENSSL_calloc(num_to_issue, sizeof(EC_JACOBIAN)); + EC_SCALAR *es = OPENSSL_calloc(num_to_issue, sizeof(EC_SCALAR)); CBB batch_cbb; CBB_zero(&batch_cbb); if (!Tps || @@ -940,19 +934,13 @@ static STACK_OF(TRUST_TOKEN) *pmbtoken_unblind( return NULL; } - if (count > ((size_t)-1) / sizeof(EC_JACOBIAN) || - count > ((size_t)-1) / sizeof(EC_SCALAR)) { - OPENSSL_PUT_ERROR(TRUST_TOKEN, ERR_R_OVERFLOW); - return NULL; - } - int ok = 0; STACK_OF(TRUST_TOKEN) *ret = sk_TRUST_TOKEN_new_null(); - EC_JACOBIAN *Tps = OPENSSL_malloc(count * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Sps = OPENSSL_malloc(count * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Wps = OPENSSL_malloc(count * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Wsps = OPENSSL_malloc(count * sizeof(EC_JACOBIAN)); - EC_SCALAR *es = OPENSSL_malloc(count * sizeof(EC_SCALAR)); + EC_JACOBIAN *Tps = OPENSSL_calloc(count, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Sps = OPENSSL_calloc(count, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Wps = OPENSSL_calloc(count, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Wsps = OPENSSL_calloc(count, sizeof(EC_JACOBIAN)); + EC_SCALAR *es = OPENSSL_calloc(count, sizeof(EC_SCALAR)); CBB batch_cbb; CBB_zero(&batch_cbb); if (ret == NULL || diff --git a/crypto/trust_token/voprf.c b/crypto/trust_token/voprf.c index c2ab815b1e..504deee534 100644 --- a/crypto/trust_token/voprf.c +++ b/crypto/trust_token/voprf.c @@ -483,16 +483,10 @@ static int voprf_sign_tt(const VOPRF_METHOD *method, return 0; } - if (num_to_issue > ((size_t)-1) / sizeof(EC_JACOBIAN) || - num_to_issue > ((size_t)-1) / sizeof(EC_SCALAR)) { - OPENSSL_PUT_ERROR(TRUST_TOKEN, ERR_R_OVERFLOW); - return 0; - } - int ret = 0; - EC_JACOBIAN *BTs = OPENSSL_malloc(num_to_issue * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Zs = OPENSSL_malloc(num_to_issue * sizeof(EC_JACOBIAN)); - EC_SCALAR *es = OPENSSL_malloc(num_to_issue * sizeof(EC_SCALAR)); + EC_JACOBIAN *BTs = OPENSSL_calloc(num_to_issue, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Zs = OPENSSL_calloc(num_to_issue, sizeof(EC_JACOBIAN)); + EC_SCALAR *es = OPENSSL_calloc(num_to_issue, sizeof(EC_SCALAR)); CBB batch_cbb; CBB_zero(&batch_cbb); if (!BTs || @@ -582,17 +576,11 @@ static STACK_OF(TRUST_TOKEN) *voprf_unblind_tt( return NULL; } - if (count > ((size_t)-1) / sizeof(EC_JACOBIAN) || - count > ((size_t)-1) / sizeof(EC_SCALAR)) { - OPENSSL_PUT_ERROR(TRUST_TOKEN, ERR_R_OVERFLOW); - return NULL; - } - int ok = 0; STACK_OF(TRUST_TOKEN) *ret = sk_TRUST_TOKEN_new_null(); - EC_JACOBIAN *BTs = OPENSSL_malloc(count * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Zs = OPENSSL_malloc(count * sizeof(EC_JACOBIAN)); - EC_SCALAR *es = OPENSSL_malloc(count * sizeof(EC_SCALAR)); + EC_JACOBIAN *BTs = OPENSSL_calloc(count, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Zs = OPENSSL_calloc(count, sizeof(EC_JACOBIAN)); + EC_SCALAR *es = OPENSSL_calloc(count, sizeof(EC_SCALAR)); CBB batch_cbb; CBB_zero(&batch_cbb); if (ret == NULL || @@ -868,16 +856,10 @@ static int voprf_sign_impl(const VOPRF_METHOD *method, return 0; } - if (num_to_issue > ((size_t)-1) / sizeof(EC_JACOBIAN) || - num_to_issue > ((size_t)-1) / sizeof(EC_SCALAR)) { - OPENSSL_PUT_ERROR(TRUST_TOKEN, ERR_R_OVERFLOW); - return 0; - } - int ret = 0; - EC_JACOBIAN *BTs = OPENSSL_malloc(num_to_issue * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Zs = OPENSSL_malloc(num_to_issue * sizeof(EC_JACOBIAN)); - EC_SCALAR *dis = OPENSSL_malloc(num_to_issue * sizeof(EC_SCALAR)); + EC_JACOBIAN *BTs = OPENSSL_calloc(num_to_issue, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Zs = OPENSSL_calloc(num_to_issue, sizeof(EC_JACOBIAN)); + EC_SCALAR *dis = OPENSSL_calloc(num_to_issue, sizeof(EC_SCALAR)); if (!BTs || !Zs || !dis) { goto err; } @@ -984,17 +966,11 @@ static STACK_OF(TRUST_TOKEN) *voprf_unblind( return NULL; } - if (count > ((size_t)-1) / sizeof(EC_JACOBIAN) || - count > ((size_t)-1) / sizeof(EC_SCALAR)) { - OPENSSL_PUT_ERROR(TRUST_TOKEN, ERR_R_OVERFLOW); - return NULL; - } - int ok = 0; STACK_OF(TRUST_TOKEN) *ret = sk_TRUST_TOKEN_new_null(); - EC_JACOBIAN *BTs = OPENSSL_malloc(count * sizeof(EC_JACOBIAN)); - EC_JACOBIAN *Zs = OPENSSL_malloc(count * sizeof(EC_JACOBIAN)); - EC_SCALAR *dis = OPENSSL_malloc(count * sizeof(EC_SCALAR)); + EC_JACOBIAN *BTs = OPENSSL_calloc(count, sizeof(EC_JACOBIAN)); + EC_JACOBIAN *Zs = OPENSSL_calloc(count, sizeof(EC_JACOBIAN)); + EC_SCALAR *dis = OPENSSL_calloc(count, sizeof(EC_SCALAR)); if (ret == NULL || !BTs || !Zs || !dis) { goto err; } diff --git a/include/openssl/mem.h b/include/openssl/mem.h index cbbb11f01e..c60ea17881 100644 --- a/include/openssl/mem.h +++ b/include/openssl/mem.h @@ -85,14 +85,12 @@ OPENSSL_EXPORT void *OPENSSL_malloc(size_t size); // OPENSSL_zalloc behaves like |OPENSSL_malloc| except it also initializes the // resulting memory to zero. OPENSSL_EXPORT void *OPENSSL_zalloc(size_t size); -#endif // !_BORINGSSL_PROHIBIT_OPENSSL_MALLOC -// OPENSSL_free does nothing if |ptr| is NULL. Otherwise it zeros out the -// memory allocated at |ptr| and frees it along with the private data. -// It must only be used on on |ptr| values obtained from |OPENSSL_malloc| -OPENSSL_EXPORT void OPENSSL_free(void *ptr); +// OPENSSL_calloc is similar to a regular |calloc|, but allocates data with +// |OPENSSL_malloc|. On overflow, it will push |ERR_R_OVERFLOW| onto the error +// queue. +OPENSSL_EXPORT void *OPENSSL_calloc(size_t num, size_t size); -#ifndef _BORINGSSL_PROHIBIT_OPENSSL_MALLOC // OPENSSL_realloc returns a pointer to a buffer of |new_size| bytes that // contains the contents of |ptr|. Unlike |realloc|, a new buffer is always // allocated and the data at |ptr| is always wiped and freed. Memory is @@ -100,6 +98,11 @@ OPENSSL_EXPORT void OPENSSL_free(void *ptr); OPENSSL_EXPORT void *OPENSSL_realloc(void *ptr, size_t new_size); #endif // !_BORINGSSL_PROHIBIT_OPENSSL_MALLOC +// OPENSSL_free does nothing if |ptr| is NULL. Otherwise it zeros out the +// memory allocated at |ptr| and frees it along with the private data. +// It must only be used on on |ptr| values obtained from |OPENSSL_malloc| +OPENSSL_EXPORT void OPENSSL_free(void *ptr); + // OPENSSL_cleanse zeros out |len| bytes of memory at |ptr|. This is similar to // |memset_s| from C11. OPENSSL_EXPORT void OPENSSL_cleanse(void *ptr, size_t len); From 8d0a83da9ae3b13b19720a63d198d8c0ea805ff3 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 5 Oct 2023 11:02:03 -0400 Subject: [PATCH 05/11] Consistently use SIZE_MAX over (size_t)-1 Change-Id: Idcf76ef4b5a023cf2a3fa71565c4aaddc921183d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63385 Reviewed-by: Bob Beck Commit-Queue: David Benjamin --- crypto/evp/scrypt.c | 2 +- crypto/fipsmodule/bn/ctx.c | 2 +- crypto/fipsmodule/bn/exponentiation.c | 2 +- crypto/fipsmodule/cipher/e_aesccm.c | 2 +- crypto/fipsmodule/dh/dh.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crypto/evp/scrypt.c b/crypto/evp/scrypt.c index 7d35597378..283026726a 100644 --- a/crypto/evp/scrypt.c +++ b/crypto/evp/scrypt.c @@ -170,7 +170,7 @@ int EVP_PBE_scrypt(const char *password, size_t password_len, // Allocate and divide up the scratch space. |max_mem| fits in a size_t, which // is no bigger than uint64_t, so none of these operations may overflow. - static_assert(UINT64_MAX >= ((size_t)-1), "size_t exceeds uint64_t"); + static_assert(UINT64_MAX >= SIZE_MAX, "size_t exceeds uint64_t"); size_t B_blocks = p * 2 * r; size_t B_bytes = B_blocks * sizeof(block_t); size_t T_blocks = 2 * r; diff --git a/crypto/fipsmodule/bn/ctx.c b/crypto/fipsmodule/bn/ctx.c index 007316110c..740fb78ca7 100644 --- a/crypto/fipsmodule/bn/ctx.c +++ b/crypto/fipsmodule/bn/ctx.c @@ -210,7 +210,7 @@ static int BN_STACK_push(BN_STACK *st, size_t idx) { // This function intentionally does not push to the error queue on error. // Error-reporting is deferred to |BN_CTX_get|. size_t new_size = st->size != 0 ? st->size * 3 / 2 : BN_CTX_START_FRAMES; - if (new_size <= st->size || new_size > ((size_t)-1) / sizeof(size_t)) { + if (new_size <= st->size || new_size > SIZE_MAX / sizeof(size_t)) { return 0; } size_t *new_indexes = diff --git a/crypto/fipsmodule/bn/exponentiation.c b/crypto/fipsmodule/bn/exponentiation.c index 41c7233540..632771eb95 100644 --- a/crypto/fipsmodule/bn/exponentiation.c +++ b/crypto/fipsmodule/bn/exponentiation.c @@ -724,7 +724,7 @@ void bn_mod_exp_mont_small(BN_ULONG *r, const BN_ULONG *a, size_t num, const BN_ULONG *p, size_t num_p, const BN_MONT_CTX *mont) { if (num != (size_t)mont->N.width || num > BN_SMALL_MAX_WORDS || - num_p > ((size_t)-1) / BN_BITS2) { + num_p > SIZE_MAX / BN_BITS2) { abort(); } assert(BN_is_odd(&mont->N)); diff --git a/crypto/fipsmodule/cipher/e_aesccm.c b/crypto/fipsmodule/cipher/e_aesccm.c index c00bf61efb..295aa056a5 100644 --- a/crypto/fipsmodule/cipher/e_aesccm.c +++ b/crypto/fipsmodule/cipher/e_aesccm.c @@ -86,7 +86,7 @@ static int CRYPTO_ccm128_init(struct ccm128_context *ctx, const AES_KEY *key, } static size_t CRYPTO_ccm128_max_input(const struct ccm128_context *ctx) { - return ctx->L >= sizeof(size_t) ? (size_t)-1 + return ctx->L >= sizeof(size_t) ? SIZE_MAX : (((size_t)1) << (ctx->L * 8)) - 1; } diff --git a/crypto/fipsmodule/dh/dh.c b/crypto/fipsmodule/dh/dh.c index d57b0935f2..39c6b8e9fd 100644 --- a/crypto/fipsmodule/dh/dh.c +++ b/crypto/fipsmodule/dh/dh.c @@ -394,7 +394,7 @@ int DH_compute_key(unsigned char *out, const BIGNUM *peers_key, DH *dh) { int DH_compute_key_hashed(DH *dh, uint8_t *out, size_t *out_len, size_t max_out_len, const BIGNUM *peers_key, const EVP_MD *digest) { - *out_len = (size_t)-1; + *out_len = SIZE_MAX; const size_t digest_len = EVP_MD_size(digest); if (digest_len > max_out_len) { From a1982649e01adb75ac7dcf400920ca7064dec352 Mon Sep 17 00:00:00 2001 From: Bob Beck Date: Thu, 5 Oct 2023 16:49:17 -0600 Subject: [PATCH 06/11] sync pki to chromium 248754f767a6df29d26ebb7da231f22713924a7d Bug: chromium:1322914 Change-Id: I51d74817533e6601867c5e7ef8157caf0f7a8fb5 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63405 Auto-Submit: Bob Beck Commit-Queue: Bob Beck Commit-Queue: Adam Langley Reviewed-by: Adam Langley --- pki/signature_algorithm.h | 2 +- pki/testdata/ssl/certificates/README | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/pki/signature_algorithm.h b/pki/signature_algorithm.h index dbb4fb48c5..bf7207f355 100644 --- a/pki/signature_algorithm.h +++ b/pki/signature_algorithm.h @@ -80,7 +80,7 @@ OPENSSL_EXPORT std::optional ParseSignatureAlgorithm( // Returns the hash to be used with the tls-server-end-point channel binding // (RFC 5929) or `std::nullopt`, if not supported for this signature algorithm. -std::optional GetTlsServerEndpointDigestAlgorithm( +OPENSSL_EXPORT std::optional GetTlsServerEndpointDigestAlgorithm( SignatureAlgorithm alg); } // namespace net diff --git a/pki/testdata/ssl/certificates/README b/pki/testdata/ssl/certificates/README index 6e3db64900..dcc938b2aa 100644 --- a/pki/testdata/ssl/certificates/README +++ b/pki/testdata/ssl/certificates/README @@ -166,14 +166,6 @@ unit tests. Certs to test that the maximum validity durations set by the CA/Browser Forum Baseline Requirements are enforced. -- pre_june_2016.pem -- post_june_2016.pem -- dec_2017.pem - Certs to test that policies related to enforcing CT on Symantec are - properly gated on the issuance date. See - https://g.co/chrome/symantecpkicerts. (Note, however, that the leaf and - root do not actually form a chain.) - - may_2018.pem An 825-day certificate issued on May 1, 2018, the official start of enforcement requiring Certificate Transparency for new certificates. This From 6d3db84c47643271cb553593ee67362be3820874 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 6 Oct 2023 11:38:44 -0400 Subject: [PATCH 07/11] Add new asm files to generate_build_files.py These were added in https://boringssl-review.googlesource.com/c/boringssl/+/63185, but we forgot to add them here too. I really gotta finish unifying our build systems. Change-Id: Id98b018dd978a190be782ad24e7ff7233b87f3d7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63425 Commit-Queue: Bob Beck Auto-Submit: David Benjamin Reviewed-by: Bob Beck --- util/generate_build_files.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/util/generate_build_files.py b/util/generate_build_files.py index 1dd1629d49..9f24a2b5db 100644 --- a/util/generate_build_files.py +++ b/util/generate_build_files.py @@ -48,6 +48,8 @@ ('apple', 'x86_64'): [ 'src/third_party/fiat/asm/fiat_curve25519_adx_mul.S', 'src/third_party/fiat/asm/fiat_curve25519_adx_square.S', + 'src/third_party/fiat/asm/fiat_p256_adx_mul.S', + 'src/third_party/fiat/asm/fiat_p256_adx_sqr.S', ], ('linux', 'arm'): [ 'src/crypto/curve25519/asm/x25519-asm-arm.S', @@ -57,6 +59,8 @@ 'src/crypto/hrss/asm/poly_rq_mul.S', 'src/third_party/fiat/asm/fiat_curve25519_adx_mul.S', 'src/third_party/fiat/asm/fiat_curve25519_adx_square.S', + 'src/third_party/fiat/asm/fiat_p256_adx_mul.S', + 'src/third_party/fiat/asm/fiat_p256_adx_sqr.S', ], } From 8650dc737d6f719800f1d7702542a423b303f7c8 Mon Sep 17 00:00:00 2001 From: Pete Bentley Date: Mon, 9 Oct 2023 13:36:25 +0100 Subject: [PATCH 08/11] Fix path for modifiable libcrypto used in KAT testing. In order to run KATs, we need a copy of libcrypto which won't abort on integrity check failure, or we'd never get to the KAT. Previously this was expected to be magically present in the current directory, but nowawdays we have a build rule for it so pick it up from the build artifacts. Change-Id: Ia5359b5369475d94bbde90e81353420a0f1efea2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63445 Commit-Queue: Adam Langley Auto-Submit: Pete Bentley Reviewed-by: Adam Langley --- util/fipstools/break-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/fipstools/break-tests.sh b/util/fipstools/break-tests.sh index 695b62931b..44ca2305e9 100644 --- a/util/fipstools/break-tests.sh +++ b/util/fipstools/break-tests.sh @@ -148,7 +148,7 @@ else # Device mode TEST_FIPS_BIN="$ANDROID_PRODUCT_OUT/system/bin/test_fips" check_file "$TEST_FIPS_BIN" LIBCRYPTO_BIN="$ANDROID_PRODUCT_OUT/system/lib64/libcrypto.so" - LIBCRYPTO_BREAK_BIN="libcrypto.so" + LIBCRYPTO_BREAK_BIN="$ANDROID_PRODUCT_OUT/system/lib64/libcrypto_for_testing.so" check_file "$LIBCRYPTO_BIN" check_file "$LIBCRYPTO_BREAK_BIN" From b975f12a2fa3f313b98b0dc40a9bc9e8c0667d08 Mon Sep 17 00:00:00 2001 From: Pete Bentley Date: Mon, 9 Oct 2023 14:17:54 +0100 Subject: [PATCH 09/11] Add support for 32bit break tests on Android. Change-Id: I6fa4a146756e22fbae4b057613188f90b6db5d7a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63465 Reviewed-by: Adam Langley Auto-Submit: Pete Bentley Commit-Queue: Adam Langley --- util/fipstools/break-tests.sh | 42 ++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/util/fipstools/break-tests.sh b/util/fipstools/break-tests.sh index 44ca2305e9..736d066413 100644 --- a/util/fipstools/break-tests.sh +++ b/util/fipstools/break-tests.sh @@ -46,20 +46,24 @@ inferred_mode() { fi } +MODE=`inferred_mode` # Prefer mode from command line if present. -case "$1" in - local|device) - MODE=$1 - ;; - - "") - MODE=`inferred_mode` - ;; - - *) - usage - ;; -esac +while [ "$1" ]; do + case "$1" in + local|device) + MODE=$1 + ;; + + "32") + TEST32BIT="true" + ;; + + *) + usage + ;; + esac + shift +done check_directory() { test -d "$1" || die "Directory $1 not found." @@ -145,10 +149,16 @@ else # Device mode test "$ANDROID_BUILD_TOP" || die "'lunch aosp_arm64-eng' first" check_directory "$ANDROID_PRODUCT_OUT" - TEST_FIPS_BIN="$ANDROID_PRODUCT_OUT/system/bin/test_fips" + if [ "$TEST32BIT" ]; then + TEST_FIPS_BIN="$ANDROID_PRODUCT_OUT/system/bin/test_fips32" + LIBCRYPTO_BIN="$ANDROID_PRODUCT_OUT/system/lib/libcrypto.so" + LIBCRYPTO_BREAK_BIN="$ANDROID_PRODUCT_OUT/system/lib/libcrypto_for_testing.so" + else + TEST_FIPS_BIN="$ANDROID_PRODUCT_OUT/system/bin/test_fips" + LIBCRYPTO_BIN="$ANDROID_PRODUCT_OUT/system/lib64/libcrypto.so" + LIBCRYPTO_BREAK_BIN="$ANDROID_PRODUCT_OUT/system/lib64/libcrypto_for_testing.so" + fi check_file "$TEST_FIPS_BIN" - LIBCRYPTO_BIN="$ANDROID_PRODUCT_OUT/system/lib64/libcrypto.so" - LIBCRYPTO_BREAK_BIN="$ANDROID_PRODUCT_OUT/system/lib64/libcrypto_for_testing.so" check_file "$LIBCRYPTO_BIN" check_file "$LIBCRYPTO_BREAK_BIN" From d94be4172325d0ac239ef3046d2c609aebeaf00a Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 10 Oct 2023 11:00:52 -0400 Subject: [PATCH 10/11] Update tools on CI Also document in comments why the NDK cannot be updated right now. The libc++ update requires configuring PSTL. I matched Chromium in https://chromium-review.googlesource.com/c/chromium/src/+/4864131 Change-Id: Ic374a8a95f9f6f23266704d69c11b835cd718d94 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63505 Commit-Queue: Adam Langley Reviewed-by: Adam Langley Auto-Submit: David Benjamin --- util/bot/DEPS | 27 +++++++++++++++++++-------- util/bot/libcxx-config/__config_site | 6 ++++++ util/bot/update_clang.py | 4 ++-- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/util/bot/DEPS b/util/bot/DEPS index 8f5a412df8..b645ca34f8 100644 --- a/util/bot/DEPS +++ b/util/bot/DEPS @@ -16,6 +16,7 @@ vars = { 'chromium_git': 'https://chromium.googlesource.com', 'checkout_clang': False, + 'checkout_fuzzer': False, 'checkout_sde': False, 'checkout_nasm': False, 'checkout_libcxx': False, @@ -25,20 +26,30 @@ vars = { # cipd describe PACKAGE_NAME -version latest # infra/3pp/tools/cmake/linux-amd64 - 'cmake_version': 'version:2@3.26.4.chromium.7', + 'cmake_version': 'version:2@3.27.7.chromium.8', # infra/3pp/tools/go/linux-amd64 - 'go_version': 'version:2@1.20.5', + 'go_version': 'version:2@1.21.2', # infra/3pp/tools/perl/windows-amd64 'perl_version': 'version:2@5.32.1.1', # Update the following from # https://chromium.googlesource.com/chromium/src/+/main/DEPS - 'android_sdk_platform-tools_version': 'RSI3iwryh7URLGRgJHsCvUxj092woTPnKt4pwFcJ6L8C', - 'android_ndk_revision': '310956bd122ec2b96049f8d7398de6b717f3452e', - 'libfuzzer_revision': 'debe7d2d1982e540fbd6bd78604bf001753f9e74', - 'libcxx_revision': 'f8279b01085b800724f5c5629dc365b9f040dc53', - 'libcxxabi_revision': '899caea3814eeb45c689fc206052968943fd5cb8', + 'android_sdk_platform-tools_version': 'HWVsGs2HCKgSVv41FsOcsfJbNcB0UFiNrF6Tc4yRArYC', + 'libfuzzer_revision': '758bd21f103a501b362b1ca46fa8fcb692eaa303', + 'libcxx_revision': '8fc17971d629c19a17b006d0c4fc41e721cc2f7f', + 'libcxxabi_revision': 'db9800c042df3ee2691031a58b5e37e89a7356a3', 'ninja_version': 'version:2@1.11.1.chromium.6', + + # The Android NDK cannot be updated on CI for two reasons: + # + # Until https://crbug.com/boringssl/454 is fixed, we rely on an older NDK to + # test building without NEON instructions as the baseline. + # + # Until https://crbug.com/boringssl/653 is fixed, we cannot update past + # Chromium's version:2@r25c.cr0 package. Chromium has since switched building + # minimal CIPD packages which do not contain all the NDK files we need. We'll + # probably need to make our own NDK package. + 'android_ndk_revision': '310956bd122ec2b96049f8d7398de6b717f3452e', } deps = { @@ -82,7 +93,7 @@ deps = { }, 'boringssl/util/bot/libFuzzer': { - 'url': Var('chromium_git') + '/chromium/llvm-project/compiler-rt/lib/fuzzer.git' +'@' + Var('libfuzzer_revision'), + 'url': Var('chromium_git') + '/external/github.com/llvm/llvm-project/compiler-rt/lib/fuzzer.git' +'@' + Var('libfuzzer_revision'), 'condition': 'checkout_fuzzer', }, diff --git a/util/bot/libcxx-config/__config_site b/util/bot/libcxx-config/__config_site index f1feeab196..eb16b9d44e 100644 --- a/util/bot/libcxx-config/__config_site +++ b/util/bot/libcxx-config/__config_site @@ -3,4 +3,10 @@ #define _LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS +#if defined(__APPLE__) +#define _LIBCPP_PSTL_CPU_BACKEND_LIBDISPATCH +#else +#define _LIBCPP_PSTL_CPU_BACKEND_THREAD +#endif + #endif // BORINGSSL_LIBCXX_CONFIG_SITE_ diff --git a/util/bot/update_clang.py b/util/bot/update_clang.py index d0b188adbd..96066f056f 100644 --- a/util/bot/update_clang.py +++ b/util/bot/update_clang.py @@ -29,8 +29,8 @@ # CLANG_REVISION and CLANG_SUB_REVISION determine the build of clang # to use. These should be synced with tools/clang/scripts/update.py in # Chromium. -CLANG_REVISION = 'llvmorg-17-init-12166-g7586aeab' -CLANG_SUB_REVISION = 3 +CLANG_REVISION = 'llvmorg-18-init-7785-geef35c28' +CLANG_SUB_REVISION = 1 PACKAGE_VERSION = '%s-%s' % (CLANG_REVISION, CLANG_SUB_REVISION) From 8313e13cde1df3380317a3629f1d591635293592 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 11 Oct 2023 12:51:36 -0400 Subject: [PATCH 11/11] Replace remaining references to __ARM_ARCH__ with __ARM_ARCH Since ACLE is now widely implemented, just use the standard one. I've left a TODO with __ARM_MAX_ARCH__. Probably should just remove that one? Also deduplicate some code between arm_arch.h and asm_base.h. I think I meant to move it to asm_base.h and didn't finish the job? Change-Id: I85bb3160ec64acdabd11d741f1958ff56199c4c7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63525 Commit-Queue: David Benjamin Auto-Submit: David Benjamin Reviewed-by: Adam Langley --- crypto/chacha/asm/chacha-armv4.pl | 18 ++++++++-------- crypto/fipsmodule/aes/asm/bsaes-armv7.pl | 1 - crypto/fipsmodule/bn/asm/armv4-mont.pl | 2 +- crypto/fipsmodule/sha/asm/sha1-armv4-large.pl | 4 ++-- crypto/fipsmodule/sha/asm/sha256-armv4.pl | 14 ++++++------- crypto/fipsmodule/sha/asm/sha512-armv4.pl | 11 +++++----- include/openssl/arm_arch.h | 21 ------------------- include/openssl/asm_base.h | 11 +++++----- 8 files changed, 29 insertions(+), 53 deletions(-) diff --git a/crypto/chacha/asm/chacha-armv4.pl b/crypto/chacha/asm/chacha-armv4.pl index 5c78a9fc7e..1f5ceffb26 100755 --- a/crypto/chacha/asm/chacha-armv4.pl +++ b/crypto/chacha/asm/chacha-armv4.pl @@ -210,7 +210,7 @@ sub ROUND { .LChaCha20_ctr32: ldr r12,[sp,#0] @ pull pointer to counter and nonce stmdb sp!,{r0-r2,r4-r11,lr} -#if __ARM_ARCH__<7 && !defined(__thumb2__) +#if __ARM_ARCH<7 && !defined(__thumb2__) sub r14,pc,#16 @ ChaCha20_ctr32 #else adr r14,.LChaCha20_ctr32 @@ -292,8 +292,8 @@ sub ROUND { ldr @t[0],[sp,#4*(0)] @ load key material ldr @t[1],[sp,#4*(1)] -#if __ARM_ARCH__>=6 || !defined(__ARMEB__) -# if __ARM_ARCH__<7 +#if __ARM_ARCH>=6 || !defined(__ARMEB__) +# if __ARM_ARCH<7 orr @t[2],r12,r14 tst @t[2],#3 @ are input and output aligned? ldr @t[2],[sp,#4*(2)] @@ -319,7 +319,7 @@ sub ROUND { # endif ldrhs @t[2],[r12,#-8] ldrhs @t[3],[r12,#-4] -# if __ARM_ARCH__>=6 && defined(__ARMEB__) +# if __ARM_ARCH>=6 && defined(__ARMEB__) rev @x[0],@x[0] rev @x[1],@x[1] rev @x[2],@x[2] @@ -356,7 +356,7 @@ sub ROUND { # endif ldrhs @t[2],[r12,#-8] ldrhs @t[3],[r12,#-4] -# if __ARM_ARCH__>=6 && defined(__ARMEB__) +# if __ARM_ARCH>=6 && defined(__ARMEB__) rev @x[4],@x[4] rev @x[5],@x[5] rev @x[6],@x[6] @@ -401,7 +401,7 @@ sub ROUND { # endif ldrhs @t[2],[r12,#-8] ldrhs @t[3],[r12,#-4] -# if __ARM_ARCH__>=6 && defined(__ARMEB__) +# if __ARM_ARCH>=6 && defined(__ARMEB__) rev @x[0],@x[0] rev @x[1],@x[1] rev @x[2],@x[2] @@ -443,7 +443,7 @@ sub ROUND { # endif ldrhs @t[2],[r12,#-8] ldrhs @t[3],[r12,#-4] -# if __ARM_ARCH__>=6 && defined(__ARMEB__) +# if __ARM_ARCH>=6 && defined(__ARMEB__) rev @x[4],@x[4] rev @x[5],@x[5] rev @x[6],@x[6] @@ -474,7 +474,7 @@ sub ROUND { bhi .Loop_outer beq .Ldone -# if __ARM_ARCH__<7 +# if __ARM_ARCH<7 b .Ltail .align 4 @@ -482,7 +482,7 @@ sub ROUND { cmp @t[3],#64 @ restore flags # endif #endif -#if __ARM_ARCH__<7 +#if __ARM_ARCH<7 ldr @t[3],[sp,#4*(3)] ___ for ($i=0;$i<16;$i+=4) { diff --git a/crypto/fipsmodule/aes/asm/bsaes-armv7.pl b/crypto/fipsmodule/aes/asm/bsaes-armv7.pl index c537730f1a..fd6272d916 100644 --- a/crypto/fipsmodule/aes/asm/bsaes-armv7.pl +++ b/crypto/fipsmodule/aes/asm/bsaes-armv7.pl @@ -718,7 +718,6 @@ sub bitslice { # define VFP_ABI_FRAME 0 # define BSAES_ASM_EXTENDED_KEY # define XTS_CHAIN_TWEAK -# define __ARM_ARCH__ __LINUX_ARM_ARCH__ # define __ARM_MAX_ARCH__ 7 #endif diff --git a/crypto/fipsmodule/bn/asm/armv4-mont.pl b/crypto/fipsmodule/bn/asm/armv4-mont.pl index 207b8e4c3e..dcbaee5e99 100644 --- a/crypto/fipsmodule/bn/asm/armv4-mont.pl +++ b/crypto/fipsmodule/bn/asm/armv4-mont.pl @@ -285,7 +285,7 @@ add sp,sp,#2*4 @ skip over {r0,r2} mov r0,#1 .Labrt: -#if __ARM_ARCH__>=5 +#if __ARM_ARCH>=5 ret @ bx lr #else tst lr,#1 diff --git a/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl b/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl index 2998b89715..c52b546f72 100644 --- a/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl +++ b/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl @@ -132,7 +132,7 @@ sub Xupdate { sub BODY_00_15 { my ($a,$b,$c,$d,$e)=@_; $code.=<<___; -#if __ARM_ARCH__<7 +#if __ARM_ARCH<7 ldrb $t1,[$inp,#2] ldrb $t0,[$inp,#3] ldrb $t2,[$inp,#1] @@ -296,7 +296,7 @@ sub BODY_40_59 { teq $inp,$len bne .Lloop @ [+18], total 1307 -#if __ARM_ARCH__>=5 +#if __ARM_ARCH>=5 ldmia sp!,{r4-r12,pc} #else ldmia sp!,{r4-r12,lr} diff --git a/crypto/fipsmodule/sha/asm/sha256-armv4.pl b/crypto/fipsmodule/sha/asm/sha256-armv4.pl index 0f459e06bc..6812b27a7e 100644 --- a/crypto/fipsmodule/sha/asm/sha256-armv4.pl +++ b/crypto/fipsmodule/sha/asm/sha256-armv4.pl @@ -86,7 +86,7 @@ sub BODY_00_15 { my ($i,$a,$b,$c,$d,$e,$f,$g,$h) = @_; $code.=<<___ if ($i<16); -#if __ARM_ARCH__>=7 +#if __ARM_ARCH>=7 @ ldr $t1,[$inp],#4 @ $i # if $i==15 str $inp,[sp,#17*4] @ make room for $t4 @@ -129,7 +129,7 @@ sub BODY_00_15 { cmp $t2,#0xf2 @ done? #endif #if $i<15 -# if __ARM_ARCH__>=7 +# if __ARM_ARCH>=7 ldr $t1,[$inp],#4 @ prefetch # else ldrb $t1,[$inp,#3] @@ -179,7 +179,7 @@ sub BODY_16_XX { #ifndef __KERNEL__ # include #else -# define __ARM_ARCH__ __LINUX_ARM_ARCH__ +# define __ARM_ARCH __LINUX_ARM_ARCH__ # define __ARM_MAX_ARCH__ 7 #endif @@ -227,7 +227,7 @@ sub BODY_16_XX { .type sha256_block_data_order,%function sha256_block_data_order: .Lsha256_block_data_order: -#if __ARM_ARCH__<7 && !defined(__thumb2__) +#if __ARM_ARCH<7 && !defined(__thumb2__) sub r3,pc,#8 @ sha256_block_data_order #else adr r3,.Lsha256_block_data_order @@ -249,7 +249,7 @@ sub BODY_16_XX { sub $Ktbl,r3,#256+32 @ K256 sub sp,sp,#16*4 @ alloca(X[16]) .Loop: -# if __ARM_ARCH__>=7 +# if __ARM_ARCH>=7 ldr $t1,[$inp],#4 # else ldrb $t1,[$inp,#3] @@ -261,7 +261,7 @@ sub BODY_16_XX { $code.=".Lrounds_16_xx:\n"; for (;$i<32;$i++) { &BODY_16_XX($i,@V); unshift(@V,pop(@V)); } $code.=<<___; -#if __ARM_ARCH__>=7 +#if __ARM_ARCH>=7 ite eq @ Thumb2 thing, sanity check in ARM #endif ldreq $t3,[sp,#16*4] @ pull ctx @@ -292,7 +292,7 @@ sub BODY_16_XX { bne .Loop add sp,sp,#`16+3`*4 @ destroy frame -#if __ARM_ARCH__>=5 +#if __ARM_ARCH>=5 ldmia sp!,{r4-r11,pc} #else ldmia sp!,{r4-r11,lr} diff --git a/crypto/fipsmodule/sha/asm/sha512-armv4.pl b/crypto/fipsmodule/sha/asm/sha512-armv4.pl index 185635fcd1..d470dafadc 100644 --- a/crypto/fipsmodule/sha/asm/sha512-armv4.pl +++ b/crypto/fipsmodule/sha/asm/sha512-armv4.pl @@ -159,7 +159,7 @@ () teq $t0,#$magic ldr $t3,[sp,#$Coff+0] @ c.lo -#if __ARM_ARCH__>=7 +#if __ARM_ARCH>=7 it eq @ Thumb2 thing, sanity check in ARM #endif orreq $Ktbl,$Ktbl,#1 @@ -204,7 +204,6 @@ () # define VFP_ABI_PUSH vstmdb sp!,{d8-d15} # define VFP_ABI_POP vldmia sp!,{d8-d15} #else -# define __ARM_ARCH__ __LINUX_ARM_ARCH__ # define __ARM_MAX_ARCH__ 7 # define VFP_ABI_PUSH # define VFP_ABI_POP @@ -289,7 +288,7 @@ () .type sha512_block_data_order,%function sha512_block_data_order: .Lsha512_block_data_order: -#if __ARM_ARCH__<7 && !defined(__thumb2__) +#if __ARM_ARCH<7 && !defined(__thumb2__) sub r3,pc,#8 @ sha512_block_data_order #else adr r3,.Lsha512_block_data_order @@ -339,7 +338,7 @@ () str $Thi,[sp,#$Foff+4] .L00_15: -#if __ARM_ARCH__<7 +#if __ARM_ARCH<7 ldrb $Tlo,[$inp,#7] ldrb $t0, [$inp,#6] ldrb $t1, [$inp,#5] @@ -417,7 +416,7 @@ () ___ &BODY_00_15(0x17); $code.=<<___; -#if __ARM_ARCH__>=7 +#if __ARM_ARCH>=7 ittt eq @ Thumb2 thing, sanity check in ARM #endif ldreq $t0,[sp,#`$Xoff+8*(16-1)`+0] @@ -496,7 +495,7 @@ () bne .Loop add sp,sp,#8*9 @ destroy frame -#if __ARM_ARCH__>=5 +#if __ARM_ARCH>=5 ldmia sp!,{r4-r12,pc} #else ldmia sp!,{r4-r12,lr} diff --git a/include/openssl/arm_arch.h b/include/openssl/arm_arch.h index 60b30f5d98..f63613008a 100644 --- a/include/openssl/arm_arch.h +++ b/include/openssl/arm_arch.h @@ -79,27 +79,6 @@ // ARMV8_SHA512 indicates support for hardware SHA-512 instructions. #define ARMV8_SHA512 (1 << 6) -#if defined(__ASSEMBLER__) - -// We require the ARM assembler provide |__ARM_ARCH| from Arm C Language -// Extensions (ACLE). This is supported in GCC 4.8+ and Clang 3.2+. MSVC does -// not implement ACLE, but we require Clang's assembler on Windows. -#if !defined(__ARM_ARCH) -#error "ARM assembler must define __ARM_ARCH" -#endif - -// __ARM_ARCH__ is used by OpenSSL assembly to determine the minimum target ARM -// version. -// -// TODO(davidben): Switch the assembly to use |__ARM_ARCH| directly. -#define __ARM_ARCH__ __ARM_ARCH - -// Even when building for 32-bit ARM, support for aarch64 crypto instructions -// will be included. -#define __ARM_MAX_ARCH__ 8 - -#endif // __ASSEMBLER__ - #endif // ARM || AARCH64 #endif // OPENSSL_HEADER_ARM_ARCH_H diff --git a/include/openssl/asm_base.h b/include/openssl/asm_base.h index 9eb31818a8..e6b95dfa6b 100644 --- a/include/openssl/asm_base.h +++ b/include/openssl/asm_base.h @@ -75,14 +75,13 @@ #error "ARM assembler must define __ARM_ARCH" #endif -// __ARM_ARCH__ is used by OpenSSL assembly to determine the minimum target ARM -// version. -// -// TODO(davidben): Switch the assembly to use |__ARM_ARCH| directly. -#define __ARM_ARCH__ __ARM_ARCH - // Even when building for 32-bit ARM, support for aarch64 crypto instructions // will be included. +// +// TODO(davidben): Remove this and the corresponding ifdefs? This is only +// defined because some OpenSSL assembly files would allow disabling the NEON +// code entirely. I think we'd prefer to do that by lifting the dispatch to C +// anyway. #define __ARM_MAX_ARCH__ 8 // Support macros for