Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add patch and test job for starvation on bigger SSL records #616

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/binary-gems.yml
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,26 @@ jobs:
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
docker build --rm --platform ${{matrix.image_platform}} --build-arg from_image=${{matrix.from_image}} -t ruby-test -f spec/env/Dockerfile.${{matrix.dockerfile}} .
docker run --rm -t --network=host -v `pwd`:/build ruby-test

job_binary_yugabyte:
name: yugabyte (${{matrix.gem_platform}}
needs: rcd_build
strategy:
fail-fast: false
matrix:
include:
- gem_platform: x86_64-linux

runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Download gem-${{ matrix.gem_platform }}
uses: actions/download-artifact@v4
with:
name: binary-gem-${{ matrix.gem_platform }}
- name: Build image and Run tests
run: |
sudo apt-get install -y docker-compose
cp -v pg-*.gem misc/yugabyte/
cd misc/yugabyte
docker-compose up --abort-on-container-exit --exit-code-from pg
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
28 changes: 28 additions & 0 deletions misc/yugabyte/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
services:
yb:
build: .
container_name: yb
ports:
- "127.0.0.1:5433:5433"
volumes:
- certs:/app/generated_certs
healthcheck:
test: 'ysqlsh -h $$(hostname) -c \\conninfo || exit 1;'
interval: 2s
timeout: 30s
retries: 20
start_period: 10s

pg:
image: ruby:3.0
working_dir: /app
volumes:
- .:/app
- certs:/generated_certs
command: bash -c "gem inst pg-*.gem && ruby pg-test.rb"
depends_on:
yb:
condition: service_healthy

volumes:
certs:
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: 'yb',
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 <[email protected]>
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