On Fri, Feb 12, 2021 at 10:03:21AM +0100, Martijn van Duren wrote:
> ping
>
> On Sun, 2021-01-31 at 11:57 +0100, Martijn van Duren wrote:
> > Now that ober_scanf_elements supports '$' lets use it.
> >
> > Here's a first stab by adding it to snmpd.
> > Passing regress and a few manual checks.
> >
> > 'e' still doesn't consume the element, but I've talked it over with
> > rob@, who said that shouldn't get in the way of using this new feature.
> >
> > OK?
Looks reasonable and I guess you verified the layout of all those ASN.1
messages to ensure the $ is at the right place.
Side note: I wonder why does } not imply the $? At least now with S it
would be possible to enforce this.
I like that you closed a lot of open { format strings. What about these:
> > - if (ober_scanf_elements(usm, "{xiixpxx", &engineid, &enginelen,
> > + if (ober_scanf_elements(usm, "{xiixpxx$", &engineid, &enginelen,
Wouldn't it be better to use "{xiixpxx$}" here?
> > Index: snmpe.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> > retrieving revision 1.68
> > diff -u -p -r1.68 snmpe.c
> > --- snmpe.c 22 Jan 2021 06:33:27 -0000 1.68
> > +++ snmpe.c 31 Jan 2021 10:55:49 -0000
> > @@ -220,7 +220,7 @@ snmpe_parse(struct snmp_message *msg)
> > case SNMP_V2:
> > if (env->sc_min_seclevel != 0)
> > goto badversion;
> > - if (ober_scanf_elements(a, "se", &comn, &msg->sm_pdu) != 0)
> > + if (ober_scanf_elements(a, "seS$", &comn, &msg->sm_pdu) !=
> > 0)
> > goto parsefail;
> > if (strlcpy(msg->sm_community, comn,
> > sizeof(msg->sm_community)) >=
> > sizeof(msg->sm_community)) {
> > @@ -230,7 +230,7 @@ snmpe_parse(st? tm_udp.c
> > Index: snmpe.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> > retrieving revision 1.68
> > diff -u -p -r1.68 snmpe.c
> > --- snmpe.c 22 Jan 2021 06:33:27 -0000 1.68
> > +++ snmpe.c 31 Jan 2021 10:55:49 -0000
> > @@ -220,7 +220,7 @@ snmpe_parse(struct snmp_message *msg)
> > case SNMP_V2:
> > if (env->sc_min_seclevel != 0)
> > goto badversion;
> > - if (ober_scanf_elements(a, "se", &comn, &msg->sm_pdu) != 0)
> > + if (ober_scanf_elements(a, "seS$", &comn, &msg->sm_pdu) !=
> > 0)
> > goto parsefail;
> > if (strlcpy(msg->sm_community, comn,
> > sizeof(msg->sm_community)) >=
> > sizeof(msg->sm_community)) {
> > @@ -230,7 +230,7 @@ snmpe_parse(struct snmp_message *msg)
> > }
> > break;
> > case SNMP_V3:
> > - if (ober_scanf_elements(a, "{iisi}e",
> > + if (ober_scanf_elements(a, "{iisi$}e",
> > &msg->sm_msgid, &msg->sm_max_msg_size, &flagstr,
> > &msg->sm_secmodel, &a) != 0)
> > goto parsefail;
> > @@ -248,7 +248,7 @@ snmpe_parse(struct snmp_message *msg)
> > goto parsefail;
> > }
> >
> > - if (ober_scanf_elements(a, "{xxe",
> > + if (ober_scanf_elements(a, "{xxeS$}$",
> > &msg->sm_ctxengineid, &msg->sm_ctxengineid_len,
> > &ctxname, &len, &msg->sm_pdu) != 0)
> > goto parsefail;
> > @@ -370,7 +370,7 @@ snmpe_parse(struct snmp_message *msg)
> > }
> >
> > /* SNMP PDU */
> > - if (ober_scanf_elements(a, "iiie{et",
> > + if (ober_scanf_elements(a, "iiie{etS$}$",
> > &req, &errval, &erridx, &msg->sm_pduend,
> > &msg->sm_varbind, &class, &type) != 0) {
> > stats->snmp_silentdrops++;
> > @@ -429,7 +429,7 @@ snmpe_parsevarbinds(struct snmp_message
> >
> > for (i = 1; varbind != NULL && i < SNMPD_MAXVARBIND;
> > varbind = varbind->be_next, i++) {
> > - if (ober_scanf_elements(varbind, "{oe}", &o, &value) == -1)
> > {
> > + if (ober_scanf_elements(varbind, "{oeS$}", &o, &value) ==
> > -1) {
> > stats->snmp_inasnparseerrs++;
> > msg->sm_errstr = "invalid varbind";
> > goto varfail;
> > Index: traphandler.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 traphandler.c
> > --- traphandler.c 22 Jan 2021 06:33:27 -0000 1.20
> > +++ traphandler.c 31 Jan 2021 10:55:49 -0000
> > @@ -67,7 +67,7 @@ traphandler_parse(struct snmp_message *m
> > struct privsep *ps = &snmpd_env->sc_ps;
> > struct snmp_stats *stats = &snmpd_env->sc_stats;
> > struct ber ber = {0};
> > - struct ber_element *vblist = NULL, *elm, *elm2;
> > + struct ber_element *vblist = NULL, *elm;
> > struct ber_oid o1, o2, snmpTrapOIDOID;
> > struct ber_oid snmpTrapOID, sysUpTimeOID;
> > int sysUpTime;
> > @@ -82,7 +82,7 @@ traphandler_parse(struct snmp_message *m
> > goto done;
> > break;
> > case SNMP_C_TRAPV2:
> > - if (ober_scanf_elements(msg->sm_pdu, "{SSe}", &elm) == -1) {
> > + if (ober_scanf_elements(msg->sm_pdu, "{SSe}$", &elm) == -1)
> > {
> > stats->snmp_inasnparseerrs++;
> > goto done;
> > }
> > @@ -98,7 +98,7 @@ traphandler_parse(struct snmp_message *m
> >
> > (void)ober_string2oid("1.3.6.1.2.1.1.3.0", &sysUpTimeOID);
> > (void)ober_string2oid("1.3.6.1.6.3.1.1.4.1.0", &snmpTrapOIDOID);
> > - if (ober_scanf_elements(vblist, "{{od}{oo}", &o1, &sysUpTime, &o2,
> > + if (ober_scanf_elements(vblist, "{{od$}{oo$}", &o1, &sysUpTime, &o2,
> > &snmpTrapOID) == -1 ||
> > ober_oid_cmp(&o1, &sysUpTimeOID) != 0 ||
> > ober_oid_cmp(&o2, &snmpTrapOIDOID) != 0) {
> > @@ -107,8 +107,7 @@ traphandler_parse(struct snmp_message *m
> > }
> > (void)ober_scanf_elements(vblist, "{Se", &elm);
> > for (elm = elm->be_next; elm != NULL; elm = elm->be_next) {
> > - if (ober_scanf_elements(elm, "{oe}", &o1, &elm2) == -1 ||
> > - elm2->be_next != NULL) {
> > + if (ober_scanf_elements(elm, "{oS$}", &o1) == -1) {
> > stats->snmp_inasnparseerrs++;
> > goto done;
> > }
> > @@ -153,7 +152,7 @@ traphandler_v1translate(struct snmp_mess
> > int generic_trap, specific_trap, time_stamp;
> > int hasaddress = 0, hascommunity = 0, hasenterprise = 0;
> >
> > - if (ober_scanf_elements(msg->sm_pdu, "{oxddde", &enterprise,
> > + if (ober_scanf_elements(msg->sm_pdu, "{oxdddeS$}$", &enterprise,
> > &agent_addr, &agent_addrlen, &generic_trap, &specific_trap,
> > &time_stamp, &vblist) == -1 ||
> > agent_addrlen != 4 ||
> > @@ -379,7 +378,7 @@ trapcmd_exec(struct trapcmd *cmd, struct
> > goto out;
> >
> > for (; vb != NULL; vb = vb->be_next) {
> > - if (ober_scanf_elements(vb, "{oe}", &oid, &elm) == -1)
> > + if (ober_scanf_elements(vb, "{oeS$}", &oid, &elm) == -1)
> > goto out;
> > if ((value = smi_print_element(elm)) == NULL)
> > goto out;
> > Index: usm.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/usm.c,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 usm.c
> > --- usm.c 24 Oct 2019 12:39:27 -0000 1.17
> > +++ usm.c 31 Jan 2021 10:55:49 -0000
> > @@ -302,7 +302,7 @@ usm_decode(struct snmp_message *msg, str
> > smi_debug_elements(usm);
> > #endif
> >
> > - if (ober_scanf_elements(usm, "{xiixpxx", &engineid, &enginelen,
> > + if (ober_scanf_elements(usm, "{xiixpxx$", &engineid, &enginelen,
> > &engine_boots, &engine_time, &user, &userlen, &offs2,
> > &digest, &digestlen, &salt, &saltlen) != 0) {
> > *errp = "cannot decode USM params";
> > ? tm_udp.c
> > Index: snmpe.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> > retrieving revision 1.68
> > diff -u -p -r1.68 snmpe.c
> > --- snmpe.c 22 Jan 2021 06:33:27 -0000 1.68
> > +++ snmpe.c 31 Jan 2021 10:55:49 -0000
> > @@ -220,7 +220,7 @@ snmpe_parse(struct snmp_message *msg)
> > case SNMP_V2:
> > if (env->sc_min_seclevel != 0)
> > goto badversion;
> > - if (ober_scanf_elements(a, "se", &comn, &msg->sm_pdu) != 0)
> > + if (ober_scanf_elements(a, "seS$", &comn, &msg->sm_pdu) !=
> > 0)
> > goto parsefail;
> > if (strlcpy(msg->sm_community, comn,
> > sizeof(msg->sm_community)) >=
> > sizeof(msg->sm_community)) {
> > @@ -230,7 +230,7 @@ snmpe_parse(struct snmp_message *msg)
> > }
> > break;
> > case SNMP_V3:
> > - if (ober_scanf_elements(a, "{iisi}e",
> > + if (ober_scanf_elements(a, "{iisi$}e",
> > &msg->sm_msgid, &msg->sm_max_msg_size, &flagstr,
> > &msg->sm_secmodel, &a) != 0)
> > goto parsefail;
> > @@ -248,7 +248,7 @@ snmpe_parse(struct snmp_message *msg)
> > goto parsefail;
> > }
> >
> > - if (ober_scanf_elements(a, "{xxe",
> > + if (ober_scanf_elements(a, "{xxeS$}$",
> > &msg->sm_ctxengineid, &msg->sm_ctxengineid_len,
> > &ctxname, &len, &msg->sm_pdu) != 0)
> > goto parsefail;
> > @@ -370,7 +370,7 @@ snmpe_parse(struct snmp_message *msg)
> > }
> >
> > /* SNMP PDU */
> > - if (ober_scanf_elements(a, "iiie{et",
> > + if (ober_scanf_elements(a, "iiie{etS$}$",
> > &req, &errval, &erridx, &msg->sm_pduend,
> > &msg->sm_varbind, &class, &type) != 0) {
> > stats->snmp_silentdrops++;
> > @@ -429,7 +429,7 @@ snmpe_parsevarbinds(struct snmp_message
> >
> > for (i = 1; varbind != NULL && i < SNMPD_MAXVARBIND;
> > varbind = varbind->be_next, i++) {
> > - if (ober_scanf_elements(varbind, "{oe}", &o, &value) == -1)
> > {
> > + if (ober_scanf_elements(varbind, "{oeS$}", &o, &value) ==
> > -1) {
> > stats->snmp_inasnparseerrs++;
> > msg->sm_errstr = "invalid varbind";
> > goto varfail;
> > Index: traphandler.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 traphandler.c
> > --- traphandler.c 22 Jan 2021 06:33:27 -0000 1.20
> > +++ traphandler.c 31 Jan 2021 10:55:49 -0000
> > @@ -67,7 +67,7 @@ traphandler_parse(struct snmp_message *m
> > struct privsep *ps = &snmpd_env->sc_ps;
> > struct snmp_stats *stats = &snmpd_env->sc_stats;
> > struct ber ber = {0};
> > - struct ber_element *vblist = NULL, *elm, *elm2;
> > + struct ber_element *vblist = NULL, *elm;
> > struct ber_oid o1, o2, snmpTrapOIDOID;
> > struct ber_oid snmpTrapOID, sysUpTimeOID;
> > int sysUpTime;
> > @@ -82,7 +82,7 @@ traphandler_parse(struct snmp_message *m
> > goto done;
> > break;
> > case SNMP_C_TRAPV2:
> > - if (ober_scanf_elements(msg->sm_pdu, "{SSe}", &elm) == -1) {
> > + if (ober_scanf_elements(msg->sm_pdu, "{SSe}$", &elm) == -1)
> > {
> > stats->snmp_inasnparseerrs++;
> > goto done;
> > }
> > @@ -98,7 +98,7 @@ traphandler_parse(struct snmp_message *m
> >
> > (void)ober_string2oid("1.3.6.1.2.1.1.3.0", &sysUpTimeOID);
> > (void)ober_string2oid("1.3.6.1.6.3.1.1.4.1.0", &snmpTrapOIDOID);
> > - if (ober_scanf_elements(vblist, "{{od}{oo}", &o1, &sysUpTime, &o2,
> > + if (ober_scanf_elements(vblist, "{{od$}{oo$}", &o1, &sysUpTime, &o2,
> > &snmpTrapOID) == -1 ||
> > ober_oid_cmp(&o1, &sysUpTimeOID) != 0 ||
> > ober_oid_cmp(&o2, &snmpTrapOIDOID) != 0) {
> > @@ -107,8 +107,7 @@ traphandler_parse(struct snmp_message *m
> > }
> > (void)ober_scanf_elements(vblist, "{Se", &elm);
> > for (elm = elm->be_next; elm != NULL; elm = elm->be_next) {
> > - if (ober_scanf_elements(elm, "{oe}", &o1, &elm2) == -1 ||
> > - elm2->be_next != NULL) {
> > + if (ober_scanf_elements(elm, "{oS$}", &o1) == -1) {
> > stats->snmp_inasnparseerrs++;
> > goto done;
> > }
> > @@ -153,7 +152,7 @@ traphandler_v1translate(struct snmp_mess
> > int generic_trap, specific_trap, time_stamp;
> > int hasaddress = 0, hascommunity = 0, hasenterprise = 0;
> >
> > - if (ober_scanf_elements(msg->sm_pdu, "{oxddde", &enterprise,
> > + if (ober_scanf_elements(msg->sm_pdu, "{oxdddeS$}$", &enterprise,
> > &agent_addr, &agent_addrlen, &generic_trap, &specific_trap,
> > &time_stamp, &vblist) == -1 ||
> > agent_addrlen != 4 ||
> > @@ -379,7 +378,7 @@ trapcmd_exec(struct trapcmd *cmd, struct
> > goto out;
> >
> > for (; vb != NULL; vb = vb->be_next) {
> > - if (ober_scanf_elements(vb, "{oe}", &oid, &elm) == -1)
> > + if (ober_scanf_elements(vb, "{oeS$}", &oid, &elm) == -1)
> > goto out;
> > if ((value = smi_print_element(elm)) == NULL)
> > goto out;
> > Index: usm.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/usm.c,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 usm.c
> > --- usm.c 24 Oct 2019 12:39:27 -0000 1.17
> > +++ usm.c 31 Jan 2021 10:55:49 -0000
> > @@ -302,7 +302,7 @@ usm_decode(struct snmp_message *msg, str
> > smi_debug_elements(usm);
> > #endif
> >
> > - if (ober_scanf_elements(usm, "{xiixpxx", &engineid, &enginelen,
> > + if (ober_scanf_elements(usm, "{xiixpxx$", &engineid, &enginelen,
> > &engine_boots, &engine_time, &user, &userlen, &offs2,
> > &digest, &digestlen, &salt, &saltlen) != 0) {
> > *errp = "cannot decode USM params";
> >
> > ruct snmp_message *msg)
> > }
> > break;
> > case SNMP_V3:
> > - if (ober_scanf_elements(a, "{iisi}e",
> > + if (ober_scanf_elements(a, "{iisi$}e",
> > &msg->sm_msgid, &msg->sm_max_msg_size, &flagstr,
> > &msg->sm_secmodel, &a) != 0)
> > goto parsefail;
> > @@ -248,7 +248,7 @@ snmpe_parse(struct snmp_message *msg)
> > goto parsefail;
> > }
> >
> > - if (ober_scanf_elements(a, "{xxe",
> > + if (ober_scanf_elements(a, "{xxeS$}$",
> > &msg->sm_ctxengineid, &msg->sm_ctxengineid_len,
> > &ctxname, &len, &msg->sm_pdu) != 0)
> > goto parsefail;
> > @@ -370,7 +370,7 @@ snmpe_parse(struct snmp_message *msg)
> > }
> >
> > /* SNMP PDU */
> > - if (ober_scanf_elements(a, "iiie{et",
> > + if (ober_scanf_elements(a, "iiie{etS$}$",
> > &req, &errval, &erridx, &msg->sm_pduend,
> > &msg->sm_varbind, &class, &type) != 0) {
> > stats->snmp_silentdrops++;
> > @@ -429,7 +429,7 @@ snmpe_parsevarbinds(struct snmp_message
> >
> > for (i = 1; varbind != NULL && i < SNMPD_MAXVARBIND;
> > varbind = varbind->be_next, i++) {
> > - if (ober_scanf_elements(varbind, "{oe}", &o, &value) == -1)
> > {
> > + if (ober_scanf_elements(varbind, "{oeS$}", &o, &value) ==
> > -1) {
> > stats->snmp_inasnparseerrs++;
> > msg->sm_errstr = "invalid varbind";
> > goto varfail;
> > Index: traphandler.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 traphandler.c
> > --- traphandler.c 22 Jan 2021 06:33:27 -0000 1.20
> > +++ traphandler.c 31 Jan 2021 10:55:49 -0000
> > @@ -67,7 +67,7 @@ traphandler_parse(struct snmp_message *m
> > struct privsep *ps = &snmpd_env->sc_ps;
> > struct snmp_stats *stats = &snmpd_env->sc_stats;
> > struct ber ber = {0};
> > - struct ber_element *vblist = NULL, *elm, *elm2;
> > + struct ber_element *vblist = NULL, *elm;
> > struct ber_oid o1, o2, snmpTrapOIDOID;
> > struct ber_oid snmpTrapOID, sysUpTimeOID;
> > int sysUpTime;
> > @@ -82,7 +82,7 @@ traphandler_parse(struct snmp_message *m
> > goto done;
> > break;
> > case SNMP_C_TRAPV2:
> > - if (ober_scanf_elements(msg->sm_pdu, "{SSe}", &elm) == -1) {
> > + if (ober_scanf_elements(msg->sm_pdu, "{SSe}$", &elm) == -1)
> > {
> > stats->snmp_inasnparseerrs++;
> > goto done;
> > }
> > @@ -98,7 +98,7 @@ traphandler_parse(struct snmp_message *m
> >
> > (void)ober_string2oid("1.3.6.1.2.1.1.3.0", &sysUpTimeOID);
> > (void)ober_string2oid("1.3.6.1.6.3.1.1.4.1.0", &snmpTrapOIDOID);
> > - if (ober_scanf_elements(vblist, "{{od}{oo}", &o1, &sysUpTime, &o2,
> > + if (ober_scanf_elements(vblist, "{{od$}{oo$}", &o1, &sysUpTime, &o2,
> > &snmpTrapOID) == -1 ||
> > ober_oid_cmp(&o1, &sysUpTimeOID) != 0 ||
> > ober_oid_cmp(&o2, &snmpTrapOIDOID) != 0) {
> > @@ -107,8 +107,7 @@ traphandler_parse(struct snmp_message *m
> > }
> > (void)ober_scanf_elements(vblist, "{Se", &elm);
> > for (elm = elm->be_next; elm != NULL; elm = elm->be_next) {
> > - if (ober_scanf_elements(elm, "{oe}", &o1, &elm2) == -1 ||
> > - elm2->be_next != NULL) {
> > + if (ober_scanf_elements(elm, "{oS$}", &o1) == -1) {
> > stats->snmp_inasnparseerrs++;
> > goto done;
> > }
> > @@ -153,7 +152,7 @@ traphandler_v1translate(struct snmp_mess
> > int generic_trap, specific_trap, time_stamp;
> > int hasaddress = 0, hascommunity = 0, hasenterprise = 0;
> >
> > - if (ober_scanf_elements(msg->sm_pdu, "{oxddde", &enterprise,
> > + if (ober_scanf_elements(msg->sm_pdu, "{oxdddeS$}$", &enterprise,
> > &agent_addr, &agent_addrlen, &generic_trap, &specific_trap,
> > &time_stamp, &vblist) == -1 ||
> > agent_addrlen != 4 ||
> > @@ -379,7 +378,7 @@ trapcmd_exec(struct trapcmd *cmd, struct
> > goto out;
> >
> > for (; vb != NULL; vb = vb->be_next) {
> > - if (ober_scanf_elements(vb, "{oe}", &oid, &elm) == -1)
> > + if (ober_scanf_elements(vb, "{oeS$}", &oid, &elm) == -1)
> > goto out;
> > if ((value = smi_print_element(elm)) == NULL)
> > goto out;
> > Index: usm.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/usm.c,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 usm.c
> > --- usm.c 24 Oct 2019 12:39:27 -0000 1.17
> > +++ usm.c 31 Jan 2021 10:55:49 -0000
> > @@ -302,7 +302,7 @@ usm_decode(struct snmp_message *msg, str
> > smi_debug_elements(usm);
> > #endif
> >
> > - if (ober_scanf_elements(usm, "{xiixpxx", &engineid, &enginelen,
> > + if (ober_scanf_elements(usm, "{xiixpxx$", &engineid, &enginelen,
> > &engine_boots, &engine_time, &user, &userlen, &offs2,
> > &digest, &digestlen, &salt, &saltlen) != 0) {
> > *errp = "cannot decode USM params";
> >
> >
>
>
--
:wq Claudio
