diff --git a/rust/src/nfs/nfs.rs b/rust/src/nfs/nfs.rs index 03a91be3b3..a5d57d052b 100644 --- a/rust/src/nfs/nfs.rs +++ b/rust/src/nfs/nfs.rs @@ -1453,7 +1453,7 @@ impl NFSState { } /// Reject a partial v4 WRITE compound whose claimed length exceeds the /// limit: log the tx and skip the record so the data blob is never buffered. - fn process_partial_v4_write_request_record<'b>(&mut self, r: &RpcPacket<'b>) { + pub(crate) fn process_partial_v4_write_request_record(&mut self, r: &RpcPacket<'_>) { SCLogDebug!( "REQUEST {} partial v4 WRITE rejected: claim exceeds {}", r.hdr.xid, @@ -1499,7 +1499,7 @@ impl NFSState { /// Mark a partially buffered v4 COMPOUND request record malformed and skip it /// so the (attacker-controlled) record length is never buffered: the scanner's /// structural errors are definitive and the full parser rejects the same record. - fn process_partial_v4_malformed_request_record(&mut self, r: &RpcPacket<'_>) { + pub(crate) fn process_partial_v4_malformed_request_record(&mut self, r: &RpcPacket<'_>) { SCLogDebug!( "REQUEST {} partial v4 COMPOUND rejected: structural scan error", r.hdr.xid @@ -1535,6 +1535,39 @@ impl NFSState { ); } + /// Reject a complete v4 COMPOUND request record the parser could not read. + /// + /// The regular path only did `set_event`, which labels the most recent tx and + /// is a no-op when the state has none yet: the record was then consumed with + /// nothing logged. Run the partial path's bounded scan over the very bytes the + /// parser was given (`data`, so an RPCSEC_GSS wrapper is already unwrapped) + /// and reuse its rejects: an oversized WRITE claim gets the write-too-large + /// tx, everything else the malformed tx. A complete record has no unbuffered + /// tail, so the skip those helpers set is 0. + pub(crate) fn reject_failed_v4_compound_parse(&mut self, r: &RpcPacket<'_>, data: &[u8]) { + let max_write = unsafe { NFS_CFG_MAX_WRITE_SIZE }; + // a zero limit disables the check + if max_write != 0 + && matches!( + scan_nfs4_request_compound_write_len(data, max_write), + Nfs4CompoundScan::Oversized(_) + ) + { + SCLogDebug!( + "REQUEST {} complete v4 COMPOUND unparsed: WRITE claim exceeds {}", + r.hdr.xid, + max_write + ); + self.process_partial_v4_write_request_record(r); + return; + } + SCLogDebug!( + "REQUEST {} complete v4 COMPOUND unparsed: rejecting as malformed", + r.hdr.xid + ); + self.process_partial_v4_malformed_request_record(r); + } + /// Mark a partially buffered v4 COMPOUND reply record as malformed and /// skip it (bounded fallback; cf. /// process_partial_v4_malformed_request_record). @@ -3873,6 +3906,140 @@ mod tests { // misses may be added, however long the record promise is assert_eq!(chunk_queue_add(4096, 4096, 100), 100); } + + fn be32(v: u32) -> [u8; 4] { + v.to_be_bytes() + } + + /// A complete RPC/nfs v4 COMPOUND request record (AUTH_UNIX creds, NULL + /// verifier) carrying `compound` as program data, so the record path can be + /// driven without a stream. The RPC marker excludes itself, so it holds + /// xid + msgtype + the fixed header body + the program data. + fn v4_compound_record(xid: u32, compound: &[u8]) -> Vec { + let mut body: Vec = Vec::new(); + body.extend_from_slice(&be32(2)); // rpcver + body.extend_from_slice(&be32(100003)); // program: NFS + body.extend_from_slice(&be32(4)); // progver + body.extend_from_slice(&be32(NFSPROC4_COMPOUND)); // procedure + body.extend_from_slice(&be32(1)); // creds flavor: AUTH_UNIX + body.extend_from_slice(&be32(16)); // creds length + body.extend_from_slice(&[0u8; 16]); // stamp, machine name, uid, gid + body.extend_from_slice(&be32(0)); // verifier flavor: NULL + body.extend_from_slice(&be32(0)); // verifier length + let frag_len = (4 + 4 + body.len() + compound.len()) as u32; + let mut buf: Vec = Vec::new(); + buf.extend_from_slice(&(frag_len | 0x8000_0000).to_be_bytes()); // last frag + buf.extend_from_slice(&be32(xid)); + buf.extend_from_slice(&be32(1)); // msgtype: CALL + buf.extend_from_slice(&body); + buf.extend_from_slice(compound); + buf + } + + /// v4 compound head: empty XDR string tag + minor version + op count + fn compound_head(ops_cnt: u32) -> Vec { + let mut v = be32(0).to_vec(); // tag length 0 + v.extend_from_slice(&be32(0)); // minor version + v.extend_from_slice(&be32(ops_cnt)); + v + } + + /// v4 PUTFH op with a 16 byte handle + fn putfh_op() -> Vec { + let mut v = be32(NFSPROC4_PUTFH).to_vec(); + v.extend_from_slice(&be32(16)); + v.extend_from_slice(&[0u8; 16]); + v + } + + /// v4 WRITE op that claims `write_len` bytes but carries none + fn write_op(write_len: u32) -> Vec { + let mut v = be32(NFSPROC4_WRITE).to_vec(); + v.extend_from_slice(&be32(1)); // stateid seqid + v.extend_from_slice(&[0u8; 12]); // stateid other + v.extend_from_slice(&[0u8; 8]); // offset + v.extend_from_slice(&be32(2)); // stable: FILE_SYNC + v.extend_from_slice(&be32(write_len)); + v + } + + /// The reject a complete record gets must be observable in the transaction + /// list: `AppLayerTxData::set_event` is compiled out under cfg(test), so the + /// tx shape is what names the event (the write-too-large reject is a closed + /// file tx with the WRITE procedure, the malformed reject a plain tx with + /// the COMPOUND procedure). + fn reject_tx(state: &NFSState, expect_file_tx: bool, expect_proc: u32) { + assert_eq!( + state.transactions.len(), + 1, + "expected exactly one tx for the rejected record" + ); + let tx = &state.transactions[0]; + assert_eq!(tx.is_file_tx, expect_file_tx); + assert_eq!(tx.procedure, expect_proc); + assert!(tx.request_done && tx.response_done && tx.is_file_closed); + assert_eq!(tx.tx_data.0.files_opened, 0); + } + + #[test] + fn test_v4_complete_record_write_claim_past_record() { + let _lock = NFS_CFG_TEST_LOCK.lock().unwrap(); + unsafe { + NFS_CFG_MAX_WRITE_SIZE = 4096; + } + + // PUTFH + WRITE whose count runs past the end of the record: the parser + // stops with Incomplete, the scan sees the claim above the limit + let mut compound = compound_head(2); + compound.extend_from_slice(&putfh_op()); + compound.extend_from_slice(&write_op(65536)); + let buf = v4_compound_record(0x1234, &compound); + + let mut state = NFSState::new(); + let (_, pkt) = parse_rpc(&buf, true).expect("record parses"); + state.process_request_record_v4(&pkt); + + reject_tx(&state, true, NFSPROC4_WRITE); + assert_eq!(state.transactions[0].xid, 0x1234); + // A complete record leaves no unbuffered tail: nothing may be skipped, + // and the xid must be registered so the reply still matches the request. + assert_eq!(state.skip_ts, 0); + assert_eq!(state.ts_chunk_left, 0); + assert_eq!(state.ts_chunk_tail, 0); + assert!(state.requestmap.peek(&0x1234).is_some()); + + unsafe { + NFS_CFG_MAX_WRITE_SIZE = NFS_MAX_RW_SIZE_DEFAULT; + } + } + + #[test] + fn test_v4_complete_record_malformed_compound() { + let _lock = NFS_CFG_TEST_LOCK.lock().unwrap(); + unsafe { + NFS_CFG_MAX_WRITE_SIZE = 4096; + } + + // unknown op in a non-last position: the parser rejects it, and the + // scan cannot reach a WRITE claim either + let mut compound = compound_head(2); + compound.extend_from_slice(&be32(0xffff)); + compound.extend_from_slice(&putfh_op()); + let buf = v4_compound_record(0x4321, &compound); + + let mut state = NFSState::new(); + let (_, pkt) = parse_rpc(&buf, true).expect("record parses"); + state.process_request_record_v4(&pkt); + + reject_tx(&state, false, NFSPROC4_COMPOUND); + assert_eq!(state.transactions[0].xid, 0x4321); + assert_eq!(state.skip_ts, 0); + assert!(state.requestmap.peek(&0x4321).is_some()); + + unsafe { + NFS_CFG_MAX_WRITE_SIZE = NFS_MAX_RW_SIZE_DEFAULT; + } + } } #[test] diff --git a/rust/src/nfs/nfs4.rs b/rust/src/nfs/nfs4.rs index ae3245ef69..aace76dca7 100644 --- a/rust/src/nfs/nfs4.rs +++ b/rust/src/nfs/nfs4.rs @@ -18,6 +18,7 @@ // written by Victor Julien use nom7::bytes::streaming::take; +use nom7::error::ErrorKind; use nom7::number::streaming::be_u32; use nom7::{Err, IResult}; @@ -348,6 +349,12 @@ impl NFSState { // unwrapped below xidmap.gssapi_proc = creds.procedure; xidmap.gssapi_service = creds.service; + // privacy (0, 3) carries an encrypted compound: nothing to + // parse, and it is not malformed: just keep the request state + if creds.procedure == 0 && creds.service == 3 { + self.requestmap.put(r.hdr.xid, xidmap); + return; + } if creds.procedure == 0 && creds.service == 2 { SCLogDebug!("GSS INTEGRITY: {:?}", creds); match parse_rpc_gssapi_integrity(r.prog_data) { @@ -379,11 +386,19 @@ impl NFSState { } Err(Err::Incomplete(_n)) => { SCLogDebug!("NFSPROC4_COMPOUND: INCOMPLETE {:?}", _n); + self.reject_failed_v4_compound_parse(r, data); + } + Err(Err::Error(ref e)) if e.code == ErrorKind::Tag => { + // a defined v4 op this parser does not implement: the compound is + // valid, just not parsed. No tx of its own: keeps base's event on + // the newest tx, so a lock-only flow logs nothing when it has no + // earlier tx. + SCLogDebug!("NFSPROC4_COMPOUND: unsupported op {:?}", e); self.set_event(NFSEvent::MalformedData); } Err(Err::Error(_e)) | Err(Err::Failure(_e)) => { SCLogDebug!("NFSPROC4_COMPOUND: Parsing failed: {:?}", _e); - self.set_event(NFSEvent::MalformedData); + self.reject_failed_v4_compound_parse(r, data); } }; } @@ -477,6 +492,12 @@ impl NFSState { if xidmap.procedure == NFSPROC4_COMPOUND { let mut data = r.prog_data; + // privacy (0, 3) carries an encrypted response: not parsed, + // and not malformed (the call logged no tx either) + if xidmap.gssapi_proc == 0 && xidmap.gssapi_service == 3 { + return; + } + if xidmap.gssapi_proc == 0 && xidmap.gssapi_service == 2 { SCLogDebug!("GSS INTEGRITY as set by call: {:?}", xidmap); match parse_rpc_gssapi_integrity(r.prog_data) { diff --git a/rust/src/nfs/nfs4_records.rs b/rust/src/nfs/nfs4_records.rs index d8afa921a1..55447de718 100644 --- a/rust/src/nfs/nfs4_records.rs +++ b/rust/src/nfs/nfs4_records.rs @@ -595,6 +595,12 @@ fn nfs4_req_sequence(i: &[u8]) -> IResult<&[u8], Nfs4RequestContent<'_>> { Ok((i, req)) } +/// The op codes the NFSv4 RFCs (7530, 8881, 7862, 8276) assign all fall in this +/// range: inside it an op may be one we do not parse, outside it is no op. +fn v4_request_op_is_assigned(cmd: u32) -> bool { + matches!(cmd, 3..=75) +} + fn parse_request_compound_command(i: &[u8]) -> IResult<&[u8], Nfs4RequestContent<'_>> { let (i, cmd) = be_u32(i)?; let (i, cmd_data) = match cmd { @@ -630,6 +636,11 @@ fn parse_request_compound_command(i: &[u8]) -> IResult<&[u8], Nfs4RequestContent NFSPROC4_LAYOUTRETURN => nfs4_req_layoutreturn(i)?, NFSPROC4_DESTROY_SESSION => nfs4_req_destroy_session(i)?, NFSPROC4_DESTROY_CLIENTID => nfs4_req_destroy_clientid(i)?, + // an op no parser here reads: Tag, not Switch, so the caller does not + // call valid traffic malformed (no request parser else yields Tag) + _ if v4_request_op_is_assigned(cmd) => { + return Err(Err::Error(make_error(i, ErrorKind::Tag))); + } _ => { return Err(Err::Error(make_error(i, ErrorKind::Switch))); }