Skip to content

Commit c7eb6d3

Browse files
authored
fix: handle connection's ssl check on init (#15)
1 parent caf44f6 commit c7eb6d3

4 files changed

Lines changed: 101 additions & 5 deletions

File tree

src/intercept.rs

Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,23 @@ fn single_text_response(column_name: &str, value: &str) -> PgWireResult<Vec<Resp
2424
))])
2525
}
2626

27+
/// Build a single-column, single-row BOOL text response (`t`/`f` on the wire).
28+
/// Used for the `pg_stat_ssl` probe, whose `ssl` column is a boolean; a
29+
/// VARCHAR response would carry the wrong type oid in the RowDescription and
30+
/// Npgsql decodes the column strictly by that oid.
31+
fn single_bool_response(column_name: &str, value: bool) -> PgWireResult<Vec<Response>> {
32+
let fields = Arc::new(vec![text_field(column_name, Type::BOOL)]);
33+
34+
let mut encoder = DataRowEncoder::new(Arc::clone(&fields));
35+
encoder.encode_field(&value)?;
36+
let row = encoder.take_row();
37+
38+
Ok(vec![Response::Query(QueryResponse::new(
39+
fields,
40+
stream::iter(vec![Ok(row)]),
41+
))])
42+
}
43+
2744
/// If the query can be answered locally instead of forwarded to Trino,
2845
/// build the response and return it. Returns `None` for queries that
2946
/// should pass through to Trino.
@@ -37,11 +54,15 @@ fn single_text_response(column_name: &str, value: &str) -> PgWireResult<Vec<Resp
3754
/// `calls_function`) handle the cases where prefix matching would
3855
/// misroute a user query that *contains* a catalog name as a literal or
3956
/// column reference.
57+
///
58+
/// `client_is_secure` is the TLS state of the PG client connection, used to
59+
/// answer the `pg_stat_ssl` probe truthfully (see below).
4060
pub fn try_intercept(
4161
query: &str,
4262
parsed_query: &ParsedQuery,
4363
catalog: &str,
4464
schema: &str,
65+
client_is_secure: bool,
4566
) -> Option<PgWireResult<Vec<Response>>> {
4667
let trimmed = query.trim();
4768
if trimmed.is_empty() {
@@ -95,6 +116,17 @@ pub fn try_intercept(
95116
return Some(resp);
96117
}
97118

119+
// Npgsql (Power BI) and pgjdbc probe the connection's encryption state
120+
// right after connecting with `SELECT ssl FROM pg_stat_ssl WHERE pid =
121+
// pg_backend_pid()`. Trino has no `pg_stat_ssl`, so forwarding it aborts
122+
// the session before the client runs a single user query. Answer locally
123+
// with the real TLS state of this connection; the `WHERE pid = ...` clause
124+
// is moot because a backend only ever sees its own row in pg_stat_ssl.
125+
if parsed_query.references_table("pg_stat_ssl") {
126+
tracing::trace!(query = trimmed, "Intercept: pg_stat_ssl");
127+
return Some(single_bool_response("ssl", client_is_secure));
128+
}
129+
98130
if let Some(resp) = crate::catalog::handle_catalog_query(parsed_query) {
99131
tracing::trace!(query = trimmed, "Intercept: pg_catalog");
100132
return Some(resp);
@@ -188,15 +220,15 @@ mod tests {
188220
fn assert_intercepted(query: &str) {
189221
let parsed_query = ParsedQuery::new(query);
190222
assert!(
191-
try_intercept(query, &parsed_query, "test_catalog", "test_schema").is_some(),
223+
try_intercept(query, &parsed_query, "test_catalog", "test_schema", false).is_some(),
192224
"expected query to be intercepted: {query}"
193225
);
194226
}
195227

196228
fn assert_not_intercepted(query: &str) {
197229
let parsed_query = ParsedQuery::new(query);
198230
assert!(
199-
try_intercept(query, &parsed_query, "test_catalog", "test_schema").is_none(),
231+
try_intercept(query, &parsed_query, "test_catalog", "test_schema", false).is_none(),
200232
"expected query to NOT be intercepted: {query}"
201233
);
202234
}
@@ -246,7 +278,7 @@ mod tests {
246278

247279
for &(query, expected) in cases {
248280
let parsed_query = ParsedQuery::new(query);
249-
let result = try_intercept(query, &parsed_query, "test_catalog", "test_schema")
281+
let result = try_intercept(query, &parsed_query, "test_catalog", "test_schema", false)
250282
.unwrap_or_else(|| panic!("SHOW not intercepted: {query}"));
251283
assert!(result.is_ok(), "SHOW returned error for: {query}");
252284

@@ -280,6 +312,45 @@ mod tests {
280312
assert_intercepted("SELECT current_setting('server_version')");
281313
}
282314

315+
/// The Npgsql/pgjdbc SSL probe is answered locally instead of forwarded
316+
/// to Trino (which has no `pg_stat_ssl` and would abort the connection).
317+
#[test]
318+
fn pg_stat_ssl_probe_intercepted() {
319+
assert_intercepted("SELECT ssl FROM pg_stat_ssl WHERE pid = pg_backend_pid()");
320+
assert_intercepted("select ssl from pg_stat_ssl");
321+
}
322+
323+
/// The `ssl` column reports the connection's real TLS state and is typed
324+
/// BOOL (not VARCHAR) so the RowDescription oid matches what the driver
325+
/// decodes.
326+
#[test]
327+
fn pg_stat_ssl_reports_tls_state_as_bool() {
328+
let query = "SELECT ssl FROM pg_stat_ssl WHERE pid = pg_backend_pid()";
329+
let parsed_query = ParsedQuery::new(query);
330+
331+
for is_secure in [true, false] {
332+
let resp = try_intercept(query, &parsed_query, "cat", "sch", is_secure)
333+
.expect("pg_stat_ssl must be intercepted")
334+
.expect("intercept must not error");
335+
assert_eq!(resp.len(), 1);
336+
match &resp[0] {
337+
Response::Query(qr) => {
338+
assert_eq!(qr.row_schema.len(), 1);
339+
assert_eq!(qr.row_schema[0].name(), "ssl");
340+
assert_eq!(qr.row_schema[0].datatype(), &Type::BOOL);
341+
}
342+
other => panic!("expected Query response, got: {other:?}"),
343+
}
344+
}
345+
}
346+
347+
/// Regression: a user table literally named `pg_stat_ssl` in a literal
348+
/// must not trip the probe intercept.
349+
#[test]
350+
fn pg_stat_ssl_in_literal_not_intercepted() {
351+
assert_not_intercepted("SELECT 'pg_stat_ssl' AS sentinel");
352+
}
353+
283354
#[test]
284355
fn regular_queries_not_intercepted() {
285356
assert_not_intercepted("SELECT 1");

src/query_extended.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
9191
let query = &portal.statement.statement;
9292
tracing::debug!(query, "Extended query execute");
9393

94+
let client_is_secure = client.is_secure();
95+
9496
let conn_state = client
9597
.session_extensions()
9698
.get::<ConnectionState>()
@@ -118,6 +120,7 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
118120
&conn_state.config,
119121
Some(&conn_state.active_query_id),
120122
Some(&portal.result_column_format),
123+
client_is_secure,
121124
)
122125
.await?;
123126
let response = responses
@@ -182,6 +185,8 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
182185
return Ok(DescribeStatementResponse::no_data());
183186
}
184187

188+
let client_is_secure = client.is_secure();
189+
185190
let conn_state = client
186191
.session_extensions()
187192
.get::<ConnectionState>()
@@ -201,6 +206,7 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
201206
&conn_state.config,
202207
Some(&conn_state.active_query_id),
203208
None,
209+
client_is_secure,
204210
)
205211
.await?;
206212
let response = responses
@@ -230,6 +236,8 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
230236
let query = &portal.statement.statement;
231237
tracing::debug!(query, "Extended query describe portal");
232238

239+
let client_is_secure = client.is_secure();
240+
233241
let conn_state = client
234242
.session_extensions()
235243
.get::<ConnectionState>()
@@ -241,6 +249,7 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
241249
&conn_state.config,
242250
Some(&conn_state.active_query_id),
243251
Some(&portal.result_column_format),
252+
client_is_secure,
244253
)
245254
.await?;
246255
let response = responses

src/query_pipeline.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ pub(crate) async fn process_query(
4949
config: &Arc<Config>,
5050
active_query_id: Option<&ActiveQueryId>,
5151
result_format: Option<&Format>,
52+
client_is_secure: bool,
5253
) -> PgWireResult<Vec<Response>> {
5354
tracing::trace!(query, "Pipeline: enter");
5455

@@ -60,15 +61,23 @@ pub(crate) async fn process_query(
6061
config,
6162
active_query_id,
6263
result_format,
64+
client_is_secure,
6365
)
6466
.await;
6567
}
6668

6769
tracing::trace!(count = pieces.len(), "Pipeline: multi-statement input");
6870
let mut out = Vec::with_capacity(pieces.len());
6971
for stmt in &pieces {
70-
match process_single_statement(stmt, trino_client, config, active_query_id, result_format)
71-
.await
72+
match process_single_statement(
73+
stmt,
74+
trino_client,
75+
config,
76+
active_query_id,
77+
result_format,
78+
client_is_secure,
79+
)
80+
.await
7281
{
7382
Ok(mut responses) => out.append(&mut responses),
7483
// User-visible errors (e.g. a Trino syntax error on statement N
@@ -107,6 +116,7 @@ async fn process_single_statement(
107116
config: &Arc<Config>,
108117
active_query_id: Option<&ActiveQueryId>,
109118
result_format: Option<&Format>,
119+
client_is_secure: bool,
110120
) -> PgWireResult<Vec<Response>> {
111121
// The query is parsed up to three times: once here (for routing
112122
// checks), once by the multi-statement splitter in the public
@@ -122,6 +132,7 @@ async fn process_single_statement(
122132
&parsed_query,
123133
&config.trino_catalog,
124134
&config.trino_schema,
135+
client_is_secure,
125136
) {
126137
tracing::trace!("Pipeline: static intercept matched");
127138
return result;

src/query_simple.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ impl SimpleQueryHandler for GatewayQueryHandler {
2626
{
2727
tracing::debug!(query, "Simple query received");
2828

29+
// Captured before borrowing session state, so the `pg_stat_ssl`
30+
// intercept can report this connection's real TLS state.
31+
let client_is_secure = client.is_secure();
32+
2933
let conn_state = client
3034
.session_extensions()
3135
.get::<ConnectionState>()
@@ -39,6 +43,7 @@ impl SimpleQueryHandler for GatewayQueryHandler {
3943
&conn_state.config,
4044
Some(&conn_state.active_query_id),
4145
None,
46+
client_is_secure,
4247
)
4348
.await;
4449
match &result {

0 commit comments

Comments
 (0)