From: Martijn van Duren Subject: snmpd: move engineid into a struct To: tech@openbsd.org Date: Sun, 9 Nov 2025 12:51:34 +0100 This diff is a bit of preparation for the next diff. Initially I went a bit too simplistic and simply put the engineid value and length in two separate variables inside the requiring structs. This diff bundles the 2 variables into a struct engineid. This also helps with the parse.y stuff. This diff is on top of the "move smi_getticks to appl_sysuptime" diff. It might apply without it, but not tested. OK? martijn@ diff c11d3fa04dc651329001615fdd8ee9886239e54a 033841d6245fb04a471b798741c0003f8568f241 commit - c11d3fa04dc651329001615fdd8ee9886239e54a commit + 033841d6245fb04a471b798741c0003f8568f241 blob - 9eb74e5dd588d6bcd95c3f15a8571805523747e4 blob + 36cf6db636c1ce72084d983ef4688328ae9a6190 --- usr.sbin/snmpd/Makefile +++ usr.sbin/snmpd/Makefile @@ -4,6 +4,7 @@ PROG= snmpd MAN= snmpd.8 snmpd.conf.5 SRCS= mib.y parse.y log.c snmpe.c application.c application_blocklist.c \ application_internal.c application_agentx.c ax.c \ + engine.c \ trap.c smi.c snmpd.c \ proc.c usm.c traphandler.c util.c blob - 22b890324bfb16a0977624bc5ca1f86cad6653ce blob + d3b17ba026afe2cb332d3ab85da99b5445e141c6 --- usr.sbin/snmpd/application_internal.c +++ usr.sbin/snmpd/application_internal.c @@ -530,8 +530,8 @@ struct ber_element * appl_internal_engine(struct ber_oid *oid) { if (ober_oid_cmp(&OID(MIB_snmpEngineID, 0), oid) == 0) - return ober_add_nstring(NULL, snmpd_env->sc_engineid, - snmpd_env->sc_engineid_len); + return ober_add_nstring(NULL, snmpd_env->sc_engineid.value, + snmpd_env->sc_engineid.length); else if (ober_oid_cmp(&OID(MIB_snmpEngineBoots, 0), oid) == 0) return ober_add_integer(NULL, snmpd_env->sc_engine_boots); else if (ober_oid_cmp(&OID(MIB_snmpEngineTime, 0), oid) == 0) blob - /dev/null blob + 7a6d4fe36f628994247b05825f9572c556859d9d (mode 644) --- /dev/null +++ usr.sbin/snmpd/engine.c @@ -0,0 +1,35 @@ +/* $OpenBSD$ */ + +/* + * Copyright (c) 2025 Martijn van Duren + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include + +#include "snmpd.h" + +int +engineid_cmp(struct engineid *e1, struct engineid *e2) +{ + size_t min; + int r; + + min = e1->length < e2->length ? e1->length : e2->length; + if ((r = memcmp(e1->value, e2->value, min)) != 0) + return r; + if (e1->length == e2->length) + return 0; + return e1->length < e2->length ? -1 : 1; +} blob - 3882b915ebd85bcac9b41fff40c89a9a2633c88b blob + 555bc4050d03fd1f3b54d4ea4a745cb9fa3cfc21 --- usr.sbin/snmpd/parse.y +++ usr.sbin/snmpd/parse.y @@ -138,9 +138,7 @@ static size_t ntrapcmds = 0; static struct trapaddress_sym *trapaddresses = NULL; static size_t ntrapaddresses = 0; -static uint8_t engineid[SNMPD_MAXENGINEIDLEN]; static int32_t enginepen; -static size_t engineidlen; static unsigned char sha256[SHA256_DIGEST_LENGTH]; @@ -165,6 +163,7 @@ typedef struct { } data; enum usmauth auth; enum usmpriv enc; + struct engineid engineid; } v; int lineno; } YYSTYPE; @@ -197,6 +196,7 @@ typedef struct { %type auth %type enc %type agentxopts agentxopt +%type engineid_local enginefmt_local enginefmt %% @@ -308,12 +308,11 @@ main : LISTEN ON listen_udptcp axm_entry); } | engineid_local { - if (conf->sc_engineid_len != 0) { + if (conf->sc_engineid.length != 0) { yyerror("Redefinition of engineid"); YYERROR; } - memcpy(conf->sc_engineid, engineid, engineidlen); - conf->sc_engineid_len = engineidlen; + conf->sc_engineid = $1; } | READONLY COMMUNITY STRING { if (strlcpy(conf->sc_rdcommunity, $3, @@ -593,46 +592,50 @@ agentxopts : /* empty */ { ; enginefmt : IP4 STRING { - struct in_addr addr; + in_addr_t addr; - engineid[engineidlen++] = SNMP_ENGINEID_FMT_IPv4; + $$ = (struct engineid){}; + $$.length = 4; + $$.value[$$.length++] = SNMP_ENGINEID_FMT_IPv4; if (inet_pton(AF_INET, $2, &addr) != 1) { yyerror("Invalid ipv4 address: %s", $2); free($2); YYERROR; } - memcpy(engineid + engineidlen, &addr, - sizeof(engineid) - engineidlen); - engineid[0] |= SNMP_ENGINEID_NEW; - engineidlen += sizeof(addr); + memcpy($$.value + $$.length, &addr, sizeof(addr)); + $$.length += sizeof(addr); + $$.value[0] |= SNMP_ENGINEID_NEW; free($2); } | IP6 STRING { - struct in6_addr addr; + uint8_t addr[16]; - engineid[engineidlen++] = SNMP_ENGINEID_FMT_IPv6; - if (inet_pton(AF_INET6, $2, &addr) != 1) { + $$ = (struct engineid){}; + $$.length = 4; + $$.value[$$.length++] = SNMP_ENGINEID_FMT_IPv6; + if (inet_pton(AF_INET6, $2, addr) != 1) { yyerror("Invalid ipv6 address: %s", $2); free($2); YYERROR; } - memcpy(engineid + engineidlen, &addr, - sizeof(engineid) - engineidlen); - engineid[0] |= SNMP_ENGINEID_NEW; - engineidlen += sizeof(addr); + memcpy($$.value + $$.length, addr, sizeof(addr)); + $$.length += sizeof(addr); + $$.value[0] |= SNMP_ENGINEID_NEW; free($2); } | MAC STRING { size_t i; + $$ = (struct engineid){}; + $$.length = 4; + $$.value[$$.length++] = SNMP_ENGINEID_FMT_MAC; if (strlen($2) != 5 * 3 + 2) { yyerror("Invalid mac address: %s", $2); free($2); YYERROR; } - engineid[engineidlen++] = SNMP_ENGINEID_FMT_MAC; for (i = 0; i < 6; i++) { - if (fromhexstr(engineid + engineidlen + i, + if (fromhexstr($$.value + $$.length + i, $2 + (i * 3), 2) == NULL || $2[i * 3 + 2] != (i < 5 ? ':' : '\0')) { yyerror("Invalid mac address: %s", $2); @@ -640,108 +643,119 @@ enginefmt : IP4 STRING { YYERROR; } } - engineid[0] |= SNMP_ENGINEID_NEW; - engineidlen += 6; + $$.length += 6; + $$.value[0] |= SNMP_ENGINEID_NEW; free($2); } | TEXT STRING { - size_t i, fmtstart; + size_t i; - engineid[engineidlen++] = SNMP_ENGINEID_FMT_TEXT; - for (i = 0, fmtstart = engineidlen; - i < sizeof(engineid) - engineidlen && $2[i] != '\0'; + $$ = (struct engineid){}; + $$.length = 4; + $$.value[$$.length++] = SNMP_ENGINEID_FMT_TEXT; + for (i = 0; + $$.length < sizeof($$.value) && $2[i] != '\0'; i++) { if (!isprint($2[i])) { yyerror("invalid text character"); free($2); YYERROR; } - engineid[fmtstart + i] = $2[i]; - engineidlen++; + $$.value[$$.length++] = $2[i]; } if (i == 0 || $2[i] != '\0') { yyerror("Invalid text length: %s", $2); free($2); YYERROR; } - engineid[0] |= SNMP_ENGINEID_NEW; + $$.value[0] |= SNMP_ENGINEID_NEW; free($2); } | OCTETS STRING { - if (strlen($2) / 2 > sizeof(engineid) - 1) { - yyerror("Invalid octets length: %s", $2); - free($2); - YYERROR; - } + $$ = (struct engineid){}; + $$.length = 4; + $$.value[$$.length++] = SNMP_ENGINEID_FMT_OCT; - engineid[engineidlen++] = SNMP_ENGINEID_FMT_OCT; - if (fromhexstr(engineid + engineidlen, $2, + if (strlen($2) / 2 > sizeof($$.value) - $$.length) { + yyerror("Invalid octets length: %s", $2); + free($2); + YYERROR; + } + if (fromhexstr($$.value + $$.length, $2, strlen($2)) == NULL) { yyerror("Invalid octets: %s", $2); free($2); YYERROR; } - engineidlen += strlen($2) / 2; - engineid[0] |= SNMP_ENGINEID_NEW; + $$.length += strlen($2) / 2; + $$.value[0] |= SNMP_ENGINEID_NEW; free($2); } | AGENTID STRING { + $$ = (struct engineid){}; + $$.length = 4; + if (strlen($2) / 2 != 8) { yyerror("Invalid agentid length: %s", $2); free($2); YYERROR; } - - if (fromhexstr(engineid + engineidlen, $2, + if (fromhexstr($$.value + $$.length, $2, strlen($2)) == NULL) { yyerror("Invalid agentid: %s", $2); free($2); YYERROR; } - engineidlen += 8; - engineid[0] |= SNMP_ENGINEID_OLD; + $$.length += 8; + $$.value[0] |= SNMP_ENGINEID_OLD; free($2); } | HOSTHASH STRING { + $$ = (struct engineid){}; + $$.length = 4; + if (enginepen != PEN_OPENBSD) { yyerror("hosthash only allowed for pen " "openbsd"); YYERROR; } - engineid[engineidlen++] = SNMP_ENGINEID_FMT_HH; - memcpy(engineid + engineidlen, + $$.value[$$.length++] = SNMP_ENGINEID_FMT_HH; + memcpy($$.value + $$.length, SHA256($2, strlen($2), sha256), - sizeof(engineid) - engineidlen); - engineidlen = sizeof(engineid); - engineid[0] |= SNMP_ENGINEID_NEW; + sizeof($$.value) - $$.length); + $$.length = sizeof($$.value); + $$.value[0] |= SNMP_ENGINEID_NEW; free($2); } | NUMBER STRING { + $$ = (struct engineid){}; + $$.length = 4; + if (enginepen == PEN_OPENBSD) { yyerror("%lld is only allowed when pen is not " "openbsd", $1); YYERROR; } - if ($1 < 128 || $1 > 255) { yyerror("Invalid format number: %lld\n", $1); YYERROR; } - if (strlen($2) / 2 > sizeof(engineid) - 1) { + $$.value[$$.length++] = (uint8_t)$1; + + if (strlen($2) / 2 > sizeof($$.value) - $$.length) { yyerror("Invalid octets length: %s", $2); free($2); YYERROR; } - engineid[engineidlen++] = (uint8_t)$1; - if (fromhexstr(engineid + engineidlen, $2, + if (fromhexstr($$.value + $$.length, $2, strlen($2)) == NULL) { yyerror("Invalid octets: %s", $2); free($2); YYERROR; } - engineidlen += strlen($2) / 2; - engineid[0] |= SNMP_ENGINEID_NEW; + $$.length += strlen($2) / 2; + $$.value[0] |= SNMP_ENGINEID_NEW; free($2); } ; @@ -761,12 +775,14 @@ enginefmt_local : enginefmt YYERROR; } - engineid[engineidlen++] = SNMP_ENGINEID_FMT_HH; - memcpy(engineid + engineidlen, + $$ = (struct engineid){}; + $$.length = 4; + $$.value[$$.length++] = SNMP_ENGINEID_FMT_HH; + memcpy($$.value + $$.length, SHA256(hostname, strlen(hostname), sha256), - sizeof(engineid) - engineidlen); - engineidlen = sizeof(engineid); - engineid[0] |= SNMP_ENGINEID_NEW; + sizeof($$.value) - $$.length); + $$.length = sizeof($$.value); + $$.value[0] |= SNMP_ENGINEID_NEW; } ; @@ -792,9 +808,14 @@ pen : /* empty */ { engineid_local : ENGINEID pen { uint32_t npen = htonl(enginepen); - memcpy(engineid, &npen, sizeof(enginepen)); - engineidlen = sizeof(enginepen); - } enginefmt_local + memcpy($$.value, &npen, sizeof(npen)); + $$.length = sizeof(npen); + } enginefmt_local { + $$.value[0] |= $4.value[0] & SNMP_ENGINEID_NEW; + memcpy($$.value + $$.length, $4.value + $$.length, + $4.length - $$.length); + $$.length = $4.length; + } system : SYSTEM sysmib ; @@ -1854,18 +1875,18 @@ parse_config(const char *filename, u_int flags) /* Must be identical to enginefmt_local:HOSTHASH */ - if (conf->sc_engineid_len == 0) { + if (conf->sc_engineid.length == 0) { if (gethostname(hostname, sizeof(hostname)) == -1) fatal("gethostname"); - memcpy(conf->sc_engineid, &npen, sizeof(npen)); - conf->sc_engineid_len += sizeof(npen); - conf->sc_engineid[conf->sc_engineid_len++] |= + memcpy(conf->sc_engineid.value, &npen, sizeof(npen)); + conf->sc_engineid.length += sizeof(npen); + conf->sc_engineid.value[conf->sc_engineid.length++] = SNMP_ENGINEID_FMT_HH; - memcpy(conf->sc_engineid + conf->sc_engineid_len, + memcpy(conf->sc_engineid.value + conf->sc_engineid.length, SHA256(hostname, strlen(hostname), sha256), - sizeof(conf->sc_engineid) - conf->sc_engineid_len); - conf->sc_engineid_len = sizeof(conf->sc_engineid); - conf->sc_engineid[0] |= SNMP_ENGINEID_NEW; + sizeof(conf->sc_engineid.value) - conf->sc_engineid.length); + conf->sc_engineid.length = sizeof(conf->sc_engineid.value); + conf->sc_engineid.value[0] |= SNMP_ENGINEID_NEW; } /* Setup default listen addresses */ blob - 44773fae763979f619296a7421a8ea87e7429a46 blob + 5fefd671b71a9cea26758de538109e8e871cb2e1 --- usr.sbin/snmpd/snmpd.h +++ usr.sbin/snmpd/snmpd.h @@ -179,6 +179,11 @@ struct privsep_fd { * daemon structures */ +struct engineid { + uint8_t value[SNMPD_MAXENGINEIDLEN]; + size_t length; +}; + #define MSG_HAS_AUTH(m) (((m)->sm_flags & SNMP_MSGFLAG_AUTH) != 0) #define MSG_HAS_PRIV(m) (((m)->sm_flags & SNMP_MSGFLAG_PRIV) != 0) #define MSG_SECLEVEL(m) ((m)->sm_flags & SNMP_MSGFLAG_SECMASK) @@ -219,8 +224,7 @@ struct snmp_message { long long sm_secmodel; u_int32_t sm_engine_boots; u_int32_t sm_engine_time; - uint8_t sm_ctxengineid[SNMPD_MAXENGINEIDLEN]; - size_t sm_ctxengineid_len; + struct engineid sm_ctxengineid; char sm_ctxname[SNMPD_MAXCONTEXNAMELEN+1]; /* USM */ @@ -407,8 +411,7 @@ struct snmpd { char sc_rwcommunity[SNMPD_MAXCOMMUNITYLEN]; char sc_trcommunity[SNMPD_MAXCOMMUNITYLEN]; - uint8_t sc_engineid[SNMPD_MAXENGINEIDLEN]; - size_t sc_engineid_len; + struct engineid sc_engineid; struct snmp_stats sc_stats; struct snmp_system sc_system; @@ -444,6 +447,9 @@ extern struct snmpd *snmpd_env; struct snmpd *parse_config(const char *, u_int); int cmdline_symset(char *); +/* engine.c */ +int engineid_cmp(struct engineid *, struct engineid *); + /* snmpe.c */ void snmpe(struct privsep *, struct privsep_proc *); void snmpe_shutdown(void); blob - d444c8d9b8a47d7da9242e9372d1b88241dece5b blob + 67dcfc523ad4171ad87ab71a2c21aca7cc639187 --- usr.sbin/snmpd/snmpe.c +++ usr.sbin/snmpd/snmpe.c @@ -114,7 +114,7 @@ snmpe_init(struct privsep *ps, struct privsep_proc *p, fatal("pledge"); log_info("snmpe %s: ready", - tohexstr(env->sc_engineid, env->sc_engineid_len)); + tohexstr(env->sc_engineid.value, env->sc_engineid.length)); trap_init(); } @@ -305,12 +305,14 @@ snmpe_parse(struct snmp_message *msg) } if (ober_scanf_elements(a, "{xxeS$}$", - &engineid, &msg->sm_ctxengineid_len, &ctxname, &len, + &engineid, &msg->sm_ctxengineid.length, &ctxname, &len, &msg->sm_pdu) != 0) goto parsefail; - if (msg->sm_ctxengineid_len > sizeof(msg->sm_ctxengineid)) + if (msg->sm_ctxengineid.length > + sizeof(msg->sm_ctxengineid.value)) goto parsefail; - memcpy(msg->sm_ctxengineid, engineid, msg->sm_ctxengineid_len); + memcpy(msg->sm_ctxengineid.value, engineid, + msg->sm_ctxengineid.length); if (len > SNMPD_MAXCONTEXNAMELEN) goto parsefail; memcpy(msg->sm_ctxname, ctxname, len); @@ -456,7 +458,7 @@ badversion: "request %lld", __func__, msg->sm_host, msg->sm_port, snmpe_pdutype2string(msg->sm_pdutype), msg->sm_flags, msg->sm_secmodel, msg->sm_username, - tohexstr(msg->sm_ctxengineid, msg->sm_ctxengineid_len), + tohexstr(msg->sm_ctxengineid.value, msg->sm_ctxengineid.length), msg->sm_ctxname, msg->sm_request); else log_debug("%s: %s:%hd: SNMPv%d '%s' pdutype %s request %lld", @@ -849,7 +851,7 @@ snmpe_encode(struct snmp_message *msg) pdu = epdu = ober_add_sequence(NULL); if (msg->sm_version == SNMP_V3) { if ((epdu = ober_printf_elements(epdu, "xs{", - snmpd_env->sc_engineid, snmpd_env->sc_engineid_len, + snmpd_env->sc_engineid.value, snmpd_env->sc_engineid.length, msg->sm_ctxname)) == NULL) { ober_free_elements(pdu); return -1; blob - f676ead16b9622eea34346322e562365ec058815 blob + 11446e02974afa4f87a43e7a8454f110a749b015 --- usr.sbin/snmpd/trap.c +++ usr.sbin/snmpd/trap.c @@ -109,10 +109,7 @@ trap_send(struct ber_oid *oid, struct ber_element *elm msg->sm_secmodel = SNMP_SEC_USM; msg->sm_engine_time = snmpd_engine_time(); msg->sm_engine_boots = snmpd_env->sc_engine_boots; - memcpy(msg->sm_ctxengineid, snmpd_env->sc_engineid, - snmpd_env->sc_engineid_len); - msg->sm_ctxengineid_len = - snmpd_env->sc_engineid_len; + msg->sm_ctxengineid = snmpd_env->sc_engineid; (void)strlcpy(msg->sm_username, tr->ta_usmusername, sizeof(msg->sm_username)); msg->sm_user = tr->ta_usmuser; blob - 93324b557a9aadfe6acc42c94ea2b1397bc4475a blob + b38eaa9d4b02a389eae9ea5f7c8808e4c32cf9e5 --- usr.sbin/snmpd/usm.c +++ usr.sbin/snmpd/usm.c @@ -266,10 +266,11 @@ usm_decode(struct snmp_message *msg, struct ber_elemen off_t offs, offs2; char *usmparams; size_t len; - size_t enginelen, userlen, digestlen, saltlen; + size_t userlen, digestlen, saltlen; struct ber ber; struct ber_element *usm = NULL, *next = NULL, *decr; - char *engineid; + struct engineid engineid; + char *engineidv; char *user; char *digest, *salt; u_long now; @@ -297,7 +298,7 @@ usm_decode(struct snmp_message *msg, struct ber_elemen smi_debug_elements(usm); #endif - if (ober_scanf_elements(usm, "{xiixpxx$", &engineid, &enginelen, + if (ober_scanf_elements(usm, "{xiixpxx$", &engineidv, &engineid.length, &engine_boots, &engine_time, &user, &userlen, &offs2, &digest, &digestlen, &salt, &saltlen) != 0) { *errp = "cannot decode USM params"; @@ -306,10 +307,10 @@ usm_decode(struct snmp_message *msg, struct ber_elemen } log_debug("USM: engineid '%s', engine boots %lld, engine time %lld, " - "user '%s'", tohexstr(engineid, enginelen), engine_boots, + "user '%s'", tohexstr(engineidv, engineid.length), engine_boots, engine_time, user); - if (enginelen > SNMPD_MAXENGINEIDLEN || + if (engineid.length > sizeof(engineid.value) || userlen > SNMPD_MAXUSERNAMELEN || !usm_valid_digestlen(digestlen) || (saltlen != (MSG_HAS_PRIV(msg) ? SNMP_USM_SALTLEN : 0))) { @@ -318,8 +319,8 @@ usm_decode(struct snmp_message *msg, struct ber_elemen goto done; } - if (enginelen != snmpd_env->sc_engineid_len || - memcmp(engineid, snmpd_env->sc_engineid, enginelen) != 0) { + memcpy(engineid.value, engineidv, engineid.length); + if (engineid_cmp(&snmpd_env->sc_engineid, &engineid) != 0) { *errp = "unknown engine id"; msg->sm_usmerr = OIDVAL_usmErrEngineId; stats->snmp_usmnosuchengine++; @@ -436,7 +437,7 @@ usm_encode(struct snmp_message *msg, struct ber_elemen msg->sm_engine_boots = (u_int32_t)snmpd_env->sc_engine_boots; msg->sm_engine_time = (u_int32_t)snmpd_engine_time(); if ((a = ober_printf_elements(usm, "xdds", - snmpd_env->sc_engineid, snmpd_env->sc_engineid_len, + snmpd_env->sc_engineid.value, snmpd_env->sc_engineid.length, msg->sm_engine_boots, msg->sm_engine_time, msg->sm_username)) == NULL) goto done; @@ -720,13 +721,13 @@ usm_passwd2key(const EVP_MD *md, char *passwd, int *ma assert(snmpd_env->sc_engineid_len <= SNMPD_MAXENGINEIDLEN); #endif memcpy(pwbuf, keybuf, dlen); - memcpy(pwbuf + dlen, snmpd_env->sc_engineid, - snmpd_env->sc_engineid_len); - memcpy(pwbuf + dlen + snmpd_env->sc_engineid_len, keybuf, dlen); + memcpy(pwbuf + dlen, snmpd_env->sc_engineid.value, + snmpd_env->sc_engineid.length); + memcpy(pwbuf + dlen + snmpd_env->sc_engineid.length, keybuf, dlen); if (!EVP_DigestInit_ex(ctx, md, NULL) || !EVP_DigestUpdate(ctx, pwbuf, - 2 * dlen + snmpd_env->sc_engineid_len) || + 2 * dlen + snmpd_env->sc_engineid.length) || !EVP_DigestFinal_ex(ctx, keybuf, &dlen)) { EVP_MD_CTX_free(ctx); return NULL;