From 50f52dc341ca8d0ea91f485bc1a529dcba263991 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Thu, 24 Sep 2026 11:46:32 +0200 Subject: [PATCH 1/2] confd: reject line breaks in SNMP community strings snmpd.conf is line oriented and confd writes these values verbatim, so a newline in a community or security name appended directives of the operator's choosing, granting write access to a read-only agent. Restrict the leaves that reach the file, derived from the original types so the RFC 7407 lengths still apply. /system/contact and /system/location are ietf-system free text, dropped at render instead. Signed-off-by: Joachim Wiberg --- src/confd/src/snmp.c | 33 +++++++++++++++++++++++----- src/confd/yang/confd/infix-snmp.yang | 30 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/confd/src/snmp.c b/src/confd/src/snmp.c index aa8e76224..1ee0e720e 100644 --- a/src/confd/src/snmp.c +++ b/src/confd/src/snmp.c @@ -29,6 +29,25 @@ /* Default when /snmp/engine/listen names no port. */ #define SNMP_PORT "161" +/* + * infix-snmp restricts the leaves we own so a line break cannot reach + * here, but /system/contact and /system/location belong to ietf-system + * and are free text. Drop anything that would not sit on one line of + * snmpd.conf rather than let it add a directive. + */ +static int safe(const char *str) +{ + if (!str) + return 0; + + for (; *str; str++) { + if (!isprint((unsigned char)*str)) + return 0; + } + + return 1; +} + static int is_v6(const char *addr) { return strchr(addr, ':') != NULL; @@ -157,11 +176,13 @@ static const char *community_name(struct lyd_node *community) const char *name = lydx_get_cattr(community, "text-name"); if (name) - return name; + return safe(name) ? name : NULL; if (lydx_get_cattr(community, "binary-name")) return NULL; - return lydx_get_cattr(community, "security-name"); + name = lydx_get_cattr(community, "security-name"); + + return safe(name) ? name : NULL; } /* @@ -174,7 +195,7 @@ static const char *community_secname(struct lyd_node *snmp, struct lyd_node *com const char *secname = lydx_get_cattr(community, "security-name"); const char *tag; - if (!secname || !community_name(community)) + if (!safe(secname) || !community_name(community)) return NULL; tag = lydx_get_cattr(community, "target-tag"); @@ -193,7 +214,7 @@ static void communities(FILE *fp, struct lyd_node *snmp) const char *secname, *name, *tag; secname = lydx_get_cattr(community, "security-name"); - if (!secname) + if (!safe(secname)) continue; name = community_name(community); @@ -305,10 +326,10 @@ static int generate(struct lyd_node *config, struct lyd_node *snmp) system = lydx_get_xpathf(config, XPATH_SYSTEM_); if (system) { str = lydx_get_cattr(system, "contact"); - if (str) + if (str && safe(str)) fprintf(fp, "syscontact %s\n", str); str = lydx_get_cattr(system, "location"); - if (str) + if (str && safe(str)) fprintf(fp, "syslocation %s\n", str); } diff --git a/src/confd/yang/confd/infix-snmp.yang b/src/confd/yang/confd/infix-snmp.yang index 706b60527..ec21f4022 100644 --- a/src/confd/yang/confd/infix-snmp.yang +++ b/src/confd/yang/confd/infix-snmp.yang @@ -31,6 +31,36 @@ module infix-snmp { reference "internal"; } + /* + * The agent is configured by generating snmpd.conf, which is line + * oriented, so a line break in any of these would start a directive + * of the operator's choosing. Derive from the original types so the + * lengths RFC 7407 gives them still apply. + */ + deviation "/snmp:snmp/snmp:community/snmp:index" { + deviate replace { + type snmp:identifier { + pattern '[^\n\r]*'; + } + } + } + + deviation "/snmp:snmp/snmp:community/snmp:security-name" { + deviate replace { + type snmp:security-name { + pattern '[^\n\r]*'; + } + } + } + + deviation "/snmp:snmp/snmp:community/snmp:name/snmp:text-name/snmp:text-name" { + deviate replace { + type string { + pattern '[^\n\r]*'; + } + } + } + deviation "/snmp:snmp/snmp:vacm" { deviate not-supported; description "Access control is not view-based here. Every configured From 34daad0a64ecb8328451451512128051dc372095 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Thu, 24 Sep 2026 11:46:34 +0200 Subject: [PATCH 2/2] netsnmp: build without SET support The agent is read-only by design. Remove SET from the build so a mistake in the generated VACM configuration cannot make it writable. Signed-off-by: Joachim Wiberg --- external.mk | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/external.mk b/external.mk index 20d65312e..071cfee47 100644 --- a/external.mk +++ b/external.mk @@ -18,6 +18,13 @@ endef FRR_POST_BUILD_HOOKS += FRR_POST_BUILD_HOOK +# +# The SNMP agent is read-only, see doc/snmp.md. Drop SET support from +# the build rather than leave it to the generated VACM configuration to +# withhold, so a mistake there cannot become a writable agent. +# +NETSNMP_CONF_OPTS += --enable-read-only + # # External pre-built toolchains do not carry their own license. #