From d350a0ad03678c4f860f33f5024910551d05b2f8 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 10 Nov 2026 21:21:05 -0800 Subject: [PATCH] weed-db: end startup when password authentication fails (#12550) * weed-db: end startup when password authentication fails The failure branches returned the result of writing the error response, which is nil on success, so the handshake carried on to ReadyForQuery. Co-Authored-By: Claude Opus 5.5 * weed-db: test startup with wrong and unknown credentials Co-Authored-By: Claude Opus 5.5 * weed-db: check the auth error code and fail on stray read errors in the startup test Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- weed/server/postgres/server.go | 11 ++- weed/server/postgres/server_test.go | 141 ++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 weed/server/postgres/server_test.go diff --git a/weed/server/postgres/server.go b/weed/server/postgres/server.go index 8cb34f2888..16f99a83f4 100644 --- a/weed/server/postgres/server.go +++ b/weed/server/postgres/server.go @@ -585,12 +585,17 @@ func (s *PostgreSQLServer) handlePasswordAuth(session *PostgreSQLSession) error expectedPassword, exists := s.config.Users[session.username] passwordStr := strings.TrimSuffix(string(password), "\x00") if !exists || passwordStr != expectedPassword { - return s.sendError(session, "28P01", "authentication failed for user \""+session.username+"\"") + return s.rejectAuthentication(session) } return s.sendAuthenticationOk(session) } +func (s *PostgreSQLServer) rejectAuthentication(session *PostgreSQLSession) error { + s.sendError(session, "28P01", "authentication failed for user \""+session.username+"\"") + return fmt.Errorf("authentication failed for user %q", session.username) +} + // handleMD5Auth handles MD5 password authentication func (s *PostgreSQLServer) handleMD5Auth(session *PostgreSQLSession) error { // Generate salt @@ -650,7 +655,7 @@ func (s *PostgreSQLServer) handleMD5Auth(session *PostgreSQLSession) error { // Verify MD5 hash expectedPassword, exists := s.config.Users[session.username] if !exists { - return s.sendError(session, "28P01", "authentication failed for user \""+session.username+"\"") + return s.rejectAuthentication(session) } // Calculate expected hash: md5(md5(password + username) + salt) @@ -658,7 +663,7 @@ func (s *PostgreSQLServer) handleMD5Auth(session *PostgreSQLSession) error { expected := fmt.Sprintf("md5%x", md5.Sum(append([]byte(fmt.Sprintf("%x", inner)), salt...))) if strings.TrimSuffix(string(response), "\x00") != expected { - return s.sendError(session, "28P01", "authentication failed for user \""+session.username+"\"") + return s.rejectAuthentication(session) } return s.sendAuthenticationOk(session) diff --git a/weed/server/postgres/server_test.go b/weed/server/postgres/server_test.go new file mode 100644 index 0000000000..eb1bd81305 --- /dev/null +++ b/weed/server/postgres/server_test.go @@ -0,0 +1,141 @@ +package postgres + +import ( + "bufio" + "crypto/md5" + "encoding/binary" + "fmt" + "io" + "net" + "strings" + "testing" + "time" +) + +func writePgMessage(conn net.Conn, msgType byte, payload []byte) error { + var buf []byte + if msgType != 0 { + buf = append(buf, msgType) + } + length := make([]byte, 4) + binary.BigEndian.PutUint32(length, uint32(len(payload)+4)) + buf = append(buf, length...) + buf = append(buf, payload...) + _, err := conn.Write(buf) + return err +} + +func readPgMessage(r *bufio.Reader) (byte, []byte, error) { + msgType, err := r.ReadByte() + if err != nil { + return 0, nil, err + } + length := make([]byte, 4) + if _, err := io.ReadFull(r, length); err != nil { + return 0, nil, err + } + body := make([]byte, binary.BigEndian.Uint32(length)-4) + if _, err := io.ReadFull(r, body); err != nil { + return 0, nil, err + } + return msgType, body, nil +} + +func runStartup(t *testing.T, method AuthMethod, user, password string) ([]byte, error) { + t.Helper() + s := &PostgreSQLServer{config: &PostgreSQLServerConfig{ + AuthMethod: method, + Users: map[string]string{"admin": "secret"}, + StartupTimeout: 5 * time.Second, + Database: "default", + }} + serverConn, clientConn := net.Pipe() + defer clientConn.Close() + session := &PostgreSQLSession{ + conn: serverConn, + reader: bufio.NewReader(serverConn), + writer: bufio.NewWriter(serverConn), + parameters: make(map[string]string), + } + + startupErr := make(chan error, 1) + go func() { + startupErr <- s.handleStartup(session) + serverConn.Close() + }() + + clientConn.SetDeadline(time.Now().Add(5 * time.Second)) + startup := make([]byte, 4) + binary.BigEndian.PutUint32(startup, PG_PROTOCOL_VERSION_3) + startup = append(startup, []byte("user\x00"+user+"\x00database\x00default\x00\x00")...) + if err := writePgMessage(clientConn, 0, startup); err != nil { + t.Fatalf("write startup: %v", err) + } + + reader := bufio.NewReader(clientConn) + msgType, body, err := readPgMessage(reader) + if err != nil || msgType != PG_RESP_AUTH_OK || len(body) < 4 { + t.Fatalf("expected auth request, got %q %v", msgType, err) + } + response := password + if method == AuthMD5 { + inner := md5.Sum([]byte(password + user)) + response = fmt.Sprintf("md5%x", md5.Sum(append([]byte(fmt.Sprintf("%x", inner)), body[4:8]...))) + } + if err := writePgMessage(clientConn, PG_MSG_PASSWORD, []byte(response+"\x00")); err != nil { + t.Fatalf("write password: %v", err) + } + + var received []byte + for { + msgType, body, err := readPgMessage(reader) + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("read after %q: %v", received, err) + } + if msgType == PG_RESP_ERROR && !strings.Contains(string(body), "\x00C28P01\x00") { + t.Fatalf("expected SQLSTATE 28P01, got %q", body) + } + received = append(received, msgType) + } + return received, <-startupErr +} + +func TestStartupRejectsFailedAuthentication(t *testing.T) { + cases := []struct { + name string + method AuthMethod + user string + password string + }{ + {"password wrong", AuthPassword, "admin", "wrong"}, + {"password unknown user", AuthPassword, "nobody", "secret"}, + {"md5 wrong", AuthMD5, "admin", "wrong"}, + {"md5 unknown user", AuthMD5, "nobody", "secret"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + received, err := runStartup(t, tc.method, tc.user, tc.password) + if err == nil { + t.Fatalf("startup succeeded after failed authentication, server sent %q", received) + } + if string(received) != string([]byte{PG_RESP_ERROR}) { + t.Fatalf("expected only an error response, got %q", received) + } + }) + } +} + +func TestStartupAcceptsValidPassword(t *testing.T) { + for _, method := range []AuthMethod{AuthPassword, AuthMD5} { + received, err := runStartup(t, method, "admin", "secret") + if err != nil { + t.Fatalf("method %d: startup failed: %v", method, err) + } + if len(received) == 0 || received[len(received)-1] != PG_RESP_READY { + t.Fatalf("method %d: expected ReadyForQuery last, got %q", method, received) + } + } +}