From 7c4185a72ad91c3479c8ecc2f5713bcc819d16ba Mon Sep 17 00:00:00 2001 From: David Shane Holden Date: Fri, 22 Jan 2016 19:22:20 -0500 Subject: [PATCH 1/5] lib: split peer credential loopup into it's own method --- lib/ipc_setup.c | 140 ++++++++++++++++++++++++++---------------------- 1 file changed, 75 insertions(+), 65 deletions(-) diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c index 06257c1ce..b60674beb 100644 --- a/lib/ipc_setup.c +++ b/lib/ipc_setup.c @@ -311,6 +311,80 @@ qb_ipcc_us_sock_close(int32_t sock) close(sock); } +static int32_t +qb_ipc_auth_creds(struct ipc_auth_data *data) +{ + int32_t res = 0; + + /* + * currently support getpeerucred, getpeereid, and SO_PASSCRED credential + * retrieval mechanisms for various Platforms + */ +#ifdef HAVE_GETPEERUCRED + /* + * Solaris and some BSD systems + */ + { + ucred_t *uc = NULL; + + if (getpeerucred(data->sock, &uc) == 0) { + res = 0; + data->ugp.uid = ucred_geteuid(uc); + data->ugp.gid = ucred_getegid(uc); + data->ugp.pid = ucred_getpid(uc); + ucred_free(uc); + } else { + res = -errno; + } + } +#elif HAVE_GETPEEREID + /* + * Usually MacOSX systems + */ + { + /* + * TODO get the peer's pid. + * c->pid = ?; + */ + if (getpeereid(data->sock, &data->ugp.uid, &data->ugp.gid) == 0) { + res = 0; + } else { + res = -errno; + } + } + +#elif SO_PASSCRED + /* + * Usually Linux systems + */ + { + struct ucred cred; + struct cmsghdr *cmsg; + + res = -EINVAL; + for (cmsg = CMSG_FIRSTHDR(&data->msg_recv); cmsg != NULL; + cmsg = CMSG_NXTHDR(&data->msg_recv, cmsg)) { + if (cmsg->cmsg_type != SCM_CREDENTIALS) + continue; + + memcpy(&cred, CMSG_DATA(cmsg), sizeof(struct ucred)); + res = 0; + data->ugp.pid = cred.pid; + data->ugp.uid = cred.uid; + data->ugp.gid = cred.gid; + break; + } + } +#else /* no credentials */ + data->ugp.pid = 0; + data->ugp.uid = 0; + data->ugp.gid = 0; + res = -ENOTSUP; +#endif /* no credentials */ + + return res; +} + int32_t qb_ipcc_us_setup_connect(struct qb_ipcc_connection *c, struct qb_ipc_connection_response *r) @@ -622,71 +696,7 @@ process_auth(int32_t fd, int32_t revents, void *d) goto cleanup_and_return; } - /* - * currently support getpeerucred, getpeereid, and SO_PASSCRED credential - * retrieval mechanisms for various Platforms - */ -#ifdef HAVE_GETPEERUCRED - /* - * Solaris and some BSD systems - */ - { - ucred_t *uc = NULL; - - if (getpeerucred(data->sock, &uc) == 0) { - res = 0; - data->ugp.uid = ucred_geteuid(uc); - data->ugp.gid = ucred_getegid(uc); - data->ugp.pid = ucred_getpid(uc); - ucred_free(uc); - } else { - res = -errno; - } - } -#elif HAVE_GETPEEREID - /* - * Usually MacOSX systems - */ - { - /* - * TODO get the peer's pid. - * c->pid = ?; - */ - if (getpeereid(data->sock, &data->ugp.uid, &data->ugp.gid) == 0) { - res = 0; - } else { - res = -errno; - } - } - -#elif SO_PASSCRED - /* - * Usually Linux systems - */ - { - struct ucred cred; - struct cmsghdr *cmsg; - - res = -EINVAL; - for (cmsg = CMSG_FIRSTHDR(&data->msg_recv); cmsg != NULL; - cmsg = CMSG_NXTHDR(&data->msg_recv, cmsg)) { - if (cmsg->cmsg_type != SCM_CREDENTIALS) - continue; - - memcpy(&cred, CMSG_DATA(cmsg), sizeof(struct ucred)); - res = 0; - data->ugp.pid = cred.pid; - data->ugp.uid = cred.uid; - data->ugp.gid = cred.gid; - break; - } - } -#else /* no credentials */ - data->ugp.pid = 0; - data->ugp.uid = 0; - data->ugp.gid = 0; - res = -ENOTSUP; -#endif /* no credentials */ + res = qb_ipc_auth_creds(data); cleanup_and_return: #ifdef SO_PASSCRED From 888ef2e7218056ecbb3a06105674c673fc71f555 Mon Sep 17 00:00:00 2001 From: David Shane Holden Date: Fri, 22 Jan 2016 19:30:13 -0500 Subject: [PATCH 2/5] lib: add init_ipc_auth_data() to initialize ipc_auth_data --- lib/ipc_setup.c | 93 +++++++++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 42 deletions(-) diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c index b60674beb..f3dd1c9dd 100644 --- a/lib/ipc_setup.c +++ b/lib/ipc_setup.c @@ -385,6 +385,56 @@ qb_ipc_auth_creds(struct ipc_auth_data *data) return res; } +static void +destroy_ipc_auth_data(struct ipc_auth_data *data) +{ + if (data->s) { + qb_ipcs_unref(data->s); + } + +#ifdef SO_PASSCRED + free(data->cmsg_cred); +#endif + free(data); +} + +static struct ipc_auth_data * +init_ipc_auth_data(int sock, size_t len) +{ + struct ipc_auth_data *data = calloc(1, sizeof(struct ipc_auth_data)); + + if (data == NULL) + return 0; + + data->msg_recv.msg_iov = &data->iov_recv; + data->msg_recv.msg_iovlen = 1; + data->msg_recv.msg_name = 0; + data->msg_recv.msg_namelen = 0; + +#ifdef SO_PASSCRED + data->cmsg_cred = calloc(1, CMSG_SPACE(sizeof(struct ucred))); + if (data->cmsg_cred == NULL) { + destroy_ipc_auth_data(data); + return 0; + } + data->msg_recv.msg_control = (void *)data->cmsg_cred; + data->msg_recv.msg_controllen = CMSG_SPACE(sizeof(struct ucred)); +#endif +#ifdef QB_SOLARIS + data->msg_recv.msg_accrights = 0; + data->msg_recv.msg_accrightslen = 0; +#else + data->msg_recv.msg_flags = 0; +#endif /* QB_SOLARIS */ + + data->len = len; + data->iov_recv.iov_base = &data->msg; + data->iov_recv.iov_len = data->len; + data->sock = sock; + + return data; +} + int32_t qb_ipcc_us_setup_connect(struct qb_ipcc_connection *c, struct qb_ipc_connection_response *r) @@ -642,19 +692,6 @@ handle_new_connection(struct qb_ipcs_service *s, return res; } -static void -destroy_ipc_auth_data(struct ipc_auth_data *data) -{ - if (data->s) { - qb_ipcs_unref(data->s); - } - -#ifdef SO_PASSCRED - free(data->cmsg_cred); -#endif - free(data); -} - static int32_t process_auth(int32_t fd, int32_t revents, void *d) { @@ -726,7 +763,7 @@ qb_ipcs_uc_recv_and_auth(int32_t sock, struct qb_ipcs_service *s) int on = 1; #endif - data = calloc(1, sizeof(struct ipc_auth_data)); + data = init_ipc_auth_data(sock, sizeof(struct qb_ipc_connection_request)); if (data == NULL) { close(sock); /* -ENOMEM */ @@ -736,34 +773,6 @@ qb_ipcs_uc_recv_and_auth(int32_t sock, struct qb_ipcs_service *s) data->s = s; qb_ipcs_ref(data->s); - data->msg_recv.msg_iov = &data->iov_recv; - data->msg_recv.msg_iovlen = 1; - data->msg_recv.msg_name = 0; - data->msg_recv.msg_namelen = 0; - -#ifdef SO_PASSCRED - data->cmsg_cred = calloc(1,CMSG_SPACE(sizeof(struct ucred))); - if (data->cmsg_cred == NULL) { - close(sock); - destroy_ipc_auth_data(data); - /* -ENOMEM */ - return; - } - data->msg_recv.msg_control = (void *)data->cmsg_cred; - data->msg_recv.msg_controllen = CMSG_SPACE(sizeof(struct ucred)); -#endif -#ifdef QB_SOLARIS - data->msg_recv.msg_accrights = 0; - data->msg_recv.msg_accrightslen = 0; -#else - data->msg_recv.msg_flags = 0; -#endif /* QB_SOLARIS */ - - data->len = sizeof(struct qb_ipc_connection_request); - data->iov_recv.iov_base = &data->msg; - data->iov_recv.iov_len = data->len; - data->sock = sock; - #ifdef SO_PASSCRED setsockopt(sock, SOL_SOCKET, SO_PASSCRED, &on, sizeof(on)); #endif From 267160634dc467cd5170d9c9ef04224f838a8f12 Mon Sep 17 00:00:00 2001 From: David Shane Holden Date: Fri, 22 Jan 2016 19:42:28 -0500 Subject: [PATCH 3/5] lib: store server peer credentials in qb_ipcc_connection --- lib/ipc_int.h | 3 +++ lib/ipc_setup.c | 34 ++++++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/lib/ipc_int.h b/lib/ipc_int.h index 500315e1c..95b725b94 100644 --- a/lib/ipc_int.h +++ b/lib/ipc_int.h @@ -91,6 +91,9 @@ struct qb_ipcc_funcs { struct qb_ipcc_connection { char name[NAME_MAX]; int32_t needs_sock_for_poll; + pid_t pid; + uid_t euid; + gid_t egid; struct qb_ipc_one_way setup; struct qb_ipc_one_way request; struct qb_ipc_one_way response; diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c index f3dd1c9dd..ebebc778a 100644 --- a/lib/ipc_setup.c +++ b/lib/ipc_setup.c @@ -51,7 +51,10 @@ struct ipc_auth_ugp { struct ipc_auth_data { int32_t sock; struct qb_ipcs_service *s; - struct qb_ipc_connection_request msg; + union { + struct qb_ipc_connection_request req; + struct qb_ipc_connection_response res; + } msg; struct msghdr msg_recv; struct iovec iov_recv; struct ipc_auth_ugp ugp; @@ -441,6 +444,7 @@ qb_ipcc_us_setup_connect(struct qb_ipcc_connection *c, { int32_t res; struct qb_ipc_connection_request request; + struct ipc_auth_data *data; #ifdef QB_LINUX int off = 0; int on = 1; @@ -464,21 +468,39 @@ qb_ipcc_us_setup_connect(struct qb_ipcc_connection *c, qb_ipcc_us_sock_close(c->setup.u.us.sock); return res; } + + data = init_ipc_auth_data(c->setup.u.us.sock, sizeof(struct qb_ipc_connection_response)); + if (data == NULL) { + qb_ipcc_us_sock_close(c->setup.u.us.sock); + return -1; + } + + qb_ipc_us_ready(&c->setup, NULL, -1, POLLIN); + res = qb_ipc_us_recv_msghdr(data); + #ifdef QB_LINUX setsockopt(c->setup.u.us.sock, SOL_SOCKET, SO_PASSCRED, &off, sizeof(off)); #endif - res = - qb_ipc_us_recv(&c->setup, r, - sizeof(struct qb_ipc_connection_response), -1); - if (res < 0) { + if (res != data->len) { + destroy_ipc_auth_data(data); return res; } + memcpy(r, &data->msg.res, sizeof(struct qb_ipc_connection_response)); + + qb_ipc_auth_creds(data); + c->pid = data->ugp.pid; + c->euid = data->ugp.uid; + c->egid = data->ugp.gid; + if (r->hdr.error != 0) { + destroy_ipc_auth_data(data); return r->hdr.error; } + + destroy_ipc_auth_data(data); return 0; } @@ -744,7 +766,7 @@ process_auth(int32_t fd, int32_t revents, void *d) if (res < 0) { close(data->sock); - } else if (data->msg.hdr.id == QB_IPC_MSG_AUTHENTICATE) { + } else if (data->msg.req.hdr.id == QB_IPC_MSG_AUTHENTICATE) { (void)handle_new_connection(data->s, res, data->sock, &data->msg, data->len, &data->ugp); } else { close(data->sock); From 8668d051c5e554eadf2967046ecfcccc7ec8762f Mon Sep 17 00:00:00 2001 From: David Shane Holden Date: Fri, 22 Jan 2016 20:00:57 -0500 Subject: [PATCH 4/5] ipc: set gid on unix sockets When creating a unix socket it's default gid is that of the parent directory. If the SOCKETDIR is owned by root:wheel with 1777 mode some of the pacemaker daemons end up unable to communicate with one another due to having insufficient permissions on the sockets. This can be fixed by setting the client sockets gid to the primary group of the server socket owner it's attempting to connect to. And, on the server side by setting the gid to the already captured gid stored in the connection info. This ensures that regardless of who owns the socket directory, as long as the applications have r/w access to it they should work. --- lib/ipc_socket.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c index 8d2179df3..7b3d7b007 100644 --- a/lib/ipc_socket.c +++ b/lib/ipc_socket.c @@ -60,7 +60,8 @@ set_sock_addr(struct sockaddr_un *address, const char *socket_name) static int32_t qb_ipc_dgram_sock_setup(const char *base_name, - const char *service_name, int32_t * sock_pt) + const char *service_name, int32_t * sock_pt, + gid_t gid) { int32_t request_fd; struct sockaddr_un local_address; @@ -86,6 +87,7 @@ qb_ipc_dgram_sock_setup(const char *base_name, sizeof(local_address)); #if !(defined(QB_LINUX) || defined(QB_CYGWIN)) chmod(local_address.sun_path, 0660); + chown(local_address.sun_path, getuid(), gid); #endif if (res < 0) { goto error_connect; @@ -221,12 +223,12 @@ static int32_t qb_ipc_dgram_sock_connect(const char *base_name, const char *local_name, const char *remote_name, - int32_t max_msg_size, int32_t * sock_pt) + int32_t max_msg_size, int32_t * sock_pt, gid_t gid) { char sock_path[PATH_MAX]; struct sockaddr_un remote_address; int32_t res = qb_ipc_dgram_sock_setup(base_name, local_name, - sock_pt); + sock_pt, gid); if (res < 0) { return res; } @@ -547,14 +549,14 @@ qb_ipcc_us_connect(struct qb_ipcc_connection * c, fd_hdr = -1; res = qb_ipc_dgram_sock_connect(r->response, "response", "request", - r->max_msg_size, &c->request.u.us.sock); + r->max_msg_size, &c->request.u.us.sock, c->egid); if (res != 0) { goto cleanup_hdr; } c->response.u.us.sock = c->request.u.us.sock; res = qb_ipc_dgram_sock_connect(r->response, "event", "event-tx", - r->max_msg_size, &c->event.u.us.sock); + r->max_msg_size, &c->event.u.us.sock, c->egid); if (res != 0) { goto cleanup_hdr; } @@ -776,7 +778,7 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, /* request channel */ res = qb_ipc_dgram_sock_setup(r->response, "request", - &c->request.u.us.sock); + &c->request.u.us.sock, c->egid); if (res < 0) { goto cleanup_hdr; } @@ -790,7 +792,7 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, /* event channel */ res = qb_ipc_dgram_sock_setup(r->response, "event-tx", - &c->event.u.us.sock); + &c->event.u.us.sock, c->egid); if (res < 0) { goto cleanup_hdr; } From 4f131253fcd31d360afe397006cd0ae815895dae Mon Sep 17 00:00:00 2001 From: David Shane Holden Date: Mon, 25 Jan 2016 21:27:29 -0500 Subject: [PATCH 5/5] update: per kgaillot review * remove pid/euid from qb_ipcc_connection * use proper #elif defines * return NULL instead of 0 for pointers * return -ENOMEM when malloc fails * remove redundant if check * use -1 for uid to chown() --- lib/ipc_int.h | 2 -- lib/ipc_setup.c | 22 ++++++++-------------- lib/ipc_socket.c | 2 +- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/lib/ipc_int.h b/lib/ipc_int.h index 95b725b94..f63e05361 100644 --- a/lib/ipc_int.h +++ b/lib/ipc_int.h @@ -91,8 +91,6 @@ struct qb_ipcc_funcs { struct qb_ipcc_connection { char name[NAME_MAX]; int32_t needs_sock_for_poll; - pid_t pid; - uid_t euid; gid_t egid; struct qb_ipc_one_way setup; struct qb_ipc_one_way request; diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c index ebebc778a..700de94b7 100644 --- a/lib/ipc_setup.c +++ b/lib/ipc_setup.c @@ -340,7 +340,7 @@ qb_ipc_auth_creds(struct ipc_auth_data *data) res = -errno; } } -#elif HAVE_GETPEEREID +#elif defined(HAVE_GETPEEREID) /* * Usually MacOSX systems */ @@ -356,7 +356,7 @@ qb_ipc_auth_creds(struct ipc_auth_data *data) } } -#elif SO_PASSCRED +#elif defined(SO_PASSCRED) /* * Usually Linux systems */ @@ -406,8 +406,9 @@ init_ipc_auth_data(int sock, size_t len) { struct ipc_auth_data *data = calloc(1, sizeof(struct ipc_auth_data)); - if (data == NULL) - return 0; + if (data == NULL) { + return NULL; + } data->msg_recv.msg_iov = &data->iov_recv; data->msg_recv.msg_iovlen = 1; @@ -418,7 +419,7 @@ init_ipc_auth_data(int sock, size_t len) data->cmsg_cred = calloc(1, CMSG_SPACE(sizeof(struct ucred))); if (data->cmsg_cred == NULL) { destroy_ipc_auth_data(data); - return 0; + return NULL; } data->msg_recv.msg_control = (void *)data->cmsg_cred; data->msg_recv.msg_controllen = CMSG_SPACE(sizeof(struct ucred)); @@ -472,7 +473,7 @@ qb_ipcc_us_setup_connect(struct qb_ipcc_connection *c, data = init_ipc_auth_data(c->setup.u.us.sock, sizeof(struct qb_ipc_connection_response)); if (data == NULL) { qb_ipcc_us_sock_close(c->setup.u.us.sock); - return -1; + return -ENOMEM; } qb_ipc_us_ready(&c->setup, NULL, -1, POLLIN); @@ -491,17 +492,10 @@ qb_ipcc_us_setup_connect(struct qb_ipcc_connection *c, memcpy(r, &data->msg.res, sizeof(struct qb_ipc_connection_response)); qb_ipc_auth_creds(data); - c->pid = data->ugp.pid; - c->euid = data->ugp.uid; c->egid = data->ugp.gid; - if (r->hdr.error != 0) { - destroy_ipc_auth_data(data); - return r->hdr.error; - } - destroy_ipc_auth_data(data); - return 0; + return r->hdr.error; } /* diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c index 7b3d7b007..e4a260688 100644 --- a/lib/ipc_socket.c +++ b/lib/ipc_socket.c @@ -87,7 +87,7 @@ qb_ipc_dgram_sock_setup(const char *base_name, sizeof(local_address)); #if !(defined(QB_LINUX) || defined(QB_CYGWIN)) chmod(local_address.sun_path, 0660); - chown(local_address.sun_path, getuid(), gid); + chown(local_address.sun_path, -1, gid); #endif if (res < 0) { goto error_connect;