From 1434a006ead141f4a995b7fb3ab86f1b270b3ccc Mon Sep 17 00:00:00 2001 From: martijn Date: Thu, 8 Feb 2024 17:38:41 +0000 Subject: [PATCH] Add tests to make sure that snmpd adheres to the RFC2578 section 7.1 octet string primitive limitations. The integer based ones are restricted by the AgentX protocol and can't be tested. --- regress/usr.sbin/snmpd/Makefile | 8 +- regress/usr.sbin/snmpd/backend.c | 270 ++++++++++++++++++++++++- regress/usr.sbin/snmpd/regress.h | 6 + regress/usr.sbin/snmpd/snmp.c | 27 ++- regress/usr.sbin/snmpd/snmpd_regress.c | 6 + 5 files changed, 305 insertions(+), 12 deletions(-) diff --git a/regress/usr.sbin/snmpd/Makefile b/regress/usr.sbin/snmpd/Makefile index 22bb422008d..e77825dc398 100644 --- a/regress/usr.sbin/snmpd/Makefile +++ b/regress/usr.sbin/snmpd/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.13 2024/02/08 17:09:51 martijn Exp $ +# $OpenBSD: Makefile,v 1.14 2024/02/08 17:38:41 martijn Exp $ # Regress tests for snmpd PROG = snmpd_regress @@ -146,6 +146,12 @@ BACKEND_TARGETS+= backend_get_close_overlap BACKEND_TARGETS+= backend_get_disappear BACKEND_TARGETS+= backend_get_disappear_overlap BACKEND_TARGETS+= backend_get_disappear_doublesession +BACKEND_TARGETS+= backend_get_octetstring_max +BACKEND_TARGETS+= backend_get_octetstring_too_long +BACKEND_TARGETS+= backend_get_ipaddress_too_short +BACKEND_TARGETS+= backend_get_ipaddress_too_long +BACKEND_TARGETS+= backend_get_opaque_non_ber +BACKEND_TARGETS+= backend_get_opaque_double_value BACKEND_TARGETS+= backend_getnext_selfbound BACKEND_TARGETS+= backend_getnext_lowerbound BACKEND_TARGETS+= backend_getnext_lowerbound_self diff --git a/regress/usr.sbin/snmpd/backend.c b/regress/usr.sbin/snmpd/backend.c index 86af8de4b63..689cce7f812 100644 --- a/regress/usr.sbin/snmpd/backend.c +++ b/regress/usr.sbin/snmpd/backend.c @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -294,12 +295,13 @@ backend_get_opaque(void) struct varbind varbind = { .type = TYPE_NULL, .name = OID_STRUCT(MIB_BACKEND_GET, 8, 0), - .data.octetstring.string = "\1", - .data.octetstring.len = 1 }; int32_t requestid; char buf[1024]; size_t n; + struct ber ber = {}; + struct ber_element *elm; + ssize_t len; ax_s = agentx_connect(axsocket); sessionid = agentx_open(ax_s, 0, 0, @@ -314,11 +316,20 @@ backend_get_opaque(void) n = agentx_read(ax_s, buf, sizeof(buf), 1000); agentx_get_handle(__func__, buf, n, 0, sessionid, &varbind, 1); + if ((elm = ober_add_integer(NULL, 1)) == NULL) + err(1, "ober_add_integer"); + if (ober_write_elements(&ber, elm) == -1) + err(1, "ober_write_elements"); + varbind.data.octetstring.len = ober_get_writebuf( + &ber, (void **)&varbind.data.octetstring.string); + ober_free_elements(elm); + varbind.type = TYPE_OPAQUE; agentx_response(ax_s, buf, NOERROR, 0, &varbind, 1); snmpv2_response_validate(snmp_s, 1000, community, requestid, 0, 0, &varbind, 1); + ober_free(&ber); } void @@ -1382,6 +1393,261 @@ backend_get_disappear_doublesession(void) &varbind, 1); } +void +backend_get_octetstring_max(void) +{ + struct sockaddr_storage ss; + struct sockaddr *sa = (struct sockaddr *)&ss; + socklen_t salen; + int snmp_s, ax_s; + uint32_t sessionid; + char vbbuf[65535] = {}; + struct varbind varbind = { + .type = TYPE_NULL, + .name = OID_STRUCT(MIB_BACKEND_GET, 34, 0), + .data.octetstring.string = vbbuf, + .data.octetstring.len = sizeof(vbbuf) + }; + int32_t requestid; + char buf[1024]; + size_t n; + + memset(vbbuf, 'a', sizeof(vbbuf)); + ax_s = agentx_connect(axsocket); + sessionid = agentx_open(ax_s, 0, 0, + OID_ARG(MIB_SUBAGENT_BACKEND_GET, 34), __func__); + agentx_register(ax_s, sessionid, 0, 0, 127, 0, + OID_ARG(MIB_BACKEND_GET, 34), 0); + + /* Too big for SOCK_DGRAM */ + salen = snmp_resolve(SOCK_STREAM, hostname, servname, sa); + snmp_s = snmp_connect(SOCK_STREAM, sa, salen); + requestid = snmpv2_get(snmp_s, community, 0, &varbind, 1); + + n = agentx_read(ax_s, buf, sizeof(buf), 1000); + agentx_get_handle(__func__, buf, n, 0, sessionid, &varbind, 1); + + varbind.type = TYPE_OCTETSTRING; + agentx_response(ax_s, buf, NOERROR, 0, &varbind, 1); + + snmpv2_response_validate(snmp_s, 1000, community, requestid, 0, 0, + &varbind, 1); +} + +void +backend_get_octetstring_too_long(void) +{ + struct sockaddr_storage ss; + struct sockaddr *sa = (struct sockaddr *)&ss; + socklen_t salen; + int snmp_s, ax_s; + uint32_t sessionid; + char vbbuf[65536]; + struct varbind varbind = { + .type = TYPE_NULL, + .name = OID_STRUCT(MIB_BACKEND_GET, 35, 0), + .data.octetstring.string = vbbuf, + .data.octetstring.len = sizeof(vbbuf) + }; + int32_t requestid; + char buf[1024]; + size_t n; + + memset(vbbuf, 'a', sizeof(vbbuf)); + ax_s = agentx_connect(axsocket); + sessionid = agentx_open(ax_s, 0, 0, + OID_ARG(MIB_SUBAGENT_BACKEND_GET, 35), __func__); + agentx_register(ax_s, sessionid, 0, 0, 127, 0, + OID_ARG(MIB_BACKEND_GET, 35), 0); + + salen = snmp_resolve(SOCK_STREAM, hostname, servname, sa); + snmp_s = snmp_connect(SOCK_STREAM, sa, salen); + requestid = snmpv2_get(snmp_s, community, 0, &varbind, 1); + + n = agentx_read(ax_s, buf, sizeof(buf), 1000); + agentx_get_handle(__func__, buf, n, 0, sessionid, &varbind, 1); + + varbind.type = TYPE_OCTETSTRING; + agentx_response(ax_s, buf, NOERROR, 0, &varbind, 1); + + varbind.type = TYPE_NULL; + snmpv2_response_validate(snmp_s, 1000, community, requestid, GENERR, 1, + &varbind, 1); +} + +void +backend_get_ipaddress_too_short(void) +{ + struct sockaddr_storage ss; + struct sockaddr *sa = (struct sockaddr *)&ss; + socklen_t salen; + int snmp_s, ax_s; + uint32_t sessionid; + struct varbind varbind = { + .type = TYPE_NULL, + .name = OID_STRUCT(MIB_BACKEND_GET, 36, 0), + .data.octetstring.string = "\0\0\0", + .data.octetstring.len = 3 + }; + int32_t requestid; + char buf[1024]; + size_t n; + + ax_s = agentx_connect(axsocket); + sessionid = agentx_open(ax_s, 0, 0, + OID_ARG(MIB_SUBAGENT_BACKEND_GET, 36), __func__); + agentx_register(ax_s, sessionid, 0, 0, 127, 0, + OID_ARG(MIB_BACKEND_GET, 36), 0); + + salen = snmp_resolve(SOCK_DGRAM, hostname, servname, sa); + snmp_s = snmp_connect(SOCK_DGRAM, sa, salen); + requestid = snmpv2_get(snmp_s, community, 0, &varbind, 1); + + n = agentx_read(ax_s, buf, sizeof(buf), 1000); + agentx_get_handle(__func__, buf, n, 0, sessionid, &varbind, 1); + + varbind.type = TYPE_IPADDRESS; + agentx_response(ax_s, buf, NOERROR, 0, &varbind, 1); + + varbind.type = TYPE_NULL; + snmpv2_response_validate(snmp_s, 1000, community, requestid, GENERR, 1, + &varbind, 1); +} + +void +backend_get_ipaddress_too_long(void) +{ + struct sockaddr_storage ss; + struct sockaddr *sa = (struct sockaddr *)&ss; + socklen_t salen; + int snmp_s, ax_s; + uint32_t sessionid; + struct varbind varbind = { + .type = TYPE_NULL, + .name = OID_STRUCT(MIB_BACKEND_GET, 37, 0), + .data.octetstring.string = "\0\0\0\0\0", + .data.octetstring.len = 5 + }; + int32_t requestid; + char buf[1024]; + size_t n; + + ax_s = agentx_connect(axsocket); + sessionid = agentx_open(ax_s, 0, 0, + OID_ARG(MIB_SUBAGENT_BACKEND_GET, 37), __func__); + agentx_register(ax_s, sessionid, 0, 0, 127, 0, + OID_ARG(MIB_BACKEND_GET, 37), 0); + + salen = snmp_resolve(SOCK_DGRAM, hostname, servname, sa); + snmp_s = snmp_connect(SOCK_DGRAM, sa, salen); + requestid = snmpv2_get(snmp_s, community, 0, &varbind, 1); + + n = agentx_read(ax_s, buf, sizeof(buf), 1000); + agentx_get_handle(__func__, buf, n, 0, sessionid, &varbind, 1); + + varbind.type = TYPE_IPADDRESS; + agentx_response(ax_s, buf, NOERROR, 0, &varbind, 1); + + varbind.type = TYPE_NULL; + snmpv2_response_validate(snmp_s, 1000, community, requestid, GENERR, 1, + &varbind, 1); +} + +void +backend_get_opaque_non_ber(void) +{ + struct sockaddr_storage ss; + struct sockaddr *sa = (struct sockaddr *)&ss; + socklen_t salen; + int snmp_s, ax_s; + uint32_t sessionid; + struct varbind varbind = { + .type = TYPE_NULL, + .name = OID_STRUCT(MIB_BACKEND_GET, 38, 0), + .data.octetstring.string = "\1", + .data.octetstring.len = 1 + }; + int32_t requestid; + char buf[1024]; + size_t n; + ssize_t len; + + ax_s = agentx_connect(axsocket); + sessionid = agentx_open(ax_s, 0, 0, + OID_ARG(MIB_SUBAGENT_BACKEND_GET, 38), __func__); + agentx_register(ax_s, sessionid, 0, 0, 127, 0, + OID_ARG(MIB_BACKEND_GET, 38), 0); + + salen = snmp_resolve(SOCK_DGRAM, hostname, servname, sa); + snmp_s = snmp_connect(SOCK_DGRAM, sa, salen); + requestid = snmpv2_get(snmp_s, community, 0, &varbind, 1); + + n = agentx_read(ax_s, buf, sizeof(buf), 1000); + agentx_get_handle(__func__, buf, n, 0, sessionid, &varbind, 1); + + varbind.type = TYPE_OPAQUE; + agentx_response(ax_s, buf, NOERROR, 0, &varbind, 1); + + varbind.type = TYPE_NULL; + snmpv2_response_validate(snmp_s, 1000, community, requestid, GENERR, 1, + &varbind, 1); +} + +void +backend_get_opaque_double_value(void) +{ + struct sockaddr_storage ss; + struct sockaddr *sa = (struct sockaddr *)&ss; + socklen_t salen; + int snmp_s, ax_s; + uint32_t sessionid; + char vbbuf[1024]; + struct varbind varbind = { + .type = TYPE_NULL, + .name = OID_STRUCT(MIB_BACKEND_GET, 39, 0), + .data.octetstring.string = vbbuf + }; + int32_t requestid; + void *berdata; + char buf[1024]; + size_t n; + struct ber ber = {}; + struct ber_element *elm; + ssize_t len; + + ax_s = agentx_connect(axsocket); + sessionid = agentx_open(ax_s, 0, 0, + OID_ARG(MIB_SUBAGENT_BACKEND_GET, 39), __func__); + agentx_register(ax_s, sessionid, 0, 0, 127, 0, + OID_ARG(MIB_BACKEND_GET, 39), 0); + + salen = snmp_resolve(SOCK_DGRAM, hostname, servname, sa); + snmp_s = snmp_connect(SOCK_DGRAM, sa, salen); + requestid = snmpv2_get(snmp_s, community, 0, &varbind, 1); + + n = agentx_read(ax_s, buf, sizeof(buf), 1000); + agentx_get_handle(__func__, buf, n, 0, sessionid, &varbind, 1); + + if ((elm = ober_add_integer(NULL, 1)) == NULL) + err(1, "ober_add_integer"); + if (ober_write_elements(&ber, elm) == -1) + err(1, "ober_write_elements"); + len = ober_get_writebuf(&ber, &berdata); + ober_free_elements(elm); + + memcpy(vbbuf, berdata, len); + memcpy(vbbuf + len, berdata, len); + varbind.data.octetstring.len = 2 * len; + + varbind.type = TYPE_OPAQUE; + agentx_response(ax_s, buf, NOERROR, 0, &varbind, 1); + + varbind.type = TYPE_NULL; + snmpv2_response_validate(snmp_s, 1000, community, requestid, GENERR, 1, + &varbind, 1); + ober_free(&ber); +} + void backend_getnext_selfbound(void) { diff --git a/regress/usr.sbin/snmpd/regress.h b/regress/usr.sbin/snmpd/regress.h index b63a58723d3..54f43eefcd9 100644 --- a/regress/usr.sbin/snmpd/regress.h +++ b/regress/usr.sbin/snmpd/regress.h @@ -289,6 +289,12 @@ void backend_get_close_overlap(void); void backend_get_disappear(void); void backend_get_disappear_overlap(void); void backend_get_disappear_doublesession(void); +void backend_get_octetstring_max(void); +void backend_get_octetstring_too_long(void); +void backend_get_ipaddress_too_short(void); +void backend_get_ipaddress_too_long(void); +void backend_get_opaque_non_ber(void); +void backend_get_opaque_double_value(void); void backend_getnext_selfbound(void); void backend_getnext_lowerbound(void); void backend_getnext_lowerbound_self(void); diff --git a/regress/usr.sbin/snmpd/snmp.c b/regress/usr.sbin/snmpd/snmp.c index d2e105edff0..ebdcea972c7 100644 --- a/regress/usr.sbin/snmpd/snmp.c +++ b/regress/usr.sbin/snmpd/snmp.c @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -288,7 +289,7 @@ snmpv2_response_validate(int s, int timeout, const char *community, struct varbind *varbindlist, size_t nvarbind) { struct ber_element *message, *pdu; - char buf[1024]; + char buf[100000]; size_t buflen = sizeof(buf); message = snmp_recv(s, timeout, buf, &buflen); @@ -311,22 +312,30 @@ snmp_recv(int s, int timeout, void *buf, size_t *buflen) struct ber ber = {}; struct ber_element *message; ssize_t n; + size_t ntot = 0; int ret; + again: if ((ret = poll(&pfd, 1, timeout)) == -1) err(1, "poll"); - if (ret == 0) - errx(1, "%s: timeout", __func__); - if ((n = read(s, buf, *buflen)) == -1) - err(1, "agentx read"); - - *buflen = n; + if (ret == 0) { + if (ntot == 0) + errx(1, "%s: timeout", __func__); + errc(1, ECANCELED, "%s: unable to decode message", __func__); + } + if ((n = read(s, buf + ntot, *buflen - ntot)) == -1) + err(1, "snmp read"); + ntot += n; ober_set_application(&ber, smi_application); - ober_set_readbuf(&ber, buf, n); + ober_set_readbuf(&ber, buf, ntot); - if ((message = ober_read_elements(&ber, NULL)) == NULL) + if ((message = ober_read_elements(&ber, NULL)) == NULL) { + if (errno == ECANCELED) + goto again; errx(1, "%s: unable to decode message", __func__); + } + *buflen = n; if (verbose) { printf("SNMP received(%d):\n", s); smi_debug_elements(message); diff --git a/regress/usr.sbin/snmpd/snmpd_regress.c b/regress/usr.sbin/snmpd/snmpd_regress.c index 044f843e491..2c6ff362b01 100644 --- a/regress/usr.sbin/snmpd/snmpd_regress.c +++ b/regress/usr.sbin/snmpd/snmpd_regress.c @@ -118,6 +118,12 @@ const struct { { "backend_get_disappear", backend_get_disappear }, { "backend_get_disappear_overlap", backend_get_disappear_overlap }, { "backend_get_disappear_doublesession", backend_get_disappear_doublesession }, + { "backend_get_octetstring_max", backend_get_octetstring_max }, + { "backend_get_octetstring_too_long", backend_get_octetstring_too_long }, + { "backend_get_ipaddress_too_short", backend_get_ipaddress_too_short }, + { "backend_get_ipaddress_too_long", backend_get_ipaddress_too_long }, + { "backend_get_opaque_non_ber", backend_get_opaque_non_ber }, + { "backend_get_opaque_double_value", backend_get_opaque_double_value }, { "backend_getnext_selfbound", backend_getnext_selfbound }, { "backend_getnext_lowerbound", backend_getnext_lowerbound }, { "backend_getnext_lowerbound_self", backend_getnext_lowerbound_self }, -- 2.20.1