Oops, I sent a wrong patch for ProcessFrontendResponse.
Correct one attached.
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
> While looking into pgpool source code for some reasons, I realize that
> we can enhance pgpool to implement stricter check for frontend message
> kinds. Please note that this is not a proposal, just a starting point
> of discussion. I feel there maybe a better idea for this.
>
> In ProcessFrontendResponse() we process messages from frontend in
> following order (I focus on the v3 protocol path here):
>
> 1. read message kind
> 2. read message content length
> 3. convert the result of 2 to host byte order and subtract 4 byte from
> it to get the actual message content length.
> 4. read message content using ppool_read2
>
> If the message from frontend is broken or out of sync, we could read
> garbage 4 bytes from frontend and treat it as message content
> length. If the message kind is 'Q' and the length is too short, the
> message is forwarded to PostgreSQL and PostgreSQL rejects the message
> with an error response. Since at this point the protocol is out of
> sync, subsequent message could generate random error including a hang
> in pgpool.
>
> I temporarily modified pgproto to allow to generate such a broken
> message content length, by adding new 'q' message kind (which is not
> defined in frontend/backend protocol). Following is the pgproto data.
> 'q' is similar to 'Q' (query message) but accepts message content
> lenngth + 4 in the next field to 'q'. The message is sent as 'Q'
> message.
>
> Following 'q' message supposes to send query string (9 bytes including
> NUL terminate) + 4 = 13 bytes message content length, but deliberately
> set it to 10.
> --------------------------------------------------------------------
> 'q' 10 "SELECT 1"
> 'Y'
> --------------------------------------------------------------------
>
> If I run this, I get this response.
> <= BE ErrorResponse(S ERROR V ERROR C 08P01 M invalid string in message F pqformat.c L 593 R pq_getmsgstring )
>
> This means pgpool forwarded the message to PostgreSQL and it raised an
> error.
>
> If the pgproto data includes following normal SELECT after 'q':
>
> 'Q' "SELECT 1"
> 'Y'
>
> pgpool is stuck in pool_read2, trying to read the message content
> which could be interpreted like length = 822104304 (in the example
> above).
>
> If I send the same data to directly PostgreSQL, I get following
> responses, resulted in client disconnection, instead of a hang.
>
> <= BE ErrorResponse(S ERROR V ERROR C 08P01 M invalid string in message F pqformat.c L 593 R pq_getmsgstring )
> <= BE ErrorResponse(S FATAL V FATAL C 08P01 M invalid frontend message type 32 F postgres.c L 454 R SocketBackend )
>
> This is because PostgreSQL checks the message kind and rejects the
> invalid message kind with a FATAL error. IMO, we should follow the
> PostgreSQL way so that we could avoid the hang in pool_read2 like
> this:
>
> FE=> Query (query="SELECT 1")
> <= BE ErrorResponse(S ERROR V ERROR C 08P01 M invalid string in message F pqformat.c L 593 R pq_getmsgstring )
> <= BE ReadyForQuery(I)
> FE=> Query (query="SELECT 1")
> <= BE ErrorResponse(S FATAL C XX000 M invalid frontend message type ' ' F pool_proto_modules.c L 2913 )
> read_it: read(2) returns error Connection reset by peer
>
> Patch attached.
> (Also pgproto patch attached).
>
> Regards,
> --
> Tatsuo Ishii
> SRA OSS K.K.
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
>
>