diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml new file mode 100644 index 00000000..77982927 --- /dev/null +++ b/.github/workflows/scorecards.yml @@ -0,0 +1,55 @@ +name: Scorecards supply-chain security +on: + # Only the default branch is supported. + branch_protection_rule: + schedule: + - cron: '29 11 * * 3' + push: + branches: [ main, release* ] + +# Declare default permissions as read only. +permissions: read-all + +jobs: + analysis: + name: Scorecards analysis + runs-on: ubuntu-latest + permissions: + # Needed to upload the results to code-scanning dashboard. + security-events: write + actions: read + contents: read + + steps: + - name: "Checkout code" + uses: actions/checkout@a12a3943b4bdde767164f792f33f40b04645d846 # v3.0.0 + with: + persist-credentials: false + + - name: "Run analysis" + uses: ossf/scorecard-action@c1aec4ac820532bab364f02a81873c555a0ba3a1 # v1.0.4 + with: + results_file: results.sarif + results_format: sarif + # Read-only PAT token. To create it, + # follow the steps in https://github.com/ossf/scorecard-action#pat-token-creation. + repo_token: ${{ secrets.SCORECARD_READ_TOKEN }} + # Publish the results to enable scorecard badges. For more details, see + # https://github.com/ossf/scorecard-action#publishing-results. + # For private repositories, `publish_results` will automatically be set to `false`, + # regardless of the value entered here. + publish_results: true + + # Upload the results as artifacts (optional). + - name: "Upload artifact" + uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 # v3.0.0 + with: + name: SARIF file + path: results.sarif + retention-days: 5 + + # Upload the results to GitHub's code scanning dashboard. + - name: "Upload to code-scanning" + uses: github/codeql-action/upload-sarif@5f532563584d71fdef14ee64d17bafb34f751ce5 # v1.0.26 + with: + sarif_file: results.sarif diff --git a/Dockerfile b/Dockerfile index c5087fee..b1dd8620 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -# syntax = docker/dockerfile:1.0-experimental +# syntax=docker/dockerfile:1 # Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 diff --git a/hack/Dockerfile_fips b/hack/Dockerfile_fips index 1b1bf858..1dfbc243 100644 --- a/hack/Dockerfile_fips +++ b/hack/Dockerfile_fips @@ -1,10 +1,15 @@ -# syntax = docker/dockerfile:1.0-experimental +# syntax=docker/dockerfile:1 -# Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +# Copyright 2022 the Pinniped contributors. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 # this dockerfile is used to produce a binary of Pinniped that uses -# only fips-allowable ciphers. +# only fips-allowable ciphers. Note that this is provided only as +# an example. Pinniped has no official support for fips and using +# a version built from this dockerfile may have unforseen consquences. +# Please do not create issues in regards to problems encountered by +# using this dockerfile. Using this dockerfile does not convey +# any type of fips certification. # use go-boringcrypto rather than main go FROM us-docker.pkg.dev/google.com/api-project-999119582588/go-boringcrypto/golang:1.17.8b7 as build-env @@ -13,12 +18,33 @@ WORKDIR /work COPY . . ARG GOPROXY -# Build the executable binary (CGO_ENABLED=1 is required for go boring) -# Pass in GOCACHE (build cache) and GOMODCACHE (module cache) so they -# can be re-used between image builds. +# 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 +# link any libc bits hence we pass "-linkmode=external -extldflags -static" +# to the ldflags directive. We do not pass "-s" to ldflags because we do +# not want to strip symbols - those are used to verify if we compiled correctly. +# We do not pass in GOCACHE (build cache) and GOMODCACHE (module cache) +# because there have been bugs in the Go compiler caching when using cgo +# (it will sometimes use cached artifiacts when it should not). Since we +# use gcc as the C compiler, the following warning is emitted: +# /boring/boringssl/build/../crypto/bio/socket_helper.c:55: warning: +# Using 'getaddrinfo' in statically linked applications requires at +# runtime the shared libraries from the glibc version used for linking +# This is referring to the code in +# https://github.com/google/boringssl/blob/af34f6460f0bf99dc267818f02b2936f60a30de7/crypto/bio/socket_helper.c#L55 +# which calls the getaddrinfo function. This function, even when statically linked, +# uses dlopen to dynamically fetch networking config. It is safe for us to ignore +# this warning because the go boring cypto code does not create netowrking connections: +# https://github.com/golang/go/blob/9d6ab825f6fe125f7ce630e103b887e580403802/src/crypto/internal/boring/goboringcrypto.h +# The osusergo and netgo tags are used to make sure that the Go implementations of these +# standard library packages are used instead of the libc based versions. +# We want to have no reliance on any C code other than the boring crypto bits. +# Setting GOOS=linux GOARCH=amd64 is a hard requirment for boring crypto: +# https://github.com/golang/go/blob/9d6ab825f6fe125f7ce630e103b887e580403802/misc/boring/README.md?plain=1#L95 +# Thus trying to compile the pinniped CLI with boring crypto is meaningless +# since we would not be able to ship windows and macOS binaries. RUN \ - --mount=type=cache,target=/cache/gocache \ - --mount=type=cache,target=/cache/gomodcache \ mkdir out && \ export CGO_ENABLED=1 GOOS=linux GOARCH=amd64 && \ 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/... && \ diff --git a/internal/crypto/ptls/fips_strict.go b/internal/crypto/ptls/fips_strict.go index e35a30a5..a6a3c65d 100644 --- a/internal/crypto/ptls/fips_strict.go +++ b/internal/crypto/ptls/fips_strict.go @@ -10,12 +10,15 @@ package ptls import ( - "C" "crypto/tls" - _ "crypto/tls/fipsonly" // restricts all TLS configuration to FIPS-approved settings. "crypto/x509" "runtime" + "C" // explicitly import cgo so that runtime/cgo gets linked into the kube-cert-agent + _ "crypto/tls/fipsonly" // restricts all TLS configuration to FIPS-approved settings. + + "k8s.io/apiserver/pkg/server/options" + "go.pinniped.dev/internal/plog" ) @@ -24,10 +27,7 @@ const secureServingOptionsMinTLSVersion = "VersionTLS12" const SecureTLSConfigMinTLSVersion = tls.VersionTLS12 func init() { - go func() { - version := runtime.Version() - plog.Debug("using boringcrypto in fips only mode.", "go version", version) - }() + plog.Debug("using boring crypto in fips only mode", "go version", runtime.Version()) } func Default(rootCAs *x509.CertPool) *tls.Config { @@ -46,6 +46,7 @@ func Default(rootCAs *x509.CertPool) *tls.Config { // This is all of the fips-approved ciphers. // The list is hard-coded for convenience of testing. + // This is kept in sync with the boring crypto compiler via TestFIPSCipherSuites. CipherSuites: []uint16{ tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, @@ -64,3 +65,7 @@ func Secure(rootCAs *x509.CertPool) *tls.Config { func DefaultLDAP(rootCAs *x509.CertPool) *tls.Config { return Default(rootCAs) } + +func secureServing(opts *options.SecureServingOptionsWithLoopback) { + defaultServing(opts) +} diff --git a/internal/crypto/ptls/ptls.go b/internal/crypto/ptls/ptls.go index 3eb45d41..ab331e38 100644 --- a/internal/crypto/ptls/ptls.go +++ b/internal/crypto/ptls/ptls.go @@ -82,11 +82,6 @@ func defaultServing(opts *options.SecureServingOptionsWithLoopback) { opts.MinTLSVersion = defaultServingOptionsMinTLSVersion } -func secureServing(opts *options.SecureServingOptionsWithLoopback) { - opts.MinTLSVersion = secureServingOptionsMinTLSVersion - opts.CipherSuites = nil -} - func secureClient(opts *options.RecommendedOptions, f RestConfigFunc) error { inClusterClient, inClusterConfig, err := f(nil) if err != nil { diff --git a/internal/crypto/ptls/secure.go b/internal/crypto/ptls/secure.go index 9f07b633..ddea0816 100644 --- a/internal/crypto/ptls/secure.go +++ b/internal/crypto/ptls/secure.go @@ -9,6 +9,8 @@ package ptls import ( "crypto/tls" "crypto/x509" + + "k8s.io/apiserver/pkg/server/options" ) // secureServingOptionsMinTLSVersion is the minimum tls version in the format @@ -42,3 +44,8 @@ func Secure(rootCAs *x509.CertPool) *tls.Config { } return c } + +func secureServing(opts *options.SecureServingOptionsWithLoopback) { + opts.MinTLSVersion = secureServingOptionsMinTLSVersion + opts.CipherSuites = nil +} diff --git a/internal/testutil/tlsserver/tlsserver.go b/internal/testutil/tlsserver/tlsserver.go index 21238993..1585284d 100644 --- a/internal/testutil/tlsserver/tlsserver.go +++ b/internal/testutil/tlsserver/tlsserver.go @@ -69,14 +69,6 @@ func RecordTLSHello(server *httptest.Server) { func AssertTLS(t *testing.T, r *http.Request, tlsConfigFunc ptls.ConfigFunc) { t.Helper() - tlsConfig := tlsConfigFunc(nil) - - AssertTLSConfig(t, r, tlsConfig) -} - -func AssertTLSConfig(t *testing.T, r *http.Request, tlsConfig *tls.Config) { - t.Helper() - m, ok := getCtxMap(r.Context()) require.True(t, ok) @@ -86,6 +78,8 @@ func AssertTLSConfig(t *testing.T, r *http.Request, tlsConfig *tls.Config) { info, ok := h.(*tls.ClientHelloInfo) require.True(t, ok) + tlsConfig := tlsConfigFunc(nil) + supportedVersions := []uint16{tlsConfig.MinVersion} ciphers := tlsConfig.CipherSuites diff --git a/test/integration/securetls_fips_test.go b/test/integration/securetls_fips_test.go index aed0c399..19cfea78 100644 --- a/test/integration/securetls_fips_test.go +++ b/test/integration/securetls_fips_test.go @@ -7,12 +7,8 @@ package integration import ( - "context" "crypto/tls" - "encoding/base64" - "fmt" "net/http" - "strings" "testing" "github.com/stretchr/testify/require" @@ -23,131 +19,16 @@ import ( "go.pinniped.dev/test/testlib" ) -// This test mirrors securetls_test.go, but adapted for fips mode. -// e.g. checks for only TLS 1.2 ciphers and checks for the -// list of fips-approved ciphers above. -// TLS checks safe to run in parallel with serial tests, see main_test.go. -func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) { - _ = testlib.IntegrationEnv(t) - t.Log("testing FIPs tls config") - - server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // pinniped CLI uses ptls.Secure when talking to KAS, - // although the distinction doesn't matter much in FIPs mode because - // each of the configs is a wrapper for the same base FIPs config. - secure := ptls.Secure(nil) - tlsserver.AssertTLSConfig(t, r, secure) - w.Header().Set("content-type", "application/json") - fmt.Fprint(w, `{"kind":"TokenCredentialRequest","apiVersion":"login.concierge.pinniped.dev/v1alpha1",`+ - `"status":{"credential":{"token":"some-fancy-token"}}}`) - }), tlsserver.RecordTLSHello) - - ca := tlsserver.TLSTestServerCA(server) - - pinnipedExe := testlib.PinnipedCLIPath(t) - - stdout, stderr := runPinnipedCLI(t, nil, pinnipedExe, "login", "static", - "--token", "does-not-matter", - "--concierge-authenticator-type", "webhook", - "--concierge-authenticator-name", "does-not-matter", - "--concierge-ca-bundle-data", base64.StdEncoding.EncodeToString(ca), - "--concierge-endpoint", server.URL, - "--enable-concierge", - "--credential-cache", "", - ) - - require.Empty(t, stderr) - require.Equal(t, `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1",`+ - `"spec":{"interactive":false},"status":{"expirationTimestamp":null,"token":"some-fancy-token"}} -`, stdout) -} - -// TLS checks safe to run in parallel with serial tests, see main_test.go. -func TestSecureTLSPinnipedCLIToSupervisor_Parallel(t *testing.T) { - _ = testlib.IntegrationEnv(t) - - server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // pinniped CLI uses ptls.Default when talking to supervisor, - // although the distinction doesn't matter much in FIPs mode because - // each of the configs is a wrapper for the same base FIPs config. - defaultTLS := ptls.Default(nil) - tlsserver.AssertTLSConfig(t, r, defaultTLS) - w.Header().Set("content-type", "application/json") - fmt.Fprint(w, `{"issuer":"https://not-a-good-issuer"}`) - }), tlsserver.RecordTLSHello) - - ca := tlsserver.TLSTestServerCA(server) - - pinnipedExe := testlib.PinnipedCLIPath(t) - - stdout, stderr := runPinnipedCLI(&fakeT{T: t}, nil, pinnipedExe, "login", "oidc", - "--ca-bundle-data", base64.StdEncoding.EncodeToString(ca), - "--issuer", server.URL, - "--credential-cache", "", - "--upstream-identity-provider-flow", "cli_password", - "--upstream-identity-provider-name", "does-not-matter", - "--upstream-identity-provider-type", "oidc", - ) - - require.Equal(t, `Error: could not complete Pinniped login: could not perform OIDC discovery for "`+ - server.URL+`": oidc: issuer did not match the issuer returned by provider, expected "`+ - server.URL+`" got "https://not-a-good-issuer" -`, stderr) - require.Empty(t, stdout) -} - -// TLS checks safe to run in parallel with serial tests, see main_test.go. -func TestSecureTLSConciergeAggregatedAPI_Parallel(t *testing.T) { - env := testlib.IntegrationEnv(t) - - cancelCtx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - - startKubectlPortForward(cancelCtx, t, "10446", "443", env.ConciergeAppName+"-api", env.ConciergeNamespace) - - stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10446) - - require.Empty(t, stderr) - secure := ptls.Secure(nil) - require.Contains(t, stdout, testlib.GetExpectedCiphers(secure), "stdout:\n%s", stdout) -} - -func TestSecureTLSSupervisor(t *testing.T) { // does not run in parallel because of the createSupervisorDefaultTLSCertificateSecretIfNeeded call - env := testlib.IntegrationEnv(t) - - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - - startKubectlPortForward(ctx, t, "10447", "443", env.SupervisorAppName+"-nodeport", env.SupervisorNamespace) - - stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10447) - - // supervisor's cert is ECDSA - defaultECDSAOnly := ptls.Default(nil) - ciphers := make([]uint16, 0, len(defaultECDSAOnly.CipherSuites)/3) - for _, id := range defaultECDSAOnly.CipherSuites { - id := id - if !strings.Contains(tls.CipherSuiteName(id), "_ECDSA_") { - continue - } - ciphers = append(ciphers, id) - } - defaultECDSAOnly.CipherSuites = ciphers - - require.Empty(t, stderr) - require.Contains(t, stdout, testlib.GetExpectedCiphers(defaultECDSAOnly), "stdout:\n%s", stdout) -} - -// this test ensures that if the list of default fips cipher -// suites changes, we will know. +// TestFIPSCipherSuites_Parallel ensures that if the list of default fips cipher suites changes, +// we will know. This is an integration test because we do not support build tags on unit tests. func TestFIPSCipherSuites_Parallel(t *testing.T) { _ = testlib.IntegrationEnv(t) + server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // use the default fips config which contains a hard coded list of cipher suites // that should be equal to the default list of fips cipher suites. - defaultTLS := ptls.Default(nil) // assert that the client hello response has the same tls config as this test server. - tlsserver.AssertTLSConfig(t, r, defaultTLS) + tlsserver.AssertTLS(t, r, ptls.Default) }), tlsserver.RecordTLSHello) ca := tlsserver.TLSTestServerCA(server) @@ -157,7 +38,7 @@ func TestFIPSCipherSuites_Parallel(t *testing.T) { // and therefore uses goboring's default fips ciphers. defaultConfig := &tls.Config{ RootCAs: pool, - NextProtos: ptls.Default(nil).NextProtos, + NextProtos: ptls.Default(nil).NextProtos, // we do not care about field for this test, so just make it match } transport := http.Transport{ TLSClientConfig: defaultConfig, @@ -172,20 +53,3 @@ func TestFIPSCipherSuites_Parallel(t *testing.T) { require.NoError(t, err) require.Equal(t, http.StatusOK, response.StatusCode) } - -type fakeT struct { - *testing.T -} - -func (t *fakeT) FailNow() { - t.Errorf("fakeT ignored FailNow") -} - -func (t *fakeT) Errorf(format string, args ...interface{}) { - t.Cleanup(func() { - if !t.Failed() { - return - } - t.Logf("reporting previously ignored errors since main test failed:\n"+format, args...) - }) -} diff --git a/test/integration/securetls_test.go b/test/integration/securetls_test.go index 05ef2586..e4324692 100644 --- a/test/integration/securetls_test.go +++ b/test/integration/securetls_test.go @@ -1,9 +1,6 @@ // Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -//go:build !fips_strict -// +build !fips_strict - package integration import ( @@ -27,7 +24,10 @@ func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) { _ = testlib.IntegrationEnv(t) server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Secure) // pinniped CLI uses ptls.Secure when talking to KAS + // pinniped CLI uses ptls.Secure when talking to KAS + // in FIPs mode the distinction doesn't matter much because + // each of the configs is a wrapper for the same base FIPs config + tlsserver.AssertTLS(t, r, ptls.Secure) w.Header().Set("content-type", "application/json") fmt.Fprint(w, `{"kind":"TokenCredentialRequest","apiVersion":"login.concierge.pinniped.dev/v1alpha1",`+ `"status":{"credential":{"token":"some-fancy-token"}}}`) @@ -58,7 +58,10 @@ func TestSecureTLSPinnipedCLIToSupervisor_Parallel(t *testing.T) { _ = testlib.IntegrationEnv(t) server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Default) // pinniped CLI uses ptls.Default when talking to supervisor + // pinniped CLI uses ptls.Default when talking to supervisor + // in FIPs mode the distinction doesn't matter much because + // each of the configs is a wrapper for the same base FIPs config + tlsserver.AssertTLS(t, r, ptls.Default) w.Header().Set("content-type", "application/json") fmt.Fprint(w, `{"issuer":"https://not-a-good-issuer"}`) }), tlsserver.RecordTLSHello) diff --git a/test/testlib/securetls.go b/test/testlib/securetls.go index 3777c1aa..b7f45b92 100644 --- a/test/testlib/securetls.go +++ b/test/testlib/securetls.go @@ -87,7 +87,7 @@ func GetExpectedCiphers(config *tls.Config) string { } s.WriteString("\n") } - tls12Bit = fmt.Sprintf(tls12Base, s.String(), getCipherSuitePreference()) + tls12Bit = fmt.Sprintf(tls12Base, s.String(), cipherSuitePreference) } skip13 := config.MaxVersion == tls.VersionTLS12 diff --git a/test/testlib/securetls_preference_fips.go b/test/testlib/securetls_preference_fips.go index 3da1f5df..e61fc205 100644 --- a/test/testlib/securetls_preference_fips.go +++ b/test/testlib/securetls_preference_fips.go @@ -10,6 +10,4 @@ package testlib // incorrectly shown as 'client' in some cases. // in fips-only mode, it correctly shows the cipher preference // as 'server', while in non-fips mode it shows as 'client'. -func getCipherSuitePreference() string { - return "server" -} +const cipherSuitePreference = "server" diff --git a/test/testlib/securetls_preference_nonfips.go b/test/testlib/securetls_preference_nonfips.go index 86bf0edc..002d329d 100644 --- a/test/testlib/securetls_preference_nonfips.go +++ b/test/testlib/securetls_preference_nonfips.go @@ -10,6 +10,4 @@ package testlib // incorrectly shown as 'client' in some cases. // in fips-only mode, it correctly shows the cipher preference // as 'server', while in non-fips mode it shows as 'client'. -func getCipherSuitePreference() string { - return "client" -} +const cipherSuitePreference = "client"