On Mon, 2021-03-22 at 09:00 +0000, Stuart Henderson wrote:
> On 2021/03/22 00:20, Martijn van Duren wrote:
> > There's two things I'd like to know before going further with this:
> >
> > 1) according to RFC2578 section 3.6:
> > (1) registration: the definition of a particular item is registered as
> > a particular OBJECT IDENTIFIER value, and associated with a
> > particular descriptor. After such a registration, the semantics
> > thereby associated with the value are not allowed to change, the
> > OBJECT IDENTIFIER can not be used for any other registration, and
> > the descriptor can not be changed nor associated with any other
> > registration. The following macros result in a registration:
> >
> > OBJECT-TYPE, MODULE-IDENTITY, NOTIFICATION-TYPE, OBJECT-GROUP,
> > OBJECT-IDENTITY, NOTIFICATION-GROUP, MODULE-COMPLIANCE,
> > AGENT-CAPABILITIES.
> >
> > Unless I'm misreading this section this would require a registration of
> > a new object to strictly comply with SMIv2. Is this change minor enough
> > to allow it?
>
> I suppose it depends what they mean by semantics. The type doesn't change,
> it's still an OCTET STRING, the data don't change, the only difference is
> the DISPLAY-HINT. I think in this case changing oid will cause more problems
> (breaking config for anyone aleeady monitoring this) than it solves.
>
> In terms of side-effects, the main one is that software indexing data
> storage by the name in ifDescr after display-formatting may go from hex
> bytes to readable strongs (the case with snmp_exporter->prometheus).
> But I think most cases where people actually monitor OpenBSD MIBs are
> more likely to be based on with net-snmp or possovly snmp(1) both of
> which just interpret these as STRING anyway.
>
> And in cases where this does change something it's easier to adapt to a
> display format change than an oid change.
>
> > 2) Afaik any byte sequence is technically allowed for these in pf.conf.
> > (at least for pfLabelName and pfTableName I manage to make UTF-8 work),
> > so changing this would technically mean that we wouldn't be able to
> > export these tables and labels anymore. Note that RFC2579 section 3.1
> > bullet 3 of octet-format doesn't specify how to handle invalid ascii or
> > UTF-8. In snmp(1) we went with the replacement character, but how any
> > other pieces of software would handle that, I don't know. Is this
> > change safe enough in all situations?
> >
> > Pending the answers, from my limited knowledge of pf and the MIB I
> > guess the impact on pfLogIfName and pfIfDescr is limited enough since
> > they show the actual interface name, which is always ascii anyway.
> > For pfTableName and pfLabelName it might be an idea to register a new
> > object which returns something like a best effort UTF-8 presentation
> > of the actual name and go with SnmpAdminString.
>
> Good catch about UTF-8, so these should be SnmpAdminString, I think the
> same applies about changing oid though.
For the second one I was talking about addition instead of changing,
since technically any non-NUL byte sequence can be entered. But I can
see how that's far from the intention and merely an implementation
detail.
So I think pfLogIfNamea and pfIfDescr can be DisplayString and
pfTableName and pfLabelName should be snmpAdminstring (or have
an snmpAdminString additional column). With that and the
appropriate corresponding change to REVISION/DESCRIPTION OK martijn@.
>
>
> > On Sat, 2021-03-20 at 14:29 +0000, Stuart Henderson wrote:
> > > DisplayString seems a better type for pfIfDescr and some of the other
> > > objects; this improves the display format in Prometheus snmp_exporter
> > > which uses hex values for OCTET STRING.
> > >
> > > OK?
> > >
> > >
> > > Index: OPENBSD-PF-MIB.txt
> > > ===================================================================
> > > RCS file: /cvs/src/share/snmp/OPENBSD-PF-MIB.txt,v
> > > retrieving revision 1.6
> > > diff -u -p -r1.6 OPENBSD-PF-MIB.txt
> > > --- OPENBSD-PF-MIB.txt 19 Jun 2018 10:08:45 -0000 1.6
> > > +++ OPENBSD-PF-MIB.txt 20 Mar 2021 14:24:11 -0000
> > > @@ -23,7 +23,7 @@ IMPORTS
> > > TimeTicks, enterprises
> > > FROM SNMPv2-SMI
> > >
> > > - TruthValue
> > > + TruthValue, DisplayString
> > > FROM SNMPv2-TC
> > >
> > > openBSD
> > > @@ -33,7 +33,7 @@ IMPORTS
> > > FROM SNMPv2-CONF;
> > >
> > > pfMIBObjects MODULE-IDENTITY
> > > - LAST-UPDATED "201506091728Z"
> > > + LAST-UPDATED "202103201421Z"
> > > ORGANIZATION "OpenBSD"
> > > CONTACT-INFO "
> > > Author: Joel Knight
> > > @@ -43,6 +43,8 @@ pfMIBObjects MODULE-IDENTITY
> > > DESCRIPTION "The MIB module for gathering information from
> > > OpenBSD's packet filter.
> > > "
> > > + REVISION "202103201421Z"
> > > + DESCRIPTION "Use DisplayString not OCTET STRING where appropriate"
> > > REVISION "201506091728Z"
> > > DESCRIPTION "Add separate counter for failed 'route-to' applications"
> > > REVISION "201308310446Z"
> > > @@ -300,7 +302,7 @@ pfStateRemovals OBJECT-TYPE
> > > -- pfLogInterface
> > >
> > > pfLogIfName OBJECT-TYPE
> > > - SYNTAX OCTET STRING
> > > + SYNTAX DisplayString
> > > MAX-ACCESS read-only
> > > STATUS current
> > > DESCRIPTION
> > > @@ -683,7 +685,7 @@ pfIfEntry OBJECT-TYPE
> > > PfIfEntry ::=
> > > SEQUENCE {
> > > pfIfIndex Integer32,
> > > - pfIfDescr OCTET STRING,
> > > + pfIfDescr DisplayString,
> > > pfIfType INTEGER,
> > > pfIfRefs Unsigned32,
> > > pfIfRules Unsigned32,
> > > @@ -719,7 +721,7 @@ pfIfIndex OBJECT-TYPE
> > > ::= { pfIfEntry 1 }
> > >
> > > pfIfDescr OBJECT-TYPE
> > > - SYNTAX OCTET STRING
> > > + SYNTAX DisplayString
> > > MAX-ACCESS read-only
> > > STATUS current
> > > DESCRIPTION
> > > @@ -913,7 +915,7 @@ pfTblEntry OBJECT-TYPE
> > > TblEntry ::=
> > > SEQUENCE {
> > > pfTblIndex Integer32,
> > > - pfTblName OCTET STRING,
> > > + pfTblName DisplayString,
> > > pfTblAddresses Integer32,
> > > pfTblAnchorRefs Integer32,
> > > pfTblRuleRefs Integer32,
> > > @@ -947,7 +949,7 @@ pfTblIndex OBJECT-TYPE
> > > ::= { pfTblEntry 1 }
> > >
> > > pfTblName OBJECT-TYPE
> > > - SYNTAX OCTET STRING
> > > + SYNTAX DisplayString
> > > MAX-ACCESS read-only
> > > STATUS current
> > > DESCRIPTION
> > > @@ -1363,7 +1365,7 @@ pfLabelEntry OBJECT-TYPE
> > > PfLabelEntry ::=
> > > SEQUENCE {
> > > pfLabelIndex Integer32,
> > > - pfLabelName OCTET STRING,
> > > + pfLabelName DisplayString,
> > > pfLabelEvals Counter64,
> > > pfLabelPkts Counter64,
> > > pfLabelBytes Counter64,
> > > @@ -1383,7 +1385,7 @@ pfLabelIndex OBJECT-TYPE
> > > ::= { pfLabelEntry 1 }
> > >
> > > pfLabelName OBJECT-TYPE
> > > - SYNTAX OCTET STRING
> > > + SYNTAX DisplayString
> > > MAX-ACCESS read-only
> > > STATUS current
> > > DESCRIPTION
> > >
> >
> >
>