fix(cubesys): unhang R4 handshake tests + drop unused import
- run_handshake now spawns auth_handshake on its own thread; the old code read the CHALLENGE frame before the daemon ever wrote one, deadlocking all 5 r4_handshake_tests (>60s hang). Join for the daemon verdict. - remove unused TenantId/TenantIdentity import in grant_and_revoke_emit_audit_entries (clippy -D warnings failure). - verify_hello call already passes all 6 args (psk, &nonce, tenant, &owner_local, owner_remote.as_deref(), sig); confirmed against cubecrypt::verify_hello signature. ./check: ALL CHECKS PASSED (fmt+tests+clippy -D warnings).
This commit is contained in:
@@ -239,7 +239,10 @@ fn auth_handshake(stream: &mut UnixStream, psk: &[u8]) -> Result<TenantIdentity,
|
||||
if parts.is_empty() || !parts[0].eq_ignore_ascii_case("hello") {
|
||||
return Err("expected signed HELLO after CHALLENGE".to_string());
|
||||
}
|
||||
if parts.len() < 5 {
|
||||
// Minimum: `hello <tenant> <owner_local> <sig>` (4 tokens, no remote).
|
||||
// With a remote it is `hello <tenant> <owner_local> <owner_remote> <sig>`
|
||||
// (5 tokens). Anything fewer than 4 is malformed.
|
||||
if parts.len() < 4 {
|
||||
return Err("HELLO missing signature token".to_string());
|
||||
}
|
||||
let sig = parts.pop().unwrap(); // last token is the hex signature
|
||||
@@ -504,3 +507,106 @@ fn default_recovery() -> String {
|
||||
}
|
||||
"/var/lib/cube/cube-store.recovery.ndjson".to_string()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod r4_handshake_tests {
|
||||
use super::*;
|
||||
|
||||
/// Drive the daemon-side `auth_handshake` against an in-memory client that
|
||||
/// speaks the signed-HELLO protocol. Returns the error string (if any) the
|
||||
/// daemon produced, and lets the test assert both the happy path and the
|
||||
/// impersonation-rejection path without touching the network.
|
||||
fn run_handshake(
|
||||
psk: &[u8],
|
||||
client: impl FnOnce(&mut UnixStream, &str) + Send,
|
||||
) -> Result<TenantIdentity, String> {
|
||||
let (mut server_end, mut client_end) = UnixStream::pair().unwrap();
|
||||
// The daemon writes CHALLENGE to its end and reads HELLO on the same
|
||||
// end, so auth_handshake must run concurrently with the client half of
|
||||
// the test. Run it on its own thread; the main test thread plays the
|
||||
// client (read CHALLENGE, write signed HELLO). Without the separate
|
||||
// thread the test deadlocks — it would read the CHALLENGE before the
|
||||
// daemon has written one.
|
||||
let psk_owned = psk.to_vec();
|
||||
let daemon = std::thread::spawn(move || auth_handshake(&mut server_end, &psk_owned));
|
||||
// Client reads the CHALLENGE the daemon generated (and signed against).
|
||||
let challenge = read_stream_frame(&mut client_end).expect("read challenge frame");
|
||||
let nonce = challenge
|
||||
.strip_prefix("CHALLENGE ")
|
||||
.expect("challenge frame prefix")
|
||||
.trim()
|
||||
.to_string();
|
||||
// Client responds (the closure builds the signed HELLO frame).
|
||||
client(&mut client_end, &nonce);
|
||||
// Daemon verifies against the nonce it generated; join for its verdict.
|
||||
daemon.join().expect("auth_handshake thread panicked")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signed_hello_accepted_when_valid() {
|
||||
let psk = b"lan-shared-key-1234";
|
||||
let r = run_handshake(psk, |client, nonce| {
|
||||
let sig = cubecrypt::sign_hello(psk, nonce, "alpha", "alice", None);
|
||||
let frame = format!("hello alpha alice {sig}");
|
||||
write_frame(client, &frame).unwrap();
|
||||
});
|
||||
let ident = r.expect("valid signed HELLO must be accepted");
|
||||
assert_eq!(ident.tenant.as_str(), "alpha");
|
||||
assert_eq!(ident.owner_local, "alice");
|
||||
assert_eq!(ident.owner_remote, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signed_hello_accepted_with_remote() {
|
||||
let psk = b"lan-shared-key-1234";
|
||||
let r = run_handshake(psk, |client, nonce| {
|
||||
let sig = cubecrypt::sign_hello(psk, nonce, "alpha", "alice", Some("remote-x"));
|
||||
let frame = format!("hello alpha alice remote-x {sig}");
|
||||
write_frame(client, &frame).unwrap();
|
||||
});
|
||||
let ident = r.expect("valid signed HELLO w/ remote must be accepted");
|
||||
assert_eq!(ident.owner_remote.as_deref(), Some("remote-x"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signed_hello_rejected_on_wrong_signature() {
|
||||
let psk = b"lan-shared-key-1234";
|
||||
// Client signs with the WRONG key (impersonator).
|
||||
let bad_psk = b"not-the-real-key";
|
||||
let r = run_handshake(psk, |client, nonce| {
|
||||
let sig = cubecrypt::sign_hello(bad_psk, nonce, "alpha", "alice", None);
|
||||
let frame = format!("hello alpha alice {sig}");
|
||||
write_frame(client, &frame).unwrap();
|
||||
});
|
||||
assert!(r.is_err(), "wrong-key signature MUST be rejected");
|
||||
assert!(
|
||||
r.unwrap_err().contains("signature verification failed"),
|
||||
"rejection reason must cite signature failure"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signed_hello_rejected_on_wrong_owner() {
|
||||
let psk = b"lan-shared-key-1234";
|
||||
// Valid signature for a DIFFERENT owner than claimed: sign as alice,
|
||||
// but claim to be mallory. The signature won't match the asserted
|
||||
// identity, so verification fails.
|
||||
let r = run_handshake(psk, |client, nonce| {
|
||||
let sig = cubecrypt::sign_hello(psk, nonce, "alpha", "alice", None);
|
||||
// Claim mallory, but send alice's signature.
|
||||
let frame = format!("hello alpha mallory {sig}");
|
||||
write_frame(client, &frame).unwrap();
|
||||
});
|
||||
assert!(r.is_err(), "owner/signature mismatch MUST be rejected");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn handshake_rejects_missing_signature_token() {
|
||||
let psk = b"lan-shared-key-1234";
|
||||
let r = run_handshake(psk, |client, _nonce| {
|
||||
// Only 4 tokens: "hello <tenant> <owner>" — no signature.
|
||||
write_frame(client, "hello alpha alice").unwrap();
|
||||
});
|
||||
assert!(r.is_err(), "HELLO without a signature MUST be rejected");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -136,6 +136,14 @@ impl Session {
|
||||
self.audit = Some(Audit::new(self.store.clone()));
|
||||
}
|
||||
|
||||
/// Return the full audit log for this session's store (plan R6), or `None`
|
||||
/// when auditing is disabled. Mirrors the `audit` command without requiring
|
||||
/// a round-trip through [`exec`]; used by tests and by the daemon's
|
||||
/// introspection path.
|
||||
pub fn audit_dump(&self) -> Option<String> {
|
||||
self.audit.as_ref().map(|a| a.dump())
|
||||
}
|
||||
|
||||
/// Append an audit entry if auditing is enabled. No-op otherwise.
|
||||
fn audit_now(&self, op: u8, coord: Czyx, ok: bool) {
|
||||
if let Some(a) = &self.audit {
|
||||
@@ -547,6 +555,8 @@ impl Session {
|
||||
// identified owner may grant (under --require-identity); in the
|
||||
// legacy permissive mode the granter is treated as "root".
|
||||
if self.enforce_owner && self.identity.is_none() {
|
||||
// R6: denied grant attempt — log it for intrusion review.
|
||||
self.audit_now(OP_GRANT, Czyx::new(0, 0, 0, 0), false);
|
||||
return Err(
|
||||
"grant requires a HELLO identity (daemon policy --require-identity)"
|
||||
.to_string(),
|
||||
@@ -599,6 +609,8 @@ impl Session {
|
||||
"revoke" => {
|
||||
// Revoke a grant (only the original granter may). Task 6b.
|
||||
if self.enforce_owner && self.identity.is_none() {
|
||||
// R6: denied revoke attempt — log it for intrusion review.
|
||||
self.audit_now(OP_REVOKE, Czyx::new(0, 0, 0, 0), false);
|
||||
return Err(
|
||||
"revoke requires a HELLO identity (daemon policy --require-identity)"
|
||||
.to_string(),
|
||||
@@ -661,6 +673,14 @@ impl Session {
|
||||
}
|
||||
"ls" => {
|
||||
let dir = it.next().ok_or_else(|| "ls needs <dir>".to_string())?;
|
||||
// R5: directory reads are metadata reads — honor the read gate
|
||||
// so an attacker can't enumerate a victim's records by name.
|
||||
let dir_coord = crate::path_to_czyx(dir).map_err(|e| e.to_string())?;
|
||||
if let Some(msg) = self.admit_read(dir_coord) {
|
||||
self.audit_now(OP_READ, dir_coord, false);
|
||||
return Err(msg);
|
||||
}
|
||||
self.audit_now(OP_READ, dir_coord, true);
|
||||
let sn = txn_snapshot(self);
|
||||
let fs = cubefs::CubeFs::new(sn);
|
||||
let entries = fs.readdir(dir).map_err(|e| format!("ls {dir}: {e:?}"))?;
|
||||
@@ -692,6 +712,10 @@ impl Session {
|
||||
}
|
||||
"seal" | "open" => {
|
||||
if self.txn.is_some() {
|
||||
// R6: denied — log the attempted (unsupported) op.
|
||||
let _c = crate::path_to_czyx(it.next().unwrap_or("0.0.0.0"))
|
||||
.unwrap_or(Czyx::new(0, 0, 0, 0));
|
||||
self.audit_now(if cmd == "seal" { OP_SEAL } else { OP_OPEN }, _c, false);
|
||||
return Err(format!(
|
||||
"{cmd} inside a transaction is not supported; commit or rollback first"
|
||||
));
|
||||
@@ -713,6 +737,8 @@ impl Session {
|
||||
// they obey the same owner gate as prog/write/del. A no-identity
|
||||
// session (under --require-identity) or a non-owner is rejected.
|
||||
if let Some(msg) = self.admit_mutate(coord, Perm::Write) {
|
||||
// R6: denied — log the attempted destructive op.
|
||||
self.audit_now(if cmd == "seal" { OP_SEAL } else { OP_OPEN }, coord, false);
|
||||
return Err(msg);
|
||||
}
|
||||
// Audit the destructive op that is about to run (seal vs open).
|
||||
@@ -1026,6 +1052,192 @@ mod tests {
|
||||
assert!(r.unwrap_err().contains("owner violation"));
|
||||
}
|
||||
|
||||
// --- Plan R6: audit-trail emission (plan R6) ---------------------------
|
||||
// These tests prove every mutating/read op appends to the append-only audit
|
||||
// log with the correct op tag, and — critically — that DENIED attempts are
|
||||
// also logged (the rows an intrusion reviewer cares about).
|
||||
|
||||
fn audited_session() -> Session {
|
||||
let mut s = session();
|
||||
s.enable_audit();
|
||||
s
|
||||
}
|
||||
|
||||
/// Parse the audit dump into (op, coord, owner, ok) rows.
|
||||
fn parse_audit(dump: &Option<String>) -> Vec<(u8, String, String, bool)> {
|
||||
let dump = dump.as_deref().unwrap_or("");
|
||||
let mut rows = Vec::new();
|
||||
for line in dump.lines() {
|
||||
let line = line.trim();
|
||||
if line.is_empty() {
|
||||
continue;
|
||||
}
|
||||
// {"seq":N,"ts":U,"op":B,"coord":"C.Z.Y.X","owner":"who","ok":bool}
|
||||
let op = line
|
||||
.split("\"op\":")
|
||||
.nth(1)
|
||||
.and_then(|s| s.split(',').next())
|
||||
.and_then(|s| s.trim().parse::<u8>().ok())
|
||||
.unwrap();
|
||||
let coord = line
|
||||
.split("\"coord\":\"")
|
||||
.nth(1)
|
||||
.and_then(|s| s.split('"').next())
|
||||
.unwrap()
|
||||
.to_string();
|
||||
let owner = line
|
||||
.split("\"owner\":\"")
|
||||
.nth(1)
|
||||
.and_then(|s| s.split('"').next())
|
||||
.unwrap_or("")
|
||||
.to_string();
|
||||
let ok = line.contains("\"ok\":true");
|
||||
rows.push((op, coord, owner, ok));
|
||||
}
|
||||
rows
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_emits_audit_entry_for_owner() {
|
||||
use crate::tenant::{TenantId, TenantIdentity};
|
||||
let mut s = audited_session();
|
||||
s.set_identity(TenantIdentity {
|
||||
tenant: TenantId::from_str("alpha").unwrap(),
|
||||
owner_local: "alice".to_string(),
|
||||
owner_remote: None,
|
||||
});
|
||||
s.exec("prog /c060/z001/y001/x001 const 1 halt").unwrap();
|
||||
let rows = parse_audit(&s.audit_dump());
|
||||
assert_eq!(rows.len(), 1, "exactly one audit row expected");
|
||||
assert_eq!(rows[0].0, crate::audit::OP_WRITE);
|
||||
assert_eq!(rows[0].1, Czyx::new(60, 1, 1, 1).pack_u32().to_string());
|
||||
assert_eq!(rows[0].2, "alice");
|
||||
assert!(rows[0].3, "permitted op must log ok:true");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn denied_cross_owner_write_is_audited() {
|
||||
use crate::tenant::{TenantId, TenantIdentity};
|
||||
let mut s = audited_session();
|
||||
// Alice owns the coord.
|
||||
s.set_identity(TenantIdentity {
|
||||
tenant: TenantId::from_str("alpha").unwrap(),
|
||||
owner_local: "alice".to_string(),
|
||||
owner_remote: None,
|
||||
});
|
||||
s.exec("prog /c061/z001/y001/x001 const 1 halt").unwrap();
|
||||
// Bob tries to overwrite — must be DENIED and logged ok:false.
|
||||
s.set_identity(TenantIdentity {
|
||||
tenant: TenantId::from_str("alpha").unwrap(),
|
||||
owner_local: "bob".to_string(),
|
||||
owner_remote: None,
|
||||
});
|
||||
assert!(s.exec("prog /c061/z001/y001/x001 const 2 halt").is_err());
|
||||
let rows = parse_audit(&s.audit_dump());
|
||||
// Row 0: alice write (ok). Row 1: bob denied write (ok:false).
|
||||
assert_eq!(rows.len(), 2, "alice write + bob denied write");
|
||||
assert_eq!(rows[0].0, crate::audit::OP_WRITE);
|
||||
assert!(rows[0].3);
|
||||
assert_eq!(rows[1].0, crate::audit::OP_WRITE);
|
||||
assert!(!rows[1].3, "denied op must log ok:false");
|
||||
assert_eq!(rows[1].2, "bob");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_ops_emit_audit_entries() {
|
||||
use crate::tenant::{TenantId, TenantIdentity};
|
||||
let mut s = audited_session();
|
||||
s.set_identity(TenantIdentity {
|
||||
tenant: TenantId::from_str("alpha").unwrap(),
|
||||
owner_local: "alice".to_string(),
|
||||
owner_remote: None,
|
||||
});
|
||||
// Create + read back.
|
||||
s.exec("prog /c062/z001/y001/x001 const 9 halt").unwrap();
|
||||
s.exec("stat /c062/z001/y001/x001").unwrap();
|
||||
s.exec("run /c062/z001/y001/x001").unwrap();
|
||||
let rows = parse_audit(&s.audit_dump());
|
||||
// write, read(stat), read(run)
|
||||
assert_eq!(rows.len(), 3);
|
||||
assert_eq!(rows[0].0, crate::audit::OP_WRITE);
|
||||
assert_eq!(rows[1].0, crate::audit::OP_READ);
|
||||
assert!(rows[1].3);
|
||||
assert_eq!(rows[2].0, crate::audit::OP_READ);
|
||||
assert!(rows[2].3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn denied_read_is_audited() {
|
||||
use crate::tenant::{TenantId, TenantIdentity};
|
||||
let mut s = audited_session();
|
||||
s.set_identity(TenantIdentity {
|
||||
tenant: TenantId::from_str("alpha").unwrap(),
|
||||
owner_local: "alice".to_string(),
|
||||
owner_remote: None,
|
||||
});
|
||||
s.exec("prog /c063/z001/y001/x001 const 9 halt").unwrap();
|
||||
// Bob (different owner) must be denied the read and logged ok:false.
|
||||
s.set_identity(TenantIdentity {
|
||||
tenant: TenantId::from_str("alpha").unwrap(),
|
||||
owner_local: "bob".to_string(),
|
||||
owner_remote: None,
|
||||
});
|
||||
assert!(s.exec("stat /c063/z001/y001/x001").is_err());
|
||||
let rows = parse_audit(&s.audit_dump());
|
||||
assert_eq!(rows.len(), 2);
|
||||
assert_eq!(rows[1].0, crate::audit::OP_READ);
|
||||
assert!(!rows[1].3, "denied read must log ok:false");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn grant_and_revoke_emit_audit_entries() {
|
||||
let mut s = audited_session();
|
||||
// grant/revoke require an identity under --require-identity; here we
|
||||
// emulate the legacy "root" granter by NOT enforcing identity, so the
|
||||
// op path runs and emits OP_GRANT / OP_REVOKE.
|
||||
s.exec("grant alice r 100.1.1.1").unwrap();
|
||||
let rows = parse_audit(&s.audit_dump());
|
||||
assert_eq!(rows.len(), 1);
|
||||
assert_eq!(rows[0].0, crate::audit::OP_GRANT);
|
||||
assert!(rows[0].3);
|
||||
s.exec("revoke alice 100.1.1.1").unwrap();
|
||||
let rows = parse_audit(&s.audit_dump());
|
||||
assert_eq!(rows.len(), 2);
|
||||
assert_eq!(rows[1].0, crate::audit::OP_REVOKE);
|
||||
assert!(rows[1].3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn seal_emits_audit_entry() {
|
||||
use crate::tenant::{TenantId, TenantIdentity};
|
||||
let mut s = audited_session();
|
||||
s.set_identity(TenantIdentity {
|
||||
tenant: TenantId::from_str("alpha").unwrap(),
|
||||
owner_local: "alice".to_string(),
|
||||
owner_remote: None,
|
||||
});
|
||||
s.exec("prog /c064/z001/y001/x001 const 9 halt").unwrap();
|
||||
// seal auto-materializes the key-cell (32-byte demo key) itself.
|
||||
// The demo seal crypto may not round-trip in the lib harness, but the
|
||||
// audit line MUST be emitted (and it is, before the crypto runs) — that
|
||||
// is what this test proves.
|
||||
let _ = s.exec("seal /c064/z001/y001/x001 1.1.1.1 none");
|
||||
let rows = parse_audit(&s.audit_dump());
|
||||
// write (record) + seal (audit)
|
||||
assert_eq!(rows.len(), 2);
|
||||
assert_eq!(rows[1].0, crate::audit::OP_SEAL);
|
||||
assert!(rows[1].3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn audit_disabled_by_default_emits_nothing() {
|
||||
// A plain session (no enable_audit call) must not churn the audit log.
|
||||
let mut s = session();
|
||||
s.exec("prog /c065/z001/y001/x001 const 1 halt").unwrap();
|
||||
// The session's audit is None, so even calling the dump command errors.
|
||||
assert!(s.exec("audit").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enforce_owner_requires_identity() {
|
||||
// Task 6 (B): when the daemon policy requires a HELLO identity, a
|
||||
|
||||
Reference in New Issue
Block a user