On Tue, 2016-01-19 at 10:34 -0500, Simo Sorce wrote: > On Tue, 2016-01-19 at 11:23 +0100, Lukas Slebodnik wrote: > [...] > > >+#endif /* __SSSD_UTIL_SELINUX_H__ */ > > BTW will we need this header file if we make > > struct cli_creds opaque? > > Replying to both your mails here: > Making cli_creds opaque requires using a pointer and dealing with > allocating it, I guess I can do that, the headers file would still be > needed in order to avoid huge ifdefs around the functions that implement > handling SELinux stuff. It makes the code a lot more readable and > searchable. > > Simo. >
Attached find patch that changes the code so that cli_creds is now opaque. This *should* work w/o the patch that changes the headers but I haven't tested it w/o that patch yet. Simo. -- Simo Sorce * Red Hat, Inc * New York
From 47d2ab3bb559ed0b0ea112c2eb8b3c4ceaa86929 Mon Sep 17 00:00:00 2001 From: Simo Sorce <s...@redhat.com> Date: Mon, 18 Jan 2016 15:21:57 -0500 Subject: [PATCH] Util: Improve code to get connection credentials Adds support to get SELINUX context and make code more abstract so that struct ucred (if availale) can be used w/o redefining uid,gid,pid to int32. Also givces a layer of indirection that may come handy if we want to imrpove the code further in the future. Resolves: https://fedorahosted.org/sssd/ticket/XXXX --- Makefile.am | 2 + src/responder/common/responder.h | 11 +++-- src/responder/common/responder_common.c | 59 ++++++++++++++++-------- src/responder/pam/pamsrv_cmd.c | 23 ++++----- src/util/util_creds.h | 82 +++++++++++++++++++++++++++++++++ 5 files changed, 141 insertions(+), 36 deletions(-) create mode 100644 src/util/util_creds.h diff --git a/Makefile.am b/Makefile.am index 407053b1a6dcd0255be76ae7f9252a671b965ea2..98f5df9ff4bbbbfbb122930c6109620a58cfd1ca 100644 --- a/Makefile.am +++ b/Makefile.am @@ -496,6 +496,7 @@ SSSD_LIBS = \ $(COLLECTION_LIBS) \ $(DHASH_LIBS) \ $(OPENLDAP_LIBS) \ + $(SELINUX_LIBS) \ $(TDB_LIBS) PYTHON_BINDINGS_LIBS = \ @@ -556,6 +557,7 @@ dist_noinst_HEADERS = \ src/util/authtok-utils.h \ src/util/util_safealign.h \ src/util/util_sss_idmap.h \ + src/util/util_creds.h \ src/monitor/monitor.h \ src/monitor/monitor_interfaces.h \ src/responder/common/responder.h \ diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h index 6b511368c9b08d1cfc828d16f57a2cde902dc82b..69eb6a1888a0c3632fc61768e12ce60c0114d22b 100644 --- a/src/responder/common/responder.h +++ b/src/responder/common/responder.h @@ -123,6 +123,8 @@ struct resp_ctx { bool shutting_down; }; +struct cli_creds; + struct cli_ctx { struct tevent_context *ev; struct resp_ctx *rctx; @@ -131,9 +133,8 @@ struct cli_ctx { tevent_fd_handler_t cfd_handler; struct sockaddr_un addr; int priv; - int32_t client_euid; - int32_t client_egid; - int32_t client_pid; + + struct cli_creds *creds; void *protocol_ctx; void *state_ctx; @@ -331,7 +332,9 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string, bool allow_sss_loop, size_t *_uid_count, uid_t **_uids); -errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count, +uid_t client_euid(struct cli_creds *creds); +errno_t check_allowed_uids(struct cli_creds *creds, + size_t allowed_uids_count, uid_t *allowed_uids); struct tevent_req * diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c index fecb72384786ea2840ed349a6afcb36efc60d492..fc074fee25f96a931649d39cccaa4fd8312ee4e7 100644 --- a/src/responder/common/responder_common.c +++ b/src/responder/common/responder_common.c @@ -42,6 +42,7 @@ #include "providers/data_provider.h" #include "monitor/monitor_interfaces.h" #include "sbus/sbus_client.h" +#include "util/util_creds.h" #ifdef HAVE_SYSTEMD #include <systemd/sd-daemon.h> @@ -88,16 +89,20 @@ static int client_destructor(struct cli_ctx *ctx) static errno_t get_client_cred(struct cli_ctx *cctx) { - cctx->client_euid = -1; - cctx->client_egid = -1; - cctx->client_pid = -1; + SEC_CTX secctx; + int ret; + + cctx->creds = talloc(cctx, struct cli_creds); + if (!cctx->creds) return ENOMEM; #ifdef HAVE_UCRED - int ret; - struct ucred client_cred; - socklen_t client_cred_len = sizeof(client_cred); + socklen_t client_cred_len = sizeof(struct ucred); - ret = getsockopt(cctx->cfd, SOL_SOCKET, SO_PEERCRED, &client_cred, + cctx->creds->ucred.uid = -1; + cctx->creds->ucred.gid = -1; + cctx->creds->ucred.pid = -1; + + ret = getsockopt(cctx->cfd, SOL_SOCKET, SO_PEERCRED, &cctx->creds->ucred, &client_cred_len); if (ret != EOK) { ret = errno; @@ -111,18 +116,34 @@ static errno_t get_client_cred(struct cli_ctx *cctx) return ENOMSG; } - cctx->client_euid = client_cred.uid; - cctx->client_egid = client_cred.gid; - cctx->client_pid = client_cred.pid; - - DEBUG(SSSDBG_TRACE_ALL, "Client creds: euid[%d] egid[%d] pid[%d].\n", - cctx->client_euid, cctx->client_egid, cctx->client_pid); + DEBUG(SSSDBG_TRACE_ALL, + "Client creds: euid[%d] egid[%d] pid[%d].\n", + cctx->creds->ucred.uid, cctx->creds->ucred.gid, + cctx->creds->ucred.pid); #endif - return EOK; + ret = SELINUX_getpeercon(cctx->cfd, &secctx); + if (ret != 0) { + DEBUG(SSSDBG_CRIT_FAILURE, + "SELINUX_getpeercon failed [%d][%s].\n", ret, strerror(ret)); + /* This is not fatal, as SELinux may simply be disabled */ + ret = EOK; + } else { + cctx->creds->selinux_ctx = SELINUX_context_new(secctx); + SELINUX_freecon(secctx); + } + + return ret; +} + +uid_t client_euid(struct cli_creds *creds) +{ + if (!creds) return -1; + return cli_creds_get_uid(creds); } -errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count, +errno_t check_allowed_uids(struct cli_creds *creds, + size_t allowed_uids_count, uid_t *allowed_uids) { size_t c; @@ -132,7 +153,7 @@ errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count, } for (c = 0; c < allowed_uids_count; c++) { - if (uid == allowed_uids[c]) { + if (client_euid(creds) == allowed_uids[c]) { return EOK; } } @@ -429,7 +450,7 @@ static void accept_fd_handler(struct tevent_context *ev, } if (rctx->allowed_uids_count != 0) { - if (cctx->client_euid == -1) { + if (client_euid(cctx->creds) == -1) { DEBUG(SSSDBG_CRIT_FAILURE, "allowed_uids configured, " \ "but platform does not support " \ "reading peer credential from the " \ @@ -439,12 +460,12 @@ static void accept_fd_handler(struct tevent_context *ev, return; } - ret = check_allowed_uids(cctx->client_euid, rctx->allowed_uids_count, + ret = check_allowed_uids(cctx->creds, rctx->allowed_uids_count, rctx->allowed_uids); if (ret != EOK) { if (ret == EACCES) { DEBUG(SSSDBG_CRIT_FAILURE, "Access denied for uid [%d].\n", - cctx->client_euid); + (int)client_euid(cctx->creds)); } else { DEBUG(SSSDBG_OP_FAILURE, "check_allowed_uids failed.\n"); } diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index ce04502b8b1ef3eeae0939f8dfb3f7ea85073e52..0a7b449fe15590a6dc1a4d744c4a49887976e9cc 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -988,14 +988,14 @@ static int pam_auth_req_destructor(struct pam_auth_req *preq) return 0; } -static bool is_uid_trusted(uint32_t uid, +static bool is_uid_trusted(struct cli_creds *creds, size_t trusted_uids_count, uid_t *trusted_uids) { - size_t i; + errno_t ret; /* root is always trusted */ - if (uid == 0) { + if (client_euid(creds) == 0) { return true; } @@ -1004,11 +1004,8 @@ static bool is_uid_trusted(uint32_t uid, return true; } - for(i = 0; i < trusted_uids_count; i++) { - if (trusted_uids[i] == uid) { - return true; - } - } + ret = check_allowed_uids(creds, trusted_uids_count, trusted_uids); + if (ret == EOK) return true; return false; } @@ -1097,13 +1094,13 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd) } pd = preq->pd; - preq->is_uid_trusted = is_uid_trusted(cctx->client_euid, + preq->is_uid_trusted = is_uid_trusted(&cctx->creds, pctx->trusted_uids_count, pctx->trusted_uids); if (!preq->is_uid_trusted) { - DEBUG(SSSDBG_MINOR_FAILURE, "uid %"PRIu32" is not trusted.\n", - cctx->client_euid); + DEBUG(SSSDBG_MINOR_FAILURE, "uid %d is not trusted.\n", + (int)client_euid(&cctx->creds)); } @@ -1782,8 +1779,8 @@ static void pam_dom_forwarder(struct pam_auth_req *preq) !is_domain_public(preq->pd->domain, pctx->public_domains, pctx->public_domains_count)) { DEBUG(SSSDBG_MINOR_FAILURE, - "Untrusted user %"PRIu32" cannot access non-public domain %s.\n", - preq->cctx->client_euid, preq->pd->domain); + "Untrusted user %d cannot access non-public domain %s.\n", + (int)client_euid(&preq->cctx->creds), preq->pd->domain); preq->pd->pam_status = PAM_PERM_DENIED; pam_reply(preq); return; diff --git a/src/util/util_creds.h b/src/util/util_creds.h new file mode 100644 index 0000000000000000000000000000000000000000..65468fa12b8c6921859574c40f5759c936a10e86 --- /dev/null +++ b/src/util/util_creds.h @@ -0,0 +1,82 @@ +/* + Authors: + Simo Sorce <s...@redhat.com> + + Copyright (C) 2016 Red Hat + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef __SSSD_UTIL_CREDS_H__ +#define __SSSD_UTIL_CREDS_H__ + +/* following code comes from gss-proxy's gp_selinux.h file */ +#ifdef HAVE_SELINUX + +#include <selinux/context.h> +#define SELINUX_CTX context_t +#include <selinux/selinux.h> +#define SEC_CTX security_context_t + +#define SELINUX_context_new context_new +#define SELINUX_context_free context_free +#define SELINUX_context_str context_str +#define SELINUX_context_type_get context_type_get +#define SELINUX_context_user_get context_user_get +#define SELINUX_context_role_get context_role_get +#define SELINUX_context_range_get context_range_get +#define SELINUX_getpeercon getpeercon +#define SELINUX_freecon freecon + +#else /* not HAVE_SELINUX */ + +#define SELINUX_CTX void * +#define SEC_CTX void * + +#define SELINUX_context_new(x) NULL +#define SELINUX_context_free(x) (x) = NULL +#define SELINUX_context_dummy_get(x) "<SELinux not compiled in>" +#define SELINUX_context_str SELINUX_context_dummy_get +#define SELINUX_context_type_get SELINUX_context_dummy_get +#define SELINUX_context_user_get SELINUX_context_dummy_get +#define SELINUX_context_role_get SELINUX_context_dummy_get +#define SELINUX_context_range_get SELINUX_context_dummy_get + +#include <errno.h> +#define SELINUX_getpeercon(x, y) -1; do { \ + *(y) = NULL; \ + errno = ENOTSUP; \ +} while(0) + +#define SELINUX_freecon(x) (x) = NULL + +#endif /* done HAVE_SELINUX */ + +#ifdef HAVE_UCRED +#include <sys/socket.h> +struct cli_creds { + struct ucred ucred; + SELINUX_CTX selinux_ctx; +}; + +#define cli_creds_get_uid(x) x->ucred.uid + +#else /* not HAVE_UCRED */ +struct cli_creds { + SELINUX_CTX selinux_ctx; +}; +#define cli_creds_get_uid(x) -1 +#endif /* done HAVE_UCRED */ + +#endif /* __SSSD_UTIL_CREDS_H__ */ -- 2.5.0
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
