Skip to content

Commit b51678b

Browse files
committed
security: isolate pgwire connection failures
1 parent d545ea4 commit b51678b

91 files changed

Lines changed: 2766 additions & 1139 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

nodedb-cluster-tests/tests/transport_security.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ fn insecure_counter_guard() -> &'static Mutex<()> {
3838
LOCK.get_or_init(|| Mutex::new(()))
3939
}
4040

41+
use nodedb_cluster::topology::{ClusterTopology, NodeInfo, NodeState};
4142
use nodedb_cluster::transport::pki_types::{CertificateDer, PrivateKeyDer, PrivatePkcs8KeyDer};
4243
use nodedb_cluster::{
4344
MacKey, NexarTransport, RaftRpcHandler, TlsCredentials, TransportCredentials,
4445
generate_node_credentials, insecure_transport_count, spki_pin_from_cert_der,
4546
};
46-
use nodedb_cluster::topology::{ClusterTopology, NodeInfo, NodeState};
4747
use nodedb_raft::message::{AppendEntriesRequest, AppendEntriesResponse, RequestVoteResponse};
4848
use nodedb_raft::transport::RaftTransport;
4949

nodedb/src/bootstrap/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ pub mod credentials;
77
pub mod data_group_recovery;
88
pub mod data_plane;
99
pub mod listeners;
10+
pub mod panic_hook;
1011
pub mod schema_rehydrate;
1112
pub mod signal;
1213
pub mod state_wiring;

nodedb/src/bootstrap/panic_hook.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
3+
//! Process-wide panic reporting that never renders application panic payloads.
4+
5+
use std::sync::OnceLock;
6+
7+
static PANIC_HOOK: OnceLock<()> = OnceLock::new();
8+
const PANIC_DIAGNOSTIC: &[u8] = b"nodedb: process panic intercepted\n";
9+
10+
#[cfg(unix)]
11+
fn report_panic() {
12+
// SAFETY: the byte slice is valid for the duration of the call and
13+
// `STDERR_FILENO` is the conventional process stderr descriptor. `write`
14+
// is allocation-free; failures and partial writes are intentionally
15+
// ignored because panic reporting must never trigger another panic.
16+
let _ = unsafe {
17+
libc::write(
18+
libc::STDERR_FILENO,
19+
PANIC_DIAGNOSTIC.as_ptr().cast(),
20+
PANIC_DIAGNOSTIC.len(),
21+
)
22+
};
23+
}
24+
25+
#[cfg(not(unix))]
26+
fn report_panic() {
27+
use std::io::Write as _;
28+
29+
// The portable fallback performs no formatting and ignores every I/O
30+
// failure. NodeDB's supported production targets use the Unix path above.
31+
let _ = std::io::stderr().write_all(PANIC_DIAGNOSTIC);
32+
}
33+
34+
/// Install the payload-redacting process panic hook exactly once.
35+
///
36+
/// The hook intentionally neither chains the default hook nor examines the
37+
/// panic payload or source location. Connection-level boundaries handle
38+
/// expected wire panics; this fixed diagnostic is the last-resort report for
39+
/// every other task.
40+
pub fn install() {
41+
PANIC_HOOK.get_or_init(|| {
42+
std::panic::set_hook(Box::new(|_| report_panic()));
43+
});
44+
}
45+
46+
#[cfg(test)]
47+
mod tests {
48+
use super::*;
49+
50+
#[test]
51+
fn local_install_state_is_idempotent_without_replacing_global_hook() {
52+
let state = OnceLock::new();
53+
assert!(state.set(()).is_ok());
54+
assert!(state.set(()).is_err());
55+
}
56+
57+
#[test]
58+
fn diagnostic_is_fixed_and_payload_free() {
59+
assert_eq!(PANIC_DIAGNOSTIC, b"nodedb: process panic intercepted\n");
60+
assert!(
61+
!PANIC_DIAGNOSTIC
62+
.windows(b"secret panic payload".len())
63+
.any(|window| window == b"secret panic payload")
64+
);
65+
}
66+
}

nodedb/src/control/notify_bus.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,11 @@ pub fn normalize_channel(channel: &str) -> String {
247247

248248
/// Handle for a session's LISTEN subscriptions.
249249
///
250-
/// Stores `(channel, session_id, receiver)` triples so the session can
251-
/// drain notifications and clean up on disconnect.
250+
/// Stores the immutable subscription tenant together with `(channel,
251+
/// session_id, receiver)` so disconnect cleanup cannot be redirected by a
252+
/// later session-identity change.
252253
pub struct ListenHandle {
254+
pub tenant_id: TenantId,
253255
pub channel: String,
254256
pub session_id: u64,
255257
pub rx: tokio::sync::mpsc::Receiver<Notification>,

nodedb/src/control/server/native/dispatch/direct_ops.rs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -354,15 +354,21 @@ async fn dispatch_single_task(
354354
// response the same way the non-staged branch below shapes it.
355355
let plan_for_staged_response = task.plan.clone();
356356

357-
let task = match route_in_tx_write(ctx.state, ctx.sessions, ctx.peer_addr, task, |stage_task| {
358-
dispatch_authorized_single_task(
359-
ctx,
360-
stage_task.tenant_id,
361-
stage_task.vshard_id,
362-
stage_task.plan,
363-
stage_task.txn_id,
364-
)
365-
})
357+
let task = match route_in_tx_write(
358+
ctx.state,
359+
ctx.sessions,
360+
ctx.peer_addr.into(),
361+
task,
362+
|stage_task| {
363+
dispatch_authorized_single_task(
364+
ctx,
365+
stage_task.tenant_id,
366+
stage_task.vshard_id,
367+
stage_task.plan,
368+
stage_task.txn_id,
369+
)
370+
},
371+
)
366372
.await
367373
{
368374
Ok(InTxnRoute::Read(routed_task)) => *routed_task,
@@ -420,7 +426,7 @@ async fn dispatch_single_task(
420426
crate::control::server::shared::session::record_reads_for_response(
421427
ctx.state,
422428
ctx.sessions,
423-
ctx.peer_addr,
429+
ctx.peer_addr.into(),
424430
ctx.tenant_id(),
425431
crate::control::server::shared::session::ResponseReads {
426432
plan: &plan_for_response,

nodedb/src/control/server/native/dispatch/graph_match.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ pub(crate) async fn handle_graph_match(
7878
crate::control::server::shared::session::record_reads_for_response(
7979
ctx.state,
8080
ctx.sessions,
81-
ctx.peer_addr,
81+
ctx.peer_addr.into(),
8282
ctx.tenant_id(),
8383
crate::control::server::shared::session::ResponseReads {
8484
plan: &plan_for_response,

nodedb/src/control/server/native/dispatch/sql.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ async fn handle_sql_inner(
146146
// DDL: try DDL router first.
147147
let txn_ctx = crate::control::server::shared::session::DmlTxnCtx {
148148
sessions: ctx.sessions,
149-
addr: ctx.peer_addr,
149+
session_id: ctx.peer_addr.into(),
150150
};
151151
if let Some(result) = crate::control::server::shared::ddl::dispatch(
152152
ctx.state,

nodedb/src/control/server/native/dispatch/sql_loop.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ pub(super) async fn run_dispatch_loop(
8787
let routed = match route_in_tx_expander(
8888
ctx.state,
8989
ctx.sessions,
90-
ctx.peer_addr,
90+
ctx.peer_addr.into(),
9191
task,
9292
|stage_task| async move {
9393
dispatch_task(ctx, stage_task)
@@ -102,7 +102,7 @@ pub(super) async fn run_dispatch_loop(
102102
route_in_tx_write(
103103
ctx.state,
104104
ctx.sessions,
105-
ctx.peer_addr,
105+
ctx.peer_addr.into(),
106106
*task,
107107
|stage_task| async move {
108108
dispatch_task(ctx, stage_task)
@@ -192,7 +192,7 @@ pub(super) async fn run_dispatch_loop(
192192
crate::control::server::shared::session::record_reads_for_response(
193193
ctx.state,
194194
ctx.sessions,
195-
ctx.peer_addr,
195+
ctx.peer_addr.into(),
196196
ctx.tenant_id(),
197197
crate::control::server::shared::session::ResponseReads {
198198
plan: &plan_for_response,

nodedb/src/control/server/native/dispatch/transaction.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ pub(crate) fn handle_begin(ctx: &DispatchCtx<'_>, seq: u64) -> NativeResponse {
7474
// ensure the session exists first (SQL "BEGIN" already does this in
7575
// `sql.rs` before calling here).
7676
ctx.sessions.ensure_session(*ctx.peer_addr);
77-
match lifecycle::run_begin(ctx.sessions, ctx.peer_addr, ctx.state) {
77+
match lifecycle::run_begin(ctx.sessions, ctx.peer_addr.into(), ctx.state) {
7878
Ok(()) => NativeResponse::status_row(seq, "BEGIN"),
7979
Err(e) => {
8080
let message = match &e {
@@ -88,15 +88,30 @@ pub(crate) fn handle_begin(ctx: &DispatchCtx<'_>, seq: u64) -> NativeResponse {
8888

8989
pub(crate) async fn handle_commit(ctx: &DispatchCtx<'_>, seq: u64) -> NativeResponse {
9090
let dp = NativeTxnDp { state: ctx.state };
91-
match commit::run_commit(ctx.sessions, ctx.peer_addr, ctx.identity, ctx.state, &dp).await {
91+
match commit::run_commit(
92+
ctx.sessions,
93+
ctx.peer_addr.into(),
94+
ctx.identity,
95+
ctx.state,
96+
&dp,
97+
)
98+
.await
99+
{
92100
CommitOutcome::Committed => NativeResponse::status_row(seq, "COMMIT"),
93101
CommitOutcome::Aborted { reason } => commit_abort_to_native(seq, &reason),
94102
}
95103
}
96104

97105
pub(crate) async fn handle_rollback(ctx: &DispatchCtx<'_>, seq: u64) -> NativeResponse {
98106
let dp = NativeTxnDp { state: ctx.state };
99-
lifecycle::run_rollback(ctx.sessions, ctx.peer_addr, ctx.identity, ctx.state, &dp).await;
107+
lifecycle::run_rollback(
108+
ctx.sessions,
109+
ctx.peer_addr.into(),
110+
ctx.identity,
111+
ctx.state,
112+
&dp,
113+
)
114+
.await;
100115
NativeResponse::status_row(seq, "ROLLBACK")
101116
}
102117

nodedb/src/control/server/native/dispatch/transaction_savepoint.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,14 @@ pub(crate) async fn handle_savepoint(
3838
) -> NativeResponse {
3939
let sp_name = sql_trimmed.split_whitespace().nth(1).unwrap_or("sp");
4040
let dp = NativeTxnDp { state: ctx.state };
41-
match savepoint_ops::run_savepoint(ctx.sessions, ctx.peer_addr, ctx.tenant_id(), &dp, sp_name)
42-
.await
41+
match savepoint_ops::run_savepoint(
42+
ctx.sessions,
43+
ctx.peer_addr.into(),
44+
ctx.tenant_id(),
45+
&dp,
46+
sp_name,
47+
)
48+
.await
4349
{
4450
Ok(()) => NativeResponse::status_row(seq, "SAVEPOINT"),
4551
Err(e) => savepoint_error_to_native(seq, &e),
@@ -53,7 +59,7 @@ pub(crate) fn handle_release_savepoint(
5359
sql_trimmed: &str,
5460
) -> NativeResponse {
5561
let sp_name = sql_trimmed.split_whitespace().last().unwrap_or("sp");
56-
match savepoint_ops::run_release_savepoint(ctx.sessions, ctx.peer_addr, sp_name) {
62+
match savepoint_ops::run_release_savepoint(ctx.sessions, ctx.peer_addr.into(), sp_name) {
5763
Ok(()) => NativeResponse::status_row(seq, "RELEASE"),
5864
Err(e) => savepoint_error_to_native(seq, &e),
5965
}
@@ -69,7 +75,7 @@ pub(crate) async fn handle_rollback_to_savepoint(
6975
let dp = NativeTxnDp { state: ctx.state };
7076
match savepoint_ops::run_rollback_to_savepoint(
7177
ctx.sessions,
72-
ctx.peer_addr,
78+
ctx.peer_addr.into(),
7379
ctx.tenant_id(),
7480
&dp,
7581
sp_name,

0 commit comments

Comments
 (0)