On Thu, Oct 03, 2019 at 08:28:02AM +0200, Martijn van Duren wrote:
> Any feedback on the relayd part?
>
> On 9/25/19 8:58 AM, Martijn van Duren wrote:
> > On 9/25/19 8:55 AM, Martijn van Duren wrote:
> >> Hello,
> >>
> >> Mischa found that relayd's agentx support is pretty much unusable for
> >> the uninitiated, because you have to know the tables beforehand.
> >>
> >> I managed to track it down to two issue with both snmpd and relayd.
> >> The fix is far from proper support for agentx, but it's good enough
> >> for going on a walk.
> >>
> > Part 2:
> > If the requested OID is a predecessor of what we support and the request
> > is getnext we should retrieve the first available mib.
> >
> > OK?
> >
> > martijn@
> >
> > Index: snmp.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/relayd/snmp.c,v
> > retrieving revision 1.29
> > diff -u -p -r1.29 snmp.c
> > --- snmp.c 28 May 2017 10:39:15 -0000 1.29
> > +++ snmp.c 25 Sep 2019 06:58:21 -0000
> > @@ -321,21 +321,27 @@ snmp_agentx_process(struct agentx_handle
> >
> > bcopy(&sr.start, &oid, sizeof(oid));
> >
> > - /*
> > - * If the requested OID is not part of the registered
> > - * MIB, return "no such object", per RFC
> > - */
> > if (snmp_oid_cmp(&relaydinfooid, &oid) == -1) {
> > - if (snmp_agentx_varbind(resp, &sr.start,
> > - AGENTX_NO_SUCH_OBJECT, NULL, 0) == -1) {
> > - log_warn("%s: unable to generate"
> > - " response", __func__);
> > - snmp_agentx_pdu_free(resp);
> > - resp = NULL;
> > + /*
> > + * If the requested OID is not part of the
> > registered
> > + * MIB, return "no such object", per RFC
> > + */
> > + if (pdu->hdr->type == AGENTX_GET) {
> > + if (snmp_agentx_varbind(resp, &sr.start,
> > + AGENTX_NO_SUCH_OBJECT, NULL, 0) ==
> > -1) {
> > + log_warn("%s: unable to
> > generate"
> > + " response", __func__);
> > + snmp_agentx_pdu_free(resp);
> > + resp = NULL;
> > + }
> > + goto reply;
> > }
> > - goto reply;
> > + bcopy(&relaydinfooid, &oid, sizeof(oid));
> > + }
Up to here it mostly makes sense but...
> > + if (oid.o_n == 9) {
> > + oid.o_id[9] = 1;
> > + oid.o_n++;
> > }
It is really hard to convince myself that this check is right. I'm also
surprised that none of the other possible snmp_oid_cmp() return values are
checked (I assume that 2 would be the usual case for a GET request and a
return of 1 should trigger an error).
I wonder if there is a better way to write this to be less magic. My
argument here is that this is example code on how to integrate agentx
supoprt into other daemons and looking at this I have to say I'm sceptical
if I want such code in e.g. bgpd.
It is however OK claudio@
> > -
> > if (oid.o_n != OIDIDX_relaydInfo + 2 + 1) {
> > /* GET requests require the exact OID */
> > if (pdu->hdr->type == AGENTX_GET)
> >
>
--
:wq Claudio
