diff --git a/pkg/sql/pgwire/auth_test.go b/pkg/sql/pgwire/auth_test.go new file mode 100644 index 000000000000..6cbf2f6112f3 --- /dev/null +++ b/pkg/sql/pgwire/auth_test.go @@ -0,0 +1,229 @@ +// Copyright 2019 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package pgwire_test + +import ( + "context" + gosql "database/sql" + "fmt" + "net" + "net/url" + "strings" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/server" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/datadriven" + "github.com/cockroachdb/errors/stdstrings" + "github.com/lib/pq" +) + +// TestAuthenticationAndHBARules exercises the authentication code +// using datadriven testing. +// +// It supports the following DSL: +// +// config [secure] [insecure] +// Only run the test file if the server is in the specified +// security mode. (The default is `config secure insecure` i.e. +// the test file is applicable to both.) +// +// sql +// +// Execute the specified SQL statement using the default root +// connection provided by StartServer(). +// +// set_hba +// +// Load the provided HBA configuration via the cluster setting +// server.host_based_authentication.configuration. +// +// connect [key=value ...] +// Attempt a SQL connection using the provided connection +// parameters using the pg "DSN notation": k/v pairs separated +// by spaces. +// The following standard pg keys are recognized: +// user - the username +// password - the password +// host - the server name/address +// port - the server port +// sslmode, sslrootcert, sslcert, sslkey - SSL parameters. +// +// The order of k/v pairs matters: if the same key is specified +// multiple times, the first occurrence takes priority. +// +// Additionally, the test runner will always _append_ a default +// value for user (root), host/port/sslrootcert from the +// initialized test server. This default configuration is placed +// at the end so that each test can override the values. +// +// The test runner also adds a default value for sslcert and +// sslkey based on the value of "user" — either when provided by +// the test, or root by default. +// +// When the user is either "root" or "testuser" (those are the +// users for which the test server generates certificates), +// sslmode also gets a default of "verify-full". For other +// users, sslmode is initialized by default to "verify-ca". +// +// For each of these directives, he expected output can be either "ok" +// (no error) or "ERRROR:" followed by the expected error string. +// +func TestAuthenticationAndHBARules(t *testing.T) { + defer leaktest.AfterTest(t)() + + testutils.RunTrueAndFalse(t, "insecure", func(t *testing.T, insecure bool) { + hbaRunTest(t, insecure) + }) +} + +func hbaRunTest(t *testing.T, insecure bool) { + datadriven.Walk(t, "testdata/auth", func(t *testing.T, path string) { + s, conn, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: insecure}) + defer s.Stopper().Stop(context.TODO()) + + if _, err := conn.ExecContext(context.Background(), `CREATE USER $1`, server.TestUser); err != nil { + t.Fatal(err) + } + + datadriven.RunTest(t, path, func(t *testing.T, td *datadriven.TestData) string { + switch td.Cmd { + case "config": + allowed := false + for _, a := range td.CmdArgs { + switch a.Key { + case "secure": + allowed = allowed || !insecure + case "insecure": + allowed = allowed || insecure + default: + t.Fatalf("unknown configuration: %s", a.Key) + } + } + if !allowed { + t.Skip("Test file not applicable at this security level.") + } + + case "set_hba": + _, err := conn.ExecContext(context.Background(), + `SET CLUSTER SETTING server.host_based_authentication.configuration = $1`, td.Input) + return fmtErr(err) + + case "sql": + _, err := conn.ExecContext(context.Background(), td.Input) + return fmtErr(err) + + case "connect": + // Prepare a connection string using the server's default. + // What is the user requested by the test? + user := security.RootUser + if td.HasArg("user") { + td.ScanArgs(t, "user", &user) + } + + // We want the certs to be present in the filesystem for this test. + // However, certs are only generated for users "root" and "testuser" specifically. + sqlURL, cleanupFn := sqlutils.PGUrlWithOptionalClientCerts( + t, s.ServingSQLAddr(), t.Name(), url.User(user), + user == security.RootUser || user == server.TestUser /* withClientCerts */) + defer cleanupFn() + + host, port, err := net.SplitHostPort(s.ServingSQLAddr()) + if err != nil { + t.Fatal(err) + } + options, err := url.ParseQuery(sqlURL.RawQuery) + if err != nil { + t.Fatal(err) + } + + // Here we make use of the fact that pq accepts connection + // strings using the alternate postgres configuration format, + // consisting of k=v pairs separated by spaces. + // For example this is a valid connection string: + // "host=localhost port=5432 user=root" + // We also make use of the datadriven K/V parsing facility, + // which always prioritizes the first K instance in the test's + // argument list. We append the server's config parameters + // at the end, letting the test override by introducing its + // own values at the beginning. + args := append(td.CmdArgs, + datadriven.CmdArg{Key: "user", Vals: []string{sqlURL.User.Username()}}, + datadriven.CmdArg{Key: "host", Vals: []string{host}}, + datadriven.CmdArg{Key: "port", Vals: []string{port}}, + ) + for key := range options { + args = append(args, + datadriven.CmdArg{Key: key, Vals: []string{options.Get(key)}}) + } + // Now turn the cmdargs into a dsn. + var dsnBuf strings.Builder + sp := "" + seenKeys := map[string]struct{}{} + for _, a := range args { + if _, ok := seenKeys[a.Key]; ok { + continue + } + seenKeys[a.Key] = struct{}{} + val := "" + if len(a.Vals) > 0 { + val = a.Vals[0] + } + fmt.Fprintf(&dsnBuf, "%s%s=%s", sp, a.Key, val) + sp = " " + } + dsn := dsnBuf.String() + + // Finally, connect and test the connection. + dbSQL, err := gosql.Open("postgres", dsn) + if err != nil { + return fmtErr(err) + } + defer dbSQL.Close() + _, err = dbSQL.Exec("SHOW TABLES") + return fmtErr(err) + + default: + td.Fatalf(t, "unknown command: %s", td.Cmd) + } + return "" + }) + }) +} + +func fmtErr(err error) string { + if err != nil { + errStr := "" + if pqErr, ok := err.(*pq.Error); ok { + errStr = pqErr.Message + if pqErr.Code != pgcode.Uncategorized { + errStr += fmt.Sprintf(" (SQLSTATE %s)", pqErr.Code) + } + if pqErr.Hint != "" { + hint := strings.Replace(pqErr.Hint, stdstrings.IssueReferral, "", 1) + errStr += "\nHINT: " + hint + } + if pqErr.Detail != "" { + errStr += "\nDETAIL: " + pqErr.Detail + } + } else { + errStr = err.Error() + } + return "ERROR: " + errStr + } + return "ok" +} diff --git a/pkg/sql/pgwire/pgwire_test.go b/pkg/sql/pgwire/pgwire_test.go index 110a18c14fb5..8a9ab849fcc8 100644 --- a/pkg/sql/pgwire/pgwire_test.go +++ b/pkg/sql/pgwire/pgwire_test.go @@ -42,10 +42,10 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/errors" "github.com/jackc/pgx" "github.com/jackc/pgx/pgproto3" "github.com/lib/pq" - "github.com/pkg/errors" ) func wrongArgCountString(want, got int) string { @@ -1941,168 +1941,6 @@ func TestPGWireAuth(t *testing.T) { }) } -func TestHBA(t *testing.T) { - defer leaktest.AfterTest(t)() - - s, conn, _ := serverutils.StartServer(t, base.TestServerArgs{}) - defer s.Stopper().Stop(context.TODO()) - db := sqlutils.MakeSQLRunner(conn) - - const ( - user = "passworduser" - pass = "pass" - ) - - db.Exec(t, fmt.Sprintf(`CREATE USER %s`, server.TestUser)) - db.Exec(t, fmt.Sprintf(`CREATE USER %s WITH PASSWORD '%s'`, user, pass)) - - tests := []struct { - // The hba.conf file/setting. - conf string - // Error message when setting the config. - confErr string - // Error message of password login. - passErr string - // Error message of cert login. - certErr string - }{ - { - conf: `bad`, - confErr: "entry 1 invalid", - }, - { - conf: `#empty`, - confErr: "no entries", - }, - { - conf: `host all all 1.1.1/0 cert`, - confErr: "invalid CIDR address", - }, - { - conf: `host all all 0.0.0.0/0 invalid`, - confErr: "unknown auth method", - }, - { - conf: `host db all 0.0.0.0/0 cert`, - confErr: "database must be specified as all", - }, - { - // quoted all isn't ok since it strips the special meaning - conf: `host "all" all 0.0.0.0/0 cert`, - confErr: "database must be specified as all", - }, - { - // only the all hostname is supported - conf: `host all all hostname cert`, - confErr: "host addresses not supported", - }, - { - // valid for both specified users - conf: ` - host all testuser 0.0.0.0/0 cert - host all passworduser 0.0.0.0/0 password - `, - }, - { - // valid for both specified users - conf: ` - host all testuser,passworduser all cert-password - `, - }, - { - // the "all" user means password never is checked - conf: ` - host all testuser,all 0.0.0.0/0 cert - host all passworduser 0.0.0.0/0 password - `, - passErr: "no TLS peer certificates", - }, - { - // but double quoting removes special meaning - conf: ` - host all testuser,"all" 0.0.0.0/0 cert - host all passworduser 0.0.0.0/0 password - `, - }, - { - // disallow passwords - conf: "host all all 0.0.0.0/0 cert", - passErr: "no TLS peer certificates", - }, - { - // disallow certs - conf: "host all all 0.0.0.0/0 password", - certErr: "password authentication failed for user testuser", - }, - { - // invalid user name - conf: "host all invalid 0.0.0.0/0 cert", - certErr: "no .* entry", - passErr: "no .* entry", - }, - { - // invalid IP - conf: "host all all 0.0.0.0/32 cert", - certErr: "no .* entry", - passErr: "no .* entry", - }, - { - // allow certs from 127.*.*.* - conf: "host all all 127.0.0.0/8 cert", - passErr: "no TLS peer certificates", - }, - { - // allow certs from 128.*.*.*, but connect from 127.0.0.0 - conf: "host all all 128.0.0.0/8 cert", - certErr: "no .* entry", - passErr: "no .* entry", - }, - } - for i, tc := range tests { - t.Run(fmt.Sprint(i), func(t *testing.T) { - t.Log(tc.conf) - db.ExpectErr(t, tc.confErr, `SET CLUSTER SETTING server.host_based_authentication.configuration = $1`, tc.conf) - - t.Run("root", func(t *testing.T) { - // Authenticate as root with certificate and expect success. - rootPgURL, cleanupFn := sqlutils.PGUrl( - t, s.ServingSQLAddr(), t.Name(), url.User(security.RootUser)) - defer cleanupFn() - if err := trivialQuery(rootPgURL); err != nil { - t.Fatalf("could not auth as root: %v", err) - } - }) - - t.Run("cert", func(t *testing.T) { - testUserPgURL, cleanupFn := sqlutils.PGUrl( - t, s.ServingSQLAddr(), t.Name(), url.User(server.TestUser)) - defer cleanupFn() - err := trivialQuery(testUserPgURL) - if !testutils.IsError(err, tc.certErr) { - t.Errorf("expected err %v, got %v", tc.certErr, err) - } - }) - - t.Run("password", func(t *testing.T) { - host, port, err := net.SplitHostPort(s.ServingSQLAddr()) - if err != nil { - t.Fatal(err) - } - testURL := url.URL{ - Scheme: "postgres", - User: url.UserPassword(user, pass), - Host: net.JoinHostPort(host, port), - RawQuery: "sslmode=require", - } - err = trivialQuery(testURL) - if !testutils.IsError(err, tc.passErr) { - t.Errorf("expected err %v, got %v", tc.passErr, err) - } - }) - }) - } -} - func TestPGWireResultChange(t *testing.T) { defer leaktest.AfterTest(t)() s, _, _ := serverutils.StartServer(t, base.TestServerArgs{}) diff --git a/pkg/sql/pgwire/testdata/auth/empty_hba b/pkg/sql/pgwire/testdata/auth/empty_hba new file mode 100644 index 000000000000..76b5abd9b2b3 --- /dev/null +++ b/pkg/sql/pgwire/testdata/auth/empty_hba @@ -0,0 +1,99 @@ +# These tests exercise the code when the HBA configuration is empty. +# +# An empty config is somewhat equivalent to: +# +# host all root 0.0.0.0/0 cert +# host all all 0.0.0.0/0 cert-password +# +# The server is secure, so the test runner sets sslmode=verify-full +# by default for all test directives below with all other SSL settings loaded. +# +# When this is the case, client certificates are verified by the +# server, and password auth is not required if the client cert +# is valid. If the cert is invalid, the password becomes required. + +config secure +---- + +# Set HBA to empty in case it wasn't done before. +set_hba +---- +ok + +subtest root + +# Root can always connect regardless. +connect user=root +---- +ok + +# When no client cert is presented, the server would otherwise require +# password auth. However, root password auth is rejected in any case. +connect user=root password=foo sslmode=verify-ca sslcert= +---- +ERROR: user root must use certificate authentication instead of password authentication + +subtest end root + + +subtest normaluser_cert + +# User need no password, and we're presenting a client cert. All good. +connect user=testuser +---- +ok + +# Make the user need a password. +sql +ALTER USER testuser WITH PASSWORD 'pass'; +---- +ok + +# Password now needed, but as long as we're presenting a cert it's good. +connect user=testuser +---- +ok + +# If we don't present the client certificate, the password is required. +connect user=testuser password=invalid sslmode=verify-ca sslcert= +---- +ERROR: password authentication failed for user testuser + +connect user=testuser password=pass sslmode=verify-ca sslcert= +---- +ok + +# Reset the test user to no password. +sql +DROP USER testuser; CREATE USER testuser +---- +ok + +subtest end normaluser_cert + +subtest normaluser_nocert + +# This other test user has no default cert. +sql +CREATE USER testuser_nocert +---- +ok + +# Since there is no cert, no cert is going to be presented by the client +# and password auth becomes required. +connect user=testuser_nocert +---- +ERROR: password authentication failed for user testuser_nocert + +# Even though the user has no password, trying to present the +# empty password fails. The user simply cannot log in. +connect user=testuser_nocert password= +---- +ERROR: password authentication failed for user testuser_nocert + +sql +DROP USER testuser_nocert +---- +ok + +subtest end normaluser_nocert diff --git a/pkg/sql/pgwire/testdata/auth/hba_default_equivalence b/pkg/sql/pgwire/testdata/auth/hba_default_equivalence new file mode 100644 index 000000000000..e11b445e3141 --- /dev/null +++ b/pkg/sql/pgwire/testdata/auth/hba_default_equivalence @@ -0,0 +1,91 @@ +# This verifies that the behavior with an empty HBA config +# is equivalent to: +# host all root 0.0.0.0/0 cert +# host all all 0.0.0.0/0 cert-password +# by using that explicit string and reproducing the tests +# in the test file "empty_hba". + +config secure +---- + +set_hba +host all root 0.0.0.0/0 cert +host all all 0.0.0.0/0 cert-password +---- +ok + +subtest root + +# Root can always connect regardless. +connect user=root +---- +ok + +# The rule has method "cert" so a cert is required in any case. +connect user=root password=foo sslmode=verify-ca sslcert= +---- +ERROR: no TLS peer certificates, but required for auth + +subtest end root + +subtest normaluser_cert + +# User need no password, and we're presenting a client cert. All good. +connect user=testuser +---- +ok + +# Make the user need a password. +sql +ALTER USER testuser WITH PASSWORD 'pass'; +---- +ok + +# Password now needed, but as long as we're presenting a cert it's good. +connect user=testuser +---- +ok + +# If we don't present the client certificate, the password is required. +connect user=testuser password=invalid sslmode=verify-ca sslcert= +---- +ERROR: password authentication failed for user testuser + +connect user=testuser password=pass sslmode=verify-ca sslcert= +---- +ok + +# Reset the test user to no password. +sql +DROP USER testuser; CREATE USER testuser +---- +ok + +subtest end normaluser_cert + +subtest normaluser_nocert + +# This other test user has no default cert. +sql +CREATE USER testuser_nocert; +---- +ok + +# Since there is no cert, no cert is going to be presented by the client +# and password auth becomes required. +connect user=testuser_nocert +---- +ERROR: password authentication failed for user testuser_nocert + +# Even though the user has no password, trying to present the +# empty password fails. The user simply cannot log in. +connect user=testuser_nocert password= +---- +ERROR: password authentication failed for user testuser_nocert + +sql +DROP USER testuser_nocert +---- +ok + +subtest end normaluser_nocert diff --git a/pkg/sql/pgwire/testdata/auth/hba_host_selection b/pkg/sql/pgwire/testdata/auth/hba_host_selection new file mode 100644 index 000000000000..ffe72a332999 --- /dev/null +++ b/pkg/sql/pgwire/testdata/auth/hba_host_selection @@ -0,0 +1,46 @@ +# These tests exercise the "hostname" filter in HBA rules. + +config secure +---- + +subtest nomatch + +# If the CIDR mask does not match, auth doesn't find a rule. + +set_hba +host all all 0.0.0.0/32 cert +---- +ok + +connect user=testuser +---- +ERROR: no server.host_based_authentication.configuration entry for host "127.0.0.1", user "testuser" + +subtest nomatch/root_override + +# However, even if no rule matches root can still log in. + +connect user=root +---- +ok + +subtest end nomatch/root_override + + +subtest end nomatch + +subtest match_net + +# It's possible to use a network mask. Since the test is connecting using 127.0.0.1, +# then all the 127/8 network matches. + +set_hba +host all all 127.0.0.0/8 cert +---- +ok + +connect user=testuser +---- +ok + +subtest end match_net \ No newline at end of file diff --git a/pkg/sql/pgwire/testdata/auth/hba_syntax b/pkg/sql/pgwire/testdata/auth/hba_syntax new file mode 100644 index 000000000000..24e47d9fe68a --- /dev/null +++ b/pkg/sql/pgwire/testdata/auth/hba_syntax @@ -0,0 +1,40 @@ +set_hba +bad +---- +ERROR: entry 1 invalid + +set_hba +#empty +---- +ERROR: no entries + +set_hba +host all all 1.1.1/0 cert +---- +ERROR: invalid CIDR address: 1.1.1/0 + +set_hba +host all all 0.0.0.0/0 invalid +---- +ERROR: unknown auth method "invalid" + + +# CockroachDB does not (yet?) support per-db HBA rules. +set_hba +host db all 0.0.0.0/0 cert +---- +ERROR: database must be specified as all + + +# quoted "all" isn't ok since it strips the special meaning. +set_hba +host "all" all 0.0.0.0/0 cert +---- +ERROR: database must be specified as all + + +# CockroachDB does not (yet?) support hostname-based HBA rules. +set_hba +host all all hostname cert +---- +ERROR: host addresses not supported diff --git a/pkg/sql/pgwire/testdata/auth/hba_user_selection b/pkg/sql/pgwire/testdata/auth/hba_user_selection new file mode 100644 index 000000000000..02205f6e4310 --- /dev/null +++ b/pkg/sql/pgwire/testdata/auth/hba_user_selection @@ -0,0 +1,162 @@ +# The following tests exercise how the HBA rules match on the +# username. + +config secure +---- + +# First define some users. + +# We're going to have a "passworduser" with a password set, but no client cert. +sql +CREATE USER passworduser WITH PASSWORD 'pass' +---- +ok + + + +subtest root + +# This configuration says "only root can log in". + +set_hba +host all root 0.0.0.0/0 cert +---- +ok + +connect user=root +---- +ok + +connect user=testuser +---- +ERROR: no server.host_based_authentication.configuration entry for host "127.0.0.1", user "testuser" + +connect user=passworduser password=pass +---- +ERROR: no server.host_based_authentication.configuration entry for host "127.0.0.1", user "passworduser" + +subtest end root + + + + +subtest testuser + +# This configuration says "only testuser can log in". + +set_hba +host all testuser 0.0.0.0/0 cert +---- +ok + +connect user=testuser +---- +ok + +connect user=passworduser password=pass +---- +ERROR: no server.host_based_authentication.configuration entry for host "127.0.0.1", user "passworduser" + +# Although this is not completely true. "root" can always log in nonetheless. + +connect user=root +---- +ok + +subtest end testuser + + + +subtest side_by_side + +set_hba +host all testuser 0.0.0.0/0 cert +host all passworduser 0.0.0.0/0 cert-password +---- +ok + +connect user=testuser +---- +ok + +connect user=passworduser password=pass +---- +ok + +# "root" can still log in regardless. +connect user=root +---- +ok + +subtest end side_by_side + + + +subtest multiple + +set_hba +host all testuser,passworduser 0.0.0.0/0 cert-password +---- +ok + +connect user=testuser +---- +ok + +connect user=passworduser password=pass +---- +ok + +# "root" can still log in regardless. +connect user=root +---- +ok + + +subtest end multiple + + + +subtest priority + +# This test shows that the first rule that matches +# gets priority: in this example, the first rule +# contains "all" and thus matches everything, +# so the second rule is not matched. So a certificate +# is required for everyone. + +set_hba +host all testuser,all 0.0.0.0/0 cert +host all passworduser 0.0.0.0/0 password +---- +ok + +connect user=testuser +---- +ok + +connect user=passworduser password=pass +---- +ERROR: no TLS peer certificates, but required for auth + +# The special keyword "all" only matches when it is unquoted. + +subtest priority/unquoted_all + +set_hba +host all testuser,"all" 0.0.0.0/0 cert +host all passworduser 0.0.0.0/0 password +---- +ok + +connect user=testuser +---- +ok + +connect user=passworduser password=pass +---- +ok + +subtest end priority/unquoted_all + +subtest end priority diff --git a/pkg/sql/pgwire/testdata/auth/insecure b/pkg/sql/pgwire/testdata/auth/insecure new file mode 100644 index 000000000000..67890a6778aa --- /dev/null +++ b/pkg/sql/pgwire/testdata/auth/insecure @@ -0,0 +1,24 @@ +config insecure +---- + +# Regardless of the contents of hba.conf, +# all users can always log in, even without a cert. + +set_hba +host all root 0.0.0.0/0 cert +---- +ok + +connect user=root sslmode=disable +---- +ok + +connect user=testuser sslmode=disable +---- +ok + +# If a password is provided, it is ignored. + +connect user=testuser password=abc sslmode=disable +---- +ok diff --git a/pkg/sql/pgwire/testdata/auth/special_cases b/pkg/sql/pgwire/testdata/auth/special_cases new file mode 100644 index 000000000000..981ce46c3452 --- /dev/null +++ b/pkg/sql/pgwire/testdata/auth/special_cases @@ -0,0 +1,71 @@ +# We're going to have a "passworduser" with a password set, but no client cert. + +config secure +---- + +sql +CREATE USER passworduser WITH PASSWORD 'pass' +---- +ok + +subtest root_user_cannot_use_password + +# This test exercises that root cannot log in with +# a password even if the HBA rules say so (i.e. root is +# always forced to auth with cert). + +set_hba +host all root 0.0.0.0/0 password +---- +ok + +connect user=root password=abc sslmode=verify-ca sslcert= +---- +ERROR: no TLS peer certificates, but required for auth + +subtest end root_user_cannot_use_password + + +subtest user_has_both_cert_and_passwd + +sql +ALTER USER testuser WITH PASSWORD 'pass' +---- +ok + +subtest user_has_both_cert_and_passwd/only_cert_implies_reject_password + +# If the rule says "I want a cert" (and the user has a cert), +# then don't accept a password even if the user has one. + +set_hba +host all testuser 0.0.0.0/0 cert +---- +ok + +connect user=testuser password=pass sslmode=verify-ca sslcert= +---- +ERROR: no TLS peer certificates, but required for auth + +subtest end user_has_both_cert_and_passwd/only_cert_implies_reject_password + +subtest user_has_both_cert_and_passwd/only_password_implies_reject_cert + +set_hba +host all testuser 0.0.0.0/0 password +---- +ok + +connect user=testuser +---- +ERROR: password authentication failed for user testuser + +subtest end user_has_both_cert_and_passwd/only_password_implies_reject_cert + + +sql +DROP USER testuser; CREATE USER testuser +---- +ok + +subtest end user_has_both_cert_and_passwd diff --git a/pkg/testutils/sqlutils/pg_url.go b/pkg/testutils/sqlutils/pg_url.go index 88ab719bc55d..d4caf323faa4 100644 --- a/pkg/testutils/sqlutils/pg_url.go +++ b/pkg/testutils/sqlutils/pg_url.go @@ -37,6 +37,14 @@ import ( // Args: // prefix: A prefix to be prepended to the temp file names generated, for debugging. func PGUrl(t testing.TB, servingAddr, prefix string, user *url.Userinfo) (url.URL, func()) { + return PGUrlWithOptionalClientCerts(t, servingAddr, prefix, user, true /* withCerts */) +} + +// PGUrlWithOptionalClientCerts is like PGUrl but the caller can +// customize whether the client certificates are loaded on-disk and in the URL. +func PGUrlWithOptionalClientCerts( + t testing.TB, servingAddr, prefix string, user *url.Userinfo, withClientCerts bool, +) (url.URL, func()) { host, port, err := net.SplitHostPort(servingAddr) if err != nil { t.Fatal(err) @@ -50,19 +58,24 @@ func PGUrl(t testing.TB, servingAddr, prefix string, user *url.Userinfo) (url.UR } caPath := filepath.Join(security.EmbeddedCertsDir, security.EmbeddedCACert) - certPath := filepath.Join(security.EmbeddedCertsDir, fmt.Sprintf("client.%s.crt", user.Username())) - keyPath := filepath.Join(security.EmbeddedCertsDir, fmt.Sprintf("client.%s.key", user.Username())) - - // Copy these assets to disk from embedded strings, so this test can - // run from a standalone binary. tempCAPath := securitytest.RestrictedCopy(t, caPath, tempDir, "ca") - tempCertPath := securitytest.RestrictedCopy(t, certPath, tempDir, "cert") - tempKeyPath := securitytest.RestrictedCopy(t, keyPath, tempDir, "key") options := url.Values{} - options.Add("sslmode", "verify-full") options.Add("sslrootcert", tempCAPath) - options.Add("sslcert", tempCertPath) - options.Add("sslkey", tempKeyPath) + + if withClientCerts { + certPath := filepath.Join(security.EmbeddedCertsDir, fmt.Sprintf("client.%s.crt", user.Username())) + keyPath := filepath.Join(security.EmbeddedCertsDir, fmt.Sprintf("client.%s.key", user.Username())) + + // Copy these assets to disk from embedded strings, so this test can + // run from a standalone binary. + tempCertPath := securitytest.RestrictedCopy(t, certPath, tempDir, "cert") + tempKeyPath := securitytest.RestrictedCopy(t, keyPath, tempDir, "key") + options.Add("sslcert", tempCertPath) + options.Add("sslkey", tempKeyPath) + options.Add("sslmode", "verify-full") + } else { + options.Add("sslmode", "verify-ca") + } return url.URL{ Scheme: "postgres",