On (18/01/16 18:06), Simo Sorce wrote: >I will needed selinux support later on in the secrets work, so I looked >into how we were getting peercreds and found it could be improved by >making it a little more abstract. > >This patch also uncovered issues with header inclusion (patch sent >earlier). > >(I did not open a bug for this one) > >Simo. > >-- >Simo Sorce * Red Hat, Inc * New York
>From 7cc82eff48dabc4b15e119146f36597f4cd75827 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 | 25 ++++++++++-- > src/responder/common/responder_common.c | 50 +++++++++++++++--------- > src/responder/pam/pamsrv_cmd.c | 23 +++++------ > src/util/util_selinux.h | 68 +++++++++++++++++++++++++++++++++ > 5 files changed, 133 insertions(+), 35 deletions(-) > create mode 100644 src/util/util_selinux.h > >diff --git a/Makefile.am b/Makefile.am >index >407053b1a6dcd0255be76ae7f9252a671b965ea2..2a02add0dc1942c57dec03f4762444c48a710a10 > 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_selinux.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..4735eb7af7d65c1e822d662e7200a8a7418e191e > 100644 >--- a/src/responder/common/responder.h >+++ b/src/responder/common/responder.h >@@ -36,6 +36,7 @@ > #include "sbus/sssd_dbus.h" > #include "responder/common/negcache.h" > #include "sss_client/sss_cli.h" >+#include "util/util_selinux.h" > > extern hash_table_t *dp_requests; > >@@ -123,6 +124,21 @@ struct resp_ctx { > bool shutting_down; > }; > >+#ifdef HAVE_UCRED >+typedef struct ucred UCRED_CTX; >+#define UCRED_get_uid(x) x.uid >+ >+#else /* not HAVE_UCRED */ >+struct ucred_ctx { int none; }; >+typedef struct ucred_ctx UCRED_CTX; >+#define UCRED_get_uid(x) -1 >+#endif /* done HAVE_UCRED */ >+ >+struct cli_creds { >+ UCRED_CTX ucred; >+ SELINUX_CTX selinux_ctx; >+}; >+ > struct cli_ctx { > struct tevent_context *ev; > struct resp_ctx *rctx; >@@ -131,9 +147,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 +346,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..1417c600e35afab6298109d8be6b6dc14b4b51eb > 100644 >--- a/src/responder/common/responder_common.c >+++ b/src/responder/common/responder_common.c >@@ -88,16 +88,17 @@ 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; > > #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 +112,31 @@ 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); >+ 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) >+{ >+ return UCRED_get_uid(creds->ucred); > } > >-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 +146,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 (UCRED_get_uid(creds->ucred) == allowed_uids[c]) { > return EOK; > } > } >@@ -429,7 +443,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 +453,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_selinux.h b/src/util/util_selinux.h >new file mode 100644 >index >0000000000000000000000000000000000000000..b1b31d8ce925951265f22975d2e420098fd5d27e >--- /dev/null >+++ b/src/util/util_selinux.h >@@ -0,0 +1,68 @@ >+/* >+ 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_SELINUX_H__ >+#define __SSSD_UTIL_SELINUX_H__ >+ >+#include "util/util.h" >+ I think we needn't include "big hammer" header file "util.h" here. If you need to include header file which will enable GNU features or any other macros define at configure time then it's enough to include "config.g". >+/* 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 */ >+ >+#endif /* __SSSD_UTIL_SELINUX_H__ */ BTW will we need this header file if we make struct cli_creds opaque? LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
