Date: Fri, 13 Jan 2017 09:19:04 +0000 (UTC) From: Ngie Cooper <ngie@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r312058 - stable/10/contrib/bsnmp/snmpd Message-ID: <201701130919.v0D9J46j068598@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ngie Date: Fri Jan 13 09:19:04 2017 New Revision: 312058 URL: https://svnweb.freebsd.org/changeset/base/312058 Log: MFC r310586,r310587,r310588,r311381: r310586: Refactor transport sources a bit to facilitate changes coming down pipeline Add recv callback to transport layer to better facilitate code reuse and readability and for symmetry with send callback. Move recv_dgram and recv_stream to udp_recv and lsock_recv, respectively, and make the beforementioned functions recv callbacks for the udp and lsock transports, respectively. Consolidate the check_priv* functions in their relevant trans*.c source to limit scope/use. Note: this code is roughly based content from the submitter, although this was modified to be more of a direct move from snmpd/main.c to the trans_*.c sources, and to reduce unnecessary static function declarations. r310587: Fix definition for recv_dgram(..); it should be "ssize_t", not "int" I'm not sure why this wasn't flagged as an issue by the compiler, yet r310588: Fix return type for `ret` (recv callback) and sort variables by alignment Again, for reasons I don't yet understand, this is not being flagged by the compiler. Unlike the issue addressed in r310587, this problem existed prior to r310586 r311381: lsock_init_port: address issues with initializing sockaddr_un object - Use strlcpy to ensure p->name doesn't overflow sa.sun_path [*]. - Use SUN_LEN(..) instead of spelling out calculation longhand (inspired by comment by jmallett). Tested with: dgram and stream support with both bsnmpwalk and snmpwalk CID: 1006825 Modified: stable/10/contrib/bsnmp/snmpd/main.c stable/10/contrib/bsnmp/snmpd/snmpd.h stable/10/contrib/bsnmp/snmpd/trans_lsock.c stable/10/contrib/bsnmp/snmpd/trans_udp.c Directory Properties: stable/10/ (props changed) Modified: stable/10/contrib/bsnmp/snmpd/main.c ============================================================================== --- stable/10/contrib/bsnmp/snmpd/main.c Fri Jan 13 09:11:11 2017 (r312057) +++ stable/10/contrib/bsnmp/snmpd/main.c Fri Jan 13 09:19:04 2017 (r312058) @@ -1024,154 +1024,6 @@ snmp_input_consume(struct port_input *pi pi->length -= pi->consumed; } -static void -check_priv_dgram(struct port_input *pi, struct sockcred *cred) -{ - - /* process explicitly sends credentials */ - if (cred) - pi->priv = (cred->sc_euid == 0); - else - pi->priv = 0; -} - -static void -check_priv_stream(struct port_input *pi) -{ - struct xucred ucred; - socklen_t ucredlen; - - /* obtain the accept time credentials */ - ucredlen = sizeof(ucred); - - if (getsockopt(pi->fd, 0, LOCAL_PEERCRED, &ucred, &ucredlen) == 0 && - ucredlen >= sizeof(ucred) && ucred.cr_version == XUCRED_VERSION) - pi->priv = (ucred.cr_uid == 0); - else - pi->priv = 0; -} - -/* - * Input from a stream socket. - */ -static int -recv_stream(struct port_input *pi) -{ - struct msghdr msg; - struct iovec iov[1]; - ssize_t len; - - if (pi->buf == NULL) { - /* no buffer yet - allocate one */ - if ((pi->buf = buf_alloc(0)) == NULL) { - /* ups - could not get buffer. Return an error - * the caller must close the transport. */ - return (-1); - } - pi->buflen = buf_size(0); - pi->consumed = 0; - pi->length = 0; - } - - /* try to get a message */ - msg.msg_name = pi->peer; - msg.msg_namelen = pi->peerlen; - msg.msg_iov = iov; - msg.msg_iovlen = 1; - msg.msg_control = NULL; - msg.msg_controllen = 0; - msg.msg_flags = 0; - - iov[0].iov_base = pi->buf + pi->length; - iov[0].iov_len = pi->buflen - pi->length; - - len = recvmsg(pi->fd, &msg, 0); - - if (len == -1 || len == 0) - /* receive error */ - return (-1); - - pi->length += len; - - if (pi->cred) - check_priv_stream(pi); - - return (0); -} - -/* - * Input from a datagram socket. - * Each receive should return one datagram. - */ -static int -recv_dgram(struct port_input *pi, struct in_addr *laddr) -{ - u_char embuf[1000]; - char cbuf[CMSG_SPACE(SOCKCREDSIZE(CMGROUP_MAX)) + - CMSG_SPACE(sizeof(struct in_addr))]; - struct msghdr msg; - struct iovec iov[1]; - ssize_t len; - struct cmsghdr *cmsg; - struct sockcred *cred = NULL; - - if (pi->buf == NULL) { - /* no buffer yet - allocate one */ - if ((pi->buf = buf_alloc(0)) == NULL) { - /* ups - could not get buffer. Read away input - * and drop it */ - (void)recvfrom(pi->fd, embuf, sizeof(embuf), - 0, NULL, NULL); - /* return error */ - return (-1); - } - pi->buflen = buf_size(0); - } - - /* try to get a message */ - msg.msg_name = pi->peer; - msg.msg_namelen = pi->peerlen; - msg.msg_iov = iov; - msg.msg_iovlen = 1; - memset(cbuf, 0, sizeof(cbuf)); - msg.msg_control = cbuf; - msg.msg_controllen = sizeof(cbuf); - msg.msg_flags = 0; - - iov[0].iov_base = pi->buf; - iov[0].iov_len = pi->buflen; - - len = recvmsg(pi->fd, &msg, 0); - - if (len == -1 || len == 0) - /* receive error */ - return (-1); - - if (msg.msg_flags & MSG_TRUNC) { - /* truncated - drop */ - snmpd_stats.silentDrops++; - snmpd_stats.inTooLong++; - return (-1); - } - - pi->length = (size_t)len; - - for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; - cmsg = CMSG_NXTHDR(&msg, cmsg)) { - if (cmsg->cmsg_level == IPPROTO_IP && - cmsg->cmsg_type == IP_RECVDSTADDR) - memcpy(laddr, CMSG_DATA(cmsg), sizeof(struct in_addr)); - if (cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SCM_CREDS) - cred = (struct sockcred *)CMSG_DATA(cmsg); - } - - if (pi->cred) - check_priv_dgram(pi, cred); - - return (0); -} - /* * Input from a socket */ @@ -1183,43 +1035,15 @@ snmpd_input(struct port_input *pi, struc struct snmp_pdu pdu; enum snmpd_input_err ierr, ferr; enum snmpd_proxy_err perr; + ssize_t ret, slen; int32_t vi; - int ret; - ssize_t slen; #ifdef USE_TCPWRAPPERS char client[16]; #endif struct msghdr msg; struct iovec iov[1]; - char cbuf[CMSG_SPACE(sizeof(struct in_addr))]; - struct cmsghdr *cmsgp; - - /* get input depending on the transport */ - if (pi->stream) { - msg.msg_control = NULL; - msg.msg_controllen = 0; - - ret = recv_stream(pi); - } else { - struct in_addr *laddr; - - memset(cbuf, 0, CMSG_SPACE(sizeof(struct in_addr))); - msg.msg_control = cbuf; - msg.msg_controllen = CMSG_SPACE(sizeof(struct in_addr)); - cmsgp = CMSG_FIRSTHDR(&msg); - cmsgp->cmsg_len = CMSG_LEN(sizeof(struct in_addr)); - cmsgp->cmsg_level = IPPROTO_IP; - cmsgp->cmsg_type = IP_SENDSRCADDR; - laddr = (struct in_addr *)CMSG_DATA(cmsgp); - - ret = recv_dgram(pi, laddr); - - if (laddr->s_addr == 0) { - msg.msg_control = NULL; - msg.msg_controllen = 0; - } - } + ret = tport->transport->vtab->recv(pi); if (ret == -1) return (-1); Modified: stable/10/contrib/bsnmp/snmpd/snmpd.h ============================================================================== --- stable/10/contrib/bsnmp/snmpd/snmpd.h Fri Jan 13 09:11:11 2017 (r312057) +++ stable/10/contrib/bsnmp/snmpd/snmpd.h Fri Jan 13 09:19:04 2017 (r312058) @@ -193,6 +193,7 @@ struct transport_def { ssize_t (*send)(struct tport *, const u_char *, size_t, const struct sockaddr *, size_t); + ssize_t (*recv)(struct port_input *); }; struct transport { struct asn_oid index; /* transport table index */ Modified: stable/10/contrib/bsnmp/snmpd/trans_lsock.c ============================================================================== --- stable/10/contrib/bsnmp/snmpd/trans_lsock.c Fri Jan 13 09:11:11 2017 (r312057) +++ stable/10/contrib/bsnmp/snmpd/trans_lsock.c Fri Jan 13 09:19:04 2017 (r312058) @@ -33,6 +33,7 @@ #include <sys/types.h> #include <sys/queue.h> #include <sys/stat.h> +#include <sys/ucred.h> #include <sys/un.h> #include <errno.h> @@ -58,6 +59,7 @@ static void lsock_close_port(struct tpor static int lsock_init_port(struct tport *); static ssize_t lsock_send(struct tport *, const u_char *, size_t, const struct sockaddr *, size_t); +static ssize_t lsock_recv(struct port_input *); /* exported */ const struct transport_def lsock_trans = { @@ -67,7 +69,8 @@ const struct transport_def lsock_trans = lsock_stop, lsock_close_port, lsock_init_port, - lsock_send + lsock_send, + lsock_recv }; static struct transport *my_trans; @@ -302,10 +305,9 @@ lsock_init_port(struct tport *tp) return (SNMP_ERR_RES_UNAVAIL); } - strcpy(sa.sun_path, p->name); + strlcpy(sa.sun_path, p->name, sizeof(sa.sun_path)); sa.sun_family = AF_LOCAL; - sa.sun_len = strlen(p->name) + - offsetof(struct sockaddr_un, sun_path); + sa.sun_len = SUN_LEN(&sa); (void)remove(p->name); @@ -357,10 +359,9 @@ lsock_init_port(struct tport *tp) return (SNMP_ERR_GENERR); } - strcpy(sa.sun_path, p->name); + strlcpy(sa.sun_path, p->name, sizeof(sa.sun_path)); sa.sun_family = AF_LOCAL; - sa.sun_len = strlen(p->name) + - offsetof(struct sockaddr_un, sun_path); + sa.sun_len = SUN_LEN(&sa); (void)remove(p->name); @@ -418,6 +419,73 @@ lsock_send(struct tport *tp, const u_cha return (sendto(peer->input.fd, buf, len, 0, addr, addrlen)); } +static void +check_priv_stream(struct port_input *pi) +{ + struct xucred ucred; + socklen_t ucredlen; + + /* obtain the accept time credentials */ + ucredlen = sizeof(ucred); + + if (getsockopt(pi->fd, 0, LOCAL_PEERCRED, &ucred, &ucredlen) == 0 && + ucredlen >= sizeof(ucred) && ucred.cr_version == XUCRED_VERSION) + pi->priv = (ucred.cr_uid == 0); + else + pi->priv = 0; +} + +/* + * Receive something + */ +static ssize_t +lsock_recv(struct port_input *pi) +{ + struct msghdr msg; + struct iovec iov[1]; + ssize_t len; + + msg.msg_control = NULL; + msg.msg_controllen = 0; + + if (pi->buf == NULL) { + /* no buffer yet - allocate one */ + if ((pi->buf = buf_alloc(0)) == NULL) { + /* ups - could not get buffer. Return an error + * the caller must close the transport. */ + return (-1); + } + pi->buflen = buf_size(0); + pi->consumed = 0; + pi->length = 0; + } + + /* try to get a message */ + msg.msg_name = pi->peer; + msg.msg_namelen = pi->peerlen; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + msg.msg_control = NULL; + msg.msg_controllen = 0; + msg.msg_flags = 0; + + iov[0].iov_base = pi->buf + pi->length; + iov[0].iov_len = pi->buflen - pi->length; + + len = recvmsg(pi->fd, &msg, 0); + + if (len == -1 || len == 0) + /* receive error */ + return (-1); + + pi->length += len; + + if (pi->cred) + check_priv_stream(pi); + + return (0); +} + /* * Dependency to create a lsock port */ Modified: stable/10/contrib/bsnmp/snmpd/trans_udp.c ============================================================================== --- stable/10/contrib/bsnmp/snmpd/trans_udp.c Fri Jan 13 09:11:11 2017 (r312057) +++ stable/10/contrib/bsnmp/snmpd/trans_udp.c Fri Jan 13 09:19:04 2017 (r312058) @@ -32,6 +32,7 @@ */ #include <sys/types.h> #include <sys/queue.h> +#include <sys/ucred.h> #include <stdlib.h> #include <syslog.h> @@ -54,6 +55,7 @@ static void udp_close_port(struct tport static int udp_init_port(struct tport *); static ssize_t udp_send(struct tport *, const u_char *, size_t, const struct sockaddr *, size_t); +static ssize_t udp_recv(struct port_input *); /* exported */ const struct transport_def udp_trans = { @@ -63,7 +65,8 @@ const struct transport_def udp_trans = { udp_stop, udp_close_port, udp_init_port, - udp_send + udp_send, + udp_recv }; static struct transport *my_trans; @@ -218,6 +221,123 @@ udp_send(struct tport *tp, const u_char return (sendto(p->input.fd, buf, len, 0, addr, addrlen)); } +static void +check_priv_dgram(struct port_input *pi, struct sockcred *cred) +{ + + /* process explicitly sends credentials */ + if (cred) + pi->priv = (cred->sc_euid == 0); + else + pi->priv = 0; +} + +/* + * Input from a datagram socket. + * Each receive should return one datagram. + */ +static ssize_t +recv_dgram(struct port_input *pi, struct in_addr *laddr) +{ + u_char embuf[1000]; + char cbuf[CMSG_SPACE(SOCKCREDSIZE(CMGROUP_MAX)) + + CMSG_SPACE(sizeof(struct in_addr))]; + struct msghdr msg; + struct iovec iov[1]; + ssize_t len; + struct cmsghdr *cmsg; + struct sockcred *cred = NULL; + + if (pi->buf == NULL) { + /* no buffer yet - allocate one */ + if ((pi->buf = buf_alloc(0)) == NULL) { + /* ups - could not get buffer. Read away input + * and drop it */ + (void)recvfrom(pi->fd, embuf, sizeof(embuf), + 0, NULL, NULL); + /* return error */ + return (-1); + } + pi->buflen = buf_size(0); + } + + /* try to get a message */ + msg.msg_name = pi->peer; + msg.msg_namelen = pi->peerlen; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + memset(cbuf, 0, sizeof(cbuf)); + msg.msg_control = cbuf; + msg.msg_controllen = sizeof(cbuf); + msg.msg_flags = 0; + + iov[0].iov_base = pi->buf; + iov[0].iov_len = pi->buflen; + + len = recvmsg(pi->fd, &msg, 0); + + if (len == -1 || len == 0) + /* receive error */ + return (-1); + + if (msg.msg_flags & MSG_TRUNC) { + /* truncated - drop */ + snmpd_stats.silentDrops++; + snmpd_stats.inTooLong++; + return (-1); + } + + pi->length = (size_t)len; + + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; + cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (cmsg->cmsg_level == IPPROTO_IP && + cmsg->cmsg_type == IP_RECVDSTADDR) + memcpy(laddr, CMSG_DATA(cmsg), sizeof(struct in_addr)); + if (cmsg->cmsg_level == SOL_SOCKET && + cmsg->cmsg_type == SCM_CREDS) + cred = (struct sockcred *)CMSG_DATA(cmsg); + } + + if (pi->cred) + check_priv_dgram(pi, cred); + + return (0); +} + +/* + * Receive something + */ +static ssize_t +udp_recv(struct port_input *pi) +{ + struct in_addr *laddr; + struct msghdr msg; + char cbuf[CMSG_SPACE(sizeof(struct in_addr))]; + struct cmsghdr *cmsgp; + ssize_t ret; + + memset(cbuf, 0, sizeof(cbuf)); + + msg.msg_control = cbuf; + msg.msg_controllen = sizeof(cbuf); + + cmsgp = CMSG_FIRSTHDR(&msg); + cmsgp->cmsg_len = CMSG_LEN(sizeof(struct in_addr)); + cmsgp->cmsg_level = IPPROTO_IP; + cmsgp->cmsg_type = IP_SENDSRCADDR; + laddr = (struct in_addr *)CMSG_DATA(cmsgp); + + ret = recv_dgram(pi, laddr); + + if (laddr->s_addr == INADDR_ANY) { + msg.msg_control = NULL; + msg.msg_controllen = 0; + } + + return (ret); +} + /* * Port table */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201701130919.v0D9J46j068598>