Re: snmpd: allow sending traps with SNMPv3

2021-08-16 Thread Martijn van Duren
On Mon, 2021-08-16 at 22:45 +1000, Jonathan Matthew wrote:
> On Tue, Aug 10, 2021 at 12:58:05PM +0200, Martijn van Duren wrote:
> > On Mon, 2021-08-09 at 21:44 +0200, Martijn van Duren wrote:
> > > On Tue, 2021-07-27 at 21:28 +0200, Martijn van Duren wrote:
> > > > This diff allows sending traps in SNMPv3 messages.
> > > > It defaults to the global seclevel, but it can be specified on a per
> > > > rule basis.
> > > > 
> > > > Diff requires both previous setting engineid and ober_dup diff.
> > > > 
> > > > Tested with netsnmp's snmptrapd and my WIP diff.
> > > > 
> > > > The other 2 outstanding diffs are for receiving SNMPv3 traps.
> > > > 
> > > > OK?
> > > > 
> > > > martijn@
> > > > 
> > > Resending now that the engineid diff is in.
> > > 
> > > Still awaiting the commit of ober_dup diff[0].
> > > 
> > > OK once that one goes in?
> > > 
> > > Also, rereading the diff, splitting the trap receiver in two might be a
> > > bit clutch. Once again invoking the manpage gurus.
> > > 
> > > martijn@
> > > 
> > > [0] https://marc.info/?l=openbsd-tech&m=162698527126249&w=2
> > > 
> > The listen on diff committed this morning broke this patch.
> > Updated version
> 
> I think my only concern with this is that the config syntax changes
> incompatibly, since you now have to specify 'snmpv2c' for v2c trap
> receivers.  I can think of a few alternatives, but none of them are
> great.  What you've done here seems to be the cleanest option both in
> terms of what the config looks like and the code for processing it,
> so if we're prepared to change the config syntax, I'm happy with it.
> 
To make v3 the default is the logical conclusion of the reason moves
away from SNMPv2c in general for our SNMP stack. We've disabled SNMPv2c
for listen on in snmpd(8), including trap handle. We've removed default
communities (including "trap community"), so enabling snmpv2c on a
notify listener will not even make the receiver work without explicitly
setting that community. And similar for "trap receiver", without setting
either the global "trap community" or the "trap receiver" local
community it will not work.
It would be easy enough to enable keep snmpv2c the default for trap
receiver, but considering the general direction it just seems pointless.

In other words, this change is intentional and I assumed that people
interested in OKing this diff were aware enough of the recent changes
to understand this context of changing defaults. Sorry that that wasn't
clear.

If noone else objects I'll commit as soon as I get the green light for
the libutil bump. Thanks for checking.



Re: snmpd: allow sending traps with SNMPv3

2021-08-16 Thread Jonathan Matthew
On Tue, Aug 10, 2021 at 12:58:05PM +0200, Martijn van Duren wrote:
> On Mon, 2021-08-09 at 21:44 +0200, Martijn van Duren wrote:
> > On Tue, 2021-07-27 at 21:28 +0200, Martijn van Duren wrote:
> > > This diff allows sending traps in SNMPv3 messages.
> > > It defaults to the global seclevel, but it can be specified on a per
> > > rule basis.
> > > 
> > > Diff requires both previous setting engineid and ober_dup diff.
> > > 
> > > Tested with netsnmp's snmptrapd and my WIP diff.
> > > 
> > > The other 2 outstanding diffs are for receiving SNMPv3 traps.
> > > 
> > > OK?
> > > 
> > > martijn@
> > > 
> > Resending now that the engineid diff is in.
> > 
> > Still awaiting the commit of ober_dup diff[0].
> > 
> > OK once that one goes in?
> > 
> > Also, rereading the diff, splitting the trap receiver in two might be a
> > bit clutch. Once again invoking the manpage gurus.
> > 
> > martijn@
> > 
> > [0] https://marc.info/?l=openbsd-tech&m=162698527126249&w=2
> > 
> The listen on diff committed this morning broke this patch.
> Updated version

I think my only concern with this is that the config syntax changes
incompatibly, since you now have to specify 'snmpv2c' for v2c trap
receivers.  I can think of a few alternatives, but none of them are
great.  What you've done here seems to be the cleanest option both in
terms of what the config looks like and the code for processing it,
so if we're prepared to change the config syntax, I'm happy with it.



Re: snmpd: allow sending traps with SNMPv3

2021-08-10 Thread Martijn van Duren
On Mon, 2021-08-09 at 21:44 +0200, Martijn van Duren wrote:
> On Tue, 2021-07-27 at 21:28 +0200, Martijn van Duren wrote:
> > This diff allows sending traps in SNMPv3 messages.
> > It defaults to the global seclevel, but it can be specified on a per
> > rule basis.
> > 
> > Diff requires both previous setting engineid and ober_dup diff.
> > 
> > Tested with netsnmp's snmptrapd and my WIP diff.
> > 
> > The other 2 outstanding diffs are for receiving SNMPv3 traps.
> > 
> > OK?
> > 
> > martijn@
> > 
> Resending now that the engineid diff is in.
> 
> Still awaiting the commit of ober_dup diff[0].
> 
> OK once that one goes in?
> 
> Also, rereading the diff, splitting the trap receiver in two might be a
> bit clutch. Once again invoking the manpage gurus.
> 
> martijn@
> 
> [0] https://marc.info/?l=openbsd-tech&m=162698527126249&w=2
> 
The listen on diff committed this morning broke this patch.
Updated version

In case someone wants a quick example of how to test this:
$ cat snmpd.conf
trap receiver 127.0.0.1 user test
user test authkey test1234 auth hmac-sha256 enckey test1234 enc aes
$ doas ./snmpd -df snmpd.conf  
startup
snmpe: listening on udp 0.0.0.0:161
snmpe: listening on udp :::161
snmpe 800075cb81afa75034471524f4b8c3608f47d7f5dff8f28584e99f87d8854128: ready
^C
$ doas cat /etc/snmp/snmptrapd.conf
snmpTrapdAddr 127.0.0.1
createUser -e 
0x800075cb81afa75034471524f4b8c3608f47d7f5dff8f28584e99f87d8854128 test SHA256 
test1234 AES test1234
authUser log test
$ doas snmptrapd -fLe &
[1] 72884
trapd> NET-SNMP version 5.9
$ doas ./snmpd -df snmpd.conf
snmpd> startup
snmpd> snmpe: listening on udp 0.0.0.0:161
snmpd> snmpe: listening on udp :::161
snmpd> snmpe 800075cb81afa75034471524f4b8c3608f47d7f5dff8f28584e99f87d8854128: 
ready
trapd> 2021-08-10 12:50:21 localhost [UDP: [127.0.0.1]:42772->[0.0.0.0]:0]:
trapd> SNMPv2-MIB::sysUpTime.0 = Timeticks: (2) 0:00:00.02 
SNMPv2-MIB::snmpTrapOID.0 = OID: SNMPv2-MIB::coldStart.0

Don't forget to replace my engineid with your personal one in snmptrapd.conf.

martijn@

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v
retrieving revision 1.66
diff -u -p -r1.66 parse.y
--- parse.y 10 Aug 2021 06:49:33 -  1.66
+++ parse.y 10 Aug 2021 10:57:36 -
@@ -134,11 +134,11 @@ typedef struct {
 %token HANDLE DEFAULT SRCADDR TCP UDP PFADDRFILTER PORT
 %token   STRING
 %token   NUMBER
-%typehostcmn
+%typeusmuser community optcommunity
 %typelistenproto listenflag listenflags
 %typesrcaddr port
 %typeoptwrite yesno seclevel
-%type  objtype cmd
+%type  objtype cmd hostauth hostauthv3 usmauthopts usmauthopt
 %type   oid hostoid trapoid
 %type  auth
 %type   enc
@@ -243,13 +243,13 @@ main  : LISTEN ON listen_udptcp
free($3);
}
| TRAP RECEIVER host
-   | TRAP HANDLE hostcmn trapoid cmd {
-   struct trapcmd *cmd = $5.data;
+   | TRAP HANDLE trapoid cmd {
+   struct trapcmd *cmd = $4.data;
 
-   cmd->cmd_oid = $4;
+   cmd->cmd_oid = $3;
 
if (trapcmd_add(cmd) != 0) {
-   free($4);
+   free($3);
free(cmd);
yyerror("duplicate oid");
YYERROR;
@@ -268,8 +268,8 @@ main: LISTEN ON listen_udptcp
| PFADDRFILTER yesno{
conf->sc_pfaddrfilter = $2;
}
-   | SECLEVEL seclevel {
-   conf->sc_min_seclevel = $2;
+   | seclevel {
+   conf->sc_min_seclevel = $1;
}
| USER STRING   {
const char *errstr;
@@ -701,15 +701,93 @@ hostoid   : /* empty */   
{ $$ = NULL; }
| OBJECTID oid  { $$ = $2; }
;
 
-hostcmn: /* empty */   { $$ = NULL; }
-   | COMMUNITY STRING  { $$ = $2; }
+usmuser: USER STRING   {
+   if (strlen($2) > SNMPD_MAXUSERNAMELEN) {
+   yyerror("User name too long: %s", $2);
+   free($2);
+   YYERROR;
+   }
+   $$ = $2;
+   }
+   ;
+
+community  : COMMUNITY STRING  {
+   if (strlen($2) > SNMPD_MAXCOMMUNITYLEN) {
+   yyerror("Community too long: %s", $2);
+   free($2);
+   Y

Re: snmpd: allow sending traps with SNMPv3

2021-08-09 Thread Martijn van Duren
On Tue, 2021-07-27 at 21:28 +0200, Martijn van Duren wrote:
> This diff allows sending traps in SNMPv3 messages.
> It defaults to the global seclevel, but it can be specified on a per
> rule basis.
> 
> Diff requires both previous setting engineid and ober_dup diff.
> 
> Tested with netsnmp's snmptrapd and my WIP diff.
> 
> The other 2 outstanding diffs are for receiving SNMPv3 traps.
> 
> OK?
> 
> martijn@
> 
Resending now that the engineid diff is in.

Still awaiting the commit of ober_dup diff[0].

OK once that one goes in?

Also, rereading the diff, splitting the trap receiver in two might be a
bit clutch. Once again invoking the manpage gurus.

martijn@

[0] https://marc.info/?l=openbsd-tech&m=162698527126249&w=2

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v
retrieving revision 1.65
diff -u -p -r1.65 parse.y
--- parse.y 9 Aug 2021 18:14:53 -   1.65
+++ parse.y 9 Aug 2021 19:41:38 -
@@ -134,10 +134,10 @@ typedef struct {
 %token HANDLE DEFAULT SRCADDR TCP UDP PFADDRFILTER PORT
 %token   STRING
 %token   NUMBER
-%typehostcmn
+%typeusmuser community optcommunity
 %typesrcaddr port
 %typeoptwrite yesno seclevel listenopt listenopts
-%type  objtype cmd
+%type  objtype cmd hostauth hostauthv3 usmauthopts usmauthopt
 %type   oid hostoid trapoid
 %type  auth
 %type   enc
@@ -242,13 +242,13 @@ main  : LISTEN ON listenproto
free($3);
}
| TRAP RECEIVER host
-   | TRAP HANDLE hostcmn trapoid cmd {
-   struct trapcmd *cmd = $5.data;
+   | TRAP HANDLE trapoid cmd {
+   struct trapcmd *cmd = $4.data;
 
-   cmd->cmd_oid = $4;
+   cmd->cmd_oid = $3;
 
if (trapcmd_add(cmd) != 0) {
-   free($4);
+   free($3);
free(cmd);
yyerror("duplicate oid");
YYERROR;
@@ -267,8 +267,8 @@ main: LISTEN ON listenproto
| PFADDRFILTER yesno{
conf->sc_pfaddrfilter = $2;
}
-   | SECLEVEL seclevel {
-   conf->sc_min_seclevel = $2;
+   | seclevel {
+   conf->sc_min_seclevel = $1;
}
| USER STRING   {
const char *errstr;
@@ -720,15 +720,93 @@ hostoid   : /* empty */   
{ $$ = NULL; }
| OBJECTID oid  { $$ = $2; }
;
 
-hostcmn: /* empty */   { $$ = NULL; }
-   | COMMUNITY STRING  { $$ = $2; }
+usmuser: USER STRING   {
+   if (strlen($2) > SNMPD_MAXUSERNAMELEN) {
+   yyerror("User name too long: %s", $2);
+   free($2);
+   YYERROR;
+   }
+   $$ = $2;
+   }
+   ;
+
+community  : COMMUNITY STRING  {
+   if (strlen($2) > SNMPD_MAXCOMMUNITYLEN) {
+   yyerror("Community too long: %s", $2);
+   free($2);
+   YYERROR;
+   }
+   $$ = $2;
+   }
+   ;
+
+optcommunity   : /* empty */   { $$ = NULL; }
+   | community { $$ = $1; }
+   ;
+
+usmauthopt : usmuser   {
+   $$.data = $1;
+   $$.value = -1;
+   }
+   | seclevel  {
+   $$.data = 0;
+   $$.value = $1;
+   }
+   ;
+
+usmauthopts: /* empty */   {
+   $$.data = NULL;
+   $$.value = -1;
+   }
+   | usmauthopts usmauthopt{
+   if ($2.data != NULL) {
+   if ($$.data != NULL) {
+   yyerror("user redefined");
+   free($2.data);
+   YYERROR;
+   }
+   $$.data = $2.data;
+   } else {
+   if ($$.value != -1) {
+   yyerror("seclevel redefined");
+

snmpd: allow sending traps with SNMPv3

2021-07-27 Thread Martijn van Duren
This diff allows sending traps in SNMPv3 messages.
It defaults to the global seclevel, but it can be specified on a per
rule basis.

Diff requires both previous setting engineid and ober_dup diff.

Tested with netsnmp's snmptrapd and my WIP diff.

The other 2 outstanding diffs are for receiving SNMPv3 traps.

OK?

martijn@

diff --git a/parse.y b/parse.y
index fc72546..8cea00a 100644
--- a/parse.y
+++ b/parse.y
@@ -134,10 +134,10 @@ typedef struct {
 %token HANDLE DEFAULT SRCADDR TCP UDP PFADDRFILTER PORT
 %token   STRING
 %token   NUMBER
-%typehostcmn
+%typeusmuser community optcommunity
 %typesrcaddr port
 %typeoptwrite yesno seclevel listenopt listenopts
-%type  objtype cmd
+%type  objtype cmd hostauth hostauthv3 usmauthopts usmauthopt
 %type   oid hostoid trapoid
 %type  auth
 %type   enc
@@ -242,13 +242,13 @@ main  : LISTEN ON listenproto
free($3);
}
| TRAP RECEIVER host
-   | TRAP HANDLE hostcmn trapoid cmd {
-   struct trapcmd *cmd = $5.data;
+   | TRAP HANDLE trapoid cmd {
+   struct trapcmd *cmd = $4.data;
 
-   cmd->cmd_oid = $4;
+   cmd->cmd_oid = $3;
 
if (trapcmd_add(cmd) != 0) {
-   free($4);
+   free($3);
free(cmd);
yyerror("duplicate oid");
YYERROR;
@@ -267,8 +267,8 @@ main: LISTEN ON listenproto
| PFADDRFILTER yesno{
conf->sc_pfaddrfilter = $2;
}
-   | SECLEVEL seclevel {
-   conf->sc_min_seclevel = $2;
+   | seclevel {
+   conf->sc_min_seclevel = $1;
}
| USER STRING   {
const char *errstr;
@@ -720,15 +720,93 @@ hostoid   : /* empty */   
{ $$ = NULL; }
| OBJECTID oid  { $$ = $2; }
;
 
-hostcmn: /* empty */   { $$ = NULL; }
-   | COMMUNITY STRING  { $$ = $2; }
+usmuser: USER STRING   {
+   if (strlen($2) > SNMPD_MAXUSERNAMELEN) {
+   yyerror("User name too long: %s", $2);
+   free($2);
+   YYERROR;
+   }
+   $$ = $2;
+   }
+   ;
+
+community  : COMMUNITY STRING  {
+   if (strlen($2) > SNMPD_MAXCOMMUNITYLEN) {
+   yyerror("Community too long: %s", $2);
+   free($2);
+   YYERROR;
+   }
+   $$ = $2;
+   }
+   ;
+
+optcommunity   : /* empty */   { $$ = NULL; }
+   | community { $$ = $1; }
+   ;
+
+usmauthopt : usmuser   {
+   $$.data = $1;
+   $$.value = -1;
+   }
+   | seclevel  {
+   $$.data = 0;
+   $$.value = $1;
+   }
+   ;
+
+usmauthopts: /* empty */   {
+   $$.data = NULL;
+   $$.value = -1;
+   }
+   | usmauthopts usmauthopt{
+   if ($2.data != NULL) {
+   if ($$.data != NULL) {
+   yyerror("user redefined");
+   free($2.data);
+   YYERROR;
+   }
+   $$.data = $2.data;
+   } else {
+   if ($$.value != -1) {
+   yyerror("seclevel redefined");
+   YYERROR;
+   }
+   $$.value = $2.value;
+   }
+   }
+   ;
+
+hostauthv3 : usmauthopts   {
+   if ($1.data == NULL) {
+   yyerror("user missing");
+   YYERROR;
+   }
+   $$.data = $1.data;
+   $$.value = $1.value;
+   }
+   ;
+
+hostauth   : hostauthv3{
+