Let the config parser make use of the mib_string2oid().
authormartijn <martijn@openbsd.org>
Tue, 6 Feb 2024 15:36:11 +0000 (15:36 +0000)
committermartijn <martijn@openbsd.org>
Tue, 6 Feb 2024 15:36:11 +0000 (15:36 +0000)
If a descriptor is not found in the loaded MIB files it falls back to
the old smi_oid2string(), which then throws a deprecation warning. This
won't trigger for most cases in the default install, but the
UCD-DISKIO-MIB and dependencies aren't included (yet?) (which can be
fixed by manually including them via "mib directory") and there's a
couple of misspellings (e.g. mib_2 vs mib-2, and
usmStatsNotInTimeWindow vs usmStatsNotInTimeWindows).

Feedback and OK tb@

usr.sbin/snmpd/parse.y
usr.sbin/snmpd/snmpd.h
usr.sbin/snmpd/trap.c
usr.sbin/snmpd/traphandler.c

index 6eeb5c8..f3b0925 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: parse.y,v 1.88 2024/02/06 12:44:27 martijn Exp $      */
+/*     $OpenBSD: parse.y,v 1.89 2024/02/06 15:36:11 martijn Exp $      */
 
 /*
  * Copyright (c) 2007, 2008, 2012 Reyk Floeter <reyk@openbsd.org>
@@ -97,16 +97,53 @@ struct sym {
 int             symset(const char *, const char *, int);
 char           *symget(const char *);
 
+struct oid_sym {
+       char                    *descriptor;
+       char                     file[PATH_MAX];
+       int                      lineno;
+};
+
+struct object_sym {
+       struct oid_sym           oid;
+       char                    *name;
+       int                      isint;
+       union {
+               int32_t          intval;
+               char            *sval;
+       };
+};
+
+struct trapcmd_sym {
+       struct oid_sym   oid;
+       struct trapcmd  *cmd;
+};
+
+struct trapaddress_sym {
+       struct oid_sym   oid;
+       struct trap_address *tr;
+};
+
 struct snmpd                   *conf = NULL;
 static int                      errors = 0;
 static struct usmuser          *user = NULL;
 static int                      mibparsed = 0;
-static struct ber_oid          *smi_object;
+
+static struct oid_sym          *blocklist = NULL;
+static size_t                   nblocklist = 0;
+static struct oid_sym           sysoid = {};
+static struct object_sym       *objects = NULL;
+static size_t                   nobjects = 0;
+static struct trapcmd_sym      *trapcmds = NULL;
+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;
 
+int             resolve_oid(struct ber_oid *, struct oid_sym *);
+int             resolve_oids(void);
 int             host(const char *, const char *, int, int,
                    struct sockaddr_storage *, int);
 int             listen_add(struct sockaddr_storage *, int, int);
@@ -117,7 +154,7 @@ typedef struct {
                char            *string;
                struct host     *host;
                struct timeval   tv;
-               struct ber_oid  *oid;
+               struct oid_sym   oid;
                struct agentx_master ax;
                struct {
                        int              type;
@@ -308,31 +345,31 @@ main              : LISTEN ON listen_udptcp
                }
                | TRAP RECEIVER host
                | TRAP HANDLE trapoid cmd {
-                       struct trapcmd *cmd = $4.data;
-
-                       cmd->cmd_oid = $3;
+                       struct trapcmd_sym *ttrapcmds;
 
-                       if (trapcmd_add(cmd) != 0) {
-                               free($3);
-                               free(cmd);
-                               yyerror("duplicate oid");
+                       if ((ttrapcmds = recallocarray(trapcmds, ntrapcmds,
+                           ntrapcmds + 1, sizeof(*trapcmds))) == NULL) {
+                               yyerror("malloc");
+                               free($3.descriptor);
+                               free($4.data);
                                YYERROR;
                        }
-                       conf->sc_traphandler = 1;
+
+                       trapcmds = ttrapcmds;
+                       trapcmds[ntrapcmds].oid = $3;
+                       trapcmds[ntrapcmds++].cmd = $4.data;
                }
                | BLOCKLIST oid {
-                       struct ber_oid *blocklist;
+                       struct oid_sym *tblocklist;
 
-                       blocklist = recallocarray(conf->sc_blocklist,
-                           conf->sc_nblocklist, conf->sc_nblocklist + 1,
-                           sizeof(*blocklist));
-                       if (blocklist == NULL) {
+                       if ((tblocklist = recallocarray(blocklist, nblocklist,
+                           nblocklist + 1, sizeof(*blocklist))) == NULL) {
                                yyerror("malloc");
+                               free($2.descriptor);
                                YYERROR;
                        }
-                       conf->sc_blocklist = blocklist;
-                       blocklist[conf->sc_nblocklist++] = *$2;
-                       free($2);
+                       blocklist = tblocklist;
+                       blocklist[nblocklist++] = $2;
                }
                | RTFILTER yesno                {
                        conf->sc_rtfilter = $2;
@@ -821,13 +858,12 @@ sysmib            : CONTACT STRING                {
                        free($2);
                }
                | OBJECTID oid                  {
-                       if (conf->sc_system.sys_oid.bo_n != 0) {
+                       if (sysoid.descriptor != NULL) {
                                yyerror("system oid already defined");
-                               free($2);
+                               free($2.descriptor);
                                YYERROR;
                        }
-                       conf->sc_system.sys_oid = *$2;
-                       free($2);
+                       sysoid = $2;
                }
                | SERVICES NUMBER               {
                        if (conf->sc_system.sys_services != -1) {
@@ -846,24 +882,22 @@ sysmib            : CONTACT STRING                {
                ;
 
 object         : OBJECTID oid NAME STRING optwrite     {
-                       const char *error;
+                       struct object_sym *tobjects;
 
-                       smi_object = $2;
-                       error = smi_insert($2, $4);
-                       free($4);
-                       if (error != NULL) {
-                               yyerror("%s", error);
-                               free($2);
-                               YYERROR;
+                       if ((tobjects = recallocarray(objects, nobjects,
+                           nobjects + 1, sizeof(*objects))) == NULL) {
+                               yyerror("malloc");
+                               free($2.descriptor);
+                               free($4);
                        }
-               } objectvalue {
-                       free(smi_object);
-               }
+                       objects = tobjects;
+                       nobjects++;
+                       objects[nobjects - 1].oid = $2;
+                       objects[nobjects - 1].name = $4;
+               } objectvalue
                ;
 
 objectvalue    : INTEGER NUMBER                        {
-                       const char *error;
-
                        if ($2 < INT32_MIN) {
                                yyerror("number too small");
                                YYERROR;
@@ -872,22 +906,12 @@ objectvalue       : INTEGER NUMBER                        {
                                yyerror("number too large");
                                YYERROR;
                        }
-                       error = appl_internal_object_int(smi_object, $2);
-                       if (error != NULL) {
-                               yyerror("%s", error);
-                               YYERROR;
-                       }
+                       objects[nobjects - 1].isint = 1;
+                       objects[nobjects - 1].intval = $2;
                }
                | OCTETSTRING STRING                    {
-                       const char *error;
-
-                       error = appl_internal_object_string(smi_object, $2);
-                       if (error != NULL) {
-                               yyerror("%s", error);
-                               free($2);
-                               YYERROR;
-                       }
-                       
+                       objects[nobjects - 1].isint = 0;
+                       objects[nobjects - 1].sval = $2;
                }
                ;
 
@@ -896,37 +920,28 @@ optwrite  : READONLY                              { $$ = 0; }
                ;
 
 oid            : STRING                                {
-                       struct ber_oid  *oid;
-                       if ((oid = calloc(1, sizeof(*oid))) == NULL) {
-                               yyerror("calloc");
-                               free($1);
-                               YYERROR;
-                       }
-                       if (smi_string2oid($1, oid) == -1) {
-                               yyerror("invalid OID: %s", $1);
-                               free(oid);
-                               free($1);
-                               YYERROR;
-                       }
-                       free($1);
-                       $$ = oid;
+                       $$.descriptor = $1;
+                       strlcpy($$.file, file->name, sizeof($$.file));
+                       $$.lineno = file->lineno;
                }
                ;
 
 trapoid                : oid                                   { $$ = $1; }
                | DEFAULT                               {
-                       struct ber_oid  *sysoid;
-                       if ((sysoid =
-                           calloc(1, sizeof(*sysoid))) == NULL) {
-                               yyerror("calloc");
+                       if (($$.descriptor = strdup("1.3")) == NULL) {
+                               yyerror("malloc");
                                YYERROR;
                        }
-                       ober_string2oid("1.3", sysoid);
-                       $$ = sysoid;
+                       strlcpy($$.file, file->name, sizeof($$.file));
+                       $$.lineno = file->lineno;
                }
                ;
 
-hostoid                : /* empty */                           { $$ = NULL; }
+hostoid                : /* empty */                           {
+                       $$.descriptor = NULL;
+                       strlcpy($$.file, file->name, sizeof($$.file));
+                       $$.lineno = file->lineno;
+               }
                | OBJECTID oid                          { $$ = $2; }
                ;
 
@@ -1017,41 +1032,54 @@ srcaddr         : /* empty */                           { $$ = NULL; }
                ;
 
 hostdef                : STRING hostoid hostauth srcaddr       {
-                       struct sockaddr_storage ss;
+                       struct sockaddr_storage ss, ssl = {};
                        struct trap_address *tr;
-
-                       if ((tr = calloc(1, sizeof(*tr))) == NULL) {
-                               yyerror("calloc");
-                               YYERROR;
-                       }
+                       struct trapaddress_sym *ttrapaddresses;
 
                        if (host($1, SNMPTRAP_PORT, AF_UNSPEC, SOCK_DGRAM,
                            &ss, 1) <= 0) {
                                yyerror("invalid host: %s", $1);
                                free($1);
-                               free($2);
+                               free($2.descriptor);
                                free($3.data);
                                free($4);
-                               free(tr);
                                YYERROR;
                        }
                        free($1);
-                       memcpy(&(tr->ta_ss), &ss, sizeof(ss));
                        if ($4 != NULL) {
                                if (host($4, "0", ss.ss_family, SOCK_DGRAM,
                                    &ss, 1) <= 0) {
                                        yyerror("invalid source-address: %s",
                                            $4);
-                                       free($2);
+                                       free($2.descriptor);
                                        free($3.data);
                                        free($4);
-                                       free(tr);
                                        YYERROR;
                                }
                                free($4);
-                               memcpy(&(tr->ta_sslocal), &ss, sizeof(ss));
                        }
-                       tr->ta_oid = $2;
+
+                       ttrapaddresses = reallocarray(trapaddresses,
+                           ntrapaddresses + 1, sizeof(*trapaddresses));
+                       if (ttrapaddresses == NULL) {
+                               yyerror("malloc");
+                               free($2.descriptor);
+                               free($3.data);
+                               YYERROR;
+                       }
+                       trapaddresses = ttrapaddresses;
+                       ntrapaddresses++;
+
+                       if ((tr = calloc(1, sizeof(*tr))) == NULL) {
+                               yyerror("calloc");
+                               free($2.descriptor);
+                               free($3.data);
+                               ntrapaddresses--;
+                               YYERROR;
+                       }
+
+                       tr->ta_ss = ss;
+                       tr->ta_sslocal = ssl;
                        tr->ta_version = $3.type;
                        if ($3.type == SNMP_V2) {
                                (void)strlcpy(tr->ta_community, $3.data,
@@ -1061,7 +1089,9 @@ hostdef           : STRING hostoid hostauth srcaddr       {
                                tr->ta_usmusername = $3.data;
                                tr->ta_seclevel = $3.value;
                        }
-                       TAILQ_INSERT_TAIL(&(conf->sc_trapreceivers), tr, entry);
+
+                       trapaddresses[ntrapaddresses - 1].oid = $2;
+                       trapaddresses[ntrapaddresses - 1].tr = tr;
                }
                ;
 
@@ -1641,6 +1671,105 @@ popfile(void)
        return (file ? 0 : EOF);
 }
 
+int
+resolve_oid(struct ber_oid *dst, struct oid_sym *src)
+{
+       struct file f = { .name = src->file, };
+       const char *error;
+
+       file = &f;
+       yylval.lineno = src->lineno;
+
+       if ((error = mib_string2oid(src->descriptor, dst)) != NULL) {
+               if (smi_string2oid(src->descriptor, dst) == -1) {
+                       yyerror("%s", error);
+                       free(src->descriptor);
+                       return -1;
+               }
+               yyerror("deprecated oid format");
+       }
+       free(src->descriptor);
+
+       return 0;
+}
+
+int
+resolve_oids(void)
+{
+       struct file f;
+       struct ber_oid oid;
+       const char *error;
+       size_t i;
+
+       conf->sc_blocklist = calloc(nblocklist, sizeof(*conf->sc_blocklist));
+       if (conf->sc_blocklist == NULL)
+               fatal("malloc");
+
+       conf->sc_nblocklist = nblocklist;
+       for (i = 0; i < nblocklist; i++) {
+               if (resolve_oid(&conf->sc_blocklist[i], &blocklist[i]) == -1)
+                       return -1;
+       }
+       free(blocklist);
+
+       if (sysoid.descriptor != NULL) {
+               if (resolve_oid(&conf->sc_system.sys_oid, &sysoid) == -1)
+                       return -1;
+       }
+
+       for (i = 0; i < nobjects; i++) {
+               if (resolve_oid(&oid, &objects[i].oid) == -1)
+                       return -1;
+               file = &f;
+               f.name = objects[i].oid.file;
+               yylval.lineno = objects[i].oid.lineno;
+               if ((error = smi_insert(&oid, objects[i].name)) != NULL) {
+                       yyerror("%s", error);
+                       return -1;
+               }
+               if (objects[i].isint) {
+                       if ((error = appl_internal_object_int(
+                           &oid, objects[i].intval)) != NULL) {
+                               yyerror("%s", error);
+                               return -1;
+                       }
+               } else {
+                       if ((error = appl_internal_object_string(
+                           &oid, objects[i].sval)) != NULL) {
+                               yyerror("%s", error);
+                               return -1;
+                       }
+               }
+               free(objects[i].name);
+       }
+       free(objects);
+
+       for (i = 0; i < ntrapcmds; i++) {
+               if (resolve_oid(
+                   &trapcmds[i].cmd->cmd_oid, &trapcmds[i].oid) == -1)
+                       return -1;
+               f.name = trapcmds[i].oid.file;
+               yylval.lineno = trapcmds[i].oid.lineno;
+               file = &f;
+               if (trapcmd_add(trapcmds[i].cmd) != 0) {
+                       yyerror("duplicate oid");
+                       return -1;
+               }
+       }
+       free(trapcmds);
+
+       for (i = 0; i < ntrapaddresses; i++) {
+               if (resolve_oid(
+                   &trapaddresses[i].tr->ta_oid, &trapaddresses[i].oid) == -1)
+                       return -1;
+               TAILQ_INSERT_TAIL(&conf->sc_trapreceivers,
+                   trapaddresses[i].tr, entry);
+       }
+       free(trapaddresses);
+
+       return 0;
+}
+
 struct snmpd *
 parse_config(const char *filename, u_int flags)
 {
@@ -1662,8 +1791,10 @@ parse_config(const char *filename, u_int flags)
 
        conf->sc_system.sys_services = -1;
        conf->sc_flags = flags;
+
        conf->sc_oidfmt =
            flags & SNMPD_F_NONAMES ?  MIB_OIDNUMERIC : MIB_OIDSYMBOLIC;
+
        conf->sc_confpath = filename;
        TAILQ_INIT(&conf->sc_addresses);
        TAILQ_INIT(&conf->sc_agentx_masters);
@@ -1683,10 +1814,20 @@ parse_config(const char *filename, u_int flags)
 
        endservent();
 
+       if (errors) {
+               free(conf);
+               return (NULL);
+       }
+
        if (!mibparsed)
                mib_parsedir("/usr/share/snmp/mibs");
        mib_resolve();
 
+       if (resolve_oids() == -1) {
+               free(conf);
+               return NULL;
+       }
+
        if (uname(&u) == -1)
                fatal("uname");
 
@@ -1742,12 +1883,12 @@ parse_config(const char *filename, u_int flags)
                if (h->flags & ADDRESS_FLAG_NOTIFY)
                        found = 1;
        }
-       if (conf->sc_traphandler && !found) {
+       if (ntrapcmds && !found) {
                log_warnx("trap handler needs at least one notify listener");
                free(conf);
                return (NULL);
        }
-       if (!conf->sc_traphandler && found) {
+       if (!ntrapcmds && found) {
                log_warnx("notify listener needs at least one trap handler");
                free(conf);
                return (NULL);
@@ -1785,11 +1926,6 @@ parse_config(const char *filename, u_int flags)
                }
        }
 
-       if (errors) {
-               free(conf);
-               return (NULL);
-       }
-
        return (conf);
 }
 
index 300b91f..d8c56ed 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: snmpd.h,v 1.118 2024/02/06 12:44:28 martijn Exp $     */
+/*     $OpenBSD: snmpd.h,v 1.119 2024/02/06 15:36:11 martijn Exp $     */
 
 /*
  * Copyright (c) 2007, 2008, 2012 Reyk Floeter <reyk@openbsd.org>
@@ -341,7 +341,7 @@ struct trap_address {
                        int              ta_seclevel;
                };
        };
-       struct ber_oid          *ta_oid;
+       struct ber_oid           ta_oid;
 
        TAILQ_ENTRY(trap_address) entry;
 };
@@ -428,7 +428,7 @@ struct snmpd {
 };
 
 struct trapcmd {
-       struct ber_oid          *cmd_oid;
+       struct ber_oid           cmd_oid;
                /* sideways return for intermediate lookups */
        struct trapcmd          *cmd_maybe;
 
index 469b330..c68ada5 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: trap.c,v 1.42 2024/02/06 12:44:28 martijn Exp $       */
+/*     $OpenBSD: trap.c,v 1.43 2024/02/06 15:36:11 martijn Exp $       */
 
 /*
  * Copyright (c) 2008 Reyk Floeter <reyk@openbsd.org>
@@ -67,9 +67,9 @@ trap_send(struct ber_oid *oid, struct ber_element *elm)
                ober_link_elements(vblist, elm);
 
        TAILQ_FOREACH(tr, &snmpd_env->sc_trapreceivers, entry) {
-               if (tr->ta_oid != NULL && tr->ta_oid->bo_n) {
+               if (tr->ta_oid.bo_n) {
                        /* The trap receiver may want only a specified MIB */
-                       r = ober_oid_cmp(oid, tr->ta_oid);
+                       r = ober_oid_cmp(oid, &tr->ta_oid);
                        if (r != 0 && r != 2)
                                continue;
                }
index 681c035..efa73c9 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: traphandler.c,v 1.26 2024/02/06 12:44:28 martijn Exp $        */
+/*     $OpenBSD: traphandler.c,v 1.27 2024/02/06 15:36:11 martijn Exp $        */
 
 /*
  * Copyright (c) 2014 Bret Stephen Lambert <blambert@openbsd.org>
@@ -333,7 +333,7 @@ trapcmd_exec(struct trapcmd *cmd, struct sockaddr *sa,
 
        if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, s) == -1) {
                log_warn("could not create pipe for OID '%s'",
-                   mib_oid2string(cmd->cmd_oid, oidbuf, sizeof(oidbuf),
+                   mib_oid2string(&cmd->cmd_oid, oidbuf, sizeof(oidbuf),
                    snmpd_env->sc_oidfmt));
                return;
        }
@@ -352,14 +352,14 @@ trapcmd_exec(struct trapcmd *cmd, struct sockaddr *sa,
 
                /* this shouldn't happen */
                log_warn("could not exec trap command for OID '%s'",
-                   mib_oid2string(cmd->cmd_oid, oidbuf, sizeof(oidbuf),
+                   mib_oid2string(&cmd->cmd_oid, oidbuf, sizeof(oidbuf),
                    snmpd_env->sc_oidfmt));
                _exit(1);
                /* NOTREACHED */
 
        case -1:
                log_warn("could not fork trap command for OID '%s'",
-                   mib_oid2string(cmd->cmd_oid, oidbuf, sizeof(oidbuf),
+                   mib_oid2string(&cmd->cmd_oid, oidbuf, sizeof(oidbuf),
                    snmpd_env->sc_oidfmt));
                close(s[0]);
                close(s[1]);
@@ -427,7 +427,7 @@ trapcmd_lookup(struct ber_oid *oid)
        struct trapcmd  key, *res;
 
        bzero(&key, sizeof(key));
-       key.cmd_oid = oid;
+       key.cmd_oid = *oid;
 
        if ((res = RB_FIND(trapcmd_tree, &trapcmd_tree, &key)) == NULL)
                res = key.cmd_maybe;
@@ -439,7 +439,7 @@ trapcmd_cmp(struct trapcmd *cmd1, struct trapcmd *cmd2)
 {
        int ret;
 
-       ret = ober_oid_cmp(cmd1->cmd_oid, cmd2->cmd_oid);
+       ret = ober_oid_cmp(&cmd1->cmd_oid, &cmd2->cmd_oid);
        switch (ret) {
        case 2:
                /* cmd1 is a child of cmd2 */
@@ -462,6 +462,5 @@ trapcmd_free(struct trapcmd *cmd)
 {
        RB_REMOVE(trapcmd_tree, &trapcmd_tree, cmd);
        free(cmd->cmd_argv);
-       free(cmd->cmd_oid);
        free(cmd);
 }