[BUG] Incorrect handling of DB errors

Versions

PGCat: 1.1.1
PostgreSQL: Postgres Pro 15 (similar to PostgreSQL 15)

Configurations

pgcat.toml

auth_query = "SELECT usename, passwd FROM public.pgcat_auth($1)"
auth_query_user = "pgcat_auth_user"
auth_query_password = ""

pg_hba.conf

hostssl all pgcat_auth_user 172.16.3.2/32 trust

Description

My pg_hba.conf file allows the pgcat_auth_user user to be allowed into the server unhindered, but only to execute the public.pgcat_auth function. But something goes wrong when PGCat does it.

I'm catching the next error:

pgcat-pgcat-1  | 2023-09-26T23:05:53.212508Z  WARN ThreadId(04) pgcat: Client disconnected with error
 ClientAuthPassthroughError("AuthPassthroughError(\"Error trying to obtain password from auth_query, ignoring hash for user 'xxxxxxxxxx'. Error: ServerStartupError(\\\"error message\\\", ServerIdentifier { username: \\\"pgcat_auth_user\\\", database: \\\"xxxxxx\\\" })\")", ClientIdentifier { application_name: "pgcat", username: "xxxxxxx", pool_name: "xxxxxxxx" })

This indicates that PGCat was unable to correctly read the error from the database server at this location:

pgcat/src/server.rs

Lines 716 to 726 in a054b45

    
           let mut error = vec![0u8; len as usize]; 
        
           match stream.read_exact(&mut error).await { 
        
               Ok(_) => (), 
        
               Err(_) => { 
        
                   return Err(Error::ServerStartupError( 
        
                       "error message".into(), 
        
                       server_identifier, 
        
                   )) 
        
               } 
        
           };

The solution is to subtract the number 5 from the len variable, so the errors are read correctly. Proofs:
https://user-images.githubusercontent.com/97305733/271855206-13c86dbe-f80d-4340-a189-9baafb1cab51.png

How to reproduce?

Provoke PostgreSQL to generate any protocol error. (at least in auth_query)

Auth query only works with md5 which was removed in Postgres 14.

Auth query only works with md5 which was removed in Postgres 14.

What can be done in such a case?

Only password auth through config is supported right now. If you want passthrough for Postgres 14+, we need to figure out if we can do passthrough with SCRAM or how to just store SCRAM strings in the config file instead of plain text passwords. Pgbouncer supports the latter, so we can take a look at their implementation for reference, but ideally, we figure out passthrough for SCRAM I think.

Only password auth through config is supported right now. If you want passthrough for Postgres 14+, we need to figure out if we can do passthrough with SCRAM or how to just store SCRAM strings in the config file instead of plain text passwords. Pgbouncer supports the latter, so we can take a look at their implementation for reference, but ideally, we figure out passthrough for SCRAM I think.

Thanks, I got it.
Even though I'm new to Rust, I'll think about how it can be done and if I come up with something, I'll be sure to push it to PR.

Reading the PostgreSQL protocol documentation, I was still puzzled why your code crashes with an error, the reason for which is the inability to read the response buffer from the database. And it's not even about SCRAM.

I've modified the source code a bit to see what error occurs and the reason why PGCat can't read it from the buffer:

In the original code, it happens here:

pgcat/src/server.rs

Lines 716 to 726 in a054b45

    
           let mut error = vec![0u8; len as usize]; 
        
           match stream.read_exact(&mut error).await { 
        
               Ok(_) => (), 
        
               Err(_) => { 
        
                   return Err(Error::ServerStartupError( 
        
                       "error message".into(), 
        
                       server_identifier, 
        
                   )) 
        
               } 
        
           };

This is happening because someone screwed up a little bit in that line:
https://github.com/postgresml/pgcat/blob/main/src/server.rs#L716

The solution is to subtract 5 bytes from the message length (len). (4 bytes are the message length + the length field itself, 1 byte is the message type).
Basically, you can see it in the screenshot.
Thus, the following should be the result:

let mut error = vec![0u8; len as usize - 4 - 1];

I'll look into this fix myself, and see what I can do with SCRAM authentication

As I expected, I now get an error message from the DB server.

Earlier, I was getting:

pgcat-pgcat-1  | 2023-09-26T23:05:53.212508Z  WARN ThreadId(04) pgcat: Client disconnected with error
 ClientAuthPassthroughError("AuthPassthroughError(\"Error trying to obtain password from auth_query, ignoring hash for user 'xxxxxxxxxx'. Error: ServerStartupError(\\\"error message\\\", ServerIdentifier { username: \\\"pgcat_auth_user\\\", database: \\\"xxxxxx\\\" })\")", ClientIdentifier { application_name: "pgcat", username: "xxxxxxx", pool_name: "xxxxxxxx" })

This was due to the end of the buffer being reached earlier than planned. I removed 5 bytes from the plan, now the buffer reads properly and I can get a normal error message.

@levkk , please give a comment

Seems reasonable. A PR is welcome, if you have a moment to provide a fix.

Seems reasonable. A PR is welcome, if you have a moment to provide a fix.

I will prepare a PR, but in the meantime please consider my other PR.

So far I'm about halfway to passthrough SCRAM-SHA-256 authentication. Looking at your code, I realize what nonsense it is. I really hope that you will not abandon your project, it really has a lot of room to grow!

Sounds good! The project is here to stay, thank you for your contributions!

	let mut error = vec![0u8; len as usize];

	match stream.read_exact(&mut error).await {
	Ok(_) => (),
	Err(_) => {
	return Err(Error::ServerStartupError(
	"error message".into(),
	server_identifier,
	))
	}
	};