From 13c00ae8c73ee9635c11059925814b351dc3593c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 30 May 2011 19:16:05 -0400 Subject: [PATCH] Fix portability bugs in use of credentials control messages for peer auth. 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 | 60 ++++++++++++++++++++-------------- src/interfaces/libpq/fe-auth.c | 22 +++++++------ 2 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 618f007827..e6ab659f4b 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -1788,7 +1788,7 @@ auth_peer(hbaPort *port) char ident_user[IDENT_USERNAME_MAX + 1]; #if defined(HAVE_GETPEEREID) - /* OpenBSD style: */ + /* OpenBSD (also Mac OS X) style: use getpeereid() */ uid_t uid; gid_t gid; struct passwd *pass; @@ -1843,7 +1843,7 @@ auth_peer(hbaPort *port) strlcpy(ident_user, pass->pw_name, IDENT_USERNAME_MAX + 1); #elif defined(HAVE_GETPEERUCRED) - /* Solaris > 10 */ + /* Solaris > 10: use getpeerucred() */ uid_t uid; struct passwd *pass; ucred_t *ucred; @@ -1878,9 +1878,7 @@ auth_peer(hbaPort *port) strlcpy(ident_user, pass->pw_name, IDENT_USERNAME_MAX + 1); #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; @@ -1894,36 +1892,35 @@ auth_peer(hbaPort *port) #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(port->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(port->sock, &msg, 0) < 0) { ereport(LOG, (errcode_for_socket_access(), @@ -1931,6 +1928,19 @@ auth_peer(hbaPort *port) return STATUS_ERROR; } + 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 STATUS_ERROR; + } + cred = (Cred *) CMSG_DATA(cmsg); pw = getpwuid(cred->cruid); diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index 6f1a163a10..094926b4e6 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -693,11 +693,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 /* @@ -713,11 +714,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