Add SNMP (v1/v2c) with a minimal local ASN.1 BER reader

First dissector needing actual ASN.1 decoding - a small local
tag/length/value reader, not a general ASN.1 decoder, just enough to
walk SNMP's own SEQUENCE/INTEGER/OCTET STRING structure. v3 wraps the
PDU in its own security-parameters header instead of a plain community
string and can be encrypted, so it's reported by version alone, the
same "don't take on real crypto" call already made for TLS/QUIC.
Community strings are shown as-is, matching FTP's PASS precedent --
v1/v2c send them in the clear regardless.

SnmpDissector takes its port in the constructor so it can be
registered twice, at 161 (agent) and 162 (trap receiver). Unlike
DHCP's 67/68, trap traffic never touches 161 on either side (ephemeral
source port straight to 162), so there's no shared port for
l7_summarize()'s dst-then-src fallback to land on - both ports need
explicit registration.

Live-verified against a real snmpd (net-snmp 5.9.5.2) on loopback: a
real snmpget GetRequest/GetResponse exchange decoded correctly with
matching request-ids across both directions, and a real snmptrap
SNMPv2-Trap on port 162 confirmed the second registered port actually
gets used.

5 files changed, 363 insertions, 1 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d08d6ad..b7fab04 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -109,6 +109,7 @@ add_executable(packeteer_tests
   tests/test_smtp.cpp
   tests/test_tftp.cpp
   tests/test_quic.cpp
+  tests/test_snmp.cpp
   tests/test_dissector.cpp
   tests/test_http.cpp
   tests/test_tls.cpp
diff --git a/PLAN.md b/PLAN.md
index 392e02e..50d4981 100644
--- a/PLAN.md
+++ b/PLAN.md
@@ -31,7 +31,7 @@ unowned buffers) via a real-world capture pipeline.
 4. [done] Bounded channel + drop-on-backpressure between capture and render
 5. [in progress] L7 dissector interface, add protocols incrementally --
    interface + DNS + HTTP + TLS SNI + mDNS + SSH banner + NTP + DHCP +
-   FTP + SMTP + TFTP + QUIC done (packeteer/l7/); ARP/VLAN/IGMP done
+   FTP + SMTP + TFTP + QUIC + SNMP done (packeteer/l7/); ARP/VLAN/IGMP done
    at the L2/L3 level too (packeteer/net/); more protocols can still
    be added incrementally,
    by design
@@ -540,3 +540,25 @@ None currently open.
   This also serves as a real-traffic confirmation that the
   L7Registry fix works: without it, none of this would have decoded
   at all, since tls_dissector claims port 443 first.
+- SNMP (l7/snmp.hpp), scoped to v1/v2c - the first dissector needing
+  actual ASN.1 BER decoding, via a small local TLV reader (tag/length/
+  value only, not a general ASN.1 decoder: no indefinite-length
+  encoding, no multi-byte tag numbers, nothing beyond what SNMP's own
+  SEQUENCE/INTEGER/OCTET STRING structure uses). v3 wraps the PDU in
+  its own security-parameters header instead of a plain community
+  string, and the PDU can be encrypted - reported by version alone,
+  not decoded further, the same "don't take on real crypto" call as
+  TLS/QUIC. Community strings are shown as-is, not redacted: v1/v2c
+  send them in the clear regardless, same reasoning as FTP's PASS.
+  SnmpDissector takes its port in the constructor rather than a fixed
+  override, so it's registered twice - 161 (agent) and 162 (trap
+  receiver). Unlike DHCP's 67/68, there's no port shared by both
+  directions for l7_summarize()'s dst-then-src fallback to land on: a
+  trap goes from an ephemeral source port straight to 162, touching
+  161 nowhere at all, so both had to be registered explicitly rather
+  than relying on the fallback the way DHCP could.
+  Live-verified against a real snmpd (net-snmp 5.9.5.2) on loopback: a
+  real `snmpget` GetRequest/GetResponse exchange on port 161 decoded
+  correctly with matching request-ids across both directions, and a
+  real `snmptrap` SNMPv2-Trap on port 162 confirmed the second
+  registered port actually gets used, not just the first.
diff --git a/include/packeteer/l7/snmp.hpp b/include/packeteer/l7/snmp.hpp
new file mode 100644
index 0000000..0bf5011
--- /dev/null
+++ b/include/packeteer/l7/snmp.hpp
@@ -0,0 +1,178 @@
+#pragma once
+
+#include <cstdint>
+#include <cstdio>
+#include <optional>
+#include <span>
+#include <string>
+
+#include "packeteer/l7/dissector.hpp"
+
+// SNMPv1 (RFC 1157) / SNMPv2c (RFC 1901+), both wrapped in the same
+// ASN.1 BER-encoded SEQUENCE { version, community, pdu }. SNMPv3 (RFC
+// 3411+) wraps the PDU in its own security-parameters header instead
+// of a plain community string, and the PDU itself can be encrypted --
+// reported by version alone, not decoded further, the same "don't
+// take on real crypto" call already made for TLS/QUIC.
+//
+// A minimal BER TLV reader lives in the detail namespace below: just
+// enough tag/length/value walking to get through SNMP's own
+// SEQUENCE/INTEGER/OCTET STRING structure, not a general ASN.1
+// decoder (no support for indefinite-length encoding, multi-byte tag
+// numbers, or any type SNMP itself doesn't use for the fields read
+// here).
+namespace packeteer::net {
+
+inline constexpr std::uint16_t kSnmpAgentPort = 161;  // get/set requests and their responses
+inline constexpr std::uint16_t kSnmpTrapPort = 162;    // traps/informs, sent from an ephemeral port
+
+namespace detail {
+
+struct BerTlv {
+    std::uint8_t tag;
+    std::span<const unsigned char> value;
+    std::size_t next_offset;  // offset just past this TLV, relative to the buffer read from
+};
+
+inline std::optional<BerTlv> read_ber_tlv(std::span<const unsigned char> bytes,
+                                            std::size_t offset) {
+    if (offset >= bytes.size()) return std::nullopt;
+    std::uint8_t tag = bytes[offset];
+    std::size_t pos = offset + 1;
+    if (pos >= bytes.size()) return std::nullopt;
+
+    std::uint8_t len_byte = bytes[pos++];
+    std::size_t length;
+    if ((len_byte & 0x80) == 0) {
+        length = len_byte;  // short form
+    } else {
+        std::uint8_t num_len_bytes = len_byte & 0x7F;
+        // 0 here is BER's indefinite-length form (not used by DER/SNMP's
+        // canonical encoding); >4 would overflow a reasonable length
+        // for anything SNMP actually sends. Both rejected rather than
+        // guessed at.
+        if (num_len_bytes == 0 || num_len_bytes > 4) return std::nullopt;
+        if (pos + num_len_bytes > bytes.size()) return std::nullopt;
+        length = 0;
+        for (std::uint8_t i = 0; i < num_len_bytes; ++i) length = (length << 8) | bytes[pos++];
+    }
+    if (pos + length > bytes.size()) return std::nullopt;
+    return BerTlv{tag, bytes.subspan(pos, length), pos + length};
+}
+
+// BER INTEGER: big-endian two's complement. SNMP's own fields
+// (version, request-id, error-status/index) all fit well within
+// 64 bits, so a fixed-width sign-extending read is enough.
+inline std::optional<std::int64_t> read_ber_integer(std::span<const unsigned char> value) {
+    if (value.empty() || value.size() > 8) return std::nullopt;
+    std::int64_t result = (value[0] & 0x80) ? -1 : 0;
+    for (auto b : value) result = (result << 8) | b;
+    return result;
+}
+
+}  // namespace detail
+
+inline constexpr std::uint8_t kBerTagInteger = 0x02;
+inline constexpr std::uint8_t kBerTagOctetString = 0x04;
+inline constexpr std::uint8_t kBerTagSequence = 0x30;
+inline constexpr std::uint8_t kSnmpPduTrapV1 = 0xA4;  // the one PDU with no request-id field
+
+struct SnmpMessage {
+    std::int64_t version;  // wire value: 0 = v1, 1 = v2c, 3 = v3
+    // Set only for v1/v2c - v3 replaces the plain community string
+    // with its own security-parameters header (RFC 3412), and the PDU
+    // itself may be encrypted, so neither is populated for it.
+    std::optional<std::string> community;
+    std::optional<std::uint8_t> pdu_tag;
+    std::optional<std::int64_t> request_id;
+};
+
+inline std::optional<SnmpMessage> parse_snmp(std::span<const unsigned char> bytes) {
+    auto outer = detail::read_ber_tlv(bytes, 0);
+    if (!outer || outer->tag != kBerTagSequence) return std::nullopt;
+
+    auto version_tlv = detail::read_ber_tlv(outer->value, 0);
+    if (!version_tlv || version_tlv->tag != kBerTagInteger) return std::nullopt;
+    auto version = detail::read_ber_integer(version_tlv->value);
+    if (!version) return std::nullopt;
+
+    // Wire values, not sequential: v1=0, v2c=1, v3=3 (RFC 3412's
+    // msgVersion). 2 was an abandoned SNMPv2 variant (SNMPv2p/2u) that
+    // never saw wide deployment and isn't handled here either.
+    if (*version == 3) return SnmpMessage{*version, std::nullopt, std::nullopt, std::nullopt};
+    if (*version != 0 && *version != 1) return std::nullopt;
+
+    auto community_tlv = detail::read_ber_tlv(outer->value, version_tlv->next_offset);
+    if (!community_tlv || community_tlv->tag != kBerTagOctetString) return std::nullopt;
+    std::string community(reinterpret_cast<const char*>(community_tlv->value.data()),
+                           community_tlv->value.size());
+
+    auto pdu_tlv = detail::read_ber_tlv(outer->value, community_tlv->next_offset);
+    if (!pdu_tlv) return std::nullopt;
+
+    SnmpMessage msg{*version, std::move(community), pdu_tlv->tag, std::nullopt};
+
+    // Every PDU except v1's Trap-PDU starts with request-id; Trap-PDU's
+    // own first field is an enterprise OID instead (RFC 1157 4.1.6),
+    // which isn't decoded here.
+    if (pdu_tlv->tag != kSnmpPduTrapV1) {
+        if (auto request_id_tlv = detail::read_ber_tlv(pdu_tlv->value, 0)) {
+            if (request_id_tlv->tag == kBerTagInteger) {
+                msg.request_id = detail::read_ber_integer(request_id_tlv->value);
+            }
+        }
+    }
+    return msg;
+}
+
+inline std::string snmp_pdu_type_name(std::uint8_t tag) {
+    switch (tag) {
+        case 0xA0: return "GetRequest";
+        case 0xA1: return "GetNextRequest";
+        case 0xA2: return "GetResponse";
+        case 0xA3: return "SetRequest";
+        case kSnmpPduTrapV1: return "Trap";
+        case 0xA5: return "GetBulkRequest";
+        case 0xA6: return "InformRequest";
+        case 0xA7: return "SNMPv2-Trap";
+        default: {
+            char buf[16];
+            std::snprintf(buf, sizeof(buf), "pdu=0x%02x", tag);
+            return buf;
+        }
+    }
+}
+
+// Takes the port to claim in its constructor rather than a fixed
+// override, so it can be registered twice - once for 161 (agent
+// requests/responses) and once for 162 (traps, sent from an ephemeral
+// source port to the trap receiver's well-known port, so l7_summarize's
+// dst-then-src fallback can't find 161 on either side the way it can
+// for e.g. DHCP's two ports). Community strings are shown as-is, not
+// redacted: SNMPv1/v2c send them in the clear regardless, the same
+// reasoning FTP's PASS command follows here.
+class SnmpDissector : public L7Dissector {
+public:
+    explicit SnmpDissector(std::uint16_t port) : port_(port) {}
+
+    std::uint16_t port() const override { return port_; }
+
+    std::optional<std::string> summarize(std::span<const unsigned char> payload) const override {
+        auto msg = parse_snmp(payload);
+        if (!msg) return std::nullopt;
+
+        if (msg->version == 3) return std::string("SNMP v3 (encrypted/authenticated, not decoded)");
+
+        std::string out = "SNMP ";
+        out += (msg->version == 0) ? "v1 " : "v2c ";
+        out += snmp_pdu_type_name(*msg->pdu_tag);
+        out += " community=" + *msg->community;
+        if (msg->request_id) out += " request-id=" + std::to_string(*msg->request_id);
+        return out;
+    }
+
+private:
+    std::uint16_t port_;
+};
+
+}  // namespace packeteer::net
diff --git a/include/packeteer/summarize.hpp b/include/packeteer/summarize.hpp
index 57f6f6f..aeff60d 100644
--- a/include/packeteer/summarize.hpp
+++ b/include/packeteer/summarize.hpp
@@ -18,6 +18,7 @@
 #include "packeteer/l7/ntp.hpp"
 #include "packeteer/l7/quic.hpp"
 #include "packeteer/l7/smtp.hpp"
+#include "packeteer/l7/snmp.hpp"
 #include "packeteer/l7/ssh.hpp"
 #include "packeteer/l7/tftp.hpp"
 #include "packeteer/l7/tls.hpp"
@@ -111,6 +112,13 @@ inline const net::L7Registry& l7_registry() {
     static const net::SmtpDissector smtp_dissector;
     static const net::TftpDissector tftp_dissector;
     static const net::QuicDissector quic_dissector;
+    // Two instances, not one: SNMP genuinely uses two well-known ports
+    // (161 agent, 162 trap receiver), and unlike DHCP's 67/68 there's
+    // no port shared by both directions of trap traffic for
+    // l7_summarize()'s dst-then-src fallback to land on - a trap goes
+    // from an ephemeral source port to 162, never touching 161 at all.
+    static const net::SnmpDissector snmp_agent_dissector{net::kSnmpAgentPort};
+    static const net::SnmpDissector snmp_trap_dissector{net::kSnmpTrapPort};
     static const net::L7Registry registry = [] {
         net::L7Registry r;
         r.add(&dns_dissector);
@@ -130,6 +138,8 @@ inline const net::L7Registry& l7_registry() {
         // actually matter for correctness, just for which one gets
         // tried first.
         r.add(&quic_dissector);
+        r.add(&snmp_agent_dissector);
+        r.add(&snmp_trap_dissector);
         return r;
     }();
     return registry;
diff --git a/tests/test_snmp.cpp b/tests/test_snmp.cpp
new file mode 100644
index 0000000..d9ccf0c
--- /dev/null
+++ b/tests/test_snmp.cpp
@@ -0,0 +1,151 @@
+#include <doctest/doctest.h>
+
+#include <vector>
+
+#include "packeteer/l7/snmp.hpp"
+
+using namespace packeteer::net;
+
+namespace {
+
+std::vector<unsigned char> ber_length(std::size_t n) {
+    if (n < 0x80) return {static_cast<unsigned char>(n)};
+    std::vector<unsigned char> bytes;
+    while (n) {
+        bytes.insert(bytes.begin(), static_cast<unsigned char>(n & 0xFF));
+        n >>= 8;
+    }
+    bytes.insert(bytes.begin(), static_cast<unsigned char>(0x80 | bytes.size()));
+    return bytes;
+}
+
+std::vector<unsigned char> ber_tlv(std::uint8_t tag, const std::vector<unsigned char>& value) {
+    std::vector<unsigned char> out = {tag};
+    auto len = ber_length(value.size());
+    out.insert(out.end(), len.begin(), len.end());
+    out.insert(out.end(), value.begin(), value.end());
+    return out;
+}
+
+std::vector<unsigned char> ber_int(std::int64_t n) {
+    if (n == 0) return {0x00};
+    std::vector<unsigned char> bytes;
+    bool neg = n < 0;
+    std::uint64_t val = neg ? static_cast<std::uint64_t>(~n) : static_cast<std::uint64_t>(n);
+    while (val) {
+        bytes.insert(bytes.begin(), static_cast<unsigned char>(val & 0xFF));
+        val >>= 8;
+    }
+    if (!neg && (bytes[0] & 0x80)) bytes.insert(bytes.begin(), 0x00);
+    return bytes;
+}
+
+std::vector<unsigned char> ber_octet_string(const std::string& s) {
+    return std::vector<unsigned char>(s.begin(), s.end());
+}
+
+std::vector<unsigned char> concat(std::initializer_list<std::vector<unsigned char>> parts) {
+    std::vector<unsigned char> out;
+    for (const auto& p : parts) out.insert(out.end(), p.begin(), p.end());
+    return out;
+}
+
+std::vector<unsigned char> snmp_message(std::int64_t version, const std::string& community,
+                                          std::uint8_t pdu_tag, std::int64_t request_id) {
+    auto pdu_body =
+        concat({ber_tlv(kBerTagInteger, ber_int(request_id)), ber_tlv(kBerTagInteger, ber_int(0)),
+                ber_tlv(kBerTagInteger, ber_int(0)), ber_tlv(kBerTagSequence, {})});
+    auto msg_body = concat({ber_tlv(kBerTagInteger, ber_int(version)),
+                             ber_tlv(kBerTagOctetString, ber_octet_string(community)),
+                             ber_tlv(pdu_tag, pdu_body)});
+    return ber_tlv(kBerTagSequence, msg_body);
+}
+
+}  // namespace
+
+TEST_CASE("parse_snmp decodes a v2c GetRequest") {
+    auto bytes = snmp_message(1, "public", 0xA0, 12345);
+    auto msg = parse_snmp(bytes);
+    REQUIRE(msg.has_value());
+    CHECK(msg->version == 1);
+    REQUIRE(msg->community.has_value());
+    CHECK(*msg->community == "public");
+    REQUIRE(msg->pdu_tag.has_value());
+    CHECK(*msg->pdu_tag == 0xA0);
+    REQUIRE(msg->request_id.has_value());
+    CHECK(*msg->request_id == 12345);
+}
+
+TEST_CASE("parse_snmp decodes a v1 message") {
+    auto bytes = snmp_message(0, "private", 0xA2, 7);
+    auto msg = parse_snmp(bytes);
+    REQUIRE(msg.has_value());
+    CHECK(msg->version == 0);
+    REQUIRE(msg->community.has_value());
+    CHECK(*msg->community == "private");
+}
+
+TEST_CASE("parse_snmp reports v3 by version alone, without community or pdu_tag") {
+    // A minimal v3 SEQUENCE { version } - real v3 messages carry a
+    // security-parameters header afterward, but version alone is
+    // enough to trigger the "not decoded further" path.
+    auto bytes = ber_tlv(kBerTagSequence, ber_tlv(kBerTagInteger, ber_int(3)));
+    auto msg = parse_snmp(bytes);
+    REQUIRE(msg.has_value());
+    CHECK(msg->version == 3);
+    CHECK_FALSE(msg->community.has_value());
+    CHECK_FALSE(msg->pdu_tag.has_value());
+}
+
+TEST_CASE("parse_snmp rejects an unsupported version (e.g. the abandoned SNMPv2 variant)") {
+    auto bytes = ber_tlv(kBerTagSequence, ber_tlv(kBerTagInteger, ber_int(2)));
+    CHECK_FALSE(parse_snmp(bytes).has_value());
+}
+
+TEST_CASE("parse_snmp handles a v1 Trap-PDU without a request-id field") {
+    // Trap-PDU's actual first field is an enterprise OID, not
+    // request-id - deliberately don't build one, just confirm
+    // request_id stays unset rather than misreading the OID as one.
+    auto msg_body = concat({ber_tlv(kBerTagInteger, ber_int(0)),
+                             ber_tlv(kBerTagOctetString, ber_octet_string("public")),
+                             ber_tlv(kSnmpPduTrapV1, ber_tlv(0x06, {0x2b, 0x06, 0x01}))});
+    auto bytes = ber_tlv(kBerTagSequence, msg_body);
+    auto msg = parse_snmp(bytes);
+    REQUIRE(msg.has_value());
+    REQUIRE(msg->pdu_tag.has_value());
+    CHECK(*msg->pdu_tag == kSnmpPduTrapV1);
+    CHECK_FALSE(msg->request_id.has_value());
+}
+
+TEST_CASE("parse_snmp rejects a payload that isn't a BER SEQUENCE") {
+    std::vector<unsigned char> bytes = {0x02, 0x01, 0x00};
+    CHECK_FALSE(parse_snmp(bytes).has_value());
+}
+
+TEST_CASE("parse_snmp rejects a truncated message") {
+    auto full = snmp_message(1, "public", 0xA0, 1);
+    std::vector<unsigned char> truncated(full.begin(), full.begin() + 5);
+    CHECK_FALSE(parse_snmp(truncated).has_value());
+}
+
+TEST_CASE("SnmpDissector claims the port given to its constructor") {
+    SnmpDissector agent(kSnmpAgentPort);
+    SnmpDissector trap(kSnmpTrapPort);
+    CHECK(agent.port() == 161);
+    CHECK(trap.port() == 162);
+}
+
+TEST_CASE("SnmpDissector formats a v2c GetRequest") {
+    SnmpDissector dissector(kSnmpAgentPort);
+    auto summary = dissector.summarize(snmp_message(1, "public", 0xA0, 12345));
+    REQUIRE(summary.has_value());
+    CHECK(*summary == "SNMP v2c GetRequest community=public request-id=12345");
+}
+
+TEST_CASE("SnmpDissector reports v3 without a community string") {
+    SnmpDissector dissector(kSnmpAgentPort);
+    auto bytes = ber_tlv(kBerTagSequence, ber_tlv(kBerTagInteger, ber_int(3)));
+    auto summary = dissector.summarize(bytes);
+    REQUIRE(summary.has_value());
+    CHECK(*summary == "SNMP v3 (encrypted/authenticated, not decoded)");
+}