Skip to content

Commit

Permalink
MSRV cleanup; Remove mirai-annotations dependency (#609)
Browse files Browse the repository at this point in the history
* Revert MSRV violations

* Remove mirai_annotations dependency

* Clippy fixes

* Remove unsustainable test
  • Loading branch information
justsmth authored Nov 21, 2024
1 parent 9648464 commit 91689c4
Show file tree
Hide file tree
Showing 13 changed files with 25 additions and 126 deletions.
34 changes: 0 additions & 34 deletions .github/workflows/analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -205,40 +205,6 @@ jobs:
exit 0
}
mirai-analysis:
if: github.repository_owner == 'aws'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
submodules: 'recursive'
lfs: true

- uses: dtolnay/rust-toolchain@master
id: toolchain
with:
toolchain: ${{ env.MIRAI_TOOLCHAIN }}
components: rust-src, rustc-dev, llvm-tools-preview
- name: Set Rust toolchain override
run: rustup override set ${{ steps.toolchain.outputs.name }}

# https://github.com/facebookexperimental/MIRAI/blob/main/documentation/InstallationGuide.md#installing-mirai-into-cargo
- name: Install MIRAI
run: |
MIRAI_TMP_SRC=$(mktemp -d)
git clone --depth 1 --branch ${{ env.MIRAI_TAG }} https://github.com/facebookexperimental/MIRAI.git ${MIRAI_TMP_SRC}
pushd ${MIRAI_TMP_SRC}
cargo install --locked --force --path ./checker --no-default-features
popd
rm -rf ${MIRAI_TMP_SRC}
- name: Run MIRAI
working-directory: ./aws-lc-rs
run: |
cargo update
cargo update -p clap --precise 4.4.18
cargo mirai
minimal-versions:
if: github.repository_owner == 'aws'
name: Resolve the dependencies to the minimum SemVer version
Expand Down
4 changes: 3 additions & 1 deletion aws-lc-fips-sys/builder/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// Modifications copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR ISC

// Needed until MSRV >= 1.70
#![allow(clippy::unnecessary_map_or)]
#![allow(clippy::ref_option)]
// Clippy can only be run on nightly toolchain
#![cfg_attr(clippy, feature(custom_inner_attributes))]
Expand Down Expand Up @@ -550,7 +552,7 @@ fn setup_include_paths(out_dir: &Path, manifest_dir: &Path) -> PathBuf {
// iterate over all the include paths and copy them into the final output
for path in include_paths {
for child in std::fs::read_dir(path).into_iter().flatten().flatten() {
if child.file_type().is_ok_and(|t| t.is_file()) {
if child.file_type().map_or(false, |t| t.is_file()) {
let _ = std::fs::copy(
child.path(),
include_dir.join(child.path().file_name().unwrap()),
Expand Down
1 change: 0 additions & 1 deletion aws-lc-rs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ untrusted = { version = "0.7.1", optional = true }
aws-lc-sys = { version = "0.23.0", path = "../aws-lc-sys", optional = true }
aws-lc-fips-sys = { version = "0.12.0", path = "../aws-lc-fips-sys", optional = true }
zeroize = "1.7"
mirai-annotations = "1.12.0"
paste = "1.0.11"

[dev-dependencies]
Expand Down
5 changes: 1 addition & 4 deletions aws-lc-rs/src/bn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use crate::ptr::{ConstPointer, DetachableLcPtr, LcPtr};
use aws_lc::{BN_bin2bn, BN_bn2bin, BN_new, BN_num_bits, BN_num_bytes, BN_set_u64, BIGNUM};
use core::ptr::null_mut;
use mirai_annotations::unrecoverable;

impl TryFrom<&[u8]> for LcPtr<BIGNUM> {
type Error = ();
Expand Down Expand Up @@ -56,9 +55,7 @@ impl ConstPointer<BIGNUM> {
let bn_bytes = BN_num_bytes(**self);
let mut byte_vec = Vec::with_capacity(bn_bytes as usize);
let out_bytes = BN_bn2bin(**self, byte_vec.as_mut_ptr());
if out_bytes != (bn_bytes as usize) {
unrecoverable!("More bytes written than allocated.");
}
debug_assert_eq!(out_bytes, bn_bytes as usize);
byte_vec.set_len(out_bytes);
byte_vec
}
Expand Down
6 changes: 2 additions & 4 deletions aws-lc-rs/src/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ use aws_lc::{
RSA,
};

use mirai_annotations::verify_unreachable;

pub(crate) type LcPtr<T> = ManagedPointer<*mut T>;
pub(crate) type DetachableLcPtr<T> = DetachablePointer<*mut T>;

Expand Down Expand Up @@ -100,7 +98,7 @@ impl<P: Pointer> Deref for DetachablePointer<P> {
Some(pointer) => pointer,
None => {
// Safety: pointer is only None when DetachableLcPtr is detached or dropped
verify_unreachable!()
unreachable!()
}
}
}
Expand Down Expand Up @@ -131,7 +129,7 @@ impl<P: Pointer> From<DetachablePointer<P>> for ManagedPointer<P> {
Some(pointer) => ManagedPointer { pointer },
None => {
// Safety: pointer is only None when DetachableLcPtr is detached or dropped
verify_unreachable!()
unreachable!()
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions aws-lc-rs/src/rsa/encryption/oaep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use aws_lc::{
EVP_PKEY_CTX, RSA_PKCS1_OAEP_PADDING,
};
use core::{fmt::Debug, mem::size_of_val, ptr::null_mut};
use mirai_annotations::verify_unreachable;

/// RSA-OAEP with SHA1 Hash and SHA1 MGF1
pub const OAEP_SHA1_MGF1SHA1: OaepAlgorithm = OaepAlgorithm {
Expand Down Expand Up @@ -163,7 +162,7 @@ impl OaepPublicEncryptingKey {
EncryptionAlgorithmId::OaepSha256Mgf1sha256 => 32,
EncryptionAlgorithmId::OaepSha384Mgf1sha384 => 48,
EncryptionAlgorithmId::OaepSha512Mgf1sha512 => 64,
_ => verify_unreachable!(),
_ => unreachable!(),
};

// The RSA-OAEP algorithms we support use the hashing algorithm for the hash and mgf1 functions.
Expand Down
3 changes: 1 addition & 2 deletions aws-lc-rs/src/rsa/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ use core::{
// use core::ffi::c_int;
use std::os::raw::c_int;

use mirai_annotations::verify_unreachable;
#[cfg(feature = "ring-io")]
use untrusted::Input;
use zeroize::Zeroize;
Expand Down Expand Up @@ -263,7 +262,7 @@ impl KeyPair {
// https://github.com/awslabs/aws-lc/blob/main/include/openssl/rsa.h#L99
unsafe { RSA_size(*rsa.as_const()) as usize }
}
Err(_) => verify_unreachable!(),
Err(_) => unreachable!(),
}
}
}
Expand Down
18 changes: 8 additions & 10 deletions aws-lc-rs/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@

extern crate alloc;

use mirai_annotations::unrecoverable;
use std::error::Error;

use crate::{digest, error};
Expand Down Expand Up @@ -183,7 +182,7 @@ impl TestCase {
"SHA3_256" => Some(&digest::SHA3_256),
"SHA3_384" => Some(&digest::SHA3_384),
"SHA3_512" => Some(&digest::SHA3_512),
_ => unrecoverable!("Unsupported digest algorithm: {}", name),
_ => unreachable!("Unsupported digest algorithm: {}", name),
}
}

Expand Down Expand Up @@ -213,20 +212,19 @@ impl TestCase {
Some(b't') => b'\t',
Some(b'n') => b'\n',
_ => {
unrecoverable!("Invalid hex escape sequence in string.");
panic!("Invalid hex escape sequence in string.");
}
}
}
Some(b'"') => {
if s.next().is_some() {
unrecoverable!(
"characters after the closing quote of a quoted string."
);
}
assert!(
s.next().is_none(),
"characters after the closing quote of a quoted string."
);
break;
}
Some(b) => *b,
None => unrecoverable!("Missing terminating '\"' in string literal."),
None => panic!("Missing terminating '\"' in string literal."),
};
bytes.push(b);
}
Expand All @@ -236,7 +234,7 @@ impl TestCase {
match from_hex(&s) {
Ok(s) => s,
Err(err_str) => {
unrecoverable!("{} in {}", err_str, s);
panic!("{err_str} in {s}");
}
}
};
Expand Down
3 changes: 1 addition & 2 deletions aws-lc-rs/tests/aead_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use aws_lc_rs::{aead, error, test, test_file};

use aws_lc_rs::aead::{Nonce, NONCE_LEN};
use core::ops::RangeFrom;
use mirai_annotations::unrecoverable;

#[test]
fn aead_aes_gcm_128() {
Expand Down Expand Up @@ -230,7 +229,7 @@ fn test_aead<Seal, Open>(
assert_eq!(Err(error::Unspecified), o_result);
}
Some(error) => {
unrecoverable!("Unexpected error test case: {}", error);
panic!("Unexpected error test case: {error}");
}
};
}
Expand Down
7 changes: 3 additions & 4 deletions aws-lc-rs/tests/ecdsa_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use aws_lc_rs::{
signature::{self, EcdsaKeyPair, KeyPair, Signature, UnparsedPublicKey},
test, test_file,
};
use mirai_annotations::unrecoverable;

#[test]
fn ecdsa_traits() {
Expand Down Expand Up @@ -93,9 +92,9 @@ fn ecdsa_from_pkcs8_test() {
match (EcdsaKeyPair::from_pkcs8(this_asn1, &input), error) {
(Ok(_), None) => (),
(Err(e), None) => {
unrecoverable!("Failed with error \"{}\", but expected to succeed", e);
panic!("Failed with error \"{e}\", but expected to succeed");
}
(Ok(_), Some(e)) => unrecoverable!("Succeeded, but expected error \"{}\"", e),
(Ok(_), Some(e)) => panic!("Succeeded, but expected error \"{e}\""),
(Err(actual), Some(expected)) => assert_eq!(format!("{actual}"), expected),
};

Expand Down Expand Up @@ -230,7 +229,7 @@ fn test_signature_ecdsa_verify_fixed(data_file: test::File) {
("secp256k1", "SHA256") => &signature::ECDSA_P256K1_SHA256_FIXED,
("secp256k1", "SHA3-256") => &signature::ECDSA_P256K1_SHA3_256_FIXED,
_ => {
unrecoverable!("Unsupported curve+digest: {}+{}", curve_name, digest_name);
panic!("Unsupported curve+digest: {curve_name}+{digest_name}");
}
};

Expand Down
60 changes: 0 additions & 60 deletions aws-lc-rs/tests/pbkdf2_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,63 +52,3 @@ fn pbkdf2_tests() {
Ok(())
});
}

/// The API documentation specifies that derive/verify should panic, if the designated output length
/// is too long. Ring checks for an output array length while pbkdf2 is being ran, while we check
/// the array length before everything is processed.
/// Most platforms will fail to allocate this much memory, so we only test on platforms which can.
#[cfg(all(target_arch = "x86_64", target_vendor = "apple"))]
#[cfg(test)]
mod tests {
use aws_lc_rs::{digest, pbkdf2};
use core::num::NonZeroU32;
use mirai_annotations::assume;

#[test]
#[should_panic(expected = "derived key too long")]
fn pbkdf2_derive_too_long() {
let iterations = NonZeroU32::new(1_u32).unwrap();
let max_usize32 = u32::MAX as usize;
for &alg in &[
pbkdf2::PBKDF2_HMAC_SHA1,
pbkdf2::PBKDF2_HMAC_SHA256,
pbkdf2::PBKDF2_HMAC_SHA384,
pbkdf2::PBKDF2_HMAC_SHA512,
] {
let output_len = match_pbkdf2_digest(&alg).output_len;
assume!(output_len < 2048);
let mut out = vec![0u8; max_usize32 * output_len + 1];
pbkdf2::derive(alg, iterations, b"salt", b"password", &mut out);
}
}

#[test]
#[should_panic(expected = "derived key too long")]
fn pbkdf2_verify_too_long() {
let iterations = NonZeroU32::new(1_u32).unwrap();
let max_usize32 = u32::MAX as usize;
for &alg in &[
pbkdf2::PBKDF2_HMAC_SHA1,
pbkdf2::PBKDF2_HMAC_SHA256,
pbkdf2::PBKDF2_HMAC_SHA384,
pbkdf2::PBKDF2_HMAC_SHA512,
] {
let out = vec![0u8; max_usize32 * match_pbkdf2_digest(&alg).output_len + 1];
pbkdf2::verify(alg, iterations, b"salt", b"password", &out).unwrap();
}
}

fn match_pbkdf2_digest(&algorithm: &pbkdf2::Algorithm) -> &digest::Algorithm {
if algorithm == pbkdf2::PBKDF2_HMAC_SHA1 {
&digest::SHA1_FOR_LEGACY_USE_ONLY
} else if algorithm == pbkdf2::PBKDF2_HMAC_SHA256 {
&digest::SHA256
} else if algorithm == pbkdf2::PBKDF2_HMAC_SHA384 {
&digest::SHA384
} else if algorithm == pbkdf2::PBKDF2_HMAC_SHA512 {
&digest::SHA512
} else {
unreachable!()
}
}
}
2 changes: 1 addition & 1 deletion aws-lc-sys/builder/cc_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl CcBuilder {
let source_path = self.manifest_dir.join("aws-lc").join(source);
let is_asm = std::path::Path::new(source)
.extension()
.is_some_and(|ext| ext.eq("S"));
.map_or(false, |ext| ext.eq("S"));
if is_asm && target_vendor() == "apple" && target_arch() == "aarch64" {
let mut cc_preprocessor = self.create_builder();
cc_preprocessor.file(source_path);
Expand Down
5 changes: 4 additions & 1 deletion aws-lc-sys/builder/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// Modifications copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR ISC

// Needed until MSRV >= 1.70
#![allow(clippy::unnecessary_map_or)]
#![allow(clippy::ref_option)]
// Clippy can only be run on nightly toolchain
#![cfg_attr(clippy, feature(custom_inner_attributes))]
Expand Down Expand Up @@ -481,6 +483,7 @@ fn is_no_asm() -> bool {
unsafe { AWS_LC_SYS_NO_ASM }
}

#[allow(unknown_lints)]
#[allow(static_mut_refs)]
fn get_cflags() -> &'static str {
unsafe { AWS_LC_SYS_CFLAGS.as_str() }
Expand Down Expand Up @@ -656,7 +659,7 @@ fn setup_include_paths(out_dir: &Path, manifest_dir: &Path) -> PathBuf {
// iterate over all the include paths and copy them into the final output
for path in include_paths {
for child in std::fs::read_dir(path).into_iter().flatten().flatten() {
if child.file_type().is_ok_and(|t| t.is_file()) {
if child.file_type().map_or(false, |t| t.is_file()) {
let _ = std::fs::copy(
child.path(),
include_dir.join(child.path().file_name().unwrap()),
Expand Down

0 comments on commit 91689c4

Please sign in to comment.