Fix portability bugs in use of credentials control messages for peer auth.
authorTom Lane <[email protected]>
Mon, 30 May 2011 23:16:17 +0000 (19:16 -0400)
committerTom Lane <[email protected]>
Mon, 30 May 2011 23:16:17 +0000 (19:16 -0400)
Even though our existing code for handling credentials control messages has
been basically unchanged since 2001, it was fundamentally wrong: it did not
ensure proper alignment of the supplied buffer, and it was calculating
buffer sizes and message sizes incorrectly.  This led to failures on
platforms where alignment padding is relevant, for instance FreeBSD on
64-bit platforms, as seen in a recent Debian bug report passed on by
Martin Pitt (http://bugs.debian.org//cgi-bin/bugreport.cgi?bug=612888).

Rewrite to do the message-whacking using the macros specified in RFC 2292,
following a suggestion from Theo de Raadt in that thread.  Tested by me
on Debian/kFreeBSD-amd64; since OpenBSD and NetBSD document the identical
CMSG API, it should work there too.

Back-patch to all supported branches.

src/backend/libpq/auth.c
src/interfaces/libpq/fe-auth.c

index 09ab1f0b8535a56970ec3c21835464e4c50b7621..dd5f6914f7dc28f5c8bd3c3e53710a0331e5be28 100644 (file)
@@ -1649,7 +1649,7 @@ static bool
 ident_unix(int sock, char *ident_user)
 {
 #if defined(HAVE_GETPEEREID)
-   /* OpenBSD style:  */
+   /* OpenBSD (also Mac OS X) style: use getpeereid() */
    uid_t       uid;
    gid_t       gid;
    struct passwd *pass;
@@ -1708,7 +1708,7 @@ ident_unix(int sock, char *ident_user)
 
    return true;
 #elif defined(HAVE_GETPEERUCRED)
-   /* Solaris > 10 */
+   /* Solaris > 10: use getpeerucred() */
    uid_t       uid;
    struct passwd *pass;
    ucred_t    *ucred;
@@ -1745,9 +1745,7 @@ ident_unix(int sock, char *ident_user)
 
    return true;
 #elif defined(HAVE_STRUCT_CMSGCRED) || defined(HAVE_STRUCT_FCRED) || (defined(HAVE_STRUCT_SOCKCRED) && defined(LOCAL_CREDS))
-   struct msghdr msg;
-
-/* Credentials structure */
+   /* Assorted BSDen: use a credentials control message */
 #if defined(HAVE_STRUCT_CMSGCRED)
    typedef struct cmsgcred Cred;
 
@@ -1761,36 +1759,35 @@ ident_unix(int sock, char *ident_user)
 
 #define cruid sc_uid
 #endif
-   Cred       *cred;
-
-   /* Compute size without padding */
-   char        cmsgmem[ALIGN(sizeof(struct cmsghdr)) + ALIGN(sizeof(Cred))];   /* for NetBSD */
-
-   /* Point to start of first structure */
-   struct cmsghdr *cmsg = (struct cmsghdr *) cmsgmem;
 
+   struct msghdr msg;
+   struct cmsghdr *cmsg;
+   union
+   {
+       struct cmsghdr  hdr;
+       unsigned char   buf[CMSG_SPACE(sizeof(Cred))];
+   } cmsgbuf;
    struct iovec iov;
    char        buf;
+   Cred       *cred;
    struct passwd *pw;
 
-   memset(&msg, 0, sizeof(msg));
-   msg.msg_iov = &iov;
-   msg.msg_iovlen = 1;
-   msg.msg_control = (char *) cmsg;
-   msg.msg_controllen = sizeof(cmsgmem);
-   memset(cmsg, 0, sizeof(cmsgmem));
-
    /*
-    * The one character which is received here is not meaningful; its
-    * purposes is only to make sure that recvmsg() blocks long enough for the
-    * other side to send its credentials.
+    * The one character that is received here is not meaningful; its purpose
+    * is only to make sure that recvmsg() blocks long enough for the other
+    * side to send its credentials.
     */
    iov.iov_base = &buf;
    iov.iov_len = 1;
 
-   if (recvmsg(sock, &msg, 0) < 0 ||
-       cmsg->cmsg_len < sizeof(cmsgmem) ||
-       cmsg->cmsg_type != SCM_CREDS)
+   memset(&msg, 0, sizeof(msg));
+   msg.msg_iov = &iov;
+   msg.msg_iovlen = 1;
+   msg.msg_control = &cmsgbuf.buf;
+   msg.msg_controllen = sizeof(cmsgbuf.buf);
+   memset(&cmsgbuf, 0, sizeof(cmsgbuf));
+
+   if (recvmsg(sock, &msg, 0) < 0)
    {
        ereport(LOG,
                (errcode_for_socket_access(),
@@ -1798,6 +1795,19 @@ ident_unix(int sock, char *ident_user)
        return false;
    }
 
+   cmsg = CMSG_FIRSTHDR(&msg);
+   if (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC) ||
+       cmsg == NULL ||
+       cmsg->cmsg_len < CMSG_LEN(sizeof(Cred)) ||
+       cmsg->cmsg_level != SOL_SOCKET ||
+       cmsg->cmsg_type != SCM_CREDS)
+   {
+       ereport(LOG,
+               (errcode(ERRCODE_PROTOCOL_VIOLATION),
+                errmsg("could not get peer credentials: incorrect control message")));
+       return false;
+   }
+
    cred = (Cred *) CMSG_DATA(cmsg);
 
    pw = getpwuid(cred->cruid);
index 34e884ec4da3485c1d57c6cf24fc79038b45e7ec..a8dc622f073b68a7038ad0eb7b926271cedf579a 100644 (file)
@@ -694,11 +694,12 @@ pg_local_sendauth(PGconn *conn)
    struct msghdr msg;
 
 #ifdef HAVE_STRUCT_CMSGCRED
-   /* Prevent padding */
-   char        cmsgmem[sizeof(struct cmsghdr) + sizeof(struct cmsgcred)];
-
-   /* Point to start of first structure */
-   struct cmsghdr *cmsg = (struct cmsghdr *) cmsgmem;
+   struct cmsghdr *cmsg;
+   union
+   {
+       struct cmsghdr  hdr;
+       unsigned char   buf[CMSG_SPACE(sizeof(struct cmsgcred))];
+   } cmsgbuf;
 #endif
 
    /*
@@ -714,11 +715,12 @@ pg_local_sendauth(PGconn *conn)
    msg.msg_iovlen = 1;
 
 #ifdef HAVE_STRUCT_CMSGCRED
-   /* Create control header, FreeBSD */
-   msg.msg_control = cmsg;
-   msg.msg_controllen = sizeof(cmsgmem);
-   memset(cmsg, 0, sizeof(cmsgmem));
-   cmsg->cmsg_len = sizeof(cmsgmem);
+   /* FreeBSD needs us to set up a message that will be filled in by kernel */
+   memset(&cmsgbuf, 0, sizeof(cmsgbuf));
+   msg.msg_control = &cmsgbuf.buf;
+   msg.msg_controllen = sizeof(cmsgbuf.buf);
+   cmsg = CMSG_FIRSTHDR(&msg);
+   cmsg->cmsg_len = CMSG_LEN(sizeof(struct cmsgcred));
    cmsg->cmsg_level = SOL_SOCKET;
    cmsg->cmsg_type = SCM_CREDS;
 #endif