Skip to content

Commit

Permalink
Merge pull request #115202 from bdarnell/backport23.2-114709
Browse files Browse the repository at this point in the history
  • Loading branch information
jlinder authored Dec 12, 2023
2 parents 3e2cbad + 80795c9 commit bc665a3
Show file tree
Hide file tree
Showing 32 changed files with 456 additions and 7 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@
/pkg/scheduledjobs/ @cockroachdb/jobs-prs @cockroachdb/disaster-recovery
/pkg/security/ @cockroachdb/prodsec @cockroachdb/server-prs
/pkg/security/clientsecopts/ @cockroachdb/sql-foundations @cockroachdb/prodsec
/pkg/ccl/securityccl/ @cockroachdb/prodsec
#!/pkg/settings/ @cockroachdb/unowned
/pkg/spanconfig/ @cockroachdb/kv-prs @cockroachdb/sql-foundations
/pkg/spanconfig/spanconfigbounds/ @cockroachdb/sql-foundations
Expand Down
10 changes: 10 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -620,4 +620,14 @@ go_download_sdk(
},
urls = ["https://storage.googleapis.com/public-bazel-artifacts/go/20231206-175156/{}"],
version = "1.21.5fips",
# In the golang-fips toolchain, FIPS-ready crypto packages are used by default, regardless of build tags.
# The boringcrypto experiment does almost nothing in this toolchain, but it does enable the use of the
# crypto/boring.Enabled() method which is the only application-visible way to inspect whether FIPS mode
# is working correctly.
#
# The golang-fips toolchain also supports an experiment `strictfipsruntime` which causes a panic at startup
# if the kernel is in FIPS mode but OpenSSL cannot be loaded. We do not currently use this experiment
# because A) we also want to detect the case when the kernel is not in FIPS mode and B) we want to be
# able to provide additional diagnostic information such as the expected version of OpenSSL.
experiments = ["boringcrypto"],
)
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ GO_TARGETS = [
"//pkg/ccl/plpgsqlccl:plpgsqlccl",
"//pkg/ccl/schemachangerccl:schemachangerccl",
"//pkg/ccl/schemachangerccl:schemachangerccl_test",
"//pkg/ccl/securityccl/fipsccl:fipsccl",
"//pkg/ccl/serverccl/adminccl:adminccl_test",
"//pkg/ccl/serverccl/diagnosticsccl:diagnosticsccl_test",
"//pkg/ccl/serverccl/statusccl:statusccl_test",
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ go_library(
"//pkg/ccl/partitionccl",
"//pkg/ccl/pgcryptoccl",
"//pkg/ccl/plpgsqlccl",
"//pkg/ccl/securityccl/fipsccl",
"//pkg/ccl/storageccl",
"//pkg/ccl/storageccl/engineccl",
"//pkg/ccl/streamingccl/streamingest",
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/ccl_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
_ "github.com/cockroachdb/cockroach/pkg/ccl/partitionccl"
_ "github.com/cockroachdb/cockroach/pkg/ccl/pgcryptoccl"
_ "github.com/cockroachdb/cockroach/pkg/ccl/plpgsqlccl"
_ "github.com/cockroachdb/cockroach/pkg/ccl/securityccl/fipsccl"
_ "github.com/cockroachdb/cockroach/pkg/ccl/storageccl"
_ "github.com/cockroachdb/cockroach/pkg/ccl/storageccl/engineccl"
_ "github.com/cockroachdb/cockroach/pkg/ccl/streamingccl/streamingest"
Expand Down
5 changes: 5 additions & 0 deletions pkg/ccl/cliccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,19 @@ go_library(
"//pkg/base",
"//pkg/ccl/baseccl",
"//pkg/ccl/cliccl/cliflagsccl",
"//pkg/ccl/securityccl/fipsccl",
"//pkg/ccl/sqlproxyccl",
"//pkg/ccl/sqlproxyccl/tenantdirsvr",
"//pkg/ccl/storageccl/engineccl/enginepbccl",
"//pkg/ccl/utilccl",
"//pkg/ccl/workloadccl/cliccl",
"//pkg/cli",
"//pkg/cli/clierror",
"//pkg/cli/clierrorplus",
"//pkg/cli/cliflagcfg",
"//pkg/cli/cliflags",
"//pkg/cli/democluster",
"//pkg/cli/exit",
"//pkg/storage",
"//pkg/storage/enginepb",
"//pkg/util/log",
Expand All @@ -41,7 +44,9 @@ go_library(
"@com_github_cockroachdb_errors//oserror",
"@com_github_cockroachdb_pebble//vfs",
"@com_github_cockroachdb_redact//:redact",
"@com_github_olekukonko_tablewriter//:tablewriter",
"@com_github_spf13_cobra//:cobra",
"@com_github_spf13_pflag//:pflag",
],
)

Expand Down
56 changes: 56 additions & 0 deletions pkg/ccl/cliccl/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ import (
"fmt"
"os"
"path/filepath"
"runtime"
"sort"
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/ccl/baseccl"
"github.com/cockroachdb/cockroach/pkg/ccl/cliccl/cliflagsccl"
"github.com/cockroachdb/cockroach/pkg/ccl/securityccl/fipsccl"
"github.com/cockroachdb/cockroach/pkg/ccl/storageccl/engineccl/enginepbccl"
"github.com/cockroachdb/cockroach/pkg/cli"
"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
Expand All @@ -31,6 +33,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/errors/oserror"
"github.com/olekukonko/tablewriter"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -102,12 +105,24 @@ with their env type and encryption settings (if applicable).
RunE: clierrorplus.MaybeDecorateError(runList),
}

checkFipsCmd := &cobra.Command{
Use: "enterprise-check-fips",
Short: "print diagnostics for FIPS-ready configuration",
Long: `
Performs various tests of this binary's ability to operate in FIPS-ready
mode in the current environment.
`,

RunE: clierrorplus.MaybeDecorateError(runCheckFips),
}

// Add commands to the root debug command.
// We can't add them to the lists of commands (eg: DebugCmdsForPebble) as cli init() is called before us.
cli.DebugCmd.AddCommand(encryptionStatusCmd)
cli.DebugCmd.AddCommand(encryptionActiveKeyCmd)
cli.DebugCmd.AddCommand(encryptionDecryptCmd)
cli.DebugCmd.AddCommand(encryptionRegistryList)
cli.DebugCmd.AddCommand(checkFipsCmd)

// Add the encryption flag to commands that need it.
// For the encryption-status command.
Expand Down Expand Up @@ -376,3 +391,44 @@ func getActiveEncryptionkey(dir string) (string, string, error) {

return setting.EncryptionType.String(), setting.KeyId, nil
}

func runCheckFips(cmd *cobra.Command, args []string) error {
if runtime.GOOS != "linux" {
return errors.New("FIPS-ready mode is only supported on linux")
}
// Our FIPS-ready deployments have three major requirements:
// 1. This binary is built with the golang-fips toolchain and running on linux
// 2. FIPS mode is enabled in the kernel.
// 3. We can dynamically load the OpenSSL library (which must be the same major version that was present at
// build time). Verifying that the OpenSSL library is FIPS-compliant is outside the scope of this command.
table := tablewriter.NewWriter(os.Stdout)
table.SetBorder(false)
table.SetAlignment(tablewriter.ALIGN_LEFT)
emit := func(label string, status bool, detail string) {
statusSymbol := "❌"
if status {
statusSymbol = "✅"
}
table.Append([]string{label, statusSymbol, detail})
}

emit("FIPS-ready build", fipsccl.IsCompileTimeFIPSReady(), "")
buildOpenSSLVersion, soname, err := fipsccl.BuildOpenSSLVersion()
if err == nil {
table.Append([]string{"Build-time OpenSSL Version", "", buildOpenSSLVersion})
table.Append([]string{"OpenSSL library filename", "", soname})
}

isKernelEnabled, err := fipsccl.IsKernelEnabled()
detail := ""
if err != nil {
detail = err.Error()
}
emit("Kernel FIPS mode enabled", isKernelEnabled, detail)

emit("OpenSSL loaded", fipsccl.IsOpenSSLLoaded(), "")
emit("FIPS ready", fipsccl.IsFIPSReady(), "")

table.Render()
return nil
}
56 changes: 56 additions & 0 deletions pkg/ccl/cliccl/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,59 @@
package cliccl

import (
"os"
"strconv"

"github.com/cockroachdb/cockroach/pkg/ccl/securityccl/fipsccl"
"github.com/cockroachdb/cockroach/pkg/cli"
"github.com/cockroachdb/cockroach/pkg/cli/clierror"
"github.com/cockroachdb/cockroach/pkg/cli/cliflagcfg"
"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
"github.com/cockroachdb/cockroach/pkg/cli/exit"
"github.com/cockroachdb/errors"
"github.com/spf13/pflag"
)

type requireFipsFlag bool

// Type implements the pflag.Value interface.
func (f *requireFipsFlag) Type() string {
return "bool"
}

// String implements the pflag.Value interface.
func (f *requireFipsFlag) String() string {
return strconv.FormatBool(bool(*f))
}

// Set implements the pflag.Value interface.
func (f *requireFipsFlag) Set(s string) error {
v, err := strconv.ParseBool(s)
if err != nil {
return err
}
// We implement the logic of this check in the flag setter itself because it
// applies to all commands and we do not have another good way to inject
// this behavior globally (PersistentPreRun functions don't help because
// they are inherited across different levels of the command hierarchy only
// if that level does not have its own hook).
if v && !fipsccl.IsFIPSReady() {
err := errors.WithHint(errors.New("FIPS readiness checks failed"), "Run `cockroach debug enterprise-check-fips` for details")
clierror.OutputError(os.Stderr, err, true, false)
exit.WithCode(exit.UnspecifiedError())
}
*f = requireFipsFlag(v)
return nil
}

var _ pflag.Value = (*requireFipsFlag)(nil)

// IsBoolFlag implements a non-public pflag interface to indicate that this
// flag is used without an explicit value.
func (*requireFipsFlag) IsBoolFlag() bool {
return true
}

func init() {
// Multi-tenancy proxy command flags.
{
Expand Down Expand Up @@ -44,4 +92,12 @@ func init() {
// Use StringFlagDepth to avoid conflicting with the already registered KVAddrs env var.
cliflagcfg.StringFlagDepth(1, f, &testDirectorySvrContext.kvAddrs, cliflags.KVAddrs)
})

// FIPS verification flags.
cli.RegisterFlags(func() {
cmd := cli.CockroachCmd()
var requireFips = requireFipsFlag(false)
flag := cmd.PersistentFlags().VarPF(&requireFips, "enterprise-require-fips-ready", "", "abort if FIPS readiness checks fail")
flag.NoOptDefVal = "true"
})
}
14 changes: 14 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/fips_ready
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
subtest fips_ready

# We do not have the plumbing that would let test cases know whether they are
# running in a fips environment or not so this is just a very basic test to
# make sure that all the registration, oids, etc work properly.
query _
SELECT crdb_internal.fips_ready()
----
_

user testuser

statement error pq: crdb_internal\.fips_ready\(\): user testuser does not have VIEWCLUSTERSETTING system privilege
SELECT crdb_internal.fips_ready()
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/fakedist-disk/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ go_test(
"//pkg/ccl/logictestccl:testdata", # keep
],
exec_properties = {"Pool": "large"},
shard_count = 14,
shard_count = 15,
tags = [
"ccl_test",
"cpu:2",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/fakedist-disk/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/fakedist-vec-off/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ go_test(
"//pkg/ccl/logictestccl:testdata", # keep
],
exec_properties = {"Pool": "large"},
shard_count = 14,
shard_count = 15,
tags = [
"ccl_test",
"cpu:2",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/fakedist-vec-off/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/fakedist/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ go_test(
"//pkg/ccl/logictestccl:testdata", # keep
],
exec_properties = {"Pool": "large"},
shard_count = 15,
shard_count = 16,
tags = [
"ccl_test",
"cpu:2",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/fakedist/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ go_test(
"//pkg/ccl/logictestccl:testdata", # keep
],
exec_properties = {"Pool": "large"},
shard_count = 14,
shard_count = 15,
tags = [
"ccl_test",
"cpu:1",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/local-mixed-23.1/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ go_test(
"//pkg/ccl/logictestccl:testdata", # keep
],
exec_properties = {"Pool": "large"},
shard_count = 8,
shard_count = 9,
tags = [
"ccl_test",
"cpu:1",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/local-mixed-23.1/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/local-vec-off/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ go_test(
"//pkg/ccl/logictestccl:testdata", # keep
],
exec_properties = {"Pool": "large"},
shard_count = 14,
shard_count = 15,
tags = [
"ccl_test",
"cpu:1",
Expand Down
Loading

0 comments on commit bc665a3

Please sign in to comment.