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

Integrate FIPS changes into microsoft/main #501

Closed
dagood opened this issue Mar 28, 2022 · 6 comments
Closed

Integrate FIPS changes into microsoft/main #501

dagood opened this issue Mar 28, 2022 · 6 comments
Labels

Comments

@dagood
Copy link
Member

dagood commented Mar 28, 2022

One this is done, we need to make changes pretty much throughout. On the code/patch side:

  • Move platform-native crypto patches into microsoft/main.
  • Rework patches to work with GOEXPERIMENT flag.
  • If we aren't totally sure the GOEXPERIMENT toggle will disable our changes properly, make the FIPS patches only get applied optionally.
    • If our patches stay within the bounds of the boringcrypto GOEXPERIMENT flag, this probably won't be an issue. (It's broadly important in upstream Go that if the experiment flag isn't set, nothing breaks.) But if we have to make some changes that reach outside, we may want to be more cautious.

microsoft/go builds:

  • Each internal rolling build matrix most likely needs to include normal and GOEXPERIMENT builds.
    • This would be in case the flag causes bad behavior for some customers who don't need FIPS, so they can easily switch to the normal build until we can figure out the issue. (Bad behavior could be due to our patches, due to upstream changes, or a combination.)
  • The files need to be distinguished: at the moment, both boring and non-boring branches produce go{buildnum}.linux-x64.tar.gz. A build number suffix seems reasonable.
  • More data in build asset JSON to make these artifacts distinct in a way that tooling can determine later without depending very strongly on the filename convention?

microsoft/go-infra:

  • microsoft/go -> microsoft/go-images auto-update needs to handle the new artifact names, updating FIPS and non-FIPS at the same time.
  • aka.ms link generation handling.

microsoft/go-images:

  • No change, once upstream build URLs get merged into this repo, the go-images build doesn't really care where they came from.

This simplifies things: #501 (comment)

@dagood
Copy link
Member Author

dagood commented May 4, 2022

Upstream boring is merged into upstream master! golang/go@f771edd
FYI @qmuntal @jaredpar

@qmuntal
Copy link
Contributor

qmuntal commented May 5, 2022

Wow! I saw that commit when it landed but as it's description starts with REVERSE ... I just skipped it thinking they were reverting something that broke CI.

Next release is on ~August, so we have three months to get our openssl integration in sync with the new upstream changes.

@dagood
Copy link
Member Author

dagood commented May 27, 2022

So, it turns out all we need to do to maintain the behavior of our 1.18-fips Docker images is to produce a 1.19-fips image that consumes the same build as 1.19, but sets GOEXPERIMENT=opensslcrypto in the environment. (A lot of steps in the original issue description aren't necessary.) Our users could get a 1.19 image and set GOEXPERIMENT themselves, but I think for the sake of easily swapping images to produce FIPS-compliant binaries, they'll want a 1.19-fips image.

For CBL-Mariner distro builds from source, we think they can consume the 1.19 source tarball. (One that doesn't set a GOEXPERIMENT by default.) If desired, I believe they can include a line in the spec file to set GOEXPERIMENT=opensslcrypto: https://github.com/microsoft/CBL-Mariner/blob/28f679facbe1016c07a862590843108896b7af62/SPECS/golang/golang-1.16.spec#L93-L99.

More info about GOEXPERIMENT and GOFIPS behavior and some test runthroughs: #548 (comment)

@qmuntal
Copy link
Contributor

qmuntal commented Jul 11, 2022

@dagood is there any remaining that here?

@dagood
Copy link
Member Author

dagood commented Jul 11, 2022

I took a look at the microsoft/nightly 1.19 images PR again:

and saw it was missing this variant:

same build as 1.19, but sets GOEXPERIMENT=opensslcrypto in the environment

When we release 1.19(.0), I should only have to make sure to set up the microsoft/main Docker images in that way rather than the pre-1.19 way and we're good.

(I also updated the PR to get rid of an EOL distro before the .0 release, and add back in CBL-Mariner 2.0 that I had missed.)

I'd say once that go-images nightly branch PR is merged, we can close this thread.

@dagood
Copy link
Member Author

dagood commented Feb 2, 2023

Done a while ago.

@dagood dagood closed this as completed Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants