From f82c03e2ef3c382ccf600206a47bb48938582533 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 14 Sep 2026 19:57:06 +0200 Subject: [PATCH] fix: accept an acknowledgement carrying a VarBindList The subagent never registers against net-snmp. It connects, sends Open, and rejects the reply, logging "AgentX session ended: unexpected AgentX acknowledgement" before backing off 1, 2, 4, 8, 16 and 30 seconds forever. acknowledge() required the Response payload to be exactly 8 bytes. A Response PDU is sysUpTime, error and index followed by a VarBindList (RFC 2741 6.2.11), and the master may populate it. net-snmp does. Probing a live master directly over its Unix socket: OPEN -> type=18 payload_length=40 error=0 session=21 REGISTER -> type=18 payload_length=32 error=0 session=21 Type, transactionID and packetID all matched and the error was noAgentXError, so both operations had in fact succeeded and the session was granted. Only the length check refused them, so the region was registered on the master while the subagent tore the connection down. Comparing against 8 rather than requiring equality fixes it. The function only reads the fixed 8 byte block at bytes[24..28], so a longer payload needs no other handling. Reproduced on two unrelated hosts: Ubuntu 22.04 with net-snmp 5.9.1 and Debian 13 with a healthy, idle snmpd. The existing coverage misses it because the test master answers with Response::from_header, whose VarBindList is empty and whose payload is therefore exactly 8, which no real master sends. The added test drives acknowledge() over a socket pair with a net-snmp shaped reply for both Open and Register, and fails with the production error message when the comparison is restored. --- src/session.rs | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/src/session.rs b/src/session.rs index 6c25251..a86d3e0 100644 --- a/src/session.rs +++ b/src/session.rs @@ -191,8 +191,12 @@ fn reply(header: &Header) -> Response { fn acknowledge(stream: &mut UnixStream, request: &Header) -> Result
{ let (header, bytes) = receive(stream)?; + // A Response PDU carries sysUpTime, error and index, then a VarBindList the + // master is free to populate. net-snmp does: 40 payload bytes for Open and 32 + // for Register. Only the fixed 8 byte block is read below, so a longer payload + // is an ordinary acknowledgement rather than a malformed one. if header.ty != Type::Response - || header.payload_length != 8 + || header.payload_length < 8 || header.packet_id != request.packet_id || header.transaction_id != request.transaction_id || (request.ty != Type::Open && header.session_id != request.session_id) @@ -267,4 +271,46 @@ mod tests { let (response, _) = dispatch(&header, &bytes, &tables).unwrap(); assert_eq!(response.res_error, ResError::ProcessingError); } + + #[test] + fn acknowledgement_accepts_a_response_carrying_a_var_bind_list() { + // A Response PDU is sysUpTime, error and index followed by a VarBindList, + // and net-snmp sends one: probing a live master gives 40 payload bytes for + // Open and 32 for Register, never 8. Requiring exactly 8 refuses every real + // master while the handshake itself succeeded. + for (open, session_id, payload) in [(true, 21_u32, 40_usize), (false, 21_u32, 32_usize)] { + let (mut subagent, mut master) = + UnixStream::pair().expect("create a socket pair for the fake master"); + + let request = if open { + let mut request = pdu::Open::new(ID::default(), "agentx-ifstack"); + request.header.flags = NETWORK_ORDER; + request.header.packet_id = 1; + Header::from_bytes(&request.to_bytes().unwrap()).unwrap() + } else { + let mut request = pdu::Register::new(ID::try_from(TABLE.to_vec()).unwrap()); + request.header.flags = NETWORK_ORDER; + request.header.session_id = session_id; + request.header.packet_id = 2; + Header::from_bytes(&request.to_bytes().unwrap()).unwrap() + }; + + let mut response = vec![1, Type::Response.to_byte(), NETWORK_ORDER, 0]; + response.extend_from_slice(&session_id.to_be_bytes()); + response.extend_from_slice(&request.transaction_id.to_be_bytes()); + response.extend_from_slice(&request.packet_id.to_be_bytes()); + response.extend_from_slice(&(payload as u32).to_be_bytes()); + response.extend_from_slice(&31_223_731_u32.to_be_bytes()); // sysUpTime + response.extend_from_slice(&0_u16.to_be_bytes()); // res.error + response.extend_from_slice(&0_u16.to_be_bytes()); // res.index + response.resize(20 + payload, 0); // the VarBindList the master appends + master + .write_all(&response) + .expect("send the master response"); + + let acknowledged = acknowledge(&mut subagent, &request) + .expect("a master response carrying a VarBindList is a valid acknowledgement"); + assert_eq!(acknowledged.session_id, session_id); + } + } }