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.

Sign up for GitHub

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

Introduce new build tags to optionally override some min and max TLS settings #2162

Merged
merged 2 commits into from
Dec 20, 2024
Merged

Introduce new build tags to optionally override some min and max TLS settings #2162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b625b4a
introduce build tags to optionally override some TLS settings
cfryanr Dec 19, 2024
ef4b0c9
bump golang.org/x/net
cfryanr Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ ENV KUBE_GIT_VERSION=$KUBE_GIT_VERSION
ARG TARGETOS
ARG TARGETARCH

# If provided, must be a comma-separated list of Go build tags.
ARG ADDITIONAL_BUILD_TAGS

# Build the statically linked (CGO_ENABLED=0) binary.
# Mount source, build cache, and module cache for performance reasons.
# See https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/
Expand All @@ -29,8 +32,8 @@ RUN \
--mount=type=cache,target=/cache/gocache \
--mount=type=cache,target=/cache/gomodcache \
export GOCACHE=/cache/gocache GOMODCACHE=/cache/gomodcache CGO_ENABLED=0 GOOS=$TARGETOS GOARCH=$TARGETARCH && \
go build -v -trimpath -ldflags "$(hack/get-ldflags.sh) -w -s" -o /usr/local/bin/pinniped-concierge-kube-cert-agent ./cmd/pinniped-concierge-kube-cert-agent/... && \
go build -v -trimpath -ldflags "$(hack/get-ldflags.sh) -w -s" -o /usr/local/bin/pinniped-server ./cmd/pinniped-server/... && \
go build -tags $ADDITIONAL_BUILD_TAGS -v -trimpath -ldflags "$(hack/get-ldflags.sh) -w -s" -o /usr/local/bin/pinniped-concierge-kube-cert-agent ./cmd/pinniped-concierge-kube-cert-agent/... && \
go build -tags $ADDITIONAL_BUILD_TAGS -v -trimpath -ldflags "$(hack/get-ldflags.sh) -w -s" -o /usr/local/bin/pinniped-server ./cmd/pinniped-server/... && \
ln -s /usr/local/bin/pinniped-server /usr/local/bin/pinniped-concierge && \
ln -s /usr/local/bin/pinniped-server /usr/local/bin/pinniped-supervisor && \
ln -s /usr/local/bin/pinniped-server /usr/local/bin/local-user-authenticator
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ require (
go.uber.org/mock v0.5.0
go.uber.org/zap v1.27.0
golang.org/x/crypto v0.31.0
golang.org/x/net v0.32.0
golang.org/x/net v0.33.0
golang.org/x/oauth2 v0.24.0
golang.org/x/sync v0.10.0
golang.org/x/term v0.27.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,8 +766,8 @@ golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns=
golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44=
golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
golang.org/x/net v0.32.0 h1:ZqPmj8Kzc+Y6e0+skZsuACbx+wzMgo5MQsJh9Qd6aYI=
golang.org/x/net v0.32.0/go.mod h1:CwU0IoeOlnQQWJ6ioyFrfRuomB8GKF6KbYXZVyeXNfs=
golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
Expand Down
7 changes: 5 additions & 2 deletions hack/Dockerfile_fips
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ ENV KUBE_GIT_VERSION=$KUBE_GIT_VERSION
ARG TARGETOS
ARG TARGETARCH

# If provided, must be a comma-separated list of Go build tags.
ARG ADDITIONAL_BUILD_TAGS

# Build the executable binary (CGO_ENABLED=1 is required for go boring).
# Even though we need cgo to call the boring crypto C functions, these
# functions are statically linked into the binary. We also want to statically
Expand All @@ -59,8 +62,8 @@ RUN \
--mount=type=cache,target=/cache/gocache \
--mount=type=cache,target=/cache/gomodcache \
export GOCACHE=/cache/gocache GOMODCACHE=/cache/gomodcache CGO_ENABLED=1 GOOS=$TARGETOS GOARCH=$TARGETARCH GOEXPERIMENT=boringcrypto && \
go build -tags fips_strict,osusergo,netgo -v -trimpath -ldflags "$(hack/get-ldflags.sh) -w -linkmode=external -extldflags -static" -o /usr/local/bin/pinniped-concierge-kube-cert-agent ./cmd/pinniped-concierge-kube-cert-agent/... && \
go build -tags fips_strict,osusergo,netgo -v -trimpath -ldflags "$(hack/get-ldflags.sh) -w -linkmode=external -extldflags -static" -o /usr/local/bin/pinniped-server ./cmd/pinniped-server/... && \
go build -tags fips_strict,osusergo,netgo,$ADDITIONAL_BUILD_TAGS -v -trimpath -ldflags "$(hack/get-ldflags.sh) -w -linkmode=external -extldflags -static" -o /usr/local/bin/pinniped-concierge-kube-cert-agent ./cmd/pinniped-concierge-kube-cert-agent/... && \
go build -tags fips_strict,osusergo,netgo,$ADDITIONAL_BUILD_TAGS -v -trimpath -ldflags "$(hack/get-ldflags.sh) -w -linkmode=external -extldflags -static" -o /usr/local/bin/pinniped-server ./cmd/pinniped-server/... && \
ln -s /usr/local/bin/pinniped-server /usr/local/bin/pinniped-concierge && \
ln -s /usr/local/bin/pinniped-server /usr/local/bin/pinniped-supervisor && \
ln -s /usr/local/bin/pinniped-server /usr/local/bin/local-user-authenticator
Expand Down
10 changes: 10 additions & 0 deletions internal/crypto/ptls/default_profile_max_tls_version_for_fips_13.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

//go:build fips_enable_tls13_max_for_default_profile

package ptls

import "crypto/tls"

const DefaultProfileMaxTLSVersionForFIPS = tls.VersionTLS13
10 changes: 10 additions & 0 deletions internal/crypto/ptls/default_profile_max_tls_version_for_fips_default_value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

//go:build !fips_enable_tls13_max_for_default_profile

package ptls

import "crypto/tls"

const DefaultProfileMaxTLSVersionForFIPS = tls.VersionTLS12
14 changes: 8 additions & 6 deletions internal/crypto/ptls/profiles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,10 @@
// this init runs before we have parsed our config to determine our log level
// thus we must use a log statement that will always print instead of conditionally print
plog.Always("this server was not compiled in FIPS-only mode",
"go version", runtime.Version())
"go version", runtime.Version(),
"SecureProfileMinTLSVersionForNonFIPS", tls.VersionName(SecureProfileMinTLSVersionForNonFIPS))

Check warning on line 90 in internal/crypto/ptls/profiles.go

View check run for this annotation

Codecov / codecov/patch

internal/crypto/ptls/profiles.go#L89-L90 

Added lines #L89 - L90 were not covered by tests
}

// SecureTLSConfigMinTLSVersion is the minimum tls version in the format expected by tls.Config.
const SecureTLSConfigMinTLSVersion = tls.VersionTLS13

// Default TLS profile should be used by:
// A. servers whose clients are outside our control and who may reasonably wish to use TLS 1.2, and
// B. clients who need to interact with servers that might not support TLS 1.3.
Expand Down Expand Up @@ -127,8 +125,12 @@
// - Safari 12.1
// https://ssl-config.mozilla.org/#server=go&version=1.17.2&config=modern&guideline=5.6
c := Default(rootCAs)
c.MinVersion = SecureTLSConfigMinTLSVersion // max out the security
c.CipherSuites = nil // TLS 1.3 ciphers are not configurable
// Max out the security by requiring TLS 1.3 by default. Allow it to be overridden by a build tag.
c.MinVersion = SecureProfileMinTLSVersionForNonFIPS
if c.MinVersion == tls.VersionTLS13 {
// Go ignores this setting for TLS 1.3 anyway, so set this to nil just to be explicit when only supporting TLS 1.3.
c.CipherSuites = nil
}
return c
}

Expand Down
11 changes: 4 additions & 7 deletions internal/crypto/ptls/profiles_fips_strict.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,18 @@ func init() {
// this init runs before we have parsed our config to determine our log level
// thus we must use a log statement that will always print instead of conditionally print
plog.Always("this server was compiled to use boring crypto in FIPS-only mode",
"go version", runtime.Version())
"go version", runtime.Version(),
"DefaultProfileMaxTLSVersionForFIPS", tls.VersionName(DefaultProfileMaxTLSVersionForFIPS))
}

// SecureTLSConfigMinTLSVersion: see comment in profiles.go.
// Until goboring supports TLS 1.3, use TLS 1.2.
const SecureTLSConfigMinTLSVersion = tls.VersionTLS12

// Default: see comment in profiles.go.
// This chooses different cipher suites and/or TLS versions compared to non-FIPS mode.
// In FIPS mode, this will use the union of the secureCipherSuiteIDs, additionalSecureCipherSuiteIDsOnlyForLDAPClients,
// and insecureCipherSuiteIDs values defined above.
func Default(rootCAs *x509.CertPool) *tls.Config {
config := buildTLSConfig(rootCAs, allHardcodedAllowedCipherSuites(), getUserConfiguredAllowedCipherSuitesForTLSOneDotTwo())
// Until goboring supports TLS 1.3, make the max version 1.2.
config.MaxVersion = tls.VersionTLS12
// Until goboring supports TLS 1.3, make the max version 1.2 by default. Allow it to be overridden by a build tag.
config.MaxVersion = DefaultProfileMaxTLSVersionForFIPS
return config
}

Expand Down
10 changes: 10 additions & 0 deletions internal/crypto/ptls/secure_profile_min_tls_version_for_nonfips_12.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

//go:build nonfips_enable_tls12_min_for_secure_profile

package ptls

import "crypto/tls"

const SecureProfileMinTLSVersionForNonFIPS = tls.VersionTLS12
Copy link

Choose a reason for hiding this comment

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

this name does not match the value defined in internal/crypto/ptls/profiles_fips_strict.go L76,
config.MaxVersion = DefaultProfileMaxTLSVersionForFIPS

Copy link
Member Author

@cfryanr cfryanr Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @luwangVMW,

There are two new build tags:

  1. nonfips_enable_tls12_min_for_secure_profile is only for non-FIPS builds. By default, there is a global const called SecureProfileMinTLSVersionForNonFIPS set to 1.3. When the build tag is used during compilation, it changes the value of that const to be 1.2. This variable is only used in internal/crypto/ptls/profiles.go which is the file used in non-FIPS builds.
  2. fips_enable_tls13_max_for_default_profile is only for FIPS builds. By default, there is a global const called DefaultProfileMaxTLSVersionForFIPS set to 1.2. When the build tag is used during compilation, it changes the value of that const to be 1.3. This variable is only used in internal/crypto/ptls/profiles_fips_strict.go which is the file used in FIPS builds.

Side note: In the future, I expect that the official golang compiler will use a version of boringcrypto which supports TLS 1.3. The golang maintainers previously estimated that this could happen in Go 1.24 in February 2025. If that happens, then we may be able to enable TLS 1.3 by default when compiled in FIPS mode, so we may not need the setting fips_enable_tls13_max_for_default_profile described above at all because it would be the same as our new default.

10 changes: 10 additions & 0 deletions internal/crypto/ptls/secure_profile_min_tls_version_for_nonfips_default_value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

//go:build !nonfips_enable_tls12_min_for_secure_profile

package ptls

import "crypto/tls"

const SecureProfileMinTLSVersionForNonFIPS = tls.VersionTLS13
3 changes: 1 addition & 2 deletions test/integration/supervisor_discovery_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
supervisorclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned"
"go.pinniped.dev/internal/certauthority"
"go.pinniped.dev/internal/crypto/ptls"
"go.pinniped.dev/internal/here"
"go.pinniped.dev/test/testlib"
)
Expand Down Expand Up @@ -866,7 +865,7 @@ func newHTTPClient(t *testing.T, caBundle []byte, dnsOverrides map[string]string
caCertPool.AppendCertsFromPEM(caBundle)
c.Transport = &http.Transport{
DialContext: overrideDialContext,
TLSClientConfig: &tls.Config{MinVersion: ptls.SecureTLSConfigMinTLSVersion, RootCAs: caCertPool}, //nolint:gosec // this seems to be a false flag, min tls version is 1.3 in normal mode or 1.2 in fips mode
TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS12, RootCAs: caCertPool},
}
} else {
c.Transport = &http.Transport{
Expand Down
Loading