On (19/01/16 15:38), Simo Sorce wrote:
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.
I had an issue to build it correctly with ifp responder.
src/responder/ifp/ifpsrv_util.c: In function ‘ifp_req_create’:
src/responder/ifp/ifpsrv_util.c:64:30: error: passing argument 1 of
‘check_allowed_uids’ makes pointer from integer without a cast
[-Werror=int-conversion]
ret = check_allowed_uids(dbus_req->client,
^
In file included from ../sssd/src/responder/ifp/ifp_private.h:27:0,
from ../sssd/src/responder/ifp/ifpsrv_util.c:27:
src/responder/common/responder.h:334:9: note: expected ‘struct cli_creds *’ but
argument is of type ‘int64_t {aka long int}’
errno_t check_allowed_uids(struct cli_creds *creds,
^
ifp responder uses different way to obtain uid. I changed back the prototype of
check_allowed_uids.
Here is a diff on to of your patch.
diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index 419a863..3b70b69 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -331,8 +331,7 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const
char *csv_string,
size_t *_uid_count, uid_t **_uids);
uid_t client_euid(struct cli_creds *creds);
-errno_t check_allowed_uids(struct cli_creds *creds,
- size_t allowed_uids_count,
+errno_t check_allowed_uids(uid_t uid, 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 27193ce..6ac1ea2 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -138,8 +138,7 @@ uid_t client_euid(struct cli_creds *creds)
return cli_creds_get_uid(creds);
}
-errno_t check_allowed_uids(struct cli_creds *creds,
- size_t allowed_uids_count,
+errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count,
uid_t *allowed_uids)
{
size_t c;
@@ -149,7 +148,7 @@ errno_t check_allowed_uids(struct cli_creds *creds,
}
for (c = 0; c < allowed_uids_count; c++) {
- if (client_euid(creds) == allowed_uids[c]) {
+ if (uid == allowed_uids[c]) {
return EOK;
}
}
@@ -449,12 +448,13 @@ static void accept_fd_handler(struct tevent_context *ev,
return;
}
- ret = check_allowed_uids(cctx->creds, rctx->allowed_uids_count,
+ ret = check_allowed_uids(client_euid(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",
- (int)client_euid(cctx->creds));
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Access denied for uid [%"SPRIuid"].\n",
+ 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 5fe3e6b..bfc534f 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -1001,7 +1001,7 @@ static bool is_uid_trusted(struct cli_creds *creds,
return true;
}
- ret = check_allowed_uids(creds, trusted_uids_count, trusted_uids);
+ ret = check_allowed_uids(client_euid(creds), trusted_uids_count,
trusted_uids);
if (ret == EOK) return true;
return false;
@@ -1091,13 +1091,13 @@ static int pam_forwarder(struct cli_ctx *cctx, int
pam_cmd)
}
pd = preq->pd;
- preq->is_uid_trusted = is_uid_trusted(&cctx->creds,
+ 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 %d is not trusted.\n",
- (int)client_euid(&cctx->creds));
+ DEBUG(SSSDBG_MINOR_FAILURE, "uid %"SPRIuid" is not trusted.\n",
+ client_euid(cctx->creds));
}
@@ -1776,8 +1776,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 %d cannot access non-public domain %s.\n",
- (int)client_euid(&preq->cctx->creds), preq->pd->domain);
+ "Untrusted user %"SPRIuid" cannot access non-public domain
%s.\n",
+ client_euid(preq->cctx->creds), preq->pd->domain);
preq->pd->pam_status = PAM_PERM_DENIED;
pam_reply(preq);
return;
I know you are busy. I fixed few conflics + updated your patch and
added my singoff. I hope you don mind.
If you are find with attached patch then we need to find 3rd person
for review (and ACK)
