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

MacOS FIPS compliant app using FIPS OpenSSL doesn't restrict TLS ciphers to FIPS only #1521

Open
lmaliniak opened this issue Feb 4, 2025 · 10 comments
Labels
fips new-platform Support a platform new to the microsoft/go infrastructure question This issue is a question about the project

Comments

@lmaliniak
Copy link

lmaliniak commented Feb 4, 2025

I was able to build MacOS app using Microsoft Go fork after updating the patches to create openssl_darwin.go.
But, testing the Go app and capturing the advertised ciphers, they contain FIPS and non-FIPS, which is unexpected.
FIPS openssl vendor is installed and fips openssl config was verified. The FIPS mode is enabled in the app on runtime.
The gap from openssl_linux is the following:


if openssl.FIPS() {
		// FIPS mode is enabled,
		// so force FIPS mode for crypto/tls and crypto/x509.
		fipstls.Force()
	}
	sig.BoringCrypto()

Checking the code I understood that it is not allowed to import the fipstls package since it belongs to BoringCrypto. The comment in the top of the file is confusing because of the go build statement in tls.go.

//go:build boringcrypto || goexperiment.opensslcrypto || goexperiment.cngcrypto

// Package fipstls allows control over whether crypto/tls requires FIPS-approved settings.
// This package only exists with GOEXPERIMENT=boringcrypto, but the effects are independent
// of the use of BoringCrypto.

Trying to concatenate boringcrypto to the systemcrypto or opensslcrypto resulted in build error, when trying to build the application.
Trying to add the Force function in the openssl_darwin.go, it didn't have the effect of restricting the advertised TLS ciphers.
I understand that Linux is the only supported platform for BoringCrypto.
go/src/crypto/boring
[Can BoringCrypto be used only for Linux](https://stackoverflow.com/questions/75954995/can-boringcrypto-be-used-only-for-linux-linux-amd64-and-linux-arm64)

What is the recommended way to resolve above?

Thanks

@dagood
Copy link
Member

dagood commented Feb 4, 2025

This is something we're still working on documenting properly--there are some upstream Go 1.24 changes that introduce some new concepts that affect how this works.

You should be able to set GODEBUG=fips140=on at runtime to enable Go runtime FIPS mode and therefore Go TLS FIPS mode.

If you set GOFIPS140=latest at build time, then the Go runtime will always be in FIPS mode even if you don't set GODEBUG=fips140=on.

On macOS, there's no system-wide FIPS indicator like there is for Linux and Windows, so the runtime environment needs to use a Go-specific FIPS configuration environment variable to indicate that the Go runtime (and therefore Go TLS) should run in a FIPS-compliant mode.

@dagood dagood added question This issue is a question about the project fips new-platform Support a platform new to the microsoft/go infrastructure labels Feb 4, 2025
@lmaliniak
Copy link
Author

@dagood, I'll try the MacOs build of the app with FIPS openssl 3.0.9 and set GODEBUG=fips140=on at runtime.
The Microsoft build I'm using is with changes I did to enable build of MacOS using FIPS openssl.
The flags I am using for the app build are goexperiment.systemcrypto goexperiment.opensslcrypto
The version I've created is:

go version devel go1.23-f0de94ff12 Mon Jul 22 15:45:09 2024 +0000 darwin/arm64

I've set GODEBUG=fips140 and rebuilt the Go app.

Then restarted the app and captured the traffic using Wireshark. The advertised ciphers in the TLS handshake include FIPS and non-FIPS ciphers
Example:

Cipher Suites (19 suites)
Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b)
Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f)
Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 (0xc02c)
Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (0xc030)
Cipher Suite: TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca9)
Cipher Suite: TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca8)
Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA (0xc009)
Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (0xc013)
Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA (0xc00a)
Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014)
Cipher Suite: TLS_RSA_WITH_AES_128_GCM_SHA256 (0x009c)
Cipher Suite: TLS_RSA_WITH_AES_256_GCM_SHA384 (0x009d)
Cipher Suite: TLS_RSA_WITH_AES_128_CBC_SHA (0x002f)
Cipher Suite: TLS_RSA_WITH_AES_256_CBC_SHA (0x0035)
Cipher Suite: TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA (0xc012)
Cipher Suite: TLS_RSA_WITH_3DES_EDE_CBC_SHA (0x000a)
Cipher Suite: TLS_AES_128_GCM_SHA256 (0x1301)
Cipher Suite: TLS_AES_256_GCM_SHA384 (0x1302)
Cipher Suite: TLS_CHACHA20_POLY1305_SHA256 (0x1303)

@dagood
Copy link
Member

dagood commented Feb 5, 2025

Ah, I missed that you're using OpenSSL here on macOS. But same as #1520, you haven't provided enough info to reproduce. Can you try with my test app?

@lmaliniak
Copy link
Author

@dagood, yes. I'll update.

@lmaliniak
Copy link
Author

@dagood, just to make sure, was your app tested running with FIPS openssl on MacOS before?
What about the following code that is not being used?

if openssl.FIPS() {
// FIPS mode is enabled,
// so force FIPS mode for crypto/tls and crypto/x509.
fipstls.Force()
}
sig.BoringCrypto()

What is the impact on linux that won't happen on darwin?

Is there any change in the Microsoft Go fork related to support of openssl for MacOS in latest code in master?
Thanks

@dagood
Copy link
Member

dagood commented Feb 5, 2025

just to make sure, was your app tested running with FIPS openssl on MacOS before?

I haven't run this, no. I haven't gotten OpenSSL set up on macOS yet. Perhaps @gdams or @qmuntal have?

A few questions to help the repro:

  • Where is the OpenSSL 3.0.9 build from?
  • Is this based on a recent microsoft/main commit of microsoft/go? (I've been assuming this.) Or is it one of the release branches?
  • What do you get with the test app?

One thing missing to repro is source code, and using my test app is one way to fix that. Most apps should behave the same, but that also means it should be easy to make a repro program that can be freely shared. 😄

I've set GODEBUG=fips140 and rebuilt the Go app.

Then restarted the app and captured the traffic using Wireshark.

GODEBUG=fips140=on doesn't have any effect at build-time, maybe try GOFIPS140=latest instead.

So far, it sounds like this might be an issue of setting the env variables in the wrong place, like #1520.

What about the following code that is not being used?

I don't see this code, where is it?

@dagood
Copy link
Member

dagood commented Feb 5, 2025

The version I've created is:

go version devel go1.23-f0de94ff12 Mon Jul 22 15:45:09 2024 +0000 darwin/arm64

Ah, sorry, just noticed this hiding in plain sight. 😄

1.24 changes a lot. In 1.23, you should use GOFIPS=1, and you need to read the FIPS docs carefully to make note of any parts of the doc that don't apply prior to 1.24.

In 1.23, the OpenSSL backend is only supported on Linux. It's not just fipstls.Force() that's hidden behind a linux build tag, but also the initialization code for OpenSSL.

I'm unsure whether or not OpenSSL is supported on macOS in 1.24 (microsoft/main), but it's more likely.

@dagood
Copy link
Member

dagood commented Feb 5, 2025

updating the patches to create openssl_darwin.go

Ah, I also missed that you are also trying to modify 1.23. I made a lot of early assumptions based on #1520 (I thought this issue was simply a followup question) and haven't shaken them all yet.

Checking the code I understood that it is not allowed to import the fipstls package since it belongs to BoringCrypto. The comment in the top of the file is confusing because of the go build statement in tls.go.

openssl_linux.go imports crypto/internal/boring/fipstls and uses it, so I would expect openssl_darwin.go to use it too.

It's not feasible for our patches to update every comment line in the upstream code--in fact, we may avoid doing so to reduce the diff vs. upstream and avoid merge conflicts. As a rule of thumb, if you take a look at the diff between the patched repo and the commit it's based on (e.g. git diff go1.23.6 -- src/crypto/internal/boring/fipstls/tls.go) you can see we didn't touch the comment, so it's reasonably safe to assume the comment is out of date if it doesn't match up with the current logic.

@lmaliniak
Copy link
Author

lmaliniak commented Feb 5, 2025

@dagood, the answer is yes. I downloaded and installed official FIPS openssl and configured it for FIPS and did changes to build Microsoft Go fork with the openssl and even opened a PR (which you closed).
But testing it, I found that the advertised ciphers are not restricted to FIPS.

Since the 1.24 that I've tested with the CommonCrypto integration doesn't have openssl_darwin.go, the only way to try it is to update the patches and build again. Correct?
Even then, what options do I have to force the TLS ciphers based on the FIPS openssl?
OpenSSL FIPS 140-2 Security Policy
The link is in Appendix A (a little bit broken). openssl 3.0.9 download

The version I have for 1.24:
GOVERSION='devel go1.24-608acff847 Wed Jan 22 10:13:04 2025 -0800 X:allowcryptofallback'

@dagood
Copy link
Member

dagood commented Feb 5, 2025

I took another look at 1.24, and it appears it will still require a openssl_darwin.go, or generalizing openssl_linux.go and changing it to openssl.go. Some of the FIPS-related files (like crypto\internal\backend\fips140\openssl_cgo.go) don't have a linux build tag, so I would expect that you don't need to touch them (this is why I was unsure), but crypto\internal\backend\openssl_linux.go still does have the linux tag.

Apologies that we haven't taken this work into the repo--we're a relatively small team, and need to be careful to avoid adding features that might make it harder for us to maintain the features that our internal users need. We're focusing on cryptokit/commoncrypto for our internal users that target macOS.

(Also, want to make sure you see my last comment, about crypto/internal/boring/fipstls (you should be able to use it), since we posted at just about the same time and that might mess with notifications. I misunderstood this issue from the start and I think I finally caught up. Sorry about that!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fips new-platform Support a platform new to the microsoft/go infrastructure question This issue is a question about the project
Projects
None yet
Development

No branches or pull requests

2 participants