Re: snmpd agentx fixes

2017-04-13 Thread Rivo Nurges 
Hi!

I have upgraded about 20 boxes to current with the patch and everything works.

Rivo

On 10/04/2017, 03:27, "Jeremie Courreges-Anglas"  wrote:


Hi,

when implementing support for multiple listening sockets, I broke AgentX
support in a rather silly way.  AgentX support is only used by relayd in
base.

Rivo Nurges (in cc) contacted me about this.  Thanks to him and reyk@
for helping me reproduce the issue.

The config below helps reproduce the problem.

# cat /etc/snmpd.conf
socket "/var/run/agentx.sock" agentx
# cat /etc/relayd.conf
snmp
table  { 127.0.0.1 }
relay "bar" {
listen on 0.0.0.0 port 1234
forward to  check icmp
}
#

There are two diffs below.

The first part is the fix for the multiple socket support.  The issue is
that in control.c SNMP data is sent down the... AgentX socket.  This
obviously doesn't fly.  We need to know which socket to use when sending
SNMP replies, so let's store it in the snmp_message context (which
already stores the remote and local addresses).  Too obvious?

The first problem tends to hide another bug: an uninitialized variable
in trap.c.  This fix was written by Rivo Nurges.

Thoughts / ok?


Index: control.c
===
RCS file: /d/cvs/src/usr.sbin/snmpd/control.c,v
retrieving revision 1.41
diff -u -p -r1.41 control.c
--- control.c   9 Jan 2017 14:49:22 -   1.41
+++ control.c   9 Apr 2017 18:28:23 -
@@ -592,7 +592,7 @@ control_dispatch_agentx(int fd, short ev
}
}
  dispatch:
-   snmpe_dispatchmsg(msg, fd);
+   snmpe_dispatchmsg(msg);
break;
}
 
Index: snmpd.h
===
RCS file: /d/cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.74
diff -u -p -r1.74 snmpd.h
--- snmpd.h 9 Jan 2017 14:49:22 -   1.74
+++ snmpd.h 9 Apr 2017 18:28:23 -
@@ -401,6 +401,7 @@ struct pfr_buffer {
 #define MSG_REPORT(m)  (((m)->sm_flags & SNMP_MSGFLAG_REPORT) 
!= 0)
 
 struct snmp_message {
+   int  sm_sock;
struct sockaddr_storage  sm_ss;
socklen_tsm_slen;
char sm_host[HOST_NAME_MAX+1];
@@ -660,7 +661,7 @@ struct kif_arp  *karp_getaddr(struct sock
 /* snmpe.c */
 voidsnmpe(struct privsep *, struct privsep_proc *);
 voidsnmpe_shutdown(void);
-voidsnmpe_dispatchmsg(struct snmp_message *, int);
+voidsnmpe_dispatchmsg(struct snmp_message *);
 
 /* trap.c */
 voidtrap_init(void);
Index: snmpe.c
===
RCS file: /d/cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.46
diff -u -p -r1.46 snmpe.c
--- snmpe.c 18 Nov 2016 16:16:39 -  1.46
+++ snmpe.c 9 Apr 2017 18:28:23 -
@@ -42,7 +42,7 @@
 voidsnmpe_init(struct privsep *, struct privsep_proc *, void *);
 int snmpe_parse(struct snmp_message *);
 int snmpe_parsevarbinds(struct snmp_message *);
-voidsnmpe_response(int, struct snmp_message *);
+voidsnmpe_response(struct snmp_message *);
 unsigned long
 snmpe_application(struct ber_element *);
 voidsnmpe_sig_handler(int sig, short, void *);
@@ -489,6 +489,7 @@ snmpe_recvmsg(int fd, short sig, void *a
if ((msg = calloc(1, sizeof(*msg))) == NULL)
return;
 
+   msg->sm_sock = fd;
msg->sm_slen = sizeof(msg->sm_ss);
if ((len = recvfromto(fd, msg->sm_data, sizeof(msg->sm_data), 0,
(struct sockaddr *)>sm_ss, >sm_slen,
@@ -520,7 +521,7 @@ snmpe_recvmsg(int fd, short sig, void *a
if (snmpe_parse(msg) == -1) {
if (msg->sm_usmerr != 0 && MSG_REPORT(msg)) {
usm_make_report(msg);
-   snmpe_response(fd, msg);
+   snmpe_response(msg);
return;
} else {
snmp_msgfree(msg);
@@ -528,22 +529,22 @@ snmpe_recvmsg(int fd, short sig, void *a
}
}
 
-   snmpe_dispatchmsg(msg, fd);
+   snmpe_dispatchmsg(msg);
 }
 
 void
-snmpe_dispatchmsg(struct snmp_message *msg, int sock)
+snmpe_dispatchmsg(struct snmp_message *msg)
 {
if (snmpe_parsevarbinds(msg) == 1)
return;
 
/* not dispatched to subagent; respond directly */
msg->sm_context = SNMP_C_GETRESP;
-   snmpe_response(sock,

snmpd agentx fixes

2017-04-09 Thread Jeremie Courreges-Anglas 

Hi,

when implementing support for multiple listening sockets, I broke AgentX
support in a rather silly way.  AgentX support is only used by relayd in
base.

Rivo Nurges (in cc) contacted me about this.  Thanks to him and reyk@
for helping me reproduce the issue.

The config below helps reproduce the problem.

# cat /etc/snmpd.conf
socket "/var/run/agentx.sock" agentx
# cat /etc/relayd.conf
snmp
table  { 127.0.0.1 }
relay "bar" {
listen on 0.0.0.0 port 1234
forward to  check icmp
}
#

There are two diffs below.

The first part is the fix for the multiple socket support.  The issue is
that in control.c SNMP data is sent down the... AgentX socket.  This
obviously doesn't fly.  We need to know which socket to use when sending
SNMP replies, so let's store it in the snmp_message context (which
already stores the remote and local addresses).  Too obvious?

The first problem tends to hide another bug: an uninitialized variable
in trap.c.  This fix was written by Rivo Nurges.

Thoughts / ok?


Index: control.c
===
RCS file: /d/cvs/src/usr.sbin/snmpd/control.c,v
retrieving revision 1.41
diff -u -p -r1.41 control.c
--- control.c   9 Jan 2017 14:49:22 -   1.41
+++ control.c   9 Apr 2017 18:28:23 -
@@ -592,7 +592,7 @@ control_dispatch_agentx(int fd, short ev
}
}
  dispatch:
-   snmpe_dispatchmsg(msg, fd);
+   snmpe_dispatchmsg(msg);
break;
}
 
Index: snmpd.h
===
RCS file: /d/cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.74
diff -u -p -r1.74 snmpd.h
--- snmpd.h 9 Jan 2017 14:49:22 -   1.74
+++ snmpd.h 9 Apr 2017 18:28:23 -
@@ -401,6 +401,7 @@ struct pfr_buffer {
 #define MSG_REPORT(m)  (((m)->sm_flags & SNMP_MSGFLAG_REPORT) != 0)
 
 struct snmp_message {
+   int  sm_sock;
struct sockaddr_storage  sm_ss;
socklen_tsm_slen;
char sm_host[HOST_NAME_MAX+1];
@@ -660,7 +661,7 @@ struct kif_arp  *karp_getaddr(struct sock
 /* snmpe.c */
 voidsnmpe(struct privsep *, struct privsep_proc *);
 voidsnmpe_shutdown(void);
-voidsnmpe_dispatchmsg(struct snmp_message *, int);
+voidsnmpe_dispatchmsg(struct snmp_message *);
 
 /* trap.c */
 voidtrap_init(void);
Index: snmpe.c
===
RCS file: /d/cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.46
diff -u -p -r1.46 snmpe.c
--- snmpe.c 18 Nov 2016 16:16:39 -  1.46
+++ snmpe.c 9 Apr 2017 18:28:23 -
@@ -42,7 +42,7 @@
 voidsnmpe_init(struct privsep *, struct privsep_proc *, void *);
 int snmpe_parse(struct snmp_message *);
 int snmpe_parsevarbinds(struct snmp_message *);
-voidsnmpe_response(int, struct snmp_message *);
+voidsnmpe_response(struct snmp_message *);
 unsigned long
 snmpe_application(struct ber_element *);
 voidsnmpe_sig_handler(int sig, short, void *);
@@ -489,6 +489,7 @@ snmpe_recvmsg(int fd, short sig, void *a
if ((msg = calloc(1, sizeof(*msg))) == NULL)
return;
 
+   msg->sm_sock = fd;
msg->sm_slen = sizeof(msg->sm_ss);
if ((len = recvfromto(fd, msg->sm_data, sizeof(msg->sm_data), 0,
(struct sockaddr *)>sm_ss, >sm_slen,
@@ -520,7 +521,7 @@ snmpe_recvmsg(int fd, short sig, void *a
if (snmpe_parse(msg) == -1) {
if (msg->sm_usmerr != 0 && MSG_REPORT(msg)) {
usm_make_report(msg);
-   snmpe_response(fd, msg);
+   snmpe_response(msg);
return;
} else {
snmp_msgfree(msg);
@@ -528,22 +529,22 @@ snmpe_recvmsg(int fd, short sig, void *a
}
}
 
-   snmpe_dispatchmsg(msg, fd);
+   snmpe_dispatchmsg(msg);
 }
 
 void
-snmpe_dispatchmsg(struct snmp_message *msg, int sock)
+snmpe_dispatchmsg(struct snmp_message *msg)
 {
if (snmpe_parsevarbinds(msg) == 1)
return;
 
/* not dispatched to subagent; respond directly */
msg->sm_context = SNMP_C_GETRESP;
-   snmpe_response(sock, msg);
+   snmpe_response(msg);
 }
 
 void
-snmpe_response(int fd, struct snmp_message *msg)
+snmpe_response(struct snmp_message *msg)
 {
struct snmp_stats   *stats = _env->sc_stats;
u_int8_t*ptr = NULL;
@@ -580,7 +581,7 @@ snmpe_response(int fd, struct snmp_messa
goto done;
 
usm_finalize_digest(msg, ptr, len);
-   len = sendtofrom(fd, ptr, len, 0,
+   len = sendtofrom(msg->sm_sock, ptr, len, 0,
(struct sockaddr *)>sm_ss, msg->sm_slen,
(struct sockaddr *)>sm_local_ss,