Skip to content

Commit 3358ae8

Browse files
committed
security: complete authorization denial matrix
1 parent 9e9200a commit 3358ae8

18 files changed

Lines changed: 1963 additions & 108 deletions

File tree

nodedb/src/control/security/predicate_parser.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,7 @@ pub fn validate_auth_refs(predicate: &RlsPredicate) -> crate::Result<()> {
372372
"auth_method",
373373
"auth_time",
374374
"session_id",
375+
"database_id",
375376
];
376377

377378
fn check(pred: &RlsPredicate, known: &[&str]) -> crate::Result<()> {

nodedb/src/control/server/http/routes/crdt.rs

Lines changed: 116 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,15 @@ use axum::http::HeaderMap;
1111
use axum::response::IntoResponse;
1212

1313
use crate::bridge::envelope::PhysicalPlan;
14-
use crate::control::security::audit::ArcAuditEmitter;
15-
use crate::control::security::identity::Permission;
14+
use crate::control::security::audit::{ArcAuditEmitter, AuditEmitter};
15+
use crate::control::security::identity::{AuthenticatedIdentity, Permission};
16+
use crate::control::security::permission::PermissionStore;
17+
use crate::control::security::role::RoleStore;
1618
use crate::control::server::http::auth::{ApiError, AppState, resolve_identity};
1719
use crate::control::server::http::types::{HttpCrdtApplyRequest, HttpCrdtApplyResponse};
18-
use crate::control::server::shared::authorization::{authorize_collection, authorize_task_set};
20+
use crate::control::server::shared::authorization::{
21+
AuthorizationError, authorize_collection, authorize_task_set,
22+
};
1923
use crate::control::server::shared::ddl::sql_parse::hex_decode;
2024
use nodedb_physical::physical_plan::CrdtOp;
2125
use nodedb_physical::physical_task::{PhysicalTask, PostSetOp};
@@ -45,11 +49,9 @@ pub async fn crdt_apply(
4549
let identity = resolve_identity(&headers, &state, "http")?;
4650

4751
let audit = ArcAuditEmitter(std::sync::Arc::clone(&state.shared.audit));
48-
authorize_collection(
52+
preflight_crdt_apply(
4953
&identity,
50-
crate::types::DatabaseId::DEFAULT,
5154
&collection,
52-
Permission::Write,
5355
&state.shared.permissions,
5456
&state.shared.roles,
5557
&audit,
@@ -147,6 +149,25 @@ pub async fn crdt_apply(
147149
)))
148150
}
149151

152+
/// Authorize the fixed DEFAULT-database write before any CRDT apply work.
153+
fn preflight_crdt_apply(
154+
identity: &AuthenticatedIdentity,
155+
collection: &str,
156+
permissions: &PermissionStore,
157+
roles: &RoleStore,
158+
audit: &dyn AuditEmitter,
159+
) -> Result<(), AuthorizationError> {
160+
authorize_collection(
161+
identity,
162+
crate::types::DatabaseId::DEFAULT,
163+
collection,
164+
Permission::Write,
165+
permissions,
166+
roles,
167+
audit,
168+
)
169+
}
170+
150171
fn decode_bounded_delta(encoded: &str) -> Result<Vec<u8>, ApiError> {
151172
let delta = hex_decode(encoded)
152173
.ok_or_else(|| ApiError::BadRequest("invalid hex in 'delta' field".into()))?;
@@ -162,6 +183,95 @@ fn decode_bounded_delta(encoded: &str) -> Result<Vec<u8>, ApiError> {
162183
#[cfg(test)]
163184
mod tests {
164185
use super::*;
186+
use crate::control::security::audit::AuditEvent;
187+
use crate::control::security::audit::emitter::test_helpers::CapturingEmitter;
188+
use crate::control::security::identity::{AuthMethod, DatabaseSet};
189+
use crate::types::TenantId;
190+
191+
fn regular_identity() -> AuthenticatedIdentity {
192+
AuthenticatedIdentity::new_regular(
193+
7,
194+
"writer",
195+
TenantId::new(9),
196+
AuthMethod::ApiKey,
197+
Vec::new(),
198+
None,
199+
DatabaseSet::Some(smallvec::smallvec![crate::types::DatabaseId::DEFAULT]),
200+
)
201+
}
202+
203+
fn assert_one_audit_denial(audit: &CapturingEmitter) {
204+
let events = audit.recorded();
205+
assert_eq!(events.len(), 1);
206+
assert_eq!(events[0].0, AuditEvent::PermissionDenied);
207+
}
208+
209+
#[test]
210+
fn ungranted_regular_identity_is_denied_before_authorized_work_runs() {
211+
// This endpoint has no read action or caller-selectable database, so
212+
// those authorization-matrix cells are N/A.
213+
let identity = regular_identity();
214+
let permissions = PermissionStore::new();
215+
let roles = RoleStore::new();
216+
let audit = CapturingEmitter::new();
217+
let mut authorized_work_runs = 0;
218+
let preflight = preflight_crdt_apply(&identity, "orders", &permissions, &roles, &audit);
219+
220+
if preflight.is_ok() {
221+
authorized_work_runs += 1;
222+
}
223+
224+
assert!(preflight.is_err());
225+
assert_eq!(authorized_work_runs, 0);
226+
assert_one_audit_denial(&audit);
227+
}
228+
229+
#[test]
230+
fn regular_identity_cannot_write_system_audit_log_even_with_grant() {
231+
// This endpoint has no read action or caller-selectable database, so
232+
// those authorization-matrix cells are N/A.
233+
let identity = regular_identity();
234+
let permissions = PermissionStore::new();
235+
permissions
236+
.grant(
237+
"collection:9:_system.audit_log",
238+
"user:writer",
239+
Permission::Write,
240+
"admin",
241+
None,
242+
)
243+
.expect("in-memory grant succeeds");
244+
let roles = RoleStore::new();
245+
let audit = CapturingEmitter::new();
246+
247+
assert!(
248+
preflight_crdt_apply(&identity, "_system.audit_log", &permissions, &roles, &audit,)
249+
.is_err()
250+
);
251+
assert_one_audit_denial(&audit);
252+
}
253+
254+
#[test]
255+
fn tenant_ten_grant_does_not_authorize_tenant_nine_identity() {
256+
// This endpoint has no read action or caller-selectable database, so
257+
// those authorization-matrix cells are N/A.
258+
let identity = regular_identity();
259+
let permissions = PermissionStore::new();
260+
permissions
261+
.grant(
262+
"collection:10:orders",
263+
"user:writer",
264+
Permission::Write,
265+
"admin",
266+
None,
267+
)
268+
.expect("in-memory grant succeeds");
269+
let roles = RoleStore::new();
270+
let audit = CapturingEmitter::new();
271+
272+
assert!(preflight_crdt_apply(&identity, "orders", &permissions, &roles, &audit).is_err());
273+
assert_one_audit_denial(&audit);
274+
}
165275

166276
#[test]
167277
fn decoded_delta_limit_is_exact_and_invalid_hex_is_bad_request() {

nodedb/src/control/server/ilp_batch.rs

Lines changed: 143 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,18 +306,76 @@ fn build_ilp_calvin_tasks(
306306

307307
#[cfg(test)]
308308
mod tests {
309-
use super::{IlpPreflightFailure, preflight_ilp_batch};
310-
use crate::control::security::audit::NoopAuditEmitter;
309+
use super::{IlpPreflightFailure, flush_authenticated_ilp_batch, preflight_ilp_batch};
310+
use std::sync::{Arc, Mutex};
311+
312+
use crate::bridge::dispatch::Dispatcher;
313+
use crate::control::state::SharedState;
314+
311315
use crate::control::security::audit::emitter::test_helpers::CapturingEmitter;
316+
use crate::control::security::audit::{
317+
AuditEmitContext, AuditEmitter, AuditEvent, NoopAuditEmitter,
318+
};
312319
use crate::control::security::identity::{
313320
AuthMethod, AuthenticatedIdentity, DatabaseSet, Permission,
314321
};
315322
use crate::control::security::permission::PermissionStore;
316323
use crate::control::security::role::RoleStore;
317324
use crate::types::{DatabaseId, TenantId, VShardId};
325+
use crate::wal::WalManager;
318326
use nodedb_physical::physical_plan::{PhysicalPlan, TimeseriesOp};
319327
use nodedb_types::Surrogate;
320328

329+
#[derive(Clone, Debug)]
330+
struct CapturedAuditEvent {
331+
event: AuditEvent,
332+
source: String,
333+
auth_user_id: String,
334+
auth_user_name: String,
335+
tenant_id: Option<TenantId>,
336+
}
337+
338+
struct ContextCapturingEmitter {
339+
events: Mutex<Vec<CapturedAuditEvent>>,
340+
}
341+
342+
impl ContextCapturingEmitter {
343+
fn new() -> Self {
344+
Self {
345+
events: Mutex::new(Vec::new()),
346+
}
347+
}
348+
349+
fn recorded(&self) -> Vec<CapturedAuditEvent> {
350+
self.events
351+
.lock()
352+
.unwrap_or_else(|poisoned| poisoned.into_inner())
353+
.clone()
354+
}
355+
}
356+
357+
impl AuditEmitter for ContextCapturingEmitter {
358+
fn emit(
359+
&self,
360+
event: AuditEvent,
361+
source: &str,
362+
_detail: &str,
363+
context: AuditEmitContext<'_>,
364+
) {
365+
let mut events = self
366+
.events
367+
.lock()
368+
.unwrap_or_else(|poisoned| poisoned.into_inner());
369+
events.push(CapturedAuditEvent {
370+
event,
371+
source: source.into(),
372+
auth_user_id: context.auth_user_id.into(),
373+
auth_user_name: context.auth_user_name.into(),
374+
tenant_id: context.tenant_id,
375+
});
376+
}
377+
}
378+
321379
fn identity(database_id: DatabaseId) -> AuthenticatedIdentity {
322380
AuthenticatedIdentity::new_regular(
323381
7,
@@ -351,6 +409,42 @@ mod tests {
351409
)
352410
}
353411

412+
#[tokio::test]
413+
async fn authenticated_flush_rejects_unwritable_measurement_before_dispatch_or_catalog_projection()
414+
{
415+
let directory = tempfile::tempdir().expect("create ILP batch test directory");
416+
let wal = Arc::new(
417+
WalManager::open_for_testing(&directory.path().join("ilp-batch.wal"))
418+
.expect("open ILP batch test WAL"),
419+
);
420+
let (dispatcher, _data_sides) = Dispatcher::new(1, 64);
421+
let state = SharedState::new(dispatcher, wal).expect("construct ILP batch state");
422+
let database_id = DatabaseId::new(7);
423+
424+
let error = flush_authenticated_ilp_batch(
425+
&state,
426+
&identity(database_id),
427+
database_id,
428+
"cpu value=1i\n",
429+
)
430+
.await
431+
.expect_err("regular identity without write permission is rejected during preflight");
432+
433+
assert!(matches!(
434+
error,
435+
crate::Error::BadRequest { detail } if detail == "ILP batch rejected"
436+
));
437+
assert!(
438+
state
439+
.credentials
440+
.catalog()
441+
.get_collection(database_id, 9, "cpu")
442+
.expect("read catalog collection projection")
443+
.is_none(),
444+
"early authorization denial must not create a collection/schema projection"
445+
);
446+
}
447+
354448
#[test]
355449
fn groups_two_measurements_in_canonical_order_and_preserves_source_order() {
356450
let permissions = PermissionStore::new();
@@ -445,6 +539,7 @@ mod tests {
445539

446540
#[test]
447541
fn read_only_collection_is_not_sufficient_for_ilp_ingest() {
542+
// ILP is write-only; read access is not applicable to ingestion.
448543
let permissions = PermissionStore::new();
449544
permissions
450545
.grant(
@@ -462,13 +557,59 @@ mod tests {
462557
);
463558
}
464559

560+
#[test]
561+
fn system_audit_log_measurement_is_denied_to_regular_ingester() {
562+
let permissions = PermissionStore::new();
563+
grant_write(&permissions, "_system.audit_log");
564+
let audit = ContextCapturingEmitter::new();
565+
566+
assert_eq!(
567+
preflight_ilp_batch(
568+
&identity(DatabaseId::new(7)),
569+
DatabaseId::new(7),
570+
"_system.audit_log value=1i\n",
571+
&permissions,
572+
&RoleStore::new(),
573+
&audit,
574+
),
575+
Err(IlpPreflightFailure::PermissionDenied)
576+
);
577+
let events = audit.recorded();
578+
assert_eq!(events.len(), 1);
579+
assert_eq!(events[0].event, AuditEvent::PermissionDenied);
580+
assert_eq!(events[0].source, "ingester");
581+
assert_eq!(events[0].auth_user_id, "7");
582+
assert_eq!(events[0].auth_user_name, "ingester");
583+
assert_eq!(events[0].tenant_id, Some(TenantId::new(9)));
584+
}
585+
586+
#[test]
587+
fn tenant_ten_write_grant_does_not_authorize_tenant_nine_ilp_ingest() {
588+
let permissions = PermissionStore::new();
589+
permissions
590+
.grant(
591+
"collection:10:cpu",
592+
"user:ingester",
593+
Permission::Write,
594+
"admin",
595+
None,
596+
)
597+
.expect("in-memory grant succeeds");
598+
599+
assert_eq!(
600+
preflight("cpu value=1i\n", &permissions),
601+
Err(IlpPreflightFailure::PermissionDenied)
602+
);
603+
}
604+
465605
#[test]
466606
fn non_default_database_is_used_for_collection_authorization() {
467607
let permissions = PermissionStore::new();
468608
grant_write(&permissions, "cpu");
469609
let database_id = DatabaseId::new(7);
470610
let roles = RoleStore::new();
471611

612+
// Database selection is handshake-bound; ILP payload does not select it.
472613
assert_eq!(
473614
preflight_ilp_batch(
474615
&identity(DatabaseId::DEFAULT),

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,15 @@ async fn handle_sql_inner(
125125
return resp(NativeResponse::status_row(seq, "RESET"));
126126
}
127127
if upper == "DISCARD ALL" {
128+
// Recreate only disposable session state. The authenticated database
129+
// binding belongs to the connection and must survive the reset.
130+
let database_id = ctx.sessions.get_current_database(ctx.peer_addr);
128131
ctx.sessions.remove(ctx.peer_addr);
129132
ctx.sessions.ensure_session(*ctx.peer_addr);
133+
if let Some(database_id) = database_id {
134+
ctx.sessions
135+
.set_current_database(ctx.peer_addr, database_id);
136+
}
130137
return resp(NativeResponse::status_row(seq, "DISCARD ALL"));
131138
}
132139

0 commit comments

Comments
 (0)