URL: https://github.com/SSSD/sssd/pull/215 Author: jhrozek Title: #215: Support for non-POSIX users and groups Action: opened
PR body: """ This PR implements https://pagure.io/SSSD/sssd/issue/3310
The goal is to enable application users through the Apache modules or directly through the IFP interface and the PAM interface to authenticate users.
To reproduce, you can add users w/o POSIX information like this to LDAP:
dn: uid=nonposix,cn=users,cn=accounts,dc=ipa,dc=test displayName: new user uid: nonposix krbCanonicalName: nonposix@IPA.TEST objectClass: ipaobject objectClass: person objectClass: top objectClass: ipasshuser objectClass: inetorgperson objectClass: organizationalperson objectClass: krbticketpolicyaux objectClass: krbprincipalaux objectClass: inetuser objectClass: mepOriginEntry initials: nu sn: user mail: nonposix@ipa.test krbPrincipalName: nonposix@IPA.TEST givenName: new cn: new user
And optionally add the user to groups, like this: dn: cn=npgr2,cn=groups,cn=accounts,dc=ipa,dc=test objectClass: ipaobject objectClass: top objectClass: ipausergroup objectClass: groupofnames objectClass: nestedgroup cn: npgr2 member: uid=nonposix,cn=users,cn=accounts,dc=ipa,dc=test
Then, the D-Bus calls like GetUserAttrs should resolve extra attributes of the users, the groups the users are in should be resolvable as well.
In addition, PAM authentication should work against application domains as long as the service invoking the PAM conversation is listed in the 'pam_app_services' option. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/215/head:pr215 git checkout pr215
URL: https://github.com/SSSD/sssd/pull/215 Author: jhrozek Title: #215: Support for non-POSIX users and groups Action: edited
Changed field: body Original value: """ This PR implements https://pagure.io/SSSD/sssd/issue/3310
The goal is to enable application users through the Apache modules or directly through the IFP interface and the PAM interface to authenticate users.
To reproduce, you can add users w/o POSIX information like this to LDAP:
dn: uid=nonposix,cn=users,cn=accounts,dc=ipa,dc=test displayName: new user uid: nonposix krbCanonicalName: nonposix@IPA.TEST objectClass: ipaobject objectClass: person objectClass: top objectClass: ipasshuser objectClass: inetorgperson objectClass: organizationalperson objectClass: krbticketpolicyaux objectClass: krbprincipalaux objectClass: inetuser objectClass: mepOriginEntry initials: nu sn: user mail: nonposix@ipa.test krbPrincipalName: nonposix@IPA.TEST givenName: new cn: new user
And optionally add the user to groups, like this: dn: cn=npgr2,cn=groups,cn=accounts,dc=ipa,dc=test objectClass: ipaobject objectClass: top objectClass: ipausergroup objectClass: groupofnames objectClass: nestedgroup cn: npgr2 member: uid=nonposix,cn=users,cn=accounts,dc=ipa,dc=test
Then, the D-Bus calls like GetUserAttrs should resolve extra attributes of the users, the groups the users are in should be resolvable as well.
In addition, PAM authentication should work against application domains as long as the service invoking the PAM conversation is listed in the 'pam_app_services' option. """
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ CI: http://sssd-ci.duckdns.org/logs/job/65/97/summary.html """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-289805144
URL: https://github.com/SSSD/sssd/pull/215 Author: jhrozek Title: #215: Support for non-POSIX users and groups Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/215/head:pr215 git checkout pr215
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ the second iteration of patches should fix a coverity warning """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-289871457
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
sumit-bose commented: """ Please find my comments in-line. @pbrezina, I wonder if you can have a look at the CACHE_REQ patch to see if it is ok from the design point of view. """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-289901598
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
fidencio commented: """ On Tue, Mar 28, 2017 at 11:14 PM, Jakub Hrozek notifications@github.com wrote:
*@jhrozek* commented on this pull request.
In src/providers/ldap/ldap_id.c https://github.com/SSSD/sssd/pull/215#discussion_r108538847:
@@ -292,7 +297,13 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx,
} }
- if (state->use_id_mapping || filter_type == BE_FILTER_SECID) {
- if (state->non_posix == true) {
state->filter = talloc_asprintf(state,(btw as long as we agree that if (val) and if (!val) is used for bools only then I think it's OK readability wise but I still prefer ptr != NULL over !ptr
+1 here. I also prefer explicit checks for everything that's not boolean.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/SSSD/sssd/pull/215#discussion_r108538847, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG4eqh7BP99rMQWY4eKAuIELYykKJnhks5rqXhPgaJpZM4MrqPB .
"""
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-289908099
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
pbrezina commented: """ ```xml <para> POSIX domains are reachable by all services. Application domains are only reachable from the D-Bus responder (see <citerefentry> <refentrytitle>sssd-ifp</refentrytitle> <manvolnum>5</manvolnum> </citerefentry>) and the PAM responder. </para> ``` The responder name is InfoPipe or IFP not D-Bus. I would rather rephrase it either to `IFP responder` or `through the D-Bus interface`.
Otherwise the patches looks good to me. """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290050853
URL: https://github.com/SSSD/sssd/pull/215 Author: jhrozek Title: #215: Support for non-POSIX users and groups Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/215/head:pr215 git checkout pr215
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
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290113327
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ CI: http://sssd-ci.duckdns.org/logs/job/66/47/summary.html
there is a RHEL6 failure in the enumeration code. Because the test only failed on RHEL-6, I don't think it's related to the changes, but to be on the safe side, I resubmitted the patches to CI again. """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290178794
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ CI is happier now: http://sssd-ci.duckdns.org/logs/job/66/49/summary.html """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290205579
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
pbrezina commented: """ I got error in enumeration as well with my secrets patch (definitely not related), but on debian and in different test: http://sssd-ci.duckdns.org/logs/job/66/45/debian_testing/ci-build-debug/ci-m... """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290347387
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ On Thu, Mar 30, 2017 at 01:59:20AM -0700, Pavel Březina wrote:
I got error in enumeration as well with my secrets patch (definitely not related), but on debian and in different test: http://sssd-ci.duckdns.org/logs/job/66/45/debian_testing/ci-build-debug/ci-m...
And this was with the non-POSIX patches applied as well? Should I look into the enumeration issues with non-POSIX or does it mean the enumeration tests are flaky?
"""
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290353372
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
fidencio commented: """ On Thu, Mar 30, 2017 at 11:23 AM, Jakub Hrozek notifications@github.com wrote:
On Thu, Mar 30, 2017 at 01:59:20AM -0700, Pavel Březina wrote:
I got error in enumeration as well with my secrets patch (definitely not
related), but on debian and in different test:
ci-build-debug/ci-make-intgcheck.log
And this was with the non-POSIX patches applied as well? Should I look into the enumeration issues with non-POSIX or does it mean the enumeration tests are flaky?
I've personally been notice random issues with the enumeration tests lately. Usually different ones on each commit and usually just happening in one distro (and, AFAIR, it's not even consistent about in which distro it does happen).
Best Regards, -- Fabiano Fidêncio
"""
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290354411
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
pbrezina commented: """ On 03/30/2017 11:23 AM, Jakub Hrozek wrote:
On Thu, Mar 30, 2017 at 01:59:20AM -0700, Pavel Březina wrote:
I got error in enumeration as well with my secrets patch (definitely
not related), but on debian and in different test:
http://sssd-ci.duckdns.org/logs/job/66/45/debian_testing/ci-build-debug/ci-m...
And this was with the non-POSIX patches applied as well? Should I look into the enumeration issues with non-POSIX or does it mean the enumeration tests are flaky?
No, it was only with the secrets patches.
"""
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290355914
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
sumit-bose commented: """ I tested the patches with a plain LDAP setup and with and AD. In general they work as expected and since I think the current code is ok I would ACK the patches so that the following observations can be fixed later.
First I have a question about the usage of [application/...] domains. Is it expected that [application/...] requires inherit_from and cannot be configured explicitly? If I use [domain/....] and domain_type = application it work, but if I replace those two line by [application/...] SSSD won't start.
'sssctl config-check' does not like if [application/...] has other options then inherit_from, even the example from the man page causes '[rule/allowed_application_options]: Attribute 'ldap_user_extra_attrs' is not allowed in section 'application/ad-app-2'. Check for typos.'
When using [application/...] with the ad provider other domains than the one the client is joined to are treated as POSIX domains even if only the application domain is listed in in the domains option of sssd.conf.
Given the last observation it might be useful to say in the man page that currently the primary and mainly tested use-case is together with the ldap provider and more complex use cases will be evaluated in upcoming releases? """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290360748
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ On Thu, Mar 30, 2017 at 02:53:18AM -0700, sumit-bose wrote:
I tested the patches with a plain LDAP setup and with and AD. In general they work as expected and since I think the current code is ok I would ACK the patches so that the following observations can be fixed later.
First I have a question about the usage of [application/...] domains. Is it expected that [application/...] requires inherit_from and cannot be configured explicitly? If I use [domain/....] and domain_type = application it work, but if I replace those two line by [application/...] SSSD won't start.
I didn't think about testing this, frankly. I tested a separate domain with the application type which might be useful if you want to e.g. use a different bind method but no this. I think it's a valid case that can be fixed later.
'sssctl config-check' does not like if [application/...] has other options then inherit_from, even the example from the man page causes '[rule/allowed_application_options]: Attribute 'ldap_user_extra_attrs' is not allowed in section 'application/ad-app-2'. Check for typos.'
Hmm, the regex uses (domain|application) in the rules, but I'm not sure if the regex supports the OR..apparently not..
When using [application/...] with the ad provider other domains than the one the client is joined to are treated as POSIX domains even if only the application domain is listed in in the domains option of sssd.conf.
Given the last observation it might be useful to say in the man page that currently the primary and mainly tested use-case is together with the ldap provider and more complex use cases will be evaluated in upcoming releases?
Yes, this is what we talked about with the ManageIQ developers. Since for now the use-case is a replacement for their LDAP connector, I think we should document this and check later. But with the autodiscovered domains, we also need to do some tricks to rename the autodiscovered domains to avoid clashes with subdomains from POSIX domains in a mixed setup.
So if you agree, I will file three tickets for each of the cases and fix them later. I will just fix the manpage for now to make it clear only LDAP domains are supported now.
"""
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290364050
URL: https://github.com/SSSD/sssd/pull/215 Author: jhrozek Title: #215: Support for non-POSIX users and groups Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/215/head:pr215 git checkout pr215
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ the new PR just amends the manpage description of the non-POSIX domains """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290368513
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
jhrozek commented: """ * master: * 861ab44e8148208425b67c4711bc8fade10fd3ed * 3e39806177e1cd383743ff596cb96df44a6ce8c9 * ed0cdfcacc44e4e13e1524e254efa744610a87c2 * 901396366075dc3e3fcc0894345af1b51052ac69 * 5f7f249f2a8a1c7284e991aa64dbf850d482b0aa * 3e789aa0bd6b7bb6e62f91458b76753498030fb5 * 57eeec5d735c7a3bbe58299fded97414626d85f1 * b010f24f4d96d15c5c85021bb4aa83db25cd3df5 * 35f0f5ff9dac790f6c947190fcdc00d01ae9077c * cee85e8fb9534ec997e5388fce59f392cf029573 * 825e8bf2f73a815c2eceb36ae805145fcbacf74d * 6324eaf1fb321c41ca9883966118df6d45259b7e """
See the full comment at https://github.com/SSSD/sssd/pull/215#issuecomment-290392226
URL: https://github.com/SSSD/sssd/pull/215 Author: jhrozek Title: #215: Support for non-POSIX users and groups Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/215/head:pr215 git checkout pr215
URL: https://github.com/SSSD/sssd/pull/215 Title: #215: Support for non-POSIX users and groups
Label: +Pushed
sssd-devel@lists.fedorahosted.org
-
fidencio -
jhrozek
-
pbrezina
-
sumit-bose