Address review feedback on Windows FIPS hash injection

- aws-lc-rs.yml: rewrite stale Wine-setup comment. The build no longer
  runs fips_empty_main.exe; Wine is only needed for the downstream
  FIPS sanity test step that loads the cross-built DLL.
- run_windows_tests.bat: restore crypto.dll from backup before checking
  the negative-test exit code, so a failure here does not leave a
  corrupted DLL behind for subsequent local invocations.
- inject_hash.go (doWindows): require both BORINGSSL_bcm_rodata_{start,end}
  markers unconditionally. Windows FIPS is always a shared-library build
  and the runtime hashes rodata in that configuration; silently skipping
  rodata here would produce a hash that disagrees with the runtime.
- fipscommon/pe.go: add trailing newline at EOF.

Author: justsmth

.github/workflows/aws-lc-rs.yml

@@ -310,8 +310,11 @@ jobs:
         run: |
           set -ex
           # Wine binfmt allows the kernel to transparently run .exe files through
-          # Wine. This is needed for the FIPS build, which runs fips_empty_main.exe
-          # at build time to capture the integrity hash.
+          # Wine. The FIPS build itself no longer needs Wine (inject_hash.go
+          # patches the integrity hash directly into crypto.dll using the
+          # linker map), but we still need Wine to run the FIPS sanity test
+          # below, which loads the cross-built DLL so the .CRT$XCU initializer
+          # can trigger the power-on self-test.
           #
           # Ubuntu 24.04's wine64 (9.0) does not properly execute .CRT$XCU
           # initializers in cross-compiled DLLs, which prevents the FIPS

tests/ci/run_windows_tests.bat

@@ -76,13 +76,17 @@ cd /d %SRC_ROOT%
 go run util/fipstools/break-hash.go -map %BUILD_DIR%\crypto\fips_crypto.map %BUILD_DIR%\crypto\crypto.dll %BUILD_DIR%\crypto\crypto_corrupted.dll || goto error
 copy /y %BUILD_DIR%\crypto\crypto_corrupted.dll %BUILD_DIR%\crypto\crypto.dll || goto error
 %BUILD_DIR%\util\fipstools\test_fips.exe 2>nul
-if %ERRORLEVEL% equ 0 (
-    echo FIPS integrity negative test failed: test_fips should have failed with corrupted crypto.dll
-    goto error
-)
+set FIPS_NEGATIVE_RC=%ERRORLEVEL%
+@rem Restore the unmodified DLL before we decide whether the test passed or
+@rem failed, so that a failure here does not leave a corrupted crypto.dll
+@rem behind for any subsequent local invocation.
 copy /y %BUILD_DIR%\crypto\crypto.dll.bak %BUILD_DIR%\crypto\crypto.dll || goto error
 del /q %BUILD_DIR%\crypto\crypto.dll.bak
 del /q %BUILD_DIR%\crypto\crypto_corrupted.dll
+if %FIPS_NEGATIVE_RC% equ 0 (
+    echo FIPS integrity negative test failed: test_fips should have failed with corrupted crypto.dll
+    goto error
+)
 
 @echo  LOG: %date%-%time% %1 %2 FIPS validation complete
 exit /b 0

util/fipstools/fipscommon/pe.go

@@ -89,4 +89,4 @@ func (p *PEInfo) ResolveSymbolFileOffset(symbolAddrs map[string]uint64, name str
 		}
 	}
 	return 0, fmt.Errorf("RVA 0x%x for %q not found in any PE section", rva, name)
-}
+}

util/fipstools/inject_hash/inject_hash.go

@@ -330,17 +330,20 @@ func doWindows(objectBytes []byte, mapPath string) ([]byte, []byte, error) {
 		return nil, nil, err
 	}
 
-	var moduleROData []byte
+	// The Windows FIPS build is always a shared library (see the `windowsOS`
+	// branch of `do` below, which rejects static inputs). In shared builds the
+	// runtime integrity check in bcm.c hashes the rodata region in addition
+	// to the text region, so the map file must contain both rodata markers.
+	// Silently skipping rodata here would produce a hash that disagrees with
+	// what the runtime computes and the DLL would fail the power-on self-test.
 	_, hasRodataStart := symbolAddrs["BORINGSSL_bcm_rodata_start"]
 	_, hasRodataEnd := symbolAddrs["BORINGSSL_bcm_rodata_end"]
-	if hasRodataStart != hasRodataEnd {
-		return nil, nil, errors.New("rodata marker presence inconsistent")
+	if !hasRodataStart || !hasRodataEnd {
+		return nil, nil, errors.New("rodata markers missing from map file; Windows FIPS shared build requires both BORINGSSL_bcm_rodata_start and BORINGSSL_bcm_rodata_end")
 	}
-	if hasRodataStart {
-		moduleROData, err = extractRegion("BORINGSSL_bcm_rodata_start", "BORINGSSL_bcm_rodata_end")
-		if err != nil {
-			return nil, nil, err
-		}
+	moduleROData, err := extractRegion("BORINGSSL_bcm_rodata_start", "BORINGSSL_bcm_rodata_end")
+	if err != nil {
+		return nil, nil, err
 	}
 
 	return moduleText, moduleROData, nil