Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A working FIPS build for Windows #309

Merged
merged 2 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/fips.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,36 @@ jobs:
# Doc-tests fail to link with dynamic build
# See: https://github.com/rust-lang/cargo/issues/8531
run: cargo test --tests ${{ matrix.args }}
windows-fips-test:
name: aws-lc-rs windows-fips-tests
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
rust: [ stable ]
os: [ windows-2019, windows-2022 ]
args:
- --all-targets --features fips,unstable
- --all-targets --features fips,bindgen,unstable
- --release --all-targets --features fips,unstable
- --no-default-features --features fips,unstable
- --no-default-features --features fips,ring-io,unstable
- --no-default-features --features fips,ring-sig-verify,unstable
- --no-default-features --features fips,alloc,unstable
steps:
- uses: ilammy/setup-nasm@v1
- uses: actions/checkout@v3
with:
submodules: 'recursive'
- uses: actions-rs/toolchain@v1.0.7
id: toolchain
with:
toolchain: ${{ matrix.rust }}
override: true
- name: Install ninja-build tool
uses: seanmiddleditch/gha-setup-ninja@v4
- name: Run cargo test
working-directory: ./aws-lc-rs
# Doc-tests fail to link with dynamic build
# See: https://github.com/rust-lang/cargo/issues/8531
run: cargo test --tests ${{ matrix.args }}
6 changes: 5 additions & 1 deletion aws-lc-fips-sys/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ if (BUILD_SHARED_LIBS AND FIPS)
# as cmake crate will postfix the C/CXX flags after our disablement nullifying them.
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-function-sections -fno-data-sections")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-function-sections -fno-data-sections")
endif ()
endif()

add_subdirectory(aws-lc aws-lc EXCLUDE_FROM_ALL)

if (BUILD_LIBSSL)
add_definitions(-DAWS_LC_RUST_INCLUDE_SSL)
endif()
Expand All @@ -38,6 +39,7 @@ set(FINAL_ARTIFACTS_DIRECTORY ${CMAKE_BINARY_DIR}/artifacts)
# location to find the artifacts cross-platform.
set_my_target_properties(
ARCHIVE_OUTPUT_DIRECTORY ${FINAL_ARTIFACTS_DIRECTORY}
RUNTIME_OUTPUT_DIRECTORY ${FINAL_ARTIFACTS_DIRECTORY}
LIBRARY_OUTPUT_DIRECTORY ${FINAL_ARTIFACTS_DIRECTORY})

# Based on https://stackoverflow.com/a/7750816 as some generators, like MSVC, will try to prefix the output directory
Expand All @@ -46,12 +48,14 @@ foreach (OUT_NAME ${CMAKE_CONFIGURATION_TYPES})
string(TOUPPER ${OUT_NAME} OUT_NAME)
set_my_target_properties(
ARCHIVE_OUTPUT_DIRECTORY_${OUT_NAME} ${FINAL_ARTIFACTS_DIRECTORY}
RUNTIME_OUTPUT_DIRECTORY_${OUT_NAME} ${FINAL_ARTIFACTS_DIRECTORY}
LIBRARY_OUTPUT_DIRECTORY_${OUT_NAME} ${FINAL_ARTIFACTS_DIRECTORY})
endforeach ()

if (BORINGSSL_PREFIX)
if (MSVC)
set(TARGET_PREFIX "${BORINGSSL_PREFIX}")
set_my_target_properties(IMPORT_PREFIX ${TARGET_PREFIX})
else()
set(TARGET_PREFIX "lib${BORINGSSL_PREFIX}")
endif()
Expand Down
49 changes: 44 additions & 5 deletions aws-lc-fips-sys/builder/cmake_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use crate::OutputLib::{Crypto, RustWrapper, Ssl};
use crate::{target, target_arch, target_os, target_vendor, test_command, OutputLibType};
use std::collections::HashMap;
use std::env;
use std::ffi::OsStr;
use std::path::PathBuf;
Expand All @@ -15,17 +16,22 @@ pub(crate) struct CmakeBuilder {
}

fn test_perl_command() -> bool {
test_command("perl".as_ref(), &["--version".as_ref()])
test_command("perl".as_ref(), &["--version".as_ref()]).status
}

fn test_go_command() -> bool {
test_command("go".as_ref(), &["version".as_ref()])
test_command("go".as_ref(), &["version".as_ref()]).status
}

fn test_ninja_command() -> bool {
test_command("ninja".as_ref(), &["--version".as_ref()]).status
|| test_command("ninja-build".as_ref(), &["--version".as_ref()]).status
}

fn find_cmake_command() -> Option<&'static OsStr> {
if test_command("cmake3".as_ref(), &["--version".as_ref()]) {
if test_command("cmake3".as_ref(), &["--version".as_ref()]).status {
Some("cmake3".as_ref())
} else if test_command("cmake".as_ref(), &["--version".as_ref()]) {
} else if test_command("cmake".as_ref(), &["--version".as_ref()]).status {
Some("cmake".as_ref())
} else {
None
Expand Down Expand Up @@ -85,12 +91,25 @@ impl CmakeBuilder {
} else {
cmake_cfg.define("CMAKE_BUILD_TYPE", "release");
}
} else if target_os() != "windows" {
} else if target_os() == "windows" {
// The Windows/FIPS build rejects "debug" profile
// https://github.com/aws/aws-lc/blob/main/CMakeLists.txt#L656
cmake_cfg.define("CMAKE_BUILD_TYPE", "relwithdebinfo");
} else {
cmake_cfg.define("CMAKE_BUILD_TYPE", "debug");
}

if target_os() == "windows" {
cmake_cfg.generator("Ninja");
let env_map = self
.collect_vcvarsall_bat()
.map_err(|x| panic!("{}", x))
.unwrap();
for (key, value) in env_map {
cmake_cfg.env(key, value);
}
}

if let Some(prefix) = &self.build_prefix {
cmake_cfg.define("BORINGSSL_PREFIX", format!("{prefix}_"));
let include_path = self.manifest_dir.join("generated-include");
Expand Down Expand Up @@ -137,11 +156,31 @@ impl CmakeBuilder {
.configure_arg("--no-warn-unused-cli")
.build()
}

fn collect_vcvarsall_bat(&self) -> Result<HashMap<String, String>, String> {
let mut map: HashMap<String, String> = HashMap::new();
let script_path = self.manifest_dir.join("builder").join("printenv.bat");
let result = test_command(script_path.as_os_str(), &[]);
if !result.status {
return Err("Failed to run vccarsall.bat.".to_owned());
}
let lines = result.output.lines();
for line in lines {
if let Some((var, val)) = line.split_once('=') {
map.insert(var.to_string(), val.to_string());
}
}
Ok(map)
}
}

impl crate::Builder for CmakeBuilder {
fn check_dependencies(&self) -> Result<(), String> {
let mut missing_dependency = false;
if target_os() == "windows" && !test_ninja_command() {
eprintln!("Missing dependency: Ninja is required for FIPS on Windows.");
missing_dependency = true;
}
if !test_go_command() {
eprintln!("Missing dependency: Go is required for FIPS.");
missing_dependency = true;
Expand Down
29 changes: 20 additions & 9 deletions aws-lc-fips-sys/builder/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ enum OutputLibType {

impl Default for OutputLibType {
fn default() -> Self {
let build_type_result = env::var("AWS_LC_FIPS_SYS_STATIC");
if let Ok(build_type) = build_type_result {
if let Ok(build_type) = env::var("AWS_LC_FIPS_SYS_STATIC") {
eprintln!("AWS_LC_FIPS_SYS_STATIC={build_type}");
// If the environment variable is set, we ignore every other factor.
let build_type = build_type.to_lowercase();
Expand Down Expand Up @@ -116,11 +115,25 @@ fn target_platform_prefix(name: &str) -> String {
format!("{}_{}_{}", env::consts::OS, env::consts::ARCH, name)
}

fn test_command(executable: &OsStr, args: &[&OsStr]) -> bool {
if let Ok(output) = Command::new(executable).args(args).output() {
return output.status.success();
pub(crate) struct TestCommandResult {
output: Box<str>,
status: bool,
}

fn test_command(executable: &OsStr, args: &[&OsStr]) -> TestCommandResult {
if let Ok(result) = Command::new(executable).args(args).output() {
let output = String::from_utf8(result.stdout)
.unwrap_or_default()
.into_boxed_str();
return TestCommandResult {
output,
status: result.status.success(),
};
}
TestCommandResult {
output: String::new().into_boxed_str(),
status: false,
}
false
}

#[cfg(any(
Expand Down Expand Up @@ -214,9 +227,8 @@ trait Builder {
}

fn main() {
let output_lib_type = OutputLibType::default();

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"))
Expand Down Expand Up @@ -277,7 +289,6 @@ fn main() {
bindings_available,
"aws-lc-fip-sys build failed. Please enable the 'bindgen' feature on aws-lc-rs or aws-lc-fips-sys"
);

builder.build().unwrap();

println!(
Expand Down
7 changes: 7 additions & 0 deletions aws-lc-fips-sys/builder/printenv.bat
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@echo off
for /f "usebackq tokens=* delims=" %%i in (`"%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe" -latest -property installationPath`) do (
set "VS_PATH=%%i"
)
call "%VS_PATH%\VC\Auxiliary\Build\vcvarsall.bat" x64

set
9 changes: 6 additions & 3 deletions aws-lc-fips-sys/include/rust_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

#ifdef _WIN32
#define BORINGSSL_SHARED_LIBRARY
#define AWS_LC_FIPS_SYS_EXPORT __declspec(dllexport)
#else
#define AWS_LC_FIPS_SYS_EXPORT __attribute__((visibility("default")))
#endif

#include <openssl/err.h>
Expand All @@ -20,9 +23,9 @@ extern "C" {
// The following functions are wrappers over inline functions and macros in
// BoringSSL, which bindgen cannot currently correctly bind. These wrappers
// ensure changes to the functions remain in lockstep with the Rust versions.
int ERR_GET_LIB_RUST(uint32_t packed_error);
int ERR_GET_REASON_RUST(uint32_t packed_error);
int ERR_GET_FUNC_RUST(uint32_t packed_error);
AWS_LC_FIPS_SYS_EXPORT int ERR_GET_LIB_RUST(uint32_t packed_error);
AWS_LC_FIPS_SYS_EXPORT int ERR_GET_REASON_RUST(uint32_t packed_error);
AWS_LC_FIPS_SYS_EXPORT int ERR_GET_FUNC_RUST(uint32_t packed_error);


#if defined(__cplusplus)
Expand Down
4 changes: 4 additions & 0 deletions aws-lc-sys/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ add_subdirectory(aws-lc aws-lc EXCLUDE_FROM_ALL)
if (BUILD_LIBSSL)
add_definitions(-DAWS_LC_RUST_INCLUDE_SSL)
endif()

add_library(rust_wrapper rust_wrapper.c)
target_include_directories(rust_wrapper PRIVATE include)
target_link_libraries(rust_wrapper PUBLIC crypto)
Expand All @@ -31,6 +32,7 @@ set(FINAL_ARTIFACTS_DIRECTORY ${CMAKE_BINARY_DIR}/artifacts)
# location to find the artifacts cross-platform.
set_my_target_properties(
ARCHIVE_OUTPUT_DIRECTORY ${FINAL_ARTIFACTS_DIRECTORY}
RUNTIME_OUTPUT_DIRECTORY ${FINAL_ARTIFACTS_DIRECTORY}
LIBRARY_OUTPUT_DIRECTORY ${FINAL_ARTIFACTS_DIRECTORY})

# Based on https://stackoverflow.com/a/7750816 as some generators, like MSVC, will try to prefix the output directory
Expand All @@ -39,12 +41,14 @@ foreach (OUT_NAME ${CMAKE_CONFIGURATION_TYPES})
string(TOUPPER ${OUT_NAME} OUT_NAME)
set_my_target_properties(
ARCHIVE_OUTPUT_DIRECTORY_${OUT_NAME} ${FINAL_ARTIFACTS_DIRECTORY}
RUNTIME_OUTPUT_DIRECTORY_${OUT_NAME} ${FINAL_ARTIFACTS_DIRECTORY}
LIBRARY_OUTPUT_DIRECTORY_${OUT_NAME} ${FINAL_ARTIFACTS_DIRECTORY})
endforeach ()

if (BORINGSSL_PREFIX)
if (MSVC)
set(TARGET_PREFIX "${BORINGSSL_PREFIX}")
set_my_target_properties(IMPORT_PREFIX ${TARGET_PREFIX})
else()
set(TARGET_PREFIX "lib${BORINGSSL_PREFIX}")
endif()
Expand Down
4 changes: 2 additions & 2 deletions aws-lc-sys/builder/cmake_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ pub(crate) struct CmakeBuilder {
}

fn find_cmake_command() -> Option<&'static OsStr> {
if test_command("cmake3".as_ref(), &["--version".as_ref()]) {
if test_command("cmake3".as_ref(), &["--version".as_ref()]).status {
Some("cmake3".as_ref())
} else if test_command("cmake".as_ref(), &["--version".as_ref()]) {
} else if test_command("cmake".as_ref(), &["--version".as_ref()]).status {
Some("cmake".as_ref())
} else {
None
Expand Down
26 changes: 20 additions & 6 deletions aws-lc-sys/builder/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ enum OutputLibType {

impl Default for OutputLibType {
fn default() -> Self {
let build_type_result = env::var("AWS_LC_SYS_STATIC");
if let Ok(build_type) = build_type_result {
if let Ok(build_type) = env::var("AWS_LC_SYS_STATIC") {
eprintln!("AWS_LC_SYS_STATIC={build_type}");
// If the environment variable is set, we ignore every other factor.
let build_type = build_type.to_lowercase();
Expand Down Expand Up @@ -109,11 +108,26 @@ fn target_platform_prefix(name: &str) -> String {
format!("{}_{}_{}", env::consts::OS, env::consts::ARCH, name)
}

fn test_command(executable: &OsStr, args: &[&OsStr]) -> bool {
if let Ok(output) = Command::new(executable).args(args).output() {
return output.status.success();
pub(crate) struct TestCommandResult {
#[allow(dead_code)]
output: Box<str>,
status: bool,
}

fn test_command(executable: &OsStr, args: &[&OsStr]) -> TestCommandResult {
if let Ok(result) = Command::new(executable).args(args).output() {
let output = String::from_utf8(result.stdout)
.unwrap_or_default()
.into_boxed_str();
return TestCommandResult {
output,
status: result.status.success(),
};
}
TestCommandResult {
output: String::new().into_boxed_str(),
status: false,
}
false
}

#[cfg(any(
Expand Down
9 changes: 6 additions & 3 deletions aws-lc-sys/include/rust_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

#ifdef _WIN32
#define BORINGSSL_SHARED_LIBRARY
#define AWS_LC_SYS_EXPORT __declspec(dllexport)
#else
#define AWS_LC_SYS_EXPORT __attribute__((visibility("default")))
#endif

#include <openssl/err.h>
Expand All @@ -20,9 +23,9 @@ extern "C" {
// The following functions are wrappers over inline functions and macros in
// BoringSSL, which bindgen cannot currently correctly bind. These wrappers
// ensure changes to the functions remain in lockstep with the Rust versions.
int ERR_GET_LIB_RUST(uint32_t packed_error);
int ERR_GET_REASON_RUST(uint32_t packed_error);
int ERR_GET_FUNC_RUST(uint32_t packed_error);
AWS_LC_SYS_EXPORT int ERR_GET_LIB_RUST(uint32_t packed_error);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use OPENSSL_EXPORT from "openssl/base.h"? Seems like it would come in handy, so we wouldn't need duplicate definitions in both rust_wrapper.hs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that intended for use outside the library? It might work if we define BORINGSSL_IMPLEMENTATION and BORINGSSL_SHARED_LIBRARY prior to its include.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I was under the impression that these were already defined if we built with AWS-LC.

Not a blocker though, just curious if it was reusable

AWS_LC_SYS_EXPORT int ERR_GET_REASON_RUST(uint32_t packed_error);
AWS_LC_SYS_EXPORT int ERR_GET_FUNC_RUST(uint32_t packed_error);


#if defined(__cplusplus)
Expand Down
Loading