Implement PR feedback

.github/workflows/ci.yml

@@ -220,14 +220,17 @@ jobs:
           - --no-default-features --features non-fips,ring-io
           - --no-default-features --features non-fips,ring-sig-verify
           - --no-default-features --features non-fips,alloc
-          - --no-default-features --features non-fips,test_pq_kat
+          - --no-default-features --features non-fips,bindgen
         exclude:
           - rust: stable
             os: macOS-latest
             target: aarch64-unknown-linux-gnu
           - rust: stable
             os: macOS-latest
             target: i686-unknown-linux-gnu
+        include:
+          - args: --no-default-features --features non-fips,bindgen
+            envs: AWS_LC_RUST_PRIVATE_INTERNALS=1
     steps:
       - uses: actions/checkout@v3
         with:
@@ -240,6 +243,10 @@ jobs:
       - name: Run cargo test
         working-directory: ./aws-lc-rs
         run: cargo test ${{ matrix.args }}
+      - name: Run cargo test w/ environment
+        if: ${{ matrix.envs }}
+        working-directory: ./aws-lc-rs
+        run: env ${{ matrix.envs }} cargo test ${{ matrix.args }}
       - name: Run extra tests
         working-directory: ./aws-lc-rs-testing
         run: cargo test --all-targets
@@ -262,6 +269,9 @@ jobs:
           - --no-default-features --features fips,ring-sig-verify
           - --no-default-features --features fips,alloc
           - --no-default-features --features fips,bindgen
+        include:
+          - args: --no-default-features --features fips,bindgen
+            envs: AWS_LC_RUST_PRIVATE_INTERNALS=1
     steps:
       - uses: actions/checkout@v3
         with:
@@ -276,6 +286,9 @@ jobs:
         # Doc-tests fail to link with dynamic build
         # See: https://github.com/rust-lang/cargo/issues/8531
         run: cargo test --tests ${{ matrix.args }}
+      - name: Run cargo test w/ environment
+        if: ${{ matrix.envs }}
+        run: env ${{ matrix.envs }} test --tests ${{ matrix.args }}
 
   bindgen-test:
     name: aws-lc-rs bindgen-tests

aws-lc-fips-sys/builder/bindgen.rs

@@ -2,8 +2,8 @@
 // SPDX-License-Identifier: Apache-2.0 OR ISC
 
 use crate::{
-    get_aws_lc_fips_sys_includes_path, get_aws_lc_include_path, get_generated_include_path,
-    get_rust_include_path,
+    get_aws_lc_fips_sys_includes_path, get_aws_lc_include_path, get_aws_lc_rand_extra_path,
+    get_generated_include_path, get_rust_include_path, is_private_api_enabled,
 };
 use bindgen::callbacks::{ItemInfo, ParseCallbacks};
 use std::fmt::Debug;
@@ -59,6 +59,15 @@ fn prepare_clang_args(manifest_dir: &Path) -> Vec<String> {
         get_aws_lc_include_path(manifest_dir).display().to_string(),
     );
 
+    if is_private_api_enabled() {
+        clang_args.push("-I".to_string());
+        clang_args.push(
+            get_aws_lc_rand_extra_path(manifest_dir)
+                .display()
+                .to_string(),
+        );
+    }
+
     if let Some(include_paths) = get_aws_lc_fips_sys_includes_path() {
         for path in include_paths {
             add_header_include_path(&mut clang_args, path.display().to_string());
@@ -136,6 +145,12 @@ fn prepare_bindings_builder(manifest_dir: &Path, options: &BindingOptions<'_>) -
         builder = builder.clang_arg("-DAWS_LC_RUST_INCLUDE_SSL");
     }
 
+    if is_private_api_enabled() {
+        builder = builder
+            .clang_arg("-DAWS_LC_RUST_PRIVATE_INTERNALS")
+            .allowlist_file(r".*(/|\\)pq_custom_randombytes\.h");
+    }
+
     builder = builder.parse_callbacks(Box::new(StripPrefixCallback::new(options.build_prefix)));
 
     builder

aws-lc-fips-sys/builder/main.rs

@@ -23,6 +23,13 @@ pub(crate) fn get_aws_lc_include_path(manifest_dir: &Path) -> PathBuf {
     manifest_dir.join("aws-lc").join("include")
 }
 
+pub(crate) fn get_aws_lc_rand_extra_path(manifest_dir: &Path) -> PathBuf {
+    manifest_dir
+        .join("aws-lc")
+        .join("crypto")
+        .join("rand_extra")
+}
+
 pub(crate) fn get_rust_include_path(manifest_dir: &Path) -> PathBuf {
     manifest_dir.join("include")
 }
@@ -162,7 +169,7 @@ fn prepare_cmake_build(manifest_dir: &PathBuf, build_prefix: String) -> cmake::C
         cmake_cfg.define("BUILD_SHARED_LIBS", "0");
     }
 
-    let opt_level = env::var("OPT_LEVEL").unwrap_or_else(|_| "0".to_string());
+    let opt_level = get_env_flag("OPT_LEVEL", "0");
     if opt_level.ne("0") {
         if opt_level.eq("1") || opt_level.eq("2") {
             cmake_cfg.define("CMAKE_BUILD_TYPE", "relwithdebinfo");
@@ -292,9 +299,13 @@ fn main() {
 
     let mut is_bindgen_required = cfg!(feature = "bindgen");
 
-    let is_internal_generate = env::var("AWS_LC_RUST_INTERNAL_BINDGEN")
-        .unwrap_or_else(|_| String::from("0"))
-        .eq("1");
+    let is_internal_generate = is_internal_generate_enabled();
+
+    if is_internal_generate && is_private_api_enabled() {
+        panic!(
+            "AWS_LC_RUST_PRIVATE_INTERNALS=1 is not supported when AWS_LC_RUST_INTERNAL_BINDGEN=1"
+        )
+    }
 
     let pregenerated = !is_bindgen_required || is_internal_generate;
 
@@ -377,6 +388,14 @@ fn main() {
     ] {
         println!("cargo:include={}", include_path.display());
     }
+
+    if is_private_api_enabled() {
+        println!(
+            "cargo:include={}",
+            get_aws_lc_rand_extra_path(&manifest_dir).display()
+        );
+    }
+
     if let Some(include_paths) = get_aws_lc_fips_sys_includes_path() {
         for path in include_paths {
             println!("cargo:include={}", path.display());
@@ -410,3 +429,18 @@ fn check_dependencies() {
         "Required build dependency is missing. Halting build."
     );
 }
+
+fn is_internal_generate_enabled() -> bool {
+    get_env_flag("AWS_LC_RUST_INTERNAL_BINDGEN", "0").eq("1")
+}
+
+fn is_private_api_enabled() -> bool {
+    get_env_flag("AWS_LC_RUST_PRIVATE_INTERNALS", "0").eq("1")
+}
+
+fn get_env_flag<T>(key: &'static str, default: T) -> String
+where
+    T: Into<String>,
+{
+    env::var(key).unwrap_or(default.into())
+}

aws-lc-fips-sys/include/rust_wrapper.h

@@ -101,6 +101,10 @@ int ERR_GET_FUNC_RUST(uint32_t packed_error);
 #include "openssl/x509_vfy.h"
 #include "openssl/x509v3.h"
 
+#if defined(AWS_LC_RUST_PRIVATE_INTERNALS)
+#include "pq_custom_randombytes.h"
+#endif
+
 #if defined(AWS_LC_RUST_INCLUDE_SSL)
 #include "openssl/ssl.h"
 #include "openssl/ssl3.h"

aws-lc-rs/Cargo.toml

@@ -29,7 +29,6 @@ ring-io = ["dep:untrusted"]
 ring-sig-verify = ["dep:untrusted"]
 bindgen = ["aws-lc-sys?/bindgen", "aws-lc-fips-sys?/bindgen"]
 asan = ["aws-lc-sys?/asan", "aws-lc-fips-sys?/asan"]
-test_pq_kat = ["aws-lc-sys?/test_pq_random", "bindgen"]
 
 # require non-FIPS
 non-fips = ["aws-lc-sys"]

aws-lc-rs/Makefile

@@ -30,12 +30,13 @@ test:
 ifeq ($(UNAME_S),Linux)
 	cargo test --release --all-targets --features fips
 	cargo test --no-default-features --features fips
+	env AWS_LC_RUST_PRIVATE_INTERNALS=1 cargo test --no-default-features --features fips,bindgen
 endif
 	cargo test --no-default-features --features aws-lc-sys
 	cargo test --no-default-features --features aws-lc-sys,ring-sig-verify
 	cargo test --no-default-features --features aws-lc-sys,ring-io
 	cargo test --no-default-features --features aws-lc-sys,alloc
-	cargo test --no-default-features --features aws-lc-sys,test_pq_kat
+	env AWS_LC_RUST_PRIVATE_INTERNALS=1 cargo test --no-default-features --features aws-lc-sys,bindgen
 
 msrv:
 	cargo msrv verify

aws-lc-rs/build.rs

@@ -1,6 +1,8 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0 OR ISC
 
+use std::env;
+
 fn main() {
     let mutually_exclusives_count = [cfg!(feature = "non-fips"), cfg!(feature = "fips")]
         .iter()
@@ -27,8 +29,16 @@ fn main() {
         std::process::exit(1);
     }
 
-    // Force tests to run single threaded if running PQ KATs
-    if cfg!(feature = "test_pq_kat") {
+    // For PQ KEM KATs we need to enable a private aws-lc API.
+    // If AWS_LC_RUST_PRIVATE_INTERNALS=1 is set and bingen is enabled then enable our
+    // configuration to allow the tests to be built and enabled.
+    // Additionally these APIs are not thread safe. So force tests to run single threaded.
+    if env::var("AWS_LC_RUST_PRIVATE_INTERNALS")
+        .unwrap_or("0".into())
+        .eq("1")
+        && cfg!(feature = "bindgen")
+    {
+        println!("cargo:rustc-cfg=private_api");
         println!("cargo:rustc-env=RUST_TEST_THREADS=1");
     }
 }

aws-lc-rs/src/kem.rs

@@ -162,7 +162,7 @@ impl KemAlgorithm {
 #[derive(Debug)]
 pub struct KemPrivateKey {
     algorithm: &'static KemAlgorithm,
-    pkey: LcPtr<*mut EVP_PKEY>,
+    pkey: LcPtr<EVP_PKEY>,
     priv_key: Box<[u8]>,
 }
 
@@ -317,7 +317,7 @@ impl AsRef<[u8]> for KemPrivateKey {
 #[derive(Debug)]
 pub struct KemPublicKey {
     algorithm: &'static KemAlgorithm,
-    pkey: LcPtr<*mut EVP_PKEY>,
+    pkey: LcPtr<EVP_PKEY>,
     pub_key: Box<[u8]>,
 }
 
@@ -409,7 +409,7 @@ impl AsRef<[u8]> for KemPublicKey {
 
 // Returns an LcPtr to an EVP_PKEY
 #[inline]
-unsafe fn kem_key_generate(nid: c_int) -> Result<LcPtr<*mut EVP_PKEY>, Unspecified> {
+unsafe fn kem_key_generate(nid: c_int) -> Result<LcPtr<EVP_PKEY>, Unspecified> {
     let ctx = LcPtr::new(EVP_PKEY_CTX_new_id(EVP_PKEY_KEM, null_mut()))?;
     if 1 != EVP_PKEY_CTX_kem_set_params(*ctx, nid) || 1 != EVP_PKEY_keygen_init(*ctx) {
         return Err(Unspecified);
@@ -424,7 +424,7 @@ unsafe fn kem_key_generate(nid: c_int) -> Result<LcPtr<*mut EVP_PKEY>, Unspecifi
 
 #[cfg(test)]
 mod tests {
-    #[cfg(feature = "test_pq_kat")]
+    #[cfg(private_api)]
     use crate::{
         aws_lc::{
             pq_custom_randombytes_init_for_testing,
@@ -438,7 +438,7 @@ mod tests {
         kem::{KemPrivateKey, KemPublicKey, KYBER1024_R3, KYBER512_R3, KYBER768_R3},
     };
 
-    #[cfg(feature = "test_pq_kat")]
+    #[cfg(private_api)]
     #[test]
     fn test_kem_kyber512() {
         test::run(
@@ -483,7 +483,7 @@ mod tests {
         );
     }
 
-    #[cfg(feature = "test_pq_kat")]
+    #[cfg(private_api)]
     #[test]
     fn test_kem_kyber768() {
         test::run(
@@ -528,7 +528,7 @@ mod tests {
         );
     }
 
-    #[cfg(feature = "test_pq_kat")]
+    #[cfg(private_api)]
     #[test]
     fn test_kem_kyber1024() {
         test::run(

aws-lc-sys/Cargo.toml

@@ -47,7 +47,6 @@ build = "builder/main.rs"
 asan = []
 ssl = []
 bindgen = ["dep:bindgen"] # Generate the bindings on the targetted platform as a fallback mechanism.
-test_pq_random = []
 
 [build-dependencies]
 cmake = "0.1.48"

aws-lc-sys/builder/bindgen.rs

@@ -3,7 +3,7 @@
 
 use crate::{
     get_aws_lc_include_path, get_aws_lc_rand_extra_path, get_aws_lc_sys_includes_path,
-    get_generated_include_path, get_rust_include_path,
+    get_generated_include_path, get_rust_include_path, is_private_api_enabled,
 };
 use bindgen::callbacks::{ItemInfo, ParseCallbacks};
 use std::fmt::Debug;
@@ -59,7 +59,7 @@ fn prepare_clang_args(manifest_dir: &Path) -> Vec<String> {
         get_aws_lc_include_path(manifest_dir).display().to_string(),
     );
 
-    if cfg!(feature = "test_pq_random") {
+    if is_private_api_enabled() {
         clang_args.push("-I".to_string());
         clang_args.push(
             get_aws_lc_rand_extra_path(manifest_dir)
@@ -120,7 +120,6 @@ fn prepare_bindings_builder(manifest_dir: &Path, options: &BindingOptions<'_>) -
         .derive_eq(true)
         .allowlist_file(r".*(/|\\)openssl(/|\\)[^/\\]+\.h")
         .allowlist_file(r".*(/|\\)rust_wrapper\.h")
-        .allowlist_file(r".*(/|\\)pq_custom_randombytes\.h")
         .rustified_enum(r"point_conversion_form_t")
         .default_macro_constant_type(bindgen::MacroTypeVariation::Signed)
         .generate_comments(true)
@@ -146,8 +145,10 @@ fn prepare_bindings_builder(manifest_dir: &Path, options: &BindingOptions<'_>) -
         builder = builder.clang_arg("-DAWS_LC_RUST_INCLUDE_SSL");
     }
 
-    if cfg!(feature = "test_pq_random") {
-        builder = builder.clang_arg("-DAWS_LC_RUST_PQ_RANDOM_TEST");
+    if is_private_api_enabled() {
+        builder = builder
+            .clang_arg("-DAWS_LC_RUST_PRIVATE_INTERNALS")
+            .allowlist_file(r".*(/|\\)pq_custom_randombytes\.h");
     }
 
     builder = builder.parse_callbacks(Box::new(StripPrefixCallback::new(options.build_prefix)));

aws-lc-sys/builder/main.rs

@@ -156,7 +156,7 @@ fn prepare_cmake_build(manifest_dir: &PathBuf, build_prefix: String) -> cmake::C
         cmake_cfg.define("BUILD_SHARED_LIBS", "0");
     }
 
-    let opt_level = env::var("OPT_LEVEL").unwrap_or_else(|_| "0".to_string());
+    let opt_level = get_env_flag("OPT_LEVEL", "0");
     if opt_level.ne("0") {
         if opt_level.eq("1") || opt_level.eq("2") {
             cmake_cfg.define("CMAKE_BUILD_TYPE", "relwithdebinfo");
@@ -299,9 +299,13 @@ fn main() {
     let mut is_bindgen_required = cfg!(feature = "bindgen");
     let output_lib_type = OutputLibType::default();
 
-    let is_internal_generate = env::var("AWS_LC_RUST_INTERNAL_BINDGEN")
-        .unwrap_or_else(|_| String::from("0"))
-        .eq("1");
+    let is_internal_generate = is_internal_generate_enabled();
+
+    if is_internal_generate && is_private_api_enabled() {
+        panic!(
+            "AWS_LC_RUST_PRIVATE_INTERNALS=1 is not supported when AWS_LC_RUST_INTERNAL_BINDGEN=1"
+        )
+    }
 
     let pregenerated = !is_bindgen_required || is_internal_generate;
 
@@ -388,6 +392,14 @@ fn main() {
     ] {
         println!("cargo:include={}", include_path.display());
     }
+
+    if is_private_api_enabled() {
+        println!(
+            "cargo:include={}",
+            get_aws_lc_rand_extra_path(&manifest_dir).display()
+        );
+    }
+
     if let Some(include_paths) = get_aws_lc_sys_includes_path() {
         for path in include_paths {
             println!("cargo:include={}", path.display());
@@ -414,3 +426,18 @@ fn check_dependencies() {
         "Required build dependency is missing. Halting build."
     );
 }
+
+fn is_internal_generate_enabled() -> bool {
+    get_env_flag("AWS_LC_RUST_INTERNAL_BINDGEN", "0").eq("1")
+}
+
+fn is_private_api_enabled() -> bool {
+    get_env_flag("AWS_LC_RUST_PRIVATE_INTERNALS", "0").eq("1")
+}
+
+fn get_env_flag<T>(key: &'static str, default: T) -> String
+where
+    T: Into<String>,
+{
+    env::var(key).unwrap_or(default.into())
+}