Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix up fips integration #1

Merged
merged 8 commits into from
Jan 31, 2022

Conversation

jyn514
Copy link

@jyn514 jyn514 commented Jan 26, 2022

The git history for this is kind of a mess, I can clean it up if you like.

This primarily does the following things:

  • Build boringssl from source instead of requiring a pre-built version
  • Adds an automated test to make sure that fips is always enabled when the feature is set
  • Enforces in the build script that a supported version of clang is used to build boringssl when fips is enabled
  • Adds a new boringssl-fips submodule, since git submodule only allows having one canonical commit checked-in per submodule

It also makes some minor cleanups:

  • Fix tests that were broken with --feature fips
  • Adds a CI job for testing with --feature fips enabled
  • Fixes some hard-coded paths to match the source code layout in the older boringssl version
  • Uses X509_set_notBefore unconditionally rather than only when fips is enabled

I removed the BORING_BSSL_INCLUDE_PATH support because it was a little tricky to get to work properly with FIPS and it should be possible to fix independently anyway.

cc @cjmakes

The version of boringssl that's FIPS-certified doesn't have `X509_set1_notAfter`.
The only difference between that and `X509_set_notAfter` is whether they're const-correct,
which doesn't seem worth having two different code-paths.
NIST specifies that it needs to be 7.0.1; I originally tried building with clang 10 and it failed.
Theoretically this should check the versions of Go and Ninja too, but they haven't given me trouble in practice.

Example error:
```
   Compiling boring-sys v1.1.1 (/home/jnelson/work/boring/boring-sys)
error: failed to run custom build command for `boring-sys v1.1.1 (/home/jnelson/work/boring/boring-sys)`

Caused by:
  process didn't exit successfully: `/home/jnelson/work/boring/target/debug/build/boring-sys-31b8ce53031cfd83/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-env-changed=BORING_BSSL_PATH

  --- stderr
  warning: missing clang-7, trying other compilers: Permission denied (os error 13)
  warning: FIPS requires clang version 7.0.1, skipping incompatible version "clang version 10.0.0-4ubuntu1 "
  thread 'main' panicked at 'unsupported clang version "cc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0": FIPS requires clang 7.0.1', boring-sys/build.rs:216:13
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
README.md Outdated Show resolved Hide resolved
Now that build.rs checks out the appropriate commit automatically,
there's not a strong need to have the version number itself in the feature.
We can always add multiple features in the future if we support more than one FIPS-validated version.
@behrat behrat merged commit ac0234f into behrat:braden/add-fips-3678-feature Jan 31, 2022
@jyn514 jyn514 deleted the jnelson/fips branch January 31, 2022 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants