Merge #151154 #151636 #151687

151154: kvcoord: test that interceptors populate leaf state correctly r=yuzefovich a=DrewKimball

This commit adds a simple test for `txnPipeliner` and `txnWriteBuffer` to ensure that they correctly populate `roachpb.LeafTxnInputState`. This is a test-only change.

Fixes #151075

Release note: None

151636: settings: set MinPasswordLength to 14 for FIPS builds r=pritesh-lahoti,rickystewart,herkolategan a=rail

Previously, the minimum password length was 1 character. For FIPS 140-3 builds, if short passwords are allowed, this leads to server crashes when a short password is used. Under the hood, the FIPS implementation panics if a password is shorter than 14 characters. This aligns with the NIST recommendation that HMAC should have a key length of at least 112 bits, which translates to 14 ASCII characters.

This change sets the default minimum password length to 14 characters for FIPS builds.

Additionally, `cockroach demo` has been updated to ensure that the password length for the demo user is at least the minimum password length.

Epic: none
Release note (security update): Strengthened password policy controls for FIPS 140‑3 builds. The minimum allowed password length is now set to 14 characters (112 bits of entropy) in FIPS-compliant builds. This prevents crashes caused by shorter passwords, aligning with NIST key-length recommendations

151687: opt: lazily initialize helper data in join reordering r=mgartner a=mgartner

Some equivalence groups and column sets are used during join reordering
only when there are inner-join edges. These data structures are no
longer initialized if there are no inner-join edges.

Release note: None


Co-authored-by: Drew Kimball <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>

Author: 4 people

pkg/ccl/securityccl/fipsccl/fipscclbase/BUILD.bazel

@@ -5,6 +5,7 @@ go_library(
     srcs = [
         "build_boring.go",  # keep
         "build_noboring.go",
+        "consts.go",
     ],
     cgo = True,
     importpath = "github.com/cockroachdb/cockroach/pkg/ccl/securityccl/fipsccl/fipscclbase",

pkg/ccl/securityccl/fipsccl/fipscclbase/consts.go

@@ -0,0 +1,9 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package fipscclbase
+
+// FIPSMinPasswordLength is the minimum length of a password in FIPS 140-3 mode.
+const FIPSMinPasswordLength = 14

pkg/cli/clisqlclient/BUILD.bazel

@@ -22,6 +22,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/build",
+        "//pkg/ccl/securityccl/fipsccl/fipscclbase",
         "//pkg/cli/clicfg",
         "//pkg/cli/clierror",
         "//pkg/security/pprompt",

pkg/cli/clisqlclient/conn.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach-go/v2/crdb"
 	"github.com/cockroachdb/cockroach/pkg/build"
+	"github.com/cockroachdb/cockroach/pkg/ccl/securityccl/fipsccl/fipscclbase"
 	"github.com/cockroachdb/cockroach/pkg/cli/clierror"
 	"github.com/cockroachdb/cockroach/pkg/security/pprompt"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -43,7 +44,7 @@ type sqlConn struct {
 	conn         *pgx.Conn
 	reconnecting bool
 
-	// passwordMissing is true iff the url is missing a password.
+	// passwordMissing is true if the url is missing a password.
 	passwordMissing bool
 
 	// alwaysInferResultTypes is true iff the client should always use the
@@ -183,6 +184,10 @@ func (c *sqlConn) EnsureConn(ctx context.Context) error {
 	if err != nil {
 		return wrapConnError(err)
 	}
+	// Under FIPS 140-3 mode, the password must be at least 14 characters long.
+	if fipscclbase.IsFIPSReady() && !c.passwordMissing && len(base.Password) < fipscclbase.FIPSMinPasswordLength {
+		return errors.Newf("password must be at least %d characters long", fipscclbase.FIPSMinPasswordLength)
+	}
 	// Add a notice handler - re-use the cliOutputError function in this case.
 	base.OnNotice = func(_ *pgconn.PgConn, notice *pgconn.Notice) {
 		c.handleNotice(notice)
@@ -769,6 +774,10 @@ func (c *sqlConn) fillPassword() error {
 	if err != nil {
 		return err
 	}
+	// Under FIPS 140-3 mode, the password must be at least 14 characters long.
+	if fipscclbase.IsFIPSReady() && len(pwd) < fipscclbase.FIPSMinPasswordLength {
+		return errors.Newf("password must be at least %d characters long", fipscclbase.FIPSMinPasswordLength)
+	}
 	connURL.User = url.UserPassword(connURL.User.Username(), pwd)
 	c.url = connURL.String()
 	c.passwordMissing = false

pkg/cli/democluster/demo_cluster.go

@@ -7,6 +7,7 @@ package democluster
 
 import (
 	"context"
+	"crypto/rand"
 	gosql "database/sql"
 	"fmt"
 	"io"
@@ -379,7 +380,12 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
 		return err
 	}
 
-	demoPassword := genDemoPassword(demoUsername)
+	st := c.firstServer.ClusterSettings()
+	minPasswordLength := security.MinPasswordLength.Get(&st.SV)
+	demoPassword, err := genDemoPassword(demoUsername, minPasswordLength)
+	if err != nil {
+		return errors.Wrap(err, "failed to generate demo password")
+	}
 
 	// Step 8: initialize tenant servers, if enabled.
 	phaseCtx = logtags.AddTag(ctx, "phase", 8)
@@ -2121,10 +2127,21 @@ func (c *transientCluster) addDemoLoginToURL(uiURL *url.URL, includeTenantName b
 //
 // The password can be overridden via the env var
 // COCKROACH_DEMO_PASSWORD for the benefit of test automation.
-func genDemoPassword(username string) string {
+func genDemoPassword(username string, minPasswordLength int64) (string, error) {
+	if password := envutil.EnvOrDefaultString("COCKROACH_DEMO_PASSWORD", ""); password != "" {
+		if len(password) < int(minPasswordLength) {
+			return "", errors.Newf("password is too short: %s", password)
+		}
+		return password, nil
+	}
 	mypid := os.Getpid()
-	candidatePassword := fmt.Sprintf("%s%d", username, mypid)
-	return envutil.EnvOrDefaultString("COCKROACH_DEMO_PASSWORD", candidatePassword)
+	password := fmt.Sprintf("%s%d", username, mypid)
+	// If the password is too short, append random characters until it is long enough.
+	for len(password) < int(minPasswordLength) {
+		randText := strings.ToLower(rand.Text())
+		password += string(randText[0])
+	}
+	return password, nil
 }
 
 // lockDir uses a file lock to prevent concurrent writes to the

pkg/cli/interactive_tests/test_demo.tcl

@@ -169,11 +169,11 @@ eexpect eof
 end_test
 
 start_test "Check that the user can override the password."
-set ::env(COCKROACH_DEMO_PASSWORD) "hunter2"
+set ::env(COCKROACH_DEMO_PASSWORD) "hunter2hunter2hunter2hunter2"
 spawn $argv demo --no-line-editor --insecure=false --no-example-database --log-dir=logs
 eexpect "Connection parameters"
 eexpect "(sql)"
-eexpect "postgresql://demo:hunter2@"
+eexpect "postgresql://demo:hunter2hunter2hunter2hunter2@"
 eexpect "defaultdb>"
 send_eof
 eexpect eof

pkg/kv/kvclient/kvcoord/BUILD.bazel

@@ -157,6 +157,7 @@ go_test(
         "txn_interceptor_metric_recorder_test.go",
         "txn_interceptor_pipeliner_client_test.go",
         "txn_interceptor_pipeliner_test.go",
+        "txn_interceptor_populate_leaf_state_test.go",
         "txn_interceptor_seq_num_allocator_test.go",
         "txn_interceptor_span_refresher_test.go",
         "txn_interceptor_write_buffer_client_test.go",
@@ -231,6 +232,7 @@ go_test(
         "//pkg/util/encoding",
         "//pkg/util/grpcutil",
         "//pkg/util/hlc",
+        "//pkg/util/interval",
         "//pkg/util/leaktest",
         "//pkg/util/log",
         "//pkg/util/metric",

pkg/kv/kvclient/kvcoord/txn_interceptor_pipeliner_test.go

@@ -2480,6 +2480,10 @@ func (s descriptorDBRangeIterator) Error() error {
 	return nil
 }
 
+func (s descriptorDBRangeIterator) Desc() *roachpb.RangeDescriptor {
+	return &s.curDesc
+}
+
 // Test that the pipeliner rejects requests when the lock span budget is
 // exceeded, if configured to do so.
 func TestTxnPipelinerRejectAboveBudget(t *testing.T) {
@@ -2753,7 +2757,3 @@ func TestTxnPipelinerRejectAboveBudget(t *testing.T) {
 		})
 	}
 }
-
-func (s descriptorDBRangeIterator) Desc() *roachpb.RangeDescriptor {
-	return &s.curDesc
-}