[BACKPORT] FIPS build fixes, Bump Version to 2.0.7 (#1451)

* Mark bcm_redirector functions as local (#1352) * Fix delocator and FIPS static build prefixing behaviors Original: #1342 * Add aws-lc-rs fips CI test * Bump version to 2.0.7 --------- Co-authored-by: Sean McGrail <[email protected]>
aws · Feb 26, 2024 · e796835 · e796835
1 parent 5602ad1
commit e796835
Show file tree

Hide file tree

Showing 13 changed files with 286 additions and 116 deletions.
diff --git a/.github/workflows/aws-lc-rs.yml b/.github/workflows/aws-lc-rs.yml
@@ -0,0 +1,69 @@
+name: aws-lc-rs fips sanity tests
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+env:
+  GOPROXY: https://proxy.golang.org,direct
+jobs:
+  standard:
+    runs-on: ubuntu-latest
+    steps:
+
+      - uses: actions/checkout@v3
+        with:
+          repository: aws/aws-lc-rs
+          path: ./aws-lc-rs
+          submodules: false
+      - uses: actions-rs/toolchain@v1
+        with:
+          # Our aws-lc-fips-sys generation scripts require nightly.
+          toolchain: nightly
+          override: true
+      - uses: actions-rs/cargo@v1
+        with:
+          command: install
+          args: rust-script
+      - name: Install OS Dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get -y --no-install-recommends install cmake gcc clang ninja-build golang
+      - name: Remove aws-lc submodule from crate directory
+        working-directory: ./aws-lc-rs/aws-lc-fips-sys
+        run: |
+          rm -rf aws-lc
+      - uses: actions/checkout@v3
+        with:
+          path: ./aws-lc-rs/aws-lc-fips-sys/aws-lc
+      - name: Regenerate aws-lc-fips-sys crate
+        working-directory: ./aws-lc-rs/aws-lc-fips-sys
+        run: |
+          rm -rf symbols/*
+          rm -rf generated-include/openssl/*
+          ../scripts/generate/_collect_symbols_build.sh -f -c aws-lc-fips-sys
+          ../scripts/generate/_generate_prefix_headers.sh -f -c aws-lc-fips-sys
+          ../scripts/generate/_generate_bindings.sh -c aws-lc-fips-sys
+      - name: aws-lc-fips-sys build
+        working-directory: ./aws-lc-rs/aws-lc-fips-sys
+        run: |
+          cargo build
+      - name: aws-lc-fips-sys test
+        working-directory: ./aws-lc-rs/aws-lc-fips-sys
+        run: |
+          cargo test
+      - name: aws-lc-fips-sys packaging
+        working-directory: ./aws-lc-rs/aws-lc-fips-sys
+        run: |
+          cargo package --allow-dirty
+      - name: aws-lc-rs build
+        working-directory: ./aws-lc-rs/aws-lc-rs
+        run: |
+          cargo build --no-default-features --features fips
+      - name: aws-lc-rs test
+        working-directory: ./aws-lc-rs/aws-lc-rs
+        run: |
+          cargo test --no-default-features --features fips
diff --git a/crypto/fipsmodule/CMakeLists.txt b/crypto/fipsmodule/CMakeLists.txt
@@ -213,27 +213,6 @@ if((ARCH STREQUAL "x86_64" OR ARCH STREQUAL "aarch64") AND
   endif()
 endif()
 
-function(cpreprocess dest src)
-  set(TARGET "")
-  if(CMAKE_ASM_COMPILER_TARGET)
-    set(TARGET "--target=${CMAKE_ASM_COMPILER_TARGET}")
-  endif()
-
-  if(BORINGSSL_PREFIX)
-    set(PREFIX_INCLUDE "-I${PROJECT_BINARY_DIR}/symbol_prefix_include")
-  endif()
-
-  string(REGEX REPLACE "[ ]+" ";" CMAKE_ASM_FLAGS "${CMAKE_ASM_FLAGS}")
-  add_custom_command(
-    OUTPUT ${dest}
-    COMMAND ${CMAKE_ASM_COMPILER} ${TARGET} ${CMAKE_ASM_FLAGS} -E ${src} ${PREFIX_INCLUDE} -I${PROJECT_SOURCE_DIR}/include > ${dest}
-    DEPENDS
-    ${src}
-    ${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h
-    WORKING_DIRECTORY .
-  )
-endfunction()
-
 function(s2n_asm_cpreprocess dest src)
   # s2n_asm_cpreprocess differs from cpreprocess in that is does additional post-processing
   # based on s2n-bignum https://github.com/awslabs/s2n-bignum/blob/main/x86/Makefile#L264
@@ -288,30 +267,37 @@ if(FIPS_DELOCATE)
   )
   target_compile_definitions(bcm_c_generated_asm PRIVATE BORINGSSL_IMPLEMENTATION)
 
-  if(ARCH STREQUAL "aarch64")
-    # Perlasm output on Aarch64 needs to pass through the C preprocessor
-    # before it can be parsed by delocate.
-    foreach(asm ${BCM_ASM_SOURCES})
-      cpreprocess(${asm}.s ${asm})
-      list(APPEND BCM_ASM_PROCESSED_SOURCES "${asm}.s")
-    endforeach()
-  else()
-    # No preprocessing is required on other platforms.
-    set(BCM_ASM_PROCESSED_SOURCES ${BCM_ASM_SOURCES})
-  endif()
-
   add_dependencies(bcm_c_generated_asm global_target)
   add_dependencies(bcm_c_generated_asm boringssl_prefix_symbols)
-  target_include_directories(bcm_c_generated_asm BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)
+  # Important: We do not want to add the generated prefix symbols to the include path here!
+  # Delocator expects symbols to not be prefixed.
   target_include_directories(bcm_c_generated_asm PRIVATE ${PROJECT_SOURCE_DIR}/include)
   set_target_properties(bcm_c_generated_asm PROPERTIES COMPILE_OPTIONS "-S")
   set_target_properties(bcm_c_generated_asm PROPERTIES POSITION_INDEPENDENT_CODE ON)
 
+  set(TARGET "")
+  if(CMAKE_ASM_COMPILER_TARGET)
+    set(TARGET "--target=${CMAKE_ASM_COMPILER_TARGET}")
+  endif()
+
   go_executable(delocate boringssl.googlesource.com/boringssl/util/fipstools/delocate)
   add_custom_command(
     OUTPUT bcm-delocated.S
-    COMMAND ./delocate -a $<TARGET_FILE:bcm_c_generated_asm> -o bcm-delocated.S ${BCM_ASM_PROCESSED_SOURCES}
-    DEPENDS bcm_c_generated_asm delocate ${BCM_ASM_PROCESSED_SOURCES}
+    COMMAND
+    ./delocate
+    -a $<TARGET_FILE:bcm_c_generated_asm>
+    -o bcm-delocated.S
+    -cc ${CMAKE_ASM_COMPILER}
+    -cc-flags "${TARGET} ${CMAKE_ASM_FLAGS}"
+    ${PROJECT_SOURCE_DIR}/include/openssl/boringssl_prefix_symbols_asm.h
+    ${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h
+    ${BCM_ASM_SOURCES}
+    DEPENDS
+    bcm_c_generated_asm
+    delocate
+    ${BCM_ASM_SOURCES}
+    ${PROJECT_SOURCE_DIR}/include/openssl/boringssl_prefix_symbols_asm.h
+    ${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   )
 

diff --git a/crypto/fipsmodule/service_indicator/service_indicator_test.cc b/crypto/fipsmodule/service_indicator/service_indicator_test.cc
@@ -4051,7 +4051,7 @@ TEST(ServiceIndicatorTest, DRBG) {
 // Since this is running in FIPS mode it should end in FIPS
 // Update this when the AWS-LC version number is modified
 TEST(ServiceIndicatorTest, AWSLCVersionString) {
-  ASSERT_STREQ(awslc_version_string(), "AWS-LC FIPS 2.0.6");
+  ASSERT_STREQ(awslc_version_string(), "AWS-LC FIPS 2.0.7");
 }
 
 #else
@@ -4094,6 +4094,6 @@ TEST(ServiceIndicatorTest, BasicTest) {
 // Since this is not running in FIPS mode it shouldn't end in FIPS
 // Update this when the AWS-LC version number is modified
 TEST(ServiceIndicatorTest, AWSLCVersionString) {
-  ASSERT_STREQ(awslc_version_string(), "AWS-LC 2.0.6");
+  ASSERT_STREQ(awslc_version_string(), "AWS-LC 2.0.7");
 }
 #endif // AWSLC_FIPS
diff --git a/include/openssl/base.h b/include/openssl/base.h
@@ -214,7 +214,7 @@ extern "C" {
 // ServiceIndicatorTest.AWSLCVersionString
 // Note: there are two versions of this test. Only one test is compiled
 // depending on FIPS mode.
-#define AWSLC_VERSION_NUMBER_STRING "2.0.6"
+#define AWSLC_VERSION_NUMBER_STRING "2.0.7"
 
 #if defined(BORINGSSL_SHARED_LIBRARY)
 

diff --git a/util/fipstools/delocate/delocate.go b/util/fipstools/delocate/delocate.go
@@ -1703,7 +1703,7 @@ func writeAarch64Function(w stringWriter, funcName string, writeContents func(st
 	w.WriteString(".size " + funcName + ", .-" + funcName + "\n")
 }
 
-func transform(w stringWriter, inputs []inputFile) error {
+func transform(w stringWriter, includes []string, inputs []inputFile) error {
 	// symbols contains all defined symbols.
 	symbols := make(map[string]struct{})
 	// localEntrySymbols contains all symbols with a .localentry directive.
@@ -1717,6 +1717,14 @@ func transform(w stringWriter, inputs []inputFile) error {
 	// OPENSSL_ia32cap_get will be synthesized by this script.
 	symbols["OPENSSL_ia32cap_get"] = struct{}{}
 
+	for _, include := range includes {
+		relative, err := relativeHeaderIncludePath(include)
+		if err != nil {
+			return err
+		}
+		w.WriteString(fmt.Sprintf("#include <%s>\n", relative))
+	}
+
 	for _, input := range inputs {
 		forEachPath(input.ast.up, func(node *node32) {
 			symbol := input.contents[node.begin:node.end]
@@ -1977,6 +1985,21 @@ func transform(w stringWriter, inputs []inputFile) error {
 	return nil
 }
 
+// relativeHeaderIncludePath returns the relative header path for usage in #include statements.
+func relativeHeaderIncludePath(path string) (string, error) {
+	dir, err := includePathFromHeaderFilePath(path)
+	if err != nil {
+		return "", err
+	}
+
+	relative, err := filepath.Rel(dir, path)
+	if err != nil {
+		return "", err
+	}
+
+	return relative, nil
+}
+
 // preprocess runs source through the C preprocessor.
 func preprocess(cppCommand []string, path string) ([]byte, error) {
 	var args []string
@@ -2096,6 +2119,7 @@ func main() {
 		})
 	}
 
+	var includes []string
 	includePaths := make(map[string]struct{})
 
 	for i, path := range flag.Args() {
@@ -2111,6 +2135,7 @@ func main() {
 				fmt.Fprintf(os.Stderr, "%s\n", err)
 				os.Exit(1)
 			}
+			includes = append(includes, path)
 			includePaths[dir] = struct{}{}
 			continue
 		}
@@ -2139,6 +2164,9 @@ func main() {
 
 		// -E requests only preprocessing.
 		cppCommand = append(cppCommand, "-E")
+
+		// Output ‘#include’ directives in addition to the result of preprocessing.
+		cppCommand = append(cppCommand, "-dI")
 	}
 
 	if err := parseInputs(inputs, cppCommand); err != nil {
@@ -2152,7 +2180,7 @@ func main() {
 	}
 	defer out.Close()
 
-	if err := transform(out, inputs); err != nil {
+	if err := transform(out, includes, inputs); err != nil {
 		fmt.Fprintf(os.Stderr, "%s\n", err)
 		os.Exit(1)
 	}
@@ -2232,7 +2260,7 @@ func isSynthesized(symbol string) bool {
 }
 
 func redirectorName(symbol string) string {
-	return "bcm_redirector_" + symbol
+	return ".Lbcm_redirector_" + symbol
 }
 
 // sectionType returns the type of a section. I.e. a section called “.text.foo”

diff --git a/util/fipstools/delocate/delocate_test.go b/util/fipstools/delocate/delocate_test.go
@@ -28,37 +28,39 @@ var (
 )
 
 type delocateTest struct {
-	name string
-	in   []string
-	out  string
+	name     string
+	includes []string
+	inputs   []string
+	out      string
 }
 
 func (test *delocateTest) Path(file string) string {
 	return filepath.Join(*testDataDir, test.name, file)
 }
 
 var delocateTests = []delocateTest{
-	{"generic-FileDirectives", []string{"in.s"}, "out.s"},
-	{"ppc64le-GlobalEntry", []string{"in.s"}, "out.s"},
-	{"ppc64le-LoadToR0", []string{"in.s"}, "out.s"},
-	{"ppc64le-Sample2", []string{"in.s"}, "out.s"},
-	{"ppc64le-Sample", []string{"in.s"}, "out.s"},
-	{"ppc64le-TOCWithOffset", []string{"in.s"}, "out.s"},
-	{"x86_64-Basic", []string{"in.s"}, "out.s"},
-	{"x86_64-BSS", []string{"in.s"}, "out.s"},
-	{"x86_64-GOTRewrite", []string{"in.s"}, "out.s"},
-	{"x86_64-LargeMemory", []string{"in.s"}, "out.s"},
-	{"x86_64-LabelRewrite", []string{"in1.s", "in2.s"}, "out.s"},
-	{"x86_64-Sections", []string{"in.s"}, "out.s"},
-	{"x86_64-ThreeArg", []string{"in.s"}, "out.s"},
-	{"aarch64-Basic", []string{"in.s"}, "out.s"},
+	{"generic-FileDirectives", nil, []string{"in.s"}, "out.s"},
+	{"generic-Includes", []string{"/some/include/path/openssl/foo.h", "/some/include/path/openssl/bar.h"}, []string{"in.s"}, "out.s"},
+	{"ppc64le-GlobalEntry", nil, []string{"in.s"}, "out.s"},
+	{"ppc64le-LoadToR0", nil, []string{"in.s"}, "out.s"},
+	{"ppc64le-Sample2", nil, []string{"in.s"}, "out.s"},
+	{"ppc64le-Sample", nil, []string{"in.s"}, "out.s"},
+	{"ppc64le-TOCWithOffset", nil, []string{"in.s"}, "out.s"},
+	{"x86_64-Basic", nil, []string{"in.s"}, "out.s"},
+	{"x86_64-BSS", nil, []string{"in.s"}, "out.s"},
+	{"x86_64-GOTRewrite", nil, []string{"in.s"}, "out.s"},
+	{"x86_64-LargeMemory", nil, []string{"in.s"}, "out.s"},
+	{"x86_64-LabelRewrite", nil, []string{"in1.s", "in2.s"}, "out.s"},
+	{"x86_64-Sections", nil, []string{"in.s"}, "out.s"},
+	{"x86_64-ThreeArg", nil, []string{"in.s"}, "out.s"},
+	{"aarch64-Basic", nil, []string{"in.s"}, "out.s"},
 }
 
 func TestDelocate(t *testing.T) {
 	for _, test := range delocateTests {
 		t.Run(test.name, func(t *testing.T) {
 			var inputs []inputFile
-			for i, in := range test.in {
+			for i, in := range test.inputs {
 				inputs = append(inputs, inputFile{
 					index: i,
 					path:  test.Path(in),
@@ -70,7 +72,7 @@ func TestDelocate(t *testing.T) {
 			}
 
 			var buf bytes.Buffer
-			if err := transform(&buf, inputs); err != nil {
+			if err := transform(&buf, test.includes, inputs); err != nil {
 				t.Fatalf("transform failed: %s", err)
 			}
 

diff --git a/util/fipstools/delocate/testdata/aarch64-Basic/out.s b/util/fipstools/delocate/testdata/aarch64-Basic/out.s
@@ -82,7 +82,7 @@ foo:
 	bl	.Llocal_function_local_target
 
 // WAS bl remote_function
-	bl	bcm_redirector_remote_function
+	bl	.Lbcm_redirector_remote_function
 
 	bl bss_symbol_bss_get
 
@@ -108,9 +108,9 @@ foo:
 	// But 'y' is not a register prefix so far, so these should be
 	// processed as symbols.
 // WAS add y0, y0
-	add	bcm_redirector_y0, bcm_redirector_y0
+	add	.Lbcm_redirector_y0, .Lbcm_redirector_y0
 // WAS add y12, y12
-	add	bcm_redirector_y12, bcm_redirector_y12
+	add	.Lbcm_redirector_y12, .Lbcm_redirector_y12
 
 	// Make sure that the magic extension constants are recognised rather
 	// than being interpreted as symbols.
@@ -138,29 +138,29 @@ bss_symbol:
 .text
 BORINGSSL_bcm_text_end:
 .p2align 2
-.hidden bcm_redirector_remote_function
-.type bcm_redirector_remote_function, @function
-bcm_redirector_remote_function:
+.hidden .Lbcm_redirector_remote_function
+.type .Lbcm_redirector_remote_function, @function
+.Lbcm_redirector_remote_function:
 .cfi_startproc
 	b remote_function
 .cfi_endproc
-.size bcm_redirector_remote_function, .-bcm_redirector_remote_function
+.size .Lbcm_redirector_remote_function, .-.Lbcm_redirector_remote_function
 .p2align 2
-.hidden bcm_redirector_y0
-.type bcm_redirector_y0, @function
-bcm_redirector_y0:
+.hidden .Lbcm_redirector_y0
+.type .Lbcm_redirector_y0, @function
+.Lbcm_redirector_y0:
 .cfi_startproc
 	b y0
 .cfi_endproc
-.size bcm_redirector_y0, .-bcm_redirector_y0
+.size .Lbcm_redirector_y0, .-.Lbcm_redirector_y0
 .p2align 2
-.hidden bcm_redirector_y12
-.type bcm_redirector_y12, @function
-bcm_redirector_y12:
+.hidden .Lbcm_redirector_y12
+.type .Lbcm_redirector_y12, @function
+.Lbcm_redirector_y12:
 .cfi_startproc
 	b y12
 .cfi_endproc
-.size bcm_redirector_y12, .-bcm_redirector_y12
+.size .Lbcm_redirector_y12, .-.Lbcm_redirector_y12
 .p2align 2
 .hidden bss_symbol_bss_get
 .type bss_symbol_bss_get, @function