Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: snmpd: move engineid into a struct
To:
Martijn van Duren <openbsd+tech@list.imperialat.at>
Cc:
tech@openbsd.org, Jonathan Matthew <jonathan@d14n.org>
Date:
Wed, 24 Dec 2025 15:54:58 +0100

Download raw body.

Thread
  • Theo Buehler:

    snmpd: move smi_getticks to appl_sysuptime

  • On Mon, Dec 22, 2025 at 12:16:43PM +0100, Martijn van Duren wrote:
    > On 11/9/25 12:51, Martijn van Duren wrote:
    > > 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@
    > > 
    > Updated diff. jmatthew@ found that the pen vanished from the engineid.
    > Simplified the problem boils down to this:
    > foo: bar {
    > 	$$[0] = 1;
    > } baz {
    > 	$$[1] = 2
    > }
    > Where I expected the result to be {1, 2}, but I got {0, 2}. I merged
    > the two bodies, which gives a cleaner result anyway, but I'm still
    > curious how this is supposed to work. If anyone has an idea, feel free
    > to speak up.
    
    Is the engine.c in the diff below the latest version? There was a
    missing cvs add and the tree is broken with
    
    ===> usr.sbin/snmpd
    make: don't know how to make engine.c (prerequisite of: engine.o)
    Stop in usr.sbin/snmpd
    
    
    > 
    > martijn@
    > 
    > diff dbe785e855fce23d7cc7127ca7e4cb70b9645c4a 871d5adbfad53ded0f58777e5c0f2f2aab7c0e09
    > commit - dbe785e855fce23d7cc7127ca7e4cb70b9645c4a
    > commit + 871d5adbfad53ded0f58777e5c0f2f2aab7c0e09
    > 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 - e60680cf78961b4d1f39abaca101922a3cc5f1da
    > blob + 0fb33b7c810d74a712c9647c12f77ee9721ea840
    > --- 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 <martijn@openbsd.org>
    > + *
    > + * 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 <string.h>
    > +
    > +#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 + 9261eb4b8af192cfef62a2941e57b3ee116cbe1b
    > --- 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	<v.auth>	auth
    >  %type	<v.enc>		enc
    >  %type	<v.ax>		agentxopts agentxopt
    > +%type	<v.engineid>	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;
    >  		}
    >  		;
    >  
    > @@ -789,12 +805,16 @@ pen		: /* empty */			{
    >  		}
    >  		;
    >  
    > -engineid_local	: ENGINEID pen			{
    > +engineid_local	: ENGINEID pen enginefmt_local		{
    >  			uint32_t npen = htonl(enginepen);
    >  
    > -			memcpy(engineid, &npen, sizeof(enginepen));
    > -			engineidlen = sizeof(enginepen);
    > -		} enginefmt_local
    > +			memcpy($$.value, &npen, sizeof(npen));
    > +			$$.length = sizeof(npen);
    > +			$$.value[0] |= $3.value[0] & SNMP_ENGINEID_NEW;
    > +			memcpy($$.value + $$.length, $3.value + $$.length,
    > +			    $3.length - $$.length);
    > +			$$.length = $3.length;
    > +		}
    >  
    >  system		: SYSTEM sysmib
    >  		;
    > @@ -1854,18 +1874,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 - c659ee3e36b6fa8ee357341a1b2846b943a946d6
    > blob + c3d86da65b2f9bd630e8a422b5d14a206d82ffd8
    > --- 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 - 7bfccc193a8ccf8880a217d0e5b3320289e09133
    > blob + 469b8abdd94b6387a2a0991d8ea60070ad5d9a40
    > --- 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;
    > 
    
    
  • Theo Buehler:

    snmpd: move smi_getticks to appl_sysuptime