Skip to content

Commit

Permalink
Add patch and test job for starvation on bigger SSL records
Browse files Browse the repository at this point in the history
  • Loading branch information
larskanis committed Nov 28, 2024
1 parent c6d1047 commit e90c91d
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 0 deletions.
9 changes: 9 additions & 0 deletions misc/yugabyte/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM yugabytedb/yugabyte:2024.1.0.0-b129

WORKDIR /app

RUN yugabyted cert generate_server_certs --hostnames=127.0.0.1 --base_dir=.

ENTRYPOINT ["yugabyted"]
CMD ["start", "--background", "false", "--ui", "false", "--tserver_flags", "use_client_to_server_encryption=true,cert_node_filename=127.0.0.1,certs_dir=/app/generated_certs/127.0.0.1"]
VOLUME /app
45 changes: 45 additions & 0 deletions misc/yugabyte/pg-test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
require 'pg'

conn = PG.connect(
host: 'localhost',
port: 5433,
user: 'yugabyte',
dbname: 'yugabyte',
sslmode: 'require',
sslrootcert: 'app/generated_certs/127.0.0.1/ca.crt',
sslcert: 'app/generated_certs/127.0.0.1/node.127.0.0.1.crt',
sslkey: 'app/generated_certs/127.0.0.1/node.127.0.0.1.key'
)

$stdout.sync = true
# fd = File.open("pg_trace.log", "a+")
# conn.trace(fd)

begin
# Validate connection is working
res = conn.exec("SELECT version();")
res.each_row do |row|
puts "You are connected to: #{row[0]}"
end
# 53*511
# 53*767
# 53*1023
# 53*1279
# 7*1817
# 11*1487
# 13*1363
# 16*1211
# 18*1128
# 22*1984
# 27*1723

(22..53).each do |m|
(1..2048).each do |l|
hanging_query = "SELECT lpad(''::text, #{m}, '0') FROM generate_series(1, #{l});"
puts "Executing hanging query: #{hanging_query}"
conn.exec(hanging_query)
end
end
ensure
conn.close if conn
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
From ab793829a4ce473f1cc2bbc0e2a6f6753553255d Mon Sep 17 00:00:00 2001
From: Lars Kanis <lars@greiz-reinsdorf.de>
Date: Sun, 8 Sep 2024 13:59:05 +0200
Subject: [PATCH] libpq: Process buffered SSL read bytes to support records
>8kB on async API

The async API of libpq doesn't support SSL record sizes >8kB so far.
This size isn't exceeded by vanilla PostgreSQL, but by other products using
the postgres wire protocol 3.
PQconsumeInput() reads all data readable from the socket, so that the read
condition is cleared.
But it doesn't process all the data that is pending on the SSL layer.
Also a subsequent call to PQisBusy() doesn't process it, so that the client
is triggered to wait for more readable data on the socket.
But this never arrives, so that the connection blocks infinitely.

To fix this issue call pqReadData() repeatedly until there is no buffered
SSL data left to be read.

The synchronous libpq API isn't affected, since it supports arbitrary SSL
record sizes already.
---
src/interfaces/libpq/fe-exec.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 0d224a852..637894ee1 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -2006,6 +2006,19 @@ PQconsumeInput(PGconn *conn)
if (pqReadData(conn) < 0)
return 0;

+ #ifdef USE_SSL
+ /*
+ * Ensure all buffered read bytes in the SSL library are processed,
+ * which might be not the case, if the SSL record size exceeds 8k.
+ * Otherwise parseInput can't process the data.
+ */
+ while (conn->ssl_in_use && pgtls_read_pending(conn))
+ {
+ if (pqReadData(conn) < 0)
+ return 0;
+ }
+ #endif
+
/* Parsing of the data waits till later. */
return 1;
}
--
2.43.0

0 comments on commit e90c91d

Please sign in to comment.