diff --git a/pkg/acceptance/cluster/certs.go b/pkg/acceptance/cluster/certs.go index a9891c96e53b..0e149eaae852 100644 --- a/pkg/acceptance/cluster/certs.go +++ b/pkg/acceptance/cluster/certs.go @@ -39,7 +39,7 @@ func GenerateCerts(ctx context.Context) func() { // Root user. maybePanic(security.CreateClientPair( certsDir, filepath.Join(certsDir, security.EmbeddedCAKey), - 1024, 48*time.Hour, false, security.RootUserName(), true /* generate pk8 key */)) + 2048, 48*time.Hour, false, security.RootUserName(), true /* generate pk8 key */)) // Test user. maybePanic(security.CreateClientPair( diff --git a/pkg/acceptance/compose/gss/docker-compose-python.yml b/pkg/acceptance/compose/gss/docker-compose-python.yml index d07d26200131..8ab1b00278ef 100644 --- a/pkg/acceptance/compose/gss/docker-compose-python.yml +++ b/pkg/acceptance/compose/gss/docker-compose-python.yml @@ -20,13 +20,10 @@ services: build: ./python depends_on: - cockroach - command: /start.sh environment: - PGHOST=cockroach - PGPORT=26257 - PGSSLMODE=require - - PGSSLCERT=/certs/node.crt - - PGSSLKEY=/certs/node.key volumes: - ./kdc/krb5.conf:/etc/krb5.conf - ./python/start.sh:/start.sh diff --git a/pkg/acceptance/compose/gss/docker-compose.yml b/pkg/acceptance/compose/gss/docker-compose.yml index ca306ab03fef..1e3d8dd653ef 100644 --- a/pkg/acceptance/compose/gss/docker-compose.yml +++ b/pkg/acceptance/compose/gss/docker-compose.yml @@ -23,8 +23,6 @@ services: environment: - PGHOST=cockroach - PGPORT=26257 - - PGSSLCERT=/certs/node.crt - - PGSSLKEY=/certs/node.key volumes: - ./kdc/krb5.conf:/etc/krb5.conf - ./psql/gss_test.go:/test/gss_test.go diff --git a/pkg/acceptance/compose/gss/psql/gss_test.go b/pkg/acceptance/compose/gss/psql/gss_test.go index 25dc6f2bd278..601020501766 100644 --- a/pkg/acceptance/compose/gss/psql/gss_test.go +++ b/pkg/acceptance/compose/gss/psql/gss_test.go @@ -35,7 +35,7 @@ func init() { } func TestGSS(t *testing.T) { - connector, err := pq.NewConnector("user=root sslmode=require") + connector, err := pq.NewConnector("user=root password=rootpw sslmode=require") if err != nil { t.Fatal(err) } diff --git a/pkg/acceptance/compose/gss/psql/start.sh b/pkg/acceptance/compose/gss/psql/start.sh index c143406bf29c..5daaa77ced80 100755 --- a/pkg/acceptance/compose/gss/psql/start.sh +++ b/pkg/acceptance/compose/gss/psql/start.sh @@ -2,6 +2,20 @@ set -e +echo "Available certs:" +ls -l /certs + +echo "Environment:" +env + +echo "Creating a k5s token..." echo psql | kinit tester@MY.EX +echo "Preparing SQL user ahead of test" +env \ + PGSSLKEY=/certs/client.root.key \ + PGSSLCERT=/certs/client.root.crt \ + psql -c "ALTER USER root WITH PASSWORD rootpw" + +echo "Running test" ./gss.test diff --git a/pkg/acceptance/compose/gss/python/Dockerfile b/pkg/acceptance/compose/gss/python/Dockerfile index 097736f02324..adfd3121dcaa 100644 --- a/pkg/acceptance/compose/gss/python/Dockerfile +++ b/pkg/acceptance/compose/gss/python/Dockerfile @@ -11,3 +11,5 @@ WORKDIR /code COPY requirements.txt /code/ RUN pip install -r requirements.txt COPY . /code/ + +ENTRYPOINT ["/start.sh"] diff --git a/pkg/acceptance/compose/gss/python/start.sh b/pkg/acceptance/compose/gss/python/start.sh index 8463488c7ee3..e0e53227e6e9 100755 --- a/pkg/acceptance/compose/gss/python/start.sh +++ b/pkg/acceptance/compose/gss/python/start.sh @@ -2,10 +2,29 @@ set -e -psql -c "SET CLUSTER SETTING server.host_based_authentication.configuration = 'host all all all gss include_realm=0'" -psql -c "CREATE USER tester" +echo "Available certs:" +ls -l /certs + +echo "Environment:" +env +echo "Creating a k5s token..." echo psql | kinit tester@MY.EX +export PGSSLKEY=/certs/client.root.key +export PGSSLCERT=/certs/client.root.crt +export PGUSER=root + +echo "Creating test user" +psql -c "CREATE USER tester" +echo "Configuring the HBA rule prior to running the test..." +psql -c "SET CLUSTER SETTING server.host_based_authentication.configuration = 'host all all all gss include_realm=0'" + +echo "Testing the django connection..." + +unset PGSSLKEY +unset PGSSLCERT +export PGUSER=tester + # Exit with error unless we find the expected error message. python manage.py inspectdb 2>&1 | grep 'use of GSS authentication requires an enterprise license' diff --git a/pkg/ccl/sqlproxyccl/proxy_handler_test.go b/pkg/ccl/sqlproxyccl/proxy_handler_test.go index 5471017f52ab..3c30c61df4da 100644 --- a/pkg/ccl/sqlproxyccl/proxy_handler_test.go +++ b/pkg/ccl/sqlproxyccl/proxy_handler_test.go @@ -39,8 +39,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" - "github.com/jackc/pgproto3/v2" - "github.com/jackc/pgx/v4" + pgproto3 "github.com/jackc/pgproto3/v2" + pgx "github.com/jackc/pgx/v4" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -364,15 +364,22 @@ func TestProxyModifyRequestParams(t *testing.T) { te := newTester() defer te.Close() - sql, _, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) + sql, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) sql.(*server.TestServer).PGServer().TestingSetTrustClientProvidedRemoteAddr(true) defer sql.Stopper().Stop(ctx) + // Create some user with password authn. + _, err := sqlDB.Exec("CREATE USER testuser WITH PASSWORD foo123") + require.NoError(t, err) + outgoingTLSConfig, err := sql.RPCContext().GetClientTLSConfig() require.NoError(t, err) proxyOutgoingTLSConfig := outgoingTLSConfig.Clone() proxyOutgoingTLSConfig.InsecureSkipVerify = true + // We wish the proxy to work even without providing a valid TLS client cert to the SQL server. + proxyOutgoingTLSConfig.Certificates = nil + originalBackendDial := BackendDial defer testutils.TestingHook(&BackendDial, func( msg *pgproto3.StartupMessage, outgoingAddress string, tlsConfig *tls.Config, @@ -389,14 +396,14 @@ func TestProxyModifyRequestParams(t *testing.T) { // NB: This test will fail unless the user used between the proxy // and the backend is changed to a user that actually exists. delete(params, "authToken") - params["user"] = "root" + params["user"] = "testuser" return originalBackendDial(msg, sql.ServingSQLAddr(), proxyOutgoingTLSConfig) })() s, proxyAddr := newSecureProxyServer(ctx, t, sql.Stopper(), &ProxyOptions{}) - u := fmt.Sprintf("postgres://bogususer@%s/?sslmode=require&authToken=abc123&options=--cluster=dim-dog-28&sslmode=require", proxyAddr) + u := fmt.Sprintf("postgres://bogususer:foo123@%s/?sslmode=require&authToken=abc123&options=--cluster=dim-dog-28&sslmode=require", proxyAddr) te.TestConnect(ctx, t, u, func(conn *pgx.Conn) { require.Equal(t, int64(1), s.metrics.CurConnCount.Value()) require.NoError(t, runTestQuery(ctx, conn)) @@ -516,15 +523,22 @@ func TestDenylistUpdate(t *testing.T) { denyList, err := ioutil.TempFile("", "*_denylist.yml") require.NoError(t, err) - sql, _, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) + sql, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) sql.(*server.TestServer).PGServer().TestingSetTrustClientProvidedRemoteAddr(true) defer sql.Stopper().Stop(ctx) + // Create some user with password authn. + _, err = sqlDB.Exec("CREATE USER testuser WITH PASSWORD foo123") + require.NoError(t, err) + outgoingTLSConfig, err := sql.RPCContext().GetClientTLSConfig() require.NoError(t, err) proxyOutgoingTLSConfig := outgoingTLSConfig.Clone() proxyOutgoingTLSConfig.InsecureSkipVerify = true + // We wish the proxy to work even without providing a valid TLS client cert to the SQL server. + proxyOutgoingTLSConfig.Certificates = nil + originalBackendDial := BackendDial defer testutils.TestingHook(&BackendDial, func( msg *pgproto3.StartupMessage, outgoingAddress string, tlsConfig *tls.Config, @@ -554,7 +568,7 @@ func TestDenylistUpdate(t *testing.T) { }) defer func() { _ = os.Remove(denyList.Name()) }() - url := fmt.Sprintf("postgres://root:admin@%s/defaultdb_29?sslmode=require&options=--cluster=dim-dog-28&sslmode=require", addr) + url := fmt.Sprintf("postgres://testuser:foo123@%s/defaultdb_29?sslmode=require&options=--cluster=dim-dog-28&sslmode=require", addr) te.TestConnect(ctx, t, url, func(conn *pgx.Conn) { require.Eventuallyf( t, diff --git a/pkg/security/auth.go b/pkg/security/auth.go index 04f3cd5aa59c..f5dd5de42953 100644 --- a/pkg/security/auth.go +++ b/pkg/security/auth.go @@ -134,10 +134,8 @@ func UserAuthCertHook(insecureMode bool, tlsState *tls.ConnectionState) (UserAut errors.Errorf("using tenant client certificate as user certificate is not allowed") } - // The client certificate user must match the requested user, - // except if the certificate user is NodeUser, which is allowed to - // act on behalf of all other users. - if !Contains(certUsers, requestedUser.Normalized()) && !Contains(certUsers, NodeUser) { + // The client certificate user must match the requested user. + if !Contains(certUsers, requestedUser.Normalized()) { return nil, errors.Errorf("requested user is %s, but certificate is for %s", requestedUser, certUsers) } diff --git a/pkg/security/auth_test.go b/pkg/security/auth_test.go index 6e1e237606df..94c5cadf3b16 100644 --- a/pkg/security/auth_test.go +++ b/pkg/security/auth_test.go @@ -195,6 +195,8 @@ func TestAuthenticationHook(t *testing.T) { {false, "foo", security.NodeUserName(), "", true, false, false}, // Secure mode, node user. {false, security.NodeUser, security.NodeUserName(), "", true, true, true}, + // Secure mode, node cert and unrelated user. + {false, security.NodeUser, fooUser, "", true, false, false}, // Secure mode, root user. {false, security.RootUser, security.NodeUserName(), "", true, false, false}, // Secure mode, tenant cert, foo user.