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

feat(build): validate FIPS mode at build time and runtime #693

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

swiatekm
Copy link

@swiatekm swiatekm commented Aug 3, 2022

This adds an additional file to the main package which:

  1. Imports a module that is supposed to guarantee that we're always running in FIPS mode: https://go.googlesource.com/go/+/dev.boringcrypto/src/crypto/tls/fipsonly/fipsonly.go
  2. Adds an init check for whether we're using BoringSSL and prints a log line if so

It's not clear whether this is a good enough solution to attest that we're running in FIPS mode. Maybe we should also add a test that tries to use a FIPS-incompatible cipher for TLS and fails?

@github-actions github-actions bot added the go label Aug 3, 2022
@swiatekm swiatekm force-pushed the fix/fips-validation branch 2 times, most recently from be71ff1 to a7fd438 Compare August 3, 2022 11:26
@swiatekm swiatekm force-pushed the fix/fips-compliance branch from 1ca25e5 to 5899e09 Compare August 4, 2022 09:16
Base automatically changed from fix/fips-compliance to main August 4, 2022 09:34
@swiatekm swiatekm force-pushed the fix/fips-validation branch from a7fd438 to e4e556c Compare August 4, 2022 10:15
@swiatekm swiatekm force-pushed the fix/fips-validation branch from e4e556c to 597f37a Compare October 5, 2022 13:36
@swiatekm swiatekm marked this pull request as ready for review October 5, 2022 13:36
@swiatekm swiatekm requested a review from a team as a code owner October 5, 2022 13:36
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 5, 2022
Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should also add a test that tries to use a FIPS-incompatible cipher for TLS and fails?

That would be nice, but I think it can be done as separate PR

@swiatekm swiatekm force-pushed the fix/fips-validation branch from 597f37a to 2aaf635 Compare October 6, 2022 12:42
@swiatekm swiatekm enabled auto-merge (rebase) October 6, 2022 12:42
@swiatekm swiatekm merged commit acb1ee4 into main Oct 6, 2022
@swiatekm swiatekm deleted the fix/fips-validation branch October 6, 2022 12:43
@swiatekm
Copy link
Author

swiatekm commented Oct 6, 2022

Maybe we should also add a test that tries to use a FIPS-incompatible cipher for TLS and fails?

That would be nice, but I think it can be done as separate PR

I actually checked this, and it requires doing a more involved integration test - we'd need to start the collector with a receiver/exporter and make assertions about what they send over TLS.

@sumo-drosiek
Copy link
Contributor

Maybe we should also add a test that tries to use a FIPS-incompatible cipher for TLS and fails?

That would be nice, but I think it can be done as separate PR

I actually checked this, and it requires doing a more involved integration test - we'd need to start the collector with a receiver/exporter and make assertions about what they send over TLS.

So, let's create an issue 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation go
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants