krb5 commit: Refactor pktinfo code

Greg Hudson ghudson at mit.edu
Wed Feb 17 15:38:01 EST 2016


https://github.com/krb5/krb5/commit/076d6dbc50d683be540274654b35270fa8cfc289
commit 076d6dbc50d683be540274654b35270fa8cfc289
Author: Sarah Day <sarahday at mit.edu>
Date:   Wed Feb 10 14:48:44 2016 -0500

    Refactor pktinfo code
    
    Move preprocessor conditionals outside of functions and simplify
    preprocessor statements in udppktinfo.c.

 src/lib/apputils/udppktinfo.c |  410 +++++++++++++++++++++++++++++------------
 src/lib/apputils/udppktinfo.h |    5 +-
 2 files changed, 290 insertions(+), 125 deletions(-)

diff --git a/src/lib/apputils/udppktinfo.c b/src/lib/apputils/udppktinfo.c
index 383b474..aa601fa 100644
--- a/src/lib/apputils/udppktinfo.c
+++ b/src/lib/apputils/udppktinfo.c
@@ -28,9 +28,21 @@
 #include <netinet/in.h>
 #include <sys/socket.h>
 
+#if defined(IP_PKTINFO) && defined(HAVE_STRUCT_IN_PKTINFO)
+#define HAVE_IP_PKTINFO
+#endif
+
+#if defined(IPV6_PKTINFO) && defined(HAVE_STRUCT_IN6_PKTINFO)
+#define HAVE_IPV6_PKTINFO
+#endif
+
+#if defined(HAVE_IP_PKTINFO) || defined(HAVE_IPV6_PKTINFO)
+#define HAVE_PKTINFO_SUPPORT
+#endif
+
 /* Use RFC 3542 API below, but fall back from IPV6_RECVPKTINFO to IPV6_PKTINFO
  * for RFC 2292 implementations. */
-#ifndef IPV6_RECVPKTINFO
+#if !defined(IPV6_RECVPKTINFO) && defined(IPV6_PKTINFO)
 #define IPV6_RECVPKTINFO IPV6_PKTINFO
 #endif
 
@@ -40,7 +52,7 @@
 #endif /* IP_RECVPKTINFO */
 
 #if defined(CMSG_SPACE) && defined(HAVE_STRUCT_CMSGHDR) &&      \
-(defined(IP_PKTINFO) || defined(IPV6_PKTINFO))
+    defined(HAVE_PKTINFO_SUPPORT)
 union pktinfo {
 #ifdef HAVE_STRUCT_IN6_PKTINFO
     struct in6_pktinfo pi6;
@@ -50,7 +62,62 @@ union pktinfo {
 #endif
     char c;
 };
-#endif
+#endif /* HAVE_IPV6_PKTINFO && HAVE_STRUCT_CMSGHDR && HAVE_PKTINFO_SUPPORT */
+
+/*
+ * Check if a socket is bound to a wildcard address.
+ * Returns 1 if it is, 0 if it's bound to a specific address, or -1 on error
+ * with errno set to the error.
+ */
+static int
+is_socket_bound_to_wildcard(int sock)
+{
+    struct sockaddr_storage bound_addr;
+    socklen_t bound_addr_len = sizeof(bound_addr);
+
+    if (getsockname(sock, ss2sa(&bound_addr), &bound_addr_len) < 0)
+        return -1;
+
+    switch (ss2sa(&bound_addr)->sa_family) {
+    case AF_INET:
+        return ss2sin(&bound_addr)->sin_addr.s_addr == INADDR_ANY;
+    case AF_INET6:
+        return IN6_IS_ADDR_UNSPECIFIED(&ss2sin6(&bound_addr)->sin6_addr);
+    default:
+        errno = EINVAL;
+        return -1;
+    }
+}
+
+#ifdef HAVE_IP_PKTINFO
+
+#define set_ipv4_pktinfo set_ipv4_recvpktinfo
+static inline krb5_error_code
+set_ipv4_recvpktinfo(int sock)
+{
+    int sockopt = 1;
+    return setsockopt(sock, IPPROTO_IP, IP_RECVPKTINFO, &sockopt,
+                      sizeof(sockopt));
+}
+
+#else /* HAVE_IP_PKTINFO */
+#define set_ipv4_pktinfo(s) EINVAL
+#endif /* HAVE_IP_PKTINFO */
+
+#ifdef HAVE_IPV6_PKTINFO
+
+#define set_ipv6_pktinfo set_ipv6_recvpktinfo
+static inline krb5_error_code
+set_ipv6_recvpktinfo(int sock)
+{
+    int sockopt = 1;
+    return setsockopt(sock, IPPROTO_IPV6, IPV6_RECVPKTINFO, &sockopt,
+                      sizeof(sockopt));
+}
+
+#else /* HAVE_IPV6_PKTINFO */
+#define set_ipv6_pktinfo(s) EINVAL
+#endif /* HAVE_IPV6_PKTINFO */
 
 /*
  * Set pktinfo option on a socket. Takes a socket and the socket address family
@@ -62,30 +129,92 @@ union pktinfo {
 krb5_error_code
 set_pktinfo(int sock, int family)
 {
-    int sockopt = 1;
-    int option = 0, proto = 0;
-
     switch (family) {
-#if defined(IP_PKTINFO) && defined(HAVE_STRUCT_IN_PKTINFO)
     case AF_INET:
-        proto = IPPROTO_IP;
-        option = IP_RECVPKTINFO;
-        break;
-#endif
-#if defined(IPV6_PKTINFO) && defined(HAVE_STRUCT_IN6_PKTINFO)
+        return set_ipv4_pktinfo(sock);
     case AF_INET6:
-        proto = IPPROTO_IPV6;
-        option = IPV6_RECVPKTINFO;
-        break;
-#endif
+        return set_ipv6_pktinfo(sock);
     default:
         return EINVAL;
     }
-    if (setsockopt(sock, proto, option, &sockopt, sizeof(sockopt)))
-        return errno;
+}
+
+#if defined(HAVE_PKTINFO_SUPPORT) && defined(CMSG_SPACE)
+
+#ifdef HAVE_IP_PKTINFO
+
+static inline struct in_pktinfo *
+cmsg2pktinfo(struct cmsghdr *cmsgptr)
+{
+    return (struct in_pktinfo *)(void *)CMSG_DATA(cmsgptr);
+}
+
+#define check_cmsg_v4_pktinfo check_cmsg_ip_pktinfo
+static int
+check_cmsg_ip_pktinfo(struct cmsghdr *cmsgptr, struct sockaddr *to,
+                      socklen_t *tolen, aux_addressing_info *auxaddr)
+{
+    struct in_pktinfo *pktinfo;
+
+    if (cmsgptr->cmsg_level == IPPROTO_IP &&
+        cmsgptr->cmsg_type == IP_PKTINFO &&
+        *tolen >= sizeof(struct sockaddr_in)) {
+
+        memset(to, 0, sizeof(struct sockaddr_in));
+        pktinfo = cmsg2pktinfo(cmsgptr);
+        sa2sin(to)->sin_addr = pktinfo->ipi_addr;
+        sa2sin(to)->sin_family = AF_INET;
+        *tolen = sizeof(struct sockaddr_in);
+        return 1;
+    }
     return 0;
 }
 
+#else /* HAVE_IP_PKTINFO */
+#define check_cmsg_v4_pktinfo(c, t, l, a) 0
+#endif /* HAVE_IP_PKTINFO */
+
+#ifdef HAVE_IPV6_PKTINFO
+
+static inline struct in6_pktinfo *
+cmsg2pktinfo6(struct cmsghdr *cmsgptr)
+{
+    return (struct in6_pktinfo *)(void *)CMSG_DATA(cmsgptr);
+}
+
+#define check_cmsg_v6_pktinfo check_cmsg_ipv6_pktinfo
+static int
+check_cmsg_ipv6_pktinfo(struct cmsghdr *cmsgptr, struct sockaddr *to,
+                        socklen_t *tolen, aux_addressing_info *auxaddr)
+{
+    struct in6_pktinfo *pktinfo;
+
+    if (cmsgptr->cmsg_level == IPPROTO_IPV6 &&
+        cmsgptr->cmsg_type == IPV6_PKTINFO &&
+        *tolen >= sizeof(struct sockaddr_in6)) {
+
+        memset(to, 0, sizeof(struct sockaddr_in6));
+        pktinfo = cmsg2pktinfo6(cmsgptr);
+        sa2sin6(to)->sin6_addr = pktinfo->ipi6_addr;
+        sa2sin6(to)->sin6_family = AF_INET6;
+        *tolen = sizeof(struct sockaddr_in6);
+        auxaddr->ipv6_ifindex = pktinfo->ipi6_ifindex;
+        return 1;
+    }
+    return 0;
+}
+#else /* HAVE_IPV6_PKTINFO */
+#define check_cmsg_v6_pktinfo(c, t, l, a) 0
+#endif /* HAVE_IPV6_PKTINFO */
+
+static int
+check_cmsg_pktinfo(struct cmsghdr *cmsgptr, struct sockaddr *to,
+                   socklen_t *tolen, aux_addressing_info *auxaddr)
+{
+    return check_cmsg_v4_pktinfo(cmsgptr, to, tolen, auxaddr) ||
+           check_cmsg_v6_pktinfo(cmsgptr, to, tolen, auxaddr);
+}
+
 /*
  * Receive a message from a socket.
  *
@@ -106,32 +235,27 @@ set_pktinfo(int sock, int family)
  */
 krb5_error_code
 recv_from_to(int socket, void *buf, size_t len, int flags,
-             struct sockaddr *from, socklen_t *fromlen,
-             struct sockaddr *to, socklen_t *tolen,
+             struct sockaddr *from, socklen_t * fromlen,
+             struct sockaddr *to, socklen_t * tolen,
              aux_addressing_info *auxaddr)
 
 {
-#if (!defined(IP_PKTINFO) && !defined(IPV6_PKTINFO)) || !defined(CMSG_SPACE)
-    if (to && tolen) {
-        /* Clobber with something recognizeable in case we try to use
-         *          the address.  */
-        memset(to, 0x40, *tolen);
-        *tolen = 0;
-    }
-
-    return recvfrom(socket, buf, len, flags, from, fromlen);
-#else
     int r;
     struct iovec iov;
     char cmsg[CMSG_SPACE(sizeof(union pktinfo))];
     struct cmsghdr *cmsgptr;
     struct msghdr msg;
 
-    if (!to || !tolen)
+    /* Don't use pktinfo if the socket isn't bound to a wildcard address. */
+    r = is_socket_bound_to_wildcard(socket);
+    if (r < 0)
+        return errno;
+
+    if (!to || !tolen || !r)
         return recvfrom(socket, buf, len, flags, from, fromlen);
 
-    /* Clobber with something recognizeable in case we can't extract
-     *      the address but try to use it anyways.  */
+    /* Clobber with something recognizeable in case we can't extract the
+     * address but try to use it anyways. */
     memset(to, 0x40, *tolen);
 
     iov.iov_base = buf;
@@ -149,47 +273,102 @@ recv_from_to(int socket, void *buf, size_t len, int flags,
         return r;
     *fromlen = msg.msg_namelen;
 
-    /* On Darwin (and presumably all *BSD with KAME stacks),
-     *      CMSG_FIRSTHDR doesn't check for a non-zero controllen.  RFC
-     *      3542 recommends making this check, even though the (new) spec
-     *      for CMSG_FIRSTHDR says it's supposed to do the check.  */
+    /*
+     * On Darwin (and presumably all *BSD with KAME stacks), CMSG_FIRSTHDR
+     * doesn't check for a non-zero controllen.  RFC 3542 recommends making
+     * this check, even though the (new) spec for CMSG_FIRSTHDR says it's
+     * supposed to do the check.
+     */
     if (msg.msg_controllen) {
         cmsgptr = CMSG_FIRSTHDR(&msg);
         while (cmsgptr) {
-#ifdef IP_PKTINFO
-            if (cmsgptr->cmsg_level == IPPROTO_IP
-                && cmsgptr->cmsg_type == IP_PKTINFO
-                && *tolen >= sizeof(struct sockaddr_in)) {
-                struct in_pktinfo *pktinfo;
-                memset(to, 0, sizeof(struct sockaddr_in));
-                pktinfo = (struct in_pktinfo *)CMSG_DATA(cmsgptr);
-                ((struct sockaddr_in *)to)->sin_addr = pktinfo->ipi_addr;
-                ((struct sockaddr_in *)to)->sin_family = AF_INET;
-                *tolen = sizeof(struct sockaddr_in);
-                return r;
-            }
-#endif
-#if defined(IPV6_PKTINFO) && defined(HAVE_STRUCT_IN6_PKTINFO)
-            if (cmsgptr->cmsg_level == IPPROTO_IPV6
-                && cmsgptr->cmsg_type == IPV6_PKTINFO
-                && *tolen >= sizeof(struct sockaddr_in6)) {
-                struct in6_pktinfo *pktinfo;
-                memset(to, 0, sizeof(struct sockaddr_in6));
-                pktinfo = (struct in6_pktinfo *)CMSG_DATA(cmsgptr);
-                ((struct sockaddr_in6 *)to)->sin6_addr = pktinfo->ipi6_addr;
-                ((struct sockaddr_in6 *)to)->sin6_family = AF_INET6;
-                *tolen = sizeof(struct sockaddr_in6);
-                auxaddr->ipv6_ifindex = pktinfo->ipi6_ifindex;
+            if (check_cmsg_pktinfo(cmsgptr, to, tolen, auxaddr))
                 return r;
-            }
-#endif
             cmsgptr = CMSG_NXTHDR(&msg, cmsgptr);
         }
     }
     /* No info about destination addr was available.  */
     *tolen = 0;
     return r;
-#endif
+}
+
+#ifdef HAVE_IP_PKTINFO
+
+#define set_msg_from_ipv4 set_msg_from_ip_pktinfo
+static krb5_error_code
+set_msg_from_ip_pktinfo(struct msghdr *msg, struct cmsghdr *cmsgptr,
+                        struct sockaddr *from, socklen_t fromlen,
+                        aux_addressing_info *auxaddr)
+{
+    struct in_pktinfo *p = cmsg2pktinfo(cmsgptr);
+    const struct sockaddr_in *from4 = sa2sin(from);
+
+    if (fromlen != sizeof(struct sockaddr_in))
+        return EINVAL;
+    cmsgptr->cmsg_level = IPPROTO_IP;
+    cmsgptr->cmsg_type = IP_PKTINFO;
+    cmsgptr->cmsg_len = CMSG_LEN(sizeof(struct in_pktinfo));
+    p->ipi_spec_dst = from4->sin_addr;
+
+    msg->msg_controllen = CMSG_SPACE(sizeof(struct in_pktinfo));
+    return 0;
+}
+
+#else /* HAVE_IP_PKTINFO */
+#define set_msg_from_ipv4(m, c, f, l, a) EINVAL
+#endif /* HAVE_IP_PKTINFO */
+
+#ifdef HAVE_IPV6_PKTINFO
+
+#define set_msg_from_ipv6 set_msg_from_ipv6_pktinfo
+static krb5_error_code
+set_msg_from_ipv6_pktinfo(struct msghdr *msg, struct cmsghdr *cmsgptr,
+                          struct sockaddr *from, socklen_t fromlen,
+                          aux_addressing_info *auxaddr)
+{
+    struct in6_pktinfo *p = cmsg2pktinfo6(cmsgptr);
+    const struct sockaddr_in6 *from6 = sa2sin6(from);
+
+    if (fromlen != sizeof(struct sockaddr_in6))
+        return EINVAL;
+    cmsgptr->cmsg_level = IPPROTO_IPV6;
+    cmsgptr->cmsg_type = IPV6_PKTINFO;
+    cmsgptr->cmsg_len = CMSG_LEN(sizeof(struct in6_pktinfo));
+
+    p->ipi6_addr = from6->sin6_addr;
+    /*
+     * Because of the possibility of asymmetric routing, we
+     * normally don't want to specify an interface.  However,
+     * Mac OS X doesn't like sending from a link-local address
+     * (which can come up in testing at least, if you wind up
+     * with a "foo.local" name) unless we do specify the
+     * interface.
+     */
+    if (IN6_IS_ADDR_LINKLOCAL(&from6->sin6_addr))
+        p->ipi6_ifindex = auxaddr->ipv6_ifindex;
+    /* otherwise, already zero */
+
+    msg->msg_controllen = CMSG_SPACE(sizeof(struct in6_pktinfo));
+    return 0;
+}
+
+#else /* HAVE_IPV6_PKTINFO */
+#define set_msg_from_ipv6(m, c, f, l, a) EINVAL
+#endif /* HAVE_IPV6_PKTINFO */
+
+static krb5_error_code
+set_msg_from(int family, struct msghdr *msg, struct cmsghdr *cmsgptr,
+             struct sockaddr *from, socklen_t fromlen,
+             aux_addressing_info *auxaddr)
+{
+    switch (family) {
+    case AF_INET:
+        return set_msg_from_ipv4(msg, cmsgptr, from, fromlen, auxaddr);
+    case AF_INET6:
+        return set_msg_from_ipv6(msg, cmsgptr, from, fromlen, auxaddr);
+    }
+
+    return EINVAL;
 }
 
 /*
@@ -210,22 +389,22 @@ recv_from_to(int socket, void *buf, size_t len, int flags,
  */
 krb5_error_code
 send_to_from(int socket, void *buf, size_t len, int flags,
-             const struct sockaddr *to, socklen_t tolen,
-             const struct sockaddr *from, socklen_t fromlen,
-             aux_addressing_info *auxaddr)
+             const struct sockaddr *to, socklen_t tolen, struct sockaddr *from,
+             socklen_t fromlen, aux_addressing_info *auxaddr)
 {
-#if (!defined(IP_PKTINFO) && !defined(IPV6_PKTINFO)) || !defined(CMSG_SPACE)
-    return sendto(socket, buf, len, flags, to, tolen);
-#else
+    int r;
     struct iovec iov;
     struct msghdr msg;
     struct cmsghdr *cmsgptr;
     char cbuf[CMSG_SPACE(sizeof(union pktinfo))];
 
-    if (from == 0 || fromlen == 0 || from->sa_family != to->sa_family) {
-use_sendto:
-        return sendto(socket, buf, len, flags, to, tolen);
-    }
+    /* Don't use pktinfo if the socket isn't bound to a wildcard address. */
+    r = is_socket_bound_to_wildcard(socket);
+    if (r < 0)
+        return errno;
+
+    if (from == NULL || fromlen == 0 || from->sa_family != to->sa_family || !r)
+        goto use_sendto;
 
     iov.iov_base = buf;
     iov.iov_len = len;
@@ -239,58 +418,45 @@ use_sendto:
     msg.msg_iov = &iov;
     msg.msg_iovlen = 1;
     msg.msg_control = cbuf;
-    /* CMSG_FIRSTHDR needs a non-zero controllen, or it'll return NULL
-     *      on Linux.  */
+    /* CMSG_FIRSTHDR needs a non-zero controllen, or it'll return NULL on
+     * Linux. */
     msg.msg_controllen = sizeof(cbuf);
     cmsgptr = CMSG_FIRSTHDR(&msg);
     msg.msg_controllen = 0;
 
-    switch (from->sa_family) {
-#if defined(IP_PKTINFO)
-    case AF_INET:
-        if (fromlen != sizeof(struct sockaddr_in))
-            goto use_sendto;
-        cmsgptr->cmsg_level = IPPROTO_IP;
-        cmsgptr->cmsg_type = IP_PKTINFO;
-        cmsgptr->cmsg_len = CMSG_LEN(sizeof(struct in_pktinfo));
-        {
-            struct in_pktinfo *p = (struct in_pktinfo *)CMSG_DATA(cmsgptr);
-            const struct sockaddr_in *from4 = (const struct sockaddr_in *)from;
-            p->ipi_spec_dst = from4->sin_addr;
-        }
-        msg.msg_controllen = CMSG_SPACE(sizeof(struct in_pktinfo));
-        break;
-#endif
-#if defined(IPV6_PKTINFO) && defined(HAVE_STRUCT_IN6_PKTINFO)
-    case AF_INET6:
-        if (fromlen != sizeof(struct sockaddr_in6))
-            goto use_sendto;
-        cmsgptr->cmsg_level = IPPROTO_IPV6;
-        cmsgptr->cmsg_type = IPV6_PKTINFO;
-        cmsgptr->cmsg_len = CMSG_LEN(sizeof(struct in6_pktinfo));
-        {
-            struct in6_pktinfo *p = (struct in6_pktinfo *)CMSG_DATA(cmsgptr);
-            const struct sockaddr_in6 *from6 =
-                (const struct sockaddr_in6 *)from;
-            p->ipi6_addr = from6->sin6_addr;
-            /*
-             * Because of the possibility of asymmetric routing, we
-             * normally don't want to specify an interface.  However,
-             * Mac OS X doesn't like sending from a link-local address
-             * (which can come up in testing at least, if you wind up
-             * with a "foo.local" name) unless we do specify the
-             * interface.
-             */
-            if (IN6_IS_ADDR_LINKLOCAL(&from6->sin6_addr))
-                p->ipi6_ifindex = auxaddr->ipv6_ifindex;
-            /* otherwise, already zero */
-        }
-        msg.msg_controllen = CMSG_SPACE(sizeof(struct in6_pktinfo));
-        break;
-#endif
-    default:
+    if (set_msg_from(from->sa_family, &msg, cmsgptr, from, fromlen, auxaddr))
         goto use_sendto;
-    }
     return sendmsg(socket, &msg, flags);
-#endif
+
+use_sendto:
+    return sendto(socket, buf, len, flags, to, tolen);
 }
+
+#else /* HAVE_PKTINFO_SUPPORT && CMSG_SPACE */
+
+krb5_error_code
+recv_from_to(int socket, void *buf, size_t len, int flags,
+             struct sockaddr *from, socklen_t *fromlen,
+             struct sockaddr *to, socklen_t *tolen,
+             aux_addressing_info *auxaddr)
+{
+    if (to && tolen) {
+        /* Clobber with something recognizeable in case we try to use the
+         * address. */
+        memset(to, 0x40, *tolen);
+        *tolen = 0;
+    }
+
+    return recvfrom(socket, buf, len, flags, from, fromlen);
+}
+
+krb5_error_code
+send_to_from(int socket, void *buf, size_t len, int flags,
+             const struct sockaddr *to, socklen_t tolen,
+             const struct sockaddr *from, socklen_t fromlen,
+             aux_addressing_info *auxaddr)
+{
+    return sendto(socket, buf, len, flags, to, tolen);
+}
+
+#endif /* HAVE_PKTINFO_SUPPORT && CMSG_SPACE */
diff --git a/src/lib/apputils/udppktinfo.h b/src/lib/apputils/udppktinfo.h
index af37028..53a584a 100644
--- a/src/lib/apputils/udppktinfo.h
+++ b/src/lib/apputils/udppktinfo.h
@@ -52,8 +52,7 @@ recv_from_to(int socket, void *buf, size_t len, int flags,
 
 krb5_error_code
 send_to_from(int socket, void *buf, size_t len, int flags,
-             const struct sockaddr *to, socklen_t tolen,
-             const struct sockaddr *from, socklen_t fromlen,
-             aux_addressing_info *auxaddr);
+             const struct sockaddr *to, socklen_t tolen, struct sockaddr *from,
+             socklen_t fromlen, aux_addressing_info *auxaddr);
 
 #endif /* UDPPKTINFO_H */


More information about the cvs-krb5 mailing list