I see we have some errors that look unreleated to what you did. Looks good to me beyond that: not sure if it's an issue on master or if it's from being based on an old branch or what. Thanks for your contribution!

Yeah I was wondering about that.

@arnarpall it's the updated clippy complaining about the FFI names. Adding -A clippy::upper_case_acronyms here: https://github.com/cloudflare/boring/blob/master/.github/workflows/ci.yml#L60 should fix that.

@wbl shouldn't we build from a particular branch for being FIPS-compatible? Or setting the compilation flag should be sufficient enough?

And with a specified toolchain.

From my tests I did not have to do any toolchain configuration or buld from a specific branch. The fips param into cmake seems to do the trick.

 root@991ccef6aaaf:/tmp/src/build# cmake -DFIPS=1 -DFIPS_DELOCATE=1 -GNinja ..
 -- The C compiler identification is Clang 11.0.0
 -- Check for working C compiler: /usr/bin/clang
 -- Check for working C compiler: /usr/bin/clang -- works
 -- Detecting C compiler ABI info
 -- Detecting C compiler ABI info - done
 -- Detecting C compile features
 -- Detecting C compile features - done
 -- The CXX compiler identification is Clang 11.0.0
 -- Check for working CXX compiler: /usr/bin/clang++
 -- Check for working CXX compiler: /usr/bin/clang++ -- works
 -- Detecting CXX compiler ABI info
 -- Detecting CXX compiler ABI info - done
 -- Detecting CXX compile features
 -- Detecting CXX compile features - done
 -- Found Perl: /usr/bin/perl (found version "5.30.3")
 -- Checking for module 'libunwind-generic'
 --   No package 'libunwind-generic' found
 libunwind not found. Disabling unwind tests.
 -- The ASM compiler identification is Clang
 -- Found assembler: /usr/bin/clang
 -- Configuring done
 -- Generating done
 -- Build files have been written to: /tmp/src/build
 root@991ccef6aaaf:/tmp/src/build# ninja bssl
 [319/319] Linking CXX executable tool/bssl
 
 root@991ccef6aaaf:/tmp/src/build/tool# ./bssl isfips
 1

Just becase isFIPS is true doesn't mean that you have built against a FIPS 140-2 certified module.

Here is an excerpt from the document that you linked in your issue

12. Guidance and Secure Operation

12.1 Installation Instructions

The following steps shall be performed to build, compile and statically link the BoringCrypto module to
BoringSSL on the tested Operational Environments. The cryptographic module
The below tools are required in order to build and compile the module:

• Clang compiler version 7.0.1 (http://releases.llvm.org/download.html)
• Go programming language version 1.12.7 (https://golang.org/dl/)
• Ninja build system version 1.90 (https://github.com/ninja-build/ninja/releases)
  Once the above tools have been obtained, issue the following command to create a CMake toolchain file
  to specify the use of Clang:

printf "set(CMAKE_C_COMPILER \"clang\")\nset(CMAKE_CXX_COMPILER \"clang++\")\n" >
${HOME}/toolchain

The FIPS 140-2 validated release of the module can be obtained by downloading the tarball containing
the source code at the following location:
https://commondatastorage.googleapis.com/chromium-boringssl-fips/boringsslae223d6138807a13006342edfeef32e813246b39.tar.xz
or by issuing the following command:
wget https://commondatastorage.googleapis.com/chromium-boringssl-fips/boringsslae223d6138807a13006342edfeef32e813246b39.tar.xz
The set of files specified in the archive constitutes the complete set of source files of the validated
module. There shall be no additions, deletions, or alterations of this set as used during module build.
The downloaded tarball file can be verified using the below SHA-256 digest value:
3b5fdf23274d4179c2077b5e8fa625d9debd7a390aac1d165b7e47234f648bb8
By issuing the following command:
• sha256sum boringssl-ae223d6138807a13006342edfeef32e813246b39.tar.xz
After the tarball has been extracted, the following commands will compile the module:

1. cd boringssl
2. mkdir build && cd build && cmake -GNinja -DCMAKE_TOOLCHAIN_FILE=${HOME}/toolchain -DFIPS=1 -DCMAKE_BUILD_TYPE=Release ..
3. ninja
4. ninja run_tests

Upon completion of the build process. The module’s status can be verified by issuing:
• ./tool/bssl isfips
The module will print “1” if it is in a FIPS 140-2 validated mode of operation.

I have also verified that the fips mode of operation works in a library that links against build of boring with the fips feature enabled.

@arnarpall You need to explain to me how this does that. I don't think our submodule points at that version of BoringSSL for one. If we do go down this path there should be documentation. I think it would be easier to let the user point at an already built FIPS version as then whatever toolchain fun they engaged in doesn't affect the rust bit.

Hey @wbl. I'm not exactly sure what you mean that the submodule does not point to the relevant version of BoringSSL, it's right here: https://github.com/google/boringssl/tree/067cfd92f4d7da0edfa073b096d090b98a83b860/src/crypto/fipsmodule
Some docs about the fips module itself is here: https://github.com/google/boringssl/blob/067cfd92f4d7da0edfa073b096d090b98a83b860/src/crypto/fipsmodule/FIPS.md

If I'm mistaken in how this is setup then I need to revisit this.

The most recent version of BoringSSL that went through FIPS validation is git commit ae223d6138807a13006342edfeef32e813246b39, tree 39abc40f885fa27499b15d1ae5180e7e793e514f, pointed at by the tag fips-20190808. That's not the same as the submodule which points at 067cfd92f4d7da0edfa073b096d090b98a83b860.

Secondly this assumes that a more recent clang or ninja or go don't affect the certification. Reading https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Module-Validation-Program/documents/fips140-2/FIPS1402IG.pdf it's very unclear if this is the case for anyone not Google.

Thanks for the clarification @wbl

I have checked with our compliance officer and they think the toolchain won't be a problem, but the right version of BoringSSL definitely is.

@wbl so, we good to merge this?

I would say this should be fine and then we could potentially just have a branch / tag that would in fact point towards the fips validated version of the boringssl submodule.

@inikulin do you think the tag pointing to the fips version is an appropriate solution?

@arnarpall @wbl I think so

@arnarpall can we add check/instructions or probably automate switching to the FIPS-validated branch in the build script?

Shouldn't that use 2 submodules, one for the normal build and one for the FIPS version build?

I think with the latest changes in master with the addition of the BORING_BSSL_PATH is just what we need.

By pointing BORING_BSSL_PATH to a FIPS validated branch of boring SSL and building with the FIPS feature flag enabled we should be able to build the crate in complete fips compliant way.

Ok after thinking about this some more we can do 2 things.

1. Discard this pull request entirely and just rely on a pre-built binary and just point BORING_BSSL_PATH to that version.
2. React to the FIPS flag as we are doing here, with the added feature @nox suggests and initialise boring-sys/deps/boringssl with a branch of boringssl that is fips validated. Another environment variable could be added that controls the version.

I would prefer going with suggestion number 2

@inikulin Have you had any time to take a look at my proposal in my latest comment ?

React to the FIPS flag as we are doing here, with the added feature @nox suggests and initialise boring-sys/deps/boringssl with a branch of boringssl that is fips validated. Another environment variable could be added that controls the version.

@gabi-250 has run into issues with concurrent builds where if two build scripts are running at the same time with different features, it will error/cause an unexpected git state. They suggested using two different submodules instead, one for FIPS builds and one when the FIPS feature is disabled.

Closing in favor of #52.