Merge #30932

30932: security: use same error message for user DNE and bad password r=mjibson a=mjibson

This prevents attacks to determine which users exist.

Fixes #30879

Release note (bug fix): "user does not exist" and "invalid password"
errors now produce the same error message during password login.

Co-authored-by: Matt Jibson <matt.jibson@gmail.com>

pkg/cli/interactive_tests/test_secure.tcl

@@ -117,7 +117,7 @@ eexpect $prompt
 send "$argv sql --certs-dir=$certs_dir --user=eisen\r"
 eexpect "Enter password:"
 send "*****\r"
-eexpect "Error: pq: invalid password"
+eexpect "Error: pq: password authentication failed for user eisen"
 eexpect "Failed running \"sql\""
 # Check that history is scrubbed.
 send "$argv sql --certs-dir=$certs_dir\r"

pkg/security/auth.go

@@ -106,9 +106,14 @@ func UserAuthPasswordHook(insecureMode bool, password string, hashedPassword []b
 
 		// If the requested user has an empty password, disallow authentication.
 		if len(password) == 0 || CompareHashAndPassword(hashedPassword, password) != nil {
-			return errors.New("invalid password")
+			return errors.Errorf(ErrPasswordUserAuthFailed, requestedUser)
 		}
 
 		return nil
 	}
 }
+
+// ErrPasswordUserAuthFailed is the error template for failed password auth
+// of a user. It should be used when the password is incorrect or the user
+// does not exist.
+const ErrPasswordUserAuthFailed = "password authentication failed for user %s"

pkg/security/certs_test.go

@@ -423,7 +423,7 @@ func TestUseSplitCACerts(t *testing.T) {
 		expectedError            string
 	}{
 		// Success, but "node" is not a sql user.
-		{"node", security.EmbeddedCACert, "client.node", "pq: user node does not exist"},
+		{"node", security.EmbeddedCACert, "client.node", "pq: password authentication failed for user node"},
 		// Success!
 		{"root", security.EmbeddedCACert, "client.root", ""},
 		// Bad server CA: can't verify server certificate.
@@ -541,7 +541,7 @@ func TestUseWrongSplitCACerts(t *testing.T) {
 		// Certificate signed by wrong client CA.
 		{"root", security.EmbeddedCACert, "client.root", "tls: bad certificate"},
 		// Success! The node certificate still contains "CN=node" and is signed by ca.crt.
-		{"node", security.EmbeddedCACert, "node", "pq: user node does not exist"},
+		{"node", security.EmbeddedCACert, "node", "pq: password authentication failed for user node"},
 	}
 
 	for i, tc := range testCases {

pkg/sql/pgwire/conn.go

@@ -1221,7 +1221,6 @@ func (r *pgwireReader) ReadByte() (byte, error) {
 // point the sql.Session does not exist yet! If need exists to access the
 // database to look up authentication data, use the internal executor.
 func (c *conn) handleAuthentication(ctx context.Context, insecure bool) error {
-
 	sendError := func(err error) error {
 		_ /* err */ = writeErr(err, &c.msgBuilder, c.conn)
 		return err
@@ -1236,7 +1235,7 @@ func (c *conn) handleAuthentication(ctx context.Context, insecure bool) error {
 		return sendError(err)
 	}
 	if !exists {
-		return sendError(errors.Errorf("user %s does not exist", c.sessionArgs.User))
+		return sendError(errors.Errorf(security.ErrPasswordUserAuthFailed, c.sessionArgs.User))
 	}
 
 	if tlsConn, ok := c.conn.(*tls.Conn); ok {

pkg/sql/pgwire/pgwire_test.go

@@ -162,7 +162,7 @@ func TestPGWire(t *testing.T) {
 					if optUser == server.TestUser {
 						// The user TestUser has not been created so authentication
 						// will fail with a valid certificate.
-						if !testutils.IsError(err, fmt.Sprintf("pq: user %s does not exist", server.TestUser)) {
+						if !testutils.IsError(err, fmt.Sprintf("pq: password authentication failed for user %s", server.TestUser)) {
 							t.Errorf("unexpected error: %v", err)
 						}
 					} else {
@@ -199,7 +199,7 @@ func TestPGWireNonexistentUser(t *testing.T) {
 		}
 
 		err := trivialQuery(pgURL)
-		if !testutils.IsError(err, fmt.Sprintf("pq: user %s does not exist", server.TestUser)) {
+		if !testutils.IsError(err, fmt.Sprintf("pq: password authentication failed for user %s", server.TestUser)) {
 			t.Errorf("unexpected error: %v", err)
 		}
 	})
@@ -1849,7 +1849,7 @@ func TestPGWireAuth(t *testing.T) {
 				Host:     net.JoinHostPort(host, port),
 				RawQuery: "sslmode=require",
 			}
-			if err := trivialQuery(unicodeUserPgURL); !testutils.IsError(err, "pq: invalid password") {
+			if err := trivialQuery(unicodeUserPgURL); !testutils.IsError(err, "pq: password authentication failed for user") {
 				t.Fatalf("unexpected error: %v", err)
 			}
 
@@ -1883,7 +1883,7 @@ func TestPGWireAuth(t *testing.T) {
 		// Even though the correct password is supplied (empty string), this
 		// should fail because we do not support password authentication for
 		// users with empty passwords.
-		if err := trivialQuery(testUserPgURL); !testutils.IsError(err, "pq: invalid password") {
+		if err := trivialQuery(testUserPgURL); !testutils.IsError(err, "pq: password authentication failed for user") {
 			t.Fatalf("unexpected error: %v", err)
 		}
 	})