URL: https://github.com/SSSD/sssd/pull/215
Title: #215: Support for non-POSIX users and groups
jhrozek commented:
"""
I fixed the minor issues in comments and the man pages. I also fixed the issue
in the Kerberos provider with the following hunk:
```
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 0aa25ac..2faf18d 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -42,6 +42,8 @@
#include "providers/krb5/krb5_utils.h"
#include "providers/krb5/krb5_ccache.h"
+#define NON_POSIX_CCNAME_FMT "MEMORY:sssd_nonposix_dummy_%u"
+
static int krb5_mod_ccname(TALLOC_CTX *mem_ctx,
struct sysdb_ctx *sysdb,
struct sss_domain_info *domain,
@@ -317,7 +319,12 @@ static errno_t krb5_auth_prepare_ccache_name(struct
krb5child_req *kr,
case DOM_TYPE_APPLICATION:
DEBUG(SSSDBG_TRACE_FUNC,
"Domain type application, will use in-memory ccache\n");
- kr->ccname = talloc_asprintf(kr, "MEMORY:%s", kr->pd->user);
+ /* We don't care about using cryptographic randomness, just
+ * a non-predictable ccname, so using rand() here is fine
+ */
+ kr->ccname = talloc_asprintf(kr,
+ NON_POSIX_CCNAME_FMT,
+ rand() % UINT_MAX);
if (kr->ccname == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf failed.\n");
return ENOMEM;
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index e9fe185..cbbc892 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1572,6 +1572,15 @@ static krb5_error_code get_and_save_tgt(struct krb5_req
*kr,
DEBUG(SSSDBG_CONF_SETTINGS, "TGT validation is disabled.\n");
}
+ /* In a non-POSIX environment, we only care about the return code from
+ * krb5_child, so let's not even attempt to create the ccache
+ */
+ if (kr->posix_domain == false) {
+ DEBUG(SSSDBG_TRACE_LIBS,
+ "Finished authentication in a non-POSIX domain\n");
+ goto done;
+ }
+
/* If kr->ccname is cache collection (DIR:/...), we want to work
* directly with file ccache (DIR::/...), but cache collection
* should be returned back to back end.
@@ -1613,18 +1622,6 @@ static krb5_error_code get_and_save_tgt(struct krb5_req
*kr,
"add_ticket_times_and_upn_to_response failed.\n");
}
- /* In a non-POSIX environment, we only care about the return code from
- * krb5_child, so let's just destroy the credentials immediatelly
- */
- if (kr->posix_domain == false) {
- kerr = sss_krb5_cc_destroy(kr->ccname, kr->uid, kr->gid);
- if (kerr != EOK) {
- DEBUG(SSSDBG_OP_FAILURE,
- "Failed to destroy the in-memory ccache\n");
- goto done;
- }
- }
-
kerr = 0;
done:
diff --git a/src/providers/krb5/krb5_init.c b/src/providers/krb5/krb5_init.c
index 12c8dfc..66ae68f 100644
--- a/src/providers/krb5/krb5_init.c
+++ b/src/providers/krb5/krb5_init.c
@@ -136,6 +136,9 @@ errno_t sssm_krb5_init(TALLOC_CTX *mem_ctx,
return ENOMEM;
}
+ /* Only needed to generate random ccache names for non-POSIX domains */
+ srand(time(NULL) * getpid());
+
ret = sss_krb5_get_options(ctx, be_ctx->cdb, be_ctx->conf_path,
&ctx->opts);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get krb5 options [%d]: %s\n",
```
I'm not sure if the srand and rand calls are nice, if you prefer I can just use
some hardcoded name like you suggested..
And I also fixed the issue with the short boolean evaluations with the
following hunk:
```
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index e51cf80..7400dc1 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -297,7 +297,7 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx,
}
}
- if (state->non_posix == true) {
+ if (state->non_posix) {
state->filter = talloc_asprintf(state,
"(&%s(objectclass=%s)(%s=*))",
user_filter,
diff --git a/src/providers/ldap/sdap_async_enum.c
b/src/providers/ldap/sdap_async_enum.c
index 17f1cdf..91e481c 100644
--- a/src/providers/ldap/sdap_async_enum.c
+++ b/src/providers/ldap/sdap_async_enum.c
@@ -754,7 +754,7 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX
*memctx,
goto fail;
}
- if (non_posix == false && use_mapping) {
+ if (!non_posix && use_mapping) {
/* If we're ID-mapping, check for the objectSID as well */
state->filter = talloc_asprintf_append_buffer(
state->filter, "(%s=*)",
diff --git a/src/providers/ldap/sdap_async_initgroups.c
b/src/providers/ldap/sdap_async_initgroups.c
index 6c63a3e..c926ddc 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -2832,7 +2832,7 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX
*memctx,
}
}
- if (state->non_posix == true) {
+ if (state->non_posix) {
state->user_base_filter =
talloc_asprintf_append(state->user_base_filter,
")");
} else if (use_id_mapping) {
```
There was one place where the explicit form was already used for the
use_id_mapping variable, so I left both in the same long form to be consistent
in the single condition.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/215#issuecomment-290111675
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
