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>
diff --git a/crypto/fipsmodule/CMakeLists.txt b/crypto/fipsmodule/CMakeLists.txt index dea5b4f..171e6a9 100644 --- a/crypto/fipsmodule/CMakeLists.txt +++ b/crypto/fipsmodule/CMakeLists.txt
@@ -94,60 +94,10 @@ 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} )
diff --git a/crypto/fipsmodule/FIPS.md b/crypto/fipsmodule/FIPS.md index 2e0151a..06ef9fb 100644 --- a/crypto/fipsmodule/FIPS.md +++ b/crypto/fipsmodule/FIPS.md
@@ -49,7 +49,7 @@ 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. 
diff --git a/crypto/fipsmodule/bcm_hashunset_test.c b/crypto/fipsmodule/bcm_hashunset_test.c deleted file mode 100644 index c62761e..0000000 --- a/crypto/fipsmodule/bcm_hashunset_test.c +++ /dev/null
@@ -1,11 +0,0 @@ -#include <openssl/digest.h> - -#if !defined(BORINGSSL_FIPS) -#error "This file should not be built outside of the FIPS build." -#endif - -int main(void) { - /* This program only needs to trigger the FIPS power-on self-test. */ - EVP_sha256(); - return 0; -}
diff --git a/crypto/fipsmodule/inject-hash.go b/crypto/fipsmodule/inject-hash.go index 6ce5b88..7f2c9c9 100644 --- a/crypto/fipsmodule/inject-hash.go +++ b/crypto/fipsmodule/inject-hash.go
@@ -12,51 +12,25 @@ // 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() - - if err == nil { - return errors.New("binary did not fail self test") - } - - lines := strings.Split(string(out), "\n") - if len(lines) < 3 { - return fmt.Errorf("too few lines in output: %q", out) - } - - calculatedLine := lines[2] - if !strings.HasPrefix(calculatedLine, "Calculated: ") { - return errors.New("bad prefix of 3rd line: " + calculatedLine) - } - calculatedLine = calculatedLine[12:] - calculated, err := hex.DecodeString(calculatedLine) - if err != nil { - return err - } - - if len(calculated) != len(uninitHashValue) { - return fmt.Errorf("unexpected length of calculated hash: got %d, want %d", len(calculated), len(uninitHashValue)) - } - +func do(outPath, arInput string) error { arFile, err := os.Open(arInput) if err != nil { return err @@ -72,37 +46,113 @@ return fmt.Errorf("expected one file in archive, but found %d", len(ar)) } - var object []byte + var objectBytes []byte for _, contents := range ar { - object = contents + objectBytes = contents } - offset := bytes.Index(object, uninitHashValue[:]) + object, err := elf.NewFile(bytes.NewReader(objectBytes)) + if err != nil { + return errors.New("failed to parse object: " + err.Error()) + } + + // 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 + } + } + + if textSection == nil { + return errors.New("failed to find .text section in object") + } + + // 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 errors.New("failed to parse symbols: " + err.Error()) + } + + 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 + } + } + + 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) + } + + // 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) + copy(objectBytes[offset:], calculated) - if err := ioutil.WriteFile(outPath, object, 0644); err != nil { - return err - } - - 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) }
diff --git a/crypto/fipsmodule/intcheck2.png b/crypto/fipsmodule/intcheck2.png index ad1fe27..3e66267 100644 --- a/crypto/fipsmodule/intcheck2.png +++ b/crypto/fipsmodule/intcheck2.png Binary files differ
diff --git a/util/generate_build_files.py b/util/generate_build_files.py index 4225cf2..8e23040 100644 --- a/util/generate_build_files.py +++ b/util/generate_build_files.py
@@ -650,9 +650,7 @@ 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)
