Skip to content

Commit 26437b9

Browse files
committed
settings: set MinPasswordLength to 14 for FIPS builds
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. Release note: none Epic: none
1 parent cb2fe92 commit 26437b9

File tree

8 files changed

+58
-10
lines changed

8 files changed

+58
-10
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go_library(
55
srcs = [
66
"build_boring.go", # keep
77
"build_noboring.go",
8+
"consts.go",
89
],
910
cgo = True,
1011
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/securityccl/fipsccl/fipscclbase",
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Copyright 2023 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package fipscclbase
7+
8+
// FIPSMinPasswordLength is the minimum length of a password in FIPS 140-3 mode.
9+
const FIPSMinPasswordLength = 14

pkg/cli/clisqlclient/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ go_library(
2222
visibility = ["//visibility:public"],
2323
deps = [
2424
"//pkg/build",
25+
"//pkg/ccl/securityccl/fipsccl/fipscclbase",
2526
"//pkg/cli/clicfg",
2627
"//pkg/cli/clierror",
2728
"//pkg/security/pprompt",

pkg/cli/clisqlclient/conn.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"github.com/cockroachdb/cockroach-go/v2/crdb"
1919
"github.com/cockroachdb/cockroach/pkg/build"
20+
"github.com/cockroachdb/cockroach/pkg/ccl/securityccl/fipsccl/fipscclbase"
2021
"github.com/cockroachdb/cockroach/pkg/cli/clierror"
2122
"github.com/cockroachdb/cockroach/pkg/security/pprompt"
2223
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -43,7 +44,7 @@ type sqlConn struct {
4344
conn *pgx.Conn
4445
reconnecting bool
4546

46-
// passwordMissing is true iff the url is missing a password.
47+
// passwordMissing is true if the url is missing a password.
4748
passwordMissing bool
4849

4950
// alwaysInferResultTypes is true iff the client should always use the
@@ -183,6 +184,10 @@ func (c *sqlConn) EnsureConn(ctx context.Context) error {
183184
if err != nil {
184185
return wrapConnError(err)
185186
}
187+
// Under FIPS 140-3 mode, the password must be at least 14 characters long.
188+
if fipscclbase.IsFIPSReady() && !c.passwordMissing && len(base.Password) < fipscclbase.FIPSMinPasswordLength {
189+
return errors.Newf("password must be at least %d characters long", fipscclbase.FIPSMinPasswordLength)
190+
}
186191
// Add a notice handler - re-use the cliOutputError function in this case.
187192
base.OnNotice = func(_ *pgconn.PgConn, notice *pgconn.Notice) {
188193
c.handleNotice(notice)
@@ -769,6 +774,10 @@ func (c *sqlConn) fillPassword() error {
769774
if err != nil {
770775
return err
771776
}
777+
// Under FIPS 140-3 mode, the password must be at least 14 characters long.
778+
if fipscclbase.IsFIPSReady() && len(pwd) < fipscclbase.FIPSMinPasswordLength {
779+
return errors.Newf("password must be at least %d characters long", fipscclbase.FIPSMinPasswordLength)
780+
}
772781
connURL.User = url.UserPassword(connURL.User.Username(), pwd)
773782
c.url = connURL.String()
774783
c.passwordMissing = false

pkg/cli/democluster/demo_cluster.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package democluster
77

88
import (
99
"context"
10+
"crypto/rand"
1011
gosql "database/sql"
1112
"fmt"
1213
"io"
@@ -379,7 +380,12 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
379380
return err
380381
}
381382

382-
demoPassword := genDemoPassword(demoUsername)
383+
st := c.firstServer.ClusterSettings()
384+
minPasswordLength := security.MinPasswordLength.Get(&st.SV)
385+
demoPassword, err := genDemoPassword(demoUsername, minPasswordLength)
386+
if err != nil {
387+
return errors.Wrap(err, "failed to generate demo password")
388+
}
383389

384390
// Step 8: initialize tenant servers, if enabled.
385391
phaseCtx = logtags.AddTag(ctx, "phase", 8)
@@ -2121,10 +2127,21 @@ func (c *transientCluster) addDemoLoginToURL(uiURL *url.URL, includeTenantName b
21212127
//
21222128
// The password can be overridden via the env var
21232129
// COCKROACH_DEMO_PASSWORD for the benefit of test automation.
2124-
func genDemoPassword(username string) string {
2130+
func genDemoPassword(username string, minPasswordLength int64) (string, error) {
2131+
if password := envutil.EnvOrDefaultString("COCKROACH_DEMO_PASSWORD", ""); password != "" {
2132+
if len(password) < int(minPasswordLength) {
2133+
return "", errors.Newf("password is too short: %s", password)
2134+
}
2135+
return password, nil
2136+
}
21252137
mypid := os.Getpid()
2126-
candidatePassword := fmt.Sprintf("%s%d", username, mypid)
2127-
return envutil.EnvOrDefaultString("COCKROACH_DEMO_PASSWORD", candidatePassword)
2138+
password := fmt.Sprintf("%s%d", username, mypid)
2139+
// If the password is too short, append random characters until it is long enough.
2140+
for len(password) < int(minPasswordLength) {
2141+
randText := strings.ToLower(rand.Text())
2142+
password += string(randText[0])
2143+
}
2144+
return password, nil
21282145
}
21292146

21302147
// lockDir uses a file lock to prevent concurrent writes to the

pkg/cli/interactive_tests/test_demo.tcl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,11 @@ eexpect eof
169169
end_test
170170

171171
start_test "Check that the user can override the password."
172-
set ::env(COCKROACH_DEMO_PASSWORD) "hunter2"
172+
set ::env(COCKROACH_DEMO_PASSWORD) "hunter2hunter2hunter2hunter2"
173173
spawn $argv demo --no-line-editor --insecure=false --no-example-database --log-dir=logs
174174
eexpect "Connection parameters"
175175
eexpect "(sql)"
176-
eexpect "postgresql://demo:hunter2@"
176+
eexpect "postgresql://demo:hunter2hunter2hunter2hunter2@"
177177
eexpect "defaultdb>"
178178
send_eof
179179
eexpect eof

pkg/roachprod/install/cockroach.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,7 +751,7 @@ const (
751751
AuthRootCert
752752

753753
DefaultUser = "roachprod"
754-
DefaultPassword = "cockroachdb"
754+
DefaultPassword = "cockroachpassword"
755755

756756
DefaultAuthModeEnv = "ROACHPROD_DEFAULT_AUTH_MODE"
757757
)

pkg/security/password.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"runtime"
1212
"sync"
1313

14+
"github.com/cockroachdb/cockroach/pkg/ccl/securityccl/fipsccl/fipscclbase"
1415
"github.com/cockroachdb/cockroach/pkg/security/password"
1516
"github.com/cockroachdb/cockroach/pkg/settings"
1617
"github.com/cockroachdb/cockroach/pkg/util/envutil"
@@ -137,6 +138,16 @@ var AutoDetectPasswordHashes = settings.RegisterBoolSetting(
137138
true,
138139
)
139140

141+
// By default, the minimum password length is 1. In FIPS 140-3 mode, where HMAC
142+
// is required to have a key of at least 112 bits, the minimum password length
143+
// is 14 characters.
144+
var defaultMinPasswordLength = func() int64 {
145+
if fipscclbase.IsFIPSReady() {
146+
return fipscclbase.FIPSMinPasswordLength
147+
}
148+
return 1
149+
}()
150+
140151
// MinPasswordLength is the cluster setting that configures the
141152
// minimum SQL password length.
142153
var MinPasswordLength = settings.RegisterIntSetting(
@@ -145,8 +156,8 @@ var MinPasswordLength = settings.RegisterIntSetting(
145156
"the minimum length accepted for passwords set in cleartext via SQL. "+
146157
"Note that a value lower than 1 is ignored: passwords cannot be empty in any case. "+
147158
"This setting only applies when adding new users or altering an existing user's password; it will not affect existing logins.",
148-
1,
149-
settings.NonNegativeInt,
159+
defaultMinPasswordLength,
160+
settings.IntWithMinimum(defaultMinPasswordLength),
150161
settings.WithPublic)
151162

152163
// AutoUpgradePasswordHashes is the cluster setting that configures whether to

0 commit comments

Comments
 (0)