Inject FIPS hash without running module.

Previously, inject-hash would run the FIPS module in order to trigger a
failure and then extract the calculated hash value from the output. This
makes cross-compiling difficult because the build process needs to run a
binary for the target platform.

This change drops this step. Instead, inject-hash.go parses the object
file itself and calculates the hash without needing to run the module.

Change-Id: I2593daa03094b0a17b498c2e8be6915370669596
Reviewed-on: https://boringssl-review.googlesource.com/14964
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/fipsmodule/CMakeLists.txt

@@ -94,60 +94,10 @@ if(FIPS)
   set_target_properties(bcm_hashunset PROPERTIES POSITION_INDEPENDENT_CODE ON)
   set_target_properties(bcm_hashunset PROPERTIES LINKER_LANGUAGE C)
 
-  add_executable(
-    bcm_hashunset_test
-
-    bcm_hashunset_test.c
-
-    $<TARGET_OBJECTS:crypto_base>
-    $<TARGET_OBJECTS:stack>
-    $<TARGET_OBJECTS:lhash>
-    $<TARGET_OBJECTS:err>
-    $<TARGET_OBJECTS:base64>
-    $<TARGET_OBJECTS:bytestring>
-    $<TARGET_OBJECTS:pool>
-    $<TARGET_OBJECTS:digest_extra>
-    $<TARGET_OBJECTS:cipher>
-    $<TARGET_OBJECTS:modes>
-    $<TARGET_OBJECTS:aes>
-    $<TARGET_OBJECTS:des>
-    $<TARGET_OBJECTS:rc4>
-    $<TARGET_OBJECTS:conf>
-    $<TARGET_OBJECTS:chacha>
-    $<TARGET_OBJECTS:poly1305>
-    $<TARGET_OBJECTS:curve25519>
-    $<TARGET_OBJECTS:buf>
-    $<TARGET_OBJECTS:bn>
-    $<TARGET_OBJECTS:bio>
-    $<TARGET_OBJECTS:rand>
-    $<TARGET_OBJECTS:obj>
-    $<TARGET_OBJECTS:asn1>
-    $<TARGET_OBJECTS:engine>
-    $<TARGET_OBJECTS:dh>
-    $<TARGET_OBJECTS:dsa>
-    $<TARGET_OBJECTS:rsa>
-    $<TARGET_OBJECTS:ec>
-    $<TARGET_OBJECTS:ecdh>
-    $<TARGET_OBJECTS:ecdsa>
-    $<TARGET_OBJECTS:cmac>
-    $<TARGET_OBJECTS:evp>
-    $<TARGET_OBJECTS:hkdf>
-    $<TARGET_OBJECTS:pem>
-    $<TARGET_OBJECTS:x509>
-    $<TARGET_OBJECTS:x509v3>
-    $<TARGET_OBJECTS:pkcs8_lib>
-  )
-
-  target_link_libraries(bcm_hashunset_test bcm_hashunset)
-
-  if(NOT MSVC AND NOT ANDROID)
-    target_link_libraries(bcm_hashunset_test pthread)
-  endif()
-
   add_custom_command(
     OUTPUT bcm.o
-    COMMAND ${GO_EXECUTABLE} run crypto/fipsmodule/inject-hash.go crypto/fipsmodule/ar.go crypto/fipsmodule/const.go -o ${CMAKE_CURRENT_BINARY_DIR}/bcm.o -in $<TARGET_FILE:bcm_hashunset> -bin $<TARGET_FILE:bcm_hashunset_test>
-    DEPENDS bcm_hashunset_test bcm_hashunset inject-hash.go ar.go const.go
+    COMMAND ${GO_EXECUTABLE} run crypto/fipsmodule/inject-hash.go crypto/fipsmodule/ar.go crypto/fipsmodule/const.go -o ${CMAKE_CURRENT_BINARY_DIR}/bcm.o -in $<TARGET_FILE:bcm_hashunset>
+    DEPENDS bcm_hashunset inject-hash.go ar.go const.go
     WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
   )
 

crypto/fipsmodule/FIPS.md

@@ -49,7 +49,7 @@ The script performs a number of other transformations which are worth noting but
 
 In order to actually implement the integrity test, a constructor function within the module calculates an HMAC from `module_start` to `module_end` using a fixed, all-zero key. It compares the result with the known-good value added (by the script) to the unhashed portion of the text segment. If they don't match, it calls `exit` in an infinite loop.
 
-Initially the known-good value will be incorrect. Another script runs the module after an object file has been produced to get the calculated value which it then injects back into the object file.
+Initially the known-good value will be incorrect. Another script (`inject-hash.go`) calculates the correct value from the assembled object and injects it back into the object.
 
 ![build process](/crypto/fipsmodule/intcheck2.png)
 

crypto/fipsmodule/inject-hash.go

@@ -12,97 +12,147 @@
 // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
 // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
 
-// inject-hash runs a binary compiled against a FIPS module that hasn't had the
-// correct hash injected. That binary will fail the power-on integrity check
-// and write the calcualted hash value to stderr. This script parses that and
-// injects the calcualted value into the given object file.
+// inject-hash parses an archive containing a file object file. It finds a FIPS
+// module inside that object, calculates its hash and replaces the default hash
+// value in the object with the calculated value.
 package main
 
 import (
 	"bytes"
-	"encoding/hex"
+	"crypto/hmac"
+	"crypto/sha256"
+	"debug/elf"
 	"errors"
 	"flag"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
-	"os/exec"
-	"strings"
 )
 
-func do(outPath, arInput, binPath string) error {
-	cmd := exec.Command(binPath)
-	out, err := cmd.CombinedOutput()
+func do(outPath, arInput string) error {
+	arFile, err := os.Open(arInput)
+	if err != nil {
+		return err
+	}
+	defer arFile.Close()
 
-	if err == nil {
-		return errors.New("binary did not fail self test")
+	ar, err := ParseAR(arFile)
+	if err != nil {
+		return err
 	}
 
-	lines := strings.Split(string(out), "\n")
-	if len(lines) < 3 {
-		return fmt.Errorf("too few lines in output: %q", out)
+	if len(ar) != 1 {
+		return fmt.Errorf("expected one file in archive, but found %d", len(ar))
 	}
 
-	calculatedLine := lines[2]
-	if !strings.HasPrefix(calculatedLine, "Calculated: ") {
-		return errors.New("bad prefix of 3rd line: " + calculatedLine)
+	var objectBytes []byte
+	for _, contents := range ar {
+		objectBytes = contents
 	}
-	calculatedLine = calculatedLine[12:]
-	calculated, err := hex.DecodeString(calculatedLine)
+
+	object, err := elf.NewFile(bytes.NewReader(objectBytes))
 	if err != nil {
-		return err
+		return errors.New("failed to parse object: " + err.Error())
 	}
 
-	if len(calculated) != len(uninitHashValue) {
-		return fmt.Errorf("unexpected length of calculated hash: got %d, want %d", len(calculated), len(uninitHashValue))
+	// Find the .text section.
+
+	var textSection *elf.Section
+	var textSectionIndex elf.SectionIndex
+	for i, section := range object.Sections {
+		if section.Name == ".text" {
+			textSectionIndex = elf.SectionIndex(i)
+			textSection = section
+			break
+		}
 	}
 
-	arFile, err := os.Open(arInput)
-	if err != nil {
-		return err
+	if textSection == nil {
+		return errors.New("failed to find .text section in object")
 	}
-	defer arFile.Close()
 
-	ar, err := ParseAR(arFile)
+	// Find the starting and ending symbols for the module.
+
+	var startSeen, endSeen bool
+	var start, end uint64
+
+	symbols, err := object.Symbols()
 	if err != nil {
-		return err
+		return errors.New("failed to parse symbols: " + err.Error())
 	}
 
-	if len(ar) != 1 {
-		return fmt.Errorf("expected one file in archive, but found %d", len(ar))
+	for _, symbol := range symbols {
+		if symbol.Section != textSectionIndex {
+			continue
+		}
+
+		switch symbol.Name {
+		case "BORINGSSL_bcm_text_start":
+			if startSeen {
+				return errors.New("duplicate start symbol found")
+			}
+			startSeen = true
+			start = symbol.Value
+		case "BORINGSSL_bcm_text_end":
+			if endSeen {
+				return errors.New("duplicate end symbol found")
+			}
+			endSeen = true
+			end = symbol.Value
+		default:
+			continue
+		}
 	}
 
-	var object []byte
-	for _, contents := range ar {
-		object = contents
+	if !startSeen || !endSeen {
+		return errors.New("could not find module boundaries in object")
+	}
+
+	if max := textSection.Size; start > max || start > end || end > max {
+		return fmt.Errorf("invalid module boundaries: start: %x, end: %x, max: %x", start, end, max)
 	}
 
-	offset := bytes.Index(object, uninitHashValue[:])
+	// Extract the module from the .text section and hash it.
+
+	text := textSection.Open()
+	if _, err := text.Seek(int64(start), 0); err != nil {
+		return errors.New("failed to seek to module start in .text: " + err.Error())
+	}
+	moduleText := make([]byte, end-start)
+	if _, err := io.ReadFull(text, moduleText); err != nil {
+		return errors.New("failed to read .text: " + err.Error())
+	}
+
+	var zeroKey [32]byte
+	mac := hmac.New(sha256.New, zeroKey[:])
+	mac.Write(moduleText)
+	calculated := mac.Sum(nil)
+
+	// Replace the default hash value in the object with the calculated
+	// value and write it out.
+
+	offset := bytes.Index(objectBytes, uninitHashValue[:])
 	if offset < 0 {
 		return errors.New("did not find uninitialised hash value in object file")
 	}
 
-	if bytes.Index(object[offset+1:], uninitHashValue[:]) >= 0 {
+	if bytes.Index(objectBytes[offset+1:], uninitHashValue[:]) >= 0 {
 		return errors.New("found two occurrences of uninitialised hash value in object file")
 	}
 
-	copy(object[offset:], calculated)
-
-	if err := ioutil.WriteFile(outPath, object, 0644); err != nil {
-		return err
-	}
+	copy(objectBytes[offset:], calculated)
 
-	return nil
+	return ioutil.WriteFile(outPath, objectBytes, 0644)
 }
 
 func main() {
 	arInput := flag.String("in", "", "Path to a .a file")
 	outPath := flag.String("o", "", "Path to output object")
-	bin := flag.String("bin", "", "Binary compiled with the FIPS module")
 
 	flag.Parse()
 
-	if err := do(*outPath, *arInput, *bin); err != nil {
+	if err := do(*outPath, *arInput); err != nil {
 		fmt.Fprintf(os.Stderr, "%s\n", err)
 		os.Exit(1)
 	}

util/generate_build_files.py

@@ -650,9 +650,7 @@ def main(platforms):
   for path in FindCFiles(os.path.join('src', 'crypto'), OnlyTests):
     if IsGTest(path):
       crypto_test_files.append(path)
-      # bcm_hashunset_test.c is only used in the FIPS build process.
-    elif path != os.path.join('src', 'crypto', 'fipsmodule',
-                              'bcm_hashunset_test.c'):
+    else:
       test_c_files.append(path)
 
   ssl_test_files = FindCFiles(os.path.join('src', 'ssl'), OnlyTests)