Re: snmpd(8): Remove traphandler process attempt 2
ping
On Tue, 2021-01-05 at 22:24 +0100, Martijn van Duren wrote:
> With the traphandler code beaten into submission I was able to keep the
> traphandler process removal diff more manageable. This diff does a
> couple things.
> - "listen on" now accepts any combination of read, write and notify.
> This combination removes the traphandler dependency on the generic
> listen on statement, which allows an admin to run snmpd in
> traphandler-only mode. This will break existing traphandler setups,
> but considering the benefit of not having the two functionalities
> mixed is worth it to me and is fully intentional.
> The names are choosen based on viewType names from RFC 3415 (VACM)
> - The traphandler process is gone, resulting in a single dispatcher
> responsible for the initial parsing of the packets, which allows for
> better msg checking in the future. The current traphandler process
> does nothing more then what snmpe already does, but a lot crappier:
> the pledge was also too wide.
> - We now check incoming trap packets against "trap community". The
> current code does no community verification.
> - trap parsing errors are now shown in a similar fashion to the read
> and write requests.
> - traphandler now support tcp!!!
> - Minus 65 LoC
>
> With this change regress requires the following diff:
> Index: snmpd.sh
> ===
> RCS file: /cvs/src/regress/usr.sbin/snmpd/snmpd.sh,v
> retrieving revision 1.12
> diff -u -p -r1.12 snmpd.sh
> --- snmpd.sh8 Aug 2020 11:06:40 - 1.12
> +++ snmpd.sh5 Jan 2021 20:46:19 -
> @@ -65,6 +65,7 @@ cat > ${OBJDIR}/snmpd.conf < # This is config template (1) for snmpd regression testing
> # Restrict daemon to listen on localhost only
> listen on 127.0.0.1
> +listen on 127.0.0.1 notify
> listen on ::1
>
> # Specify a number of trap receivers
>
> OK?
>
> martijn@
>
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v
> retrieving revision 1.62
> diff -u -p -r1.62 parse.y
> --- parse.y 30 Oct 2020 07:43:48 - 1.62
> +++ parse.y 5 Jan 2021 21:22:12 -
> @@ -94,11 +94,10 @@ char*symget(const char *);
> struct snmpd *conf = NULL;
> static int errors = 0;
> static struct usmuser *user = NULL;
> -static char*snmpd_port = SNMPD_PORT;
>
> int host(const char *, const char *, int,
> struct sockaddr_storage *, int);
> -int listen_add(struct sockaddr_storage *, int);
> +int listen_add(struct sockaddr_storage *, int, int);
>
> typedef struct {
> union {
> @@ -121,7 +120,7 @@ typedef struct {
> %}
>
> %token INCLUDE
> -%token LISTEN ON
> +%token LISTEN ON READ WRITE NOTIFY
> %token SYSTEM CONTACT DESCR LOCATION NAME OBJECTID SERVICES RTFILTER
> %token READONLY READWRITE OCTETSTRING INTEGER COMMUNITY TRAP RECEIVER
> %token SECLEVEL NONE AUTH ENC USER AUTHKEY ENCKEY ERROR DISABLED
> @@ -130,7 +129,7 @@ typedef struct {
> %token NUMBER
> %typehostcmn
> %typesrcaddr port
> -%typeoptwrite yesno seclevel
> +%typeoptwrite yesno seclevel listenopt listenopts
> %type objtype cmd
> %type oid hostoid trapoid
> %type auth
> @@ -281,54 +280,74 @@ listenproto : UDP listen_udp
> | TCP listen_tcp
> | listen_udp
>
> -listen_udp : STRING port {
> +listenopts : /* empty */ { $$ = 0; }
> + | listenopts listenopt { $$ |= $2; }
> + ;
> +
> +listenopt : READ { $$ = ADDRESS_FLAG_READ; }
> + | WRITE { $$ = ADDRESS_FLAG_WRITE; }
> + | NOTIFY { $$ = ADDRESS_FLAG_NOTIFY; }
> + ;
> +
> +listen_udp : STRING port listenopts{
> struct sockaddr_storage ss[16];
> int nhosts, i;
> + char *port = $2;
> +
> + if (port == NULL) {
> + if ($3 == ADDRESS_FLAG_NOTIFY)
> + port = SNMPTRAP_PORT;
> + else
> + port = SNMP_PORT;
> + }
>
> - nhosts = host($1, $2, SOCK_DGRAM, ss, nitems(ss));
> + nhosts = host($1, port, SOCK_DGRAM, ss, nitems(ss));
> if (nhosts < 1) {
> yyerror("invalid address: %s", $1);
> free($1);
> - if ($2 != snmpd_port)
> - free($2);
> + free($2);
> YYERROR;
> }
> if (nhost
snmpd(8): Remove traphandler process attempt 2
With the traphandler code beaten into submission I was able to keep the
traphandler process removal diff more manageable. This diff does a
couple things.
- "listen on" now accepts any combination of read, write and notify.
This combination removes the traphandler dependency on the generic
listen on statement, which allows an admin to run snmpd in
traphandler-only mode. This will break existing traphandler setups,
but considering the benefit of not having the two functionalities
mixed is worth it to me and is fully intentional.
The names are choosen based on viewType names from RFC 3415 (VACM)
- The traphandler process is gone, resulting in a single dispatcher
responsible for the initial parsing of the packets, which allows for
better msg checking in the future. The current traphandler process
does nothing more then what snmpe already does, but a lot crappier:
the pledge was also too wide.
- We now check incoming trap packets against "trap community". The
current code does no community verification.
- trap parsing errors are now shown in a similar fashion to the read
and write requests.
- traphandler now support tcp!!!
- Minus 65 LoC
With this change regress requires the following diff:
Index: snmpd.sh
===
RCS file: /cvs/src/regress/usr.sbin/snmpd/snmpd.sh,v
retrieving revision 1.12
diff -u -p -r1.12 snmpd.sh
--- snmpd.sh8 Aug 2020 11:06:40 - 1.12
+++ snmpd.sh5 Jan 2021 20:46:19 -
@@ -65,6 +65,7 @@ cat > ${OBJDIR}/snmpd.conf < NUMBER
%typehostcmn
%typesrcaddr port
-%typeoptwrite yesno seclevel
+%typeoptwrite yesno seclevel listenopt listenopts
%type objtype cmd
%type oid hostoid trapoid
%type auth
@@ -281,54 +280,74 @@ listenproto : UDP listen_udp
| TCP listen_tcp
| listen_udp
-listen_udp : STRING port {
+listenopts : /* empty */ { $$ = 0; }
+ | listenopts listenopt { $$ |= $2; }
+ ;
+
+listenopt : READ { $$ = ADDRESS_FLAG_READ; }
+ | WRITE { $$ = ADDRESS_FLAG_WRITE; }
+ | NOTIFY { $$ = ADDRESS_FLAG_NOTIFY; }
+ ;
+
+listen_udp : STRING port listenopts{
struct sockaddr_storage ss[16];
int nhosts, i;
+ char *port = $2;
+
+ if (port == NULL) {
+ if ($3 == ADDRESS_FLAG_NOTIFY)
+ port = SNMPTRAP_PORT;
+ else
+ port = SNMP_PORT;
+ }
- nhosts = host($1, $2, SOCK_DGRAM, ss, nitems(ss));
+ nhosts = host($1, port, SOCK_DGRAM, ss, nitems(ss));
if (nhosts < 1) {
yyerror("invalid address: %s", $1);
free($1);
- if ($2 != snmpd_port)
- free($2);
+ free($2);
YYERROR;
}
if (nhosts > (int)nitems(ss))
log_warn("%s:%s resolves to more than %zu
hosts",
- $1, $2, nitems(ss));
+ $1, port, nitems(ss));
free($1);
- if ($2 != snmpd_port)
- free($2);
+ free($2);
for (i = 0; i < nhosts; i++) {
- if (listen_add(&(ss[i]), SOCK_DGRAM) == -1) {
+ if (listen_add(&(ss[i]), SOCK_DGRAM, $3) == -1)
{
yyerror("calloc");
YYERROR;
}
}
}
-listen_tcp : STRING port {
+listen_tcp : STRING port listenopts{
struct sockaddr_storage ss[16];
int nhosts, i;
+ char *port = $2;
- nhosts = host($1, $2, SOCK_STREAM, ss, nitems(ss));
+ if (port == NULL) {
+ if ($3 == ADDRESS_FLAG_NOTIFY)
+ port = SNMPTRAP_PORT;
+ else
+ port = SNMP_PORT;
+ }
+ nhosts = host($1, port, SOCK_STREAM, ss, nitems(ss));
if (nhosts < 1) {
yyerror("invalid address: %s", $1);
free($1);
- if ($2 != snmpd_port)
-
