-
Notifications
You must be signed in to change notification settings - Fork 29
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
support FIPS for Go builds #162
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
⬆️ force push to fix the FIXME |
monirul
reviewed
Mar 21, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bcressey , I added few comments, mostly related to the go code, as I'm trying to grasp the underlying intention of the code. Thanks.
rpkelly
approved these changes
Mar 25, 2024
webern
approved these changes
Mar 26, 2024
arnaldo2792
approved these changes
Mar 26, 2024
hyandell
reviewed
Mar 28, 2024
hyandell
approved these changes
Mar 29, 2024
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
The upstream project's `build-goboring.sh` script contains logic to generate the self-contained syso files expected by the Go toolchain. However, it also expects to use the host's toolchain directly, by way of `clang`, `ld`, and `objcopy` invocations. This complicates the use of the cross-compiling toolchains needed for Bottlerocket. Factor out the reusable parts of the script in a way that allows for custom toolchain use. `parse-functions.go` performs the same function as the embedded awk scripts, while `umod-amd64.s` and `umod-arm64.c` are extracted with no changes. Signed-off-by: DJ Edwards <[email protected]> Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Add two new macros to allow for parallel installation of FIPS-built binaries in `/usr/fips/bin` and `/usr/fips/libexec`. This simplifies the process of building binaries in FIPS and non-FIPS mode within the same package. Also add a buildroot policy script to check that FIPS binaries exist anywhere that non-FIPS binaries do. For now this only supports Go. Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
⬆️ force push to rebase and fix the |
jpculp
approved these changes
Mar 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue number:
Related: bottlerocket-os/bottlerocket#1667
Description of changes:
Conceptually this patch series has two halves.
The first half consists of patching Go, patching AWS-LC, building AWS-LC sysos, then building Go with those sysos instead of the stock "boringcrypto" ones. All of this is done so that
GOEXPERIMENT=boringcrypto go build ...
does the right thing. This extends the earlier unified SDK work in that we're able to replace the sysos for both amd64 and arm64, so the Go toolchain should just work for both architectures whether or not the "boringcrypto" experiment is enabled.The second half makes it easier to use
GOEXPERIMENT=boringcrypto go build ...
in downstream packaging of Go binaries. There are new macros, including a new way to set up the environment for static builds, and a lint script that checks whether all Go binaries that should be are built in a parallel version with FIPS enabled. That script is currently disabled by default: it can be enabled on a per-package basis for testing, and will be enabled by default in a future SDK release, after packaging has a chance to catch up.These changes should be backwards compatible with existing packaging - the AWS-LC bits are only used when
GOEXPERIMENT=boringcrypto
is set, the new lint doesn't run by default, and the old%set_cross_go_flags
is mostly unchanged.Testing done:
Made all the packaging changes required with a slightly different build where the lint script was on by default. (It helped me catch a ton of problems!) See https://github.com/bcressey/bottlerocket/commits/fips-and-non-fips/ for what this looks like in action with the (forthcoming) twoliter changes as well.
Built and launched
aws-k8s-1.28
AMIs where only the FIPS versions of Go binaries were used. The nodes came up, joined the cluster, ran pods, etc.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.