diff -u sssd-1.13.4/debian/changelog sssd-1.13.4/debian/changelog --- sssd-1.13.4/debian/changelog +++ sssd-1.13.4/debian/changelog @@ -1,3 +1,14 @@ +sssd (1.13.4-1ubuntu1.13) xenial; urgency=medium + + [Orion Poplawski] + * Add upstream HBAC patch. Closes LP: #1722936. + + [Andreas Hasenack] + * d/t/{common-tests,control,ldap-user-group-*-auth,login.exp,util}: add DEP8 + tests from later releases of Ubuntu (LP: #1793882) + + -- Andreas Hasenack Fri, 08 Feb 2019 15:08:44 -0200 + sssd (1.13.4-1ubuntu1.12) xenial; urgency=medium * d/p/add-back-pidfile.patch: Re-add PIDFILE entry to diff -u sssd-1.13.4/debian/patches/series sssd-1.13.4/debian/patches/series --- sssd-1.13.4/debian/patches/series +++ sssd-1.13.4/debian/patches/series @@ -10,0 +11 @@ +hbac.patch only in patch2: unchanged: --- sssd-1.13.4.orig/debian/patches/hbac.patch +++ sssd-1.13.4/debian/patches/hbac.patch @@ -0,0 +1,170 @@ +From 88f6d8ad4eef4b4fa032fd451ad732cf8201b0bf Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Jul 27 2017 07:39:58 +0000 +Subject: HBAC: Do not rely on originalMemberOf, use the sysdb memberof links instead + + +The IPA HBAC code used to read the group members from the +originalMemberOf attribute value for performance reasons. However, +especially on IPA clients trusting an AD domain, the originalMemberOf +attribute value is often not synchronized correctly. + +Instead of going through the work of maintaining both member/memberOf +and originalMemberOf, let's just do an ASQ search for the group names of +the groups the user is a member of in the cache and read their +SYSBD_NAME attribute. + +To avoid clashing between similarly-named groups in IPA and in AD, we +look at the container of the group. + +Resolves: +https://pagure.io/SSSD/sssd/issue/3382 + +(cherry picked from commit c92e49144978ad3b6c9fffa8803ebdad8f6f5b18) + +Reviewed-by: Sumit Bose +Reviewed-by: Fabiano Fidêncio + +--- + +diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c +index 72a620e..4afe44e 100644 +--- a/src/providers/ipa/ipa_hbac_common.c ++++ b/src/providers/ipa/ipa_hbac_common.c +@@ -510,14 +510,14 @@ hbac_eval_user_element(TALLOC_CTX *mem_ctx, + struct hbac_request_element **user_element) + { + errno_t ret; +- unsigned int i; + unsigned int num_groups = 0; + TALLOC_CTX *tmp_ctx; +- const char *member_dn; + struct hbac_request_element *users; +- struct ldb_message *msg; +- struct ldb_message_element *el; +- const char *attrs[] = { SYSDB_ORIG_MEMBEROF, NULL }; ++ const char *groupname; ++ struct sss_domain_info *ipa_domain; ++ struct ldb_dn *ipa_groups_basedn; ++ struct ldb_result *res; ++ int exp_comp; + + tmp_ctx = talloc_new(mem_ctx); + if (tmp_ctx == NULL) return ENOMEM; +@@ -530,56 +530,90 @@ hbac_eval_user_element(TALLOC_CTX *mem_ctx, + + users->name = username; + +- /* Read the originalMemberOf attribute +- * This will give us the list of both POSIX and +- * non-POSIX groups that this user belongs to. ++ ipa_domain = get_domains_head(domain); ++ if (ipa_domain == NULL) { ++ ret = EINVAL; ++ goto done; ++ } ++ ++ ipa_groups_basedn = ldb_dn_new_fmt(tmp_ctx, sysdb_ctx_get_ldb(domain->sysdb), ++ SYSDB_TMPL_GROUP_BASE, ipa_domain->name); ++ if (ipa_groups_basedn == NULL) { ++ ret = ENOMEM; ++ goto done; ++ } ++ ++ /* +1 because there will be a RDN preceding the base DN */ ++ exp_comp = ldb_dn_get_comp_num(ipa_groups_basedn) + 1; ++ ++ /* ++ * Get all the groups the user is a member of. ++ * This includes both POSIX and non-POSIX groups. + */ +- ret = sysdb_search_user_by_name(tmp_ctx, domain, users->name, +- attrs, &msg); ++ ret = sysdb_initgroups(tmp_ctx, domain, username, &res); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, +- "Could not determine user memberships for [%s]\n", +- users->name); ++ "sysdb_asq_search failed [%d]: %s\n", ret, sss_strerror(ret)); + goto done; + } + +- el = ldb_msg_find_element(msg, SYSDB_ORIG_MEMBEROF); +- if (el == NULL || el->num_values == 0) { ++ if (res->count == 0) { ++ /* This should not happen at this point */ ++ DEBUG(SSSDBG_MINOR_FAILURE, ++ "User [%s] not found in cache.\n", username); ++ ret = ENOENT; ++ goto done; ++ } else if (res->count == 1) { ++ /* The first item is the user entry */ + DEBUG(SSSDBG_TRACE_LIBS, "No groups for [%s]\n", users->name); + ret = create_empty_grouplist(users); + goto done; + } + DEBUG(SSSDBG_TRACE_LIBS, +- "[%d] groups for [%s]\n", el->num_values, users->name); ++ "[%u] groups for [%s]\n", res->count - 1, username); + +- users->groups = talloc_array(users, const char *, el->num_values + 1); ++ /* This also includes the sentinel, b/c we'll skip the user entry below */ ++ users->groups = talloc_array(users, const char *, res->count); + if (users->groups == NULL) { + ret = ENOMEM; + goto done; + } + +- for (i = 0; i < el->num_values; i++) { +- member_dn = (const char *)el->values[i].data; ++ /* Start counting from 1 to exclude the user entry */ ++ for (size_t i = 1; i < res->count; i++) { ++ /* Only groups from the IPA domain can be referenced from HBAC rules. To ++ * avoid evaluating groups which might even have the same name, but come ++ * from a trusted domain, we first copy the DN to a temporary one.. ++ */ ++ if (ldb_dn_get_comp_num(res->msgs[i]->dn) != exp_comp ++ || ldb_dn_compare_base(ipa_groups_basedn, ++ res->msgs[i]->dn) != 0) { ++ DEBUG(SSSDBG_FUNC_DATA, ++ "Skipping non-IPA group %s\n", ++ ldb_dn_get_linearized(res->msgs[i]->dn)); ++ continue; ++ } + +- ret = get_ipa_groupname(users->groups, domain->sysdb, member_dn, +- &users->groups[num_groups]); +- if (ret != EOK && ret != ERR_UNEXPECTED_ENTRY_TYPE) { ++ groupname = ldb_msg_find_attr_as_string(res->msgs[i], SYSDB_NAME, NULL); ++ if (groupname == NULL) { + DEBUG(SSSDBG_MINOR_FAILURE, +- "Skipping malformed entry [%s]\n", member_dn); ++ "Skipping malformed entry [%s]\n", ++ ldb_dn_get_linearized(res->msgs[i]->dn)); + continue; +- } else if (ret == EOK) { +- DEBUG(SSSDBG_TRACE_LIBS, "Added group [%s] for user [%s]\n", +- users->groups[num_groups], users->name); +- num_groups++; ++ } ++ ++ users->groups[num_groups] = talloc_strdup(users->groups, groupname); ++ if (users->groups[num_groups] == NULL) { + continue; + } +- /* Skip entries that are not groups */ +- DEBUG(SSSDBG_TRACE_INTERNAL, +- "Skipping non-group memberOf [%s]\n", member_dn); ++ ++ DEBUG(SSSDBG_TRACE_LIBS, "Added group [%s] for user [%s]\n", ++ users->groups[num_groups], users->name); ++ num_groups++; + } + users->groups[num_groups] = NULL; + +- if (num_groups < el->num_values) { ++ if (num_groups < (res->count - 1)) { + /* Shrink the array memory */ + users->groups = talloc_realloc(users, users->groups, const char *, + num_groups+1); + only in patch2: unchanged: --- sssd-1.13.4.orig/debian/tests/common-tests +++ sssd-1.13.4/debian/tests/common-tests @@ -0,0 +1,28 @@ +#!/bin/sh + +run_common_tests() { + echo "Assert local user databases do not have our LDAP test data" + check_local_user "${ldap_user}" + check_local_group "${ldap_user}" + check_local_group "${ldap_group}" + + echo "The LDAP user is known to the system via getent" + check_getent_user "${ldap_user}" + + echo "The LDAP user's private group is known to the system via getent" + check_getent_group "${ldap_user}" + + echo "The LDAP group ${ldap_group} is known to the system via getent" + check_getent_group "${ldap_group}" + + echo "The id(1) command can resolve the group membership of the LDAP user" + #$ id -Gn testuser1 + #testuser1 ldapusers + output=$(id -Gn ${ldap_user}) + # XXX couldn't find a better way to make this comparison using just /bin/sh + if [ "${output}" != "${ldap_user} ${ldap_group}" ]; then + if [ "${output}" != "${ldap_group} ${ldap_user}" ]; then + die "Output doesn't match expected group membership: ${output}" + fi + fi +} only in patch2: unchanged: --- sssd-1.13.4.orig/debian/tests/control +++ sssd-1.13.4/debian/tests/control @@ -0,0 +1,7 @@ +Tests: ldap-user-group-ldap-auth +Depends: @, slapd, ldap-utils, openssl, expect, lsb-release +Restrictions: isolation-container, needs-root, allow-stderr + +Tests: ldap-user-group-krb5-auth +Depends: @, slapd, ldap-utils, openssl, expect, lsb-release, krb5-user, krb5-admin-server, krb5-kdc +Restrictions: isolation-container, needs-root, allow-stderr only in patch2: unchanged: --- sssd-1.13.4.orig/debian/tests/ldap-user-group-krb5-auth +++ sssd-1.13.4/debian/tests/ldap-user-group-krb5-auth @@ -0,0 +1,35 @@ +#!/bin/sh + +set -ex + +. debian/tests/util +. debian/tests/common-tests + +mydomain="example.com" +myhostname="ldap.${mydomain}" +mysuffix="dc=example,dc=com" +myrealm="EXAMPLE.COM" +admin_dn="cn=admin,${mysuffix}" +admin_pw="secret" +ldap_user="testuser1" +ldap_user_pw="testuser1secret" +kerberos_principal_pw="testuser1kerberos" +ldap_group="ldapusers" + +adjust_hostname "${myhostname}" +reconfigure_slapd +generate_certs "${myhostname}" +enable_ldap_ssl +populate_ldap_rfc2307 +create_realm "${myrealm}" "${myhostname}" +create_krb_principal "${ldap_user}" "${kerberos_principal_pw}" +configure_sssd_ldap_rfc2307_krb5_auth +enable_pam_mkhomedir + +# tests begin here +run_common_tests + +# login works with the kerneros password +echo "The Kerberos principal can login on a terminal" +kdestroy > /dev/null 2>&1 || /bin/true +/usr/bin/expect -f debian/tests/login.exp "${ldap_user}" "${kerberos_principal_pw}" "${ldap_user}"@"${myrealm}" only in patch2: unchanged: --- sssd-1.13.4.orig/debian/tests/ldap-user-group-ldap-auth +++ sssd-1.13.4/debian/tests/ldap-user-group-ldap-auth @@ -0,0 +1,29 @@ +#!/bin/sh + +set -ex + +. debian/tests/util +. debian/tests/common-tests + +mydomain="example.com" +myhostname="ldap.${mydomain}" +mysuffix="dc=example,dc=com" +admin_dn="cn=admin,${mysuffix}" +admin_pw="secret" +ldap_user="testuser1" +ldap_user_pw="testuser1secret" +ldap_group="ldapusers" + +adjust_hostname "${myhostname}" +reconfigure_slapd +generate_certs "${myhostname}" +enable_ldap_ssl +populate_ldap_rfc2307 +configure_sssd_ldap_rfc2307 +enable_pam_mkhomedir + +# tests begin here +run_common_tests + +echo "The LDAP user can login on a terminal" +/usr/bin/expect -f debian/tests/login.exp "${ldap_user}" "${ldap_user_pw}" only in patch2: unchanged: --- sssd-1.13.4.orig/debian/tests/login.exp +++ sssd-1.13.4/debian/tests/login.exp @@ -0,0 +1,74 @@ +#!/usr/bin/expect + +set timeout 10 +set user [lindex $argv 0] +set password [lindex $argv 1] +set principal [lindex $argv 2] + +set distribution [exec "lsb_release" "-is"] + +if { $distribution == "Ubuntu" } { + set welcome "Welcome to" +} elseif { $distribution == "Debian" } { + set welcome "Debian GNU/Linux comes" +} else { + puts "Unsupported linux distribution $distribution" + exit 1 +} + +spawn login +expect "login:" +send "$user\r" +expect "Password:" +send "$password\r" +expect { + timeout + { + puts "Expect error: timeout after password\r\r" + exit 1 + } + "Login incorrect" + { + puts "Expect error: incorrect credentials\r\r" + exit 1 + } + "$welcome" +} +expect { + timeout + { + puts "Expect error: timeout waiting for prompt\r\r" + exit 1 + } + "$ " +} +send "id -un\r" +expect { + timeout + { + puts "Expect error: timeout waiting for 'id' result\r\r" + exit 1 + } + "$user" +} +expect { + timeout + { + puts "Expect error: timeout waiting for prompt\r\r" + exit 1 + } + "$ " +} +if { $principal != "" } { + send "klist\r" + expect { + timeout + { + puts "Expect error: timeout waiting for klist output\r\r" + exit 1 + } + "Default principal: $principal" + } +} +send "logout\r" +exit 0 only in patch2: unchanged: --- sssd-1.13.4.orig/debian/tests/util +++ sssd-1.13.4/debian/tests/util @@ -0,0 +1,259 @@ +#!/bin/sh + +reconfigure_slapd() { + debconf-set-selections << EOF +slapd slapd/domain string ${mydomain} +slapd shared/organization string ${mydomain} +slapd slapd/password1 password ${admin_pw} +slapd slapd/password2 password ${admin_pw} +EOF + rm -rf /var/backups/*slapd* /var/backups/unknown*ldapdb + dpkg-reconfigure -fnoninteractive -pcritical slapd +} + +die() { + echo "ERROR" + echo "$@" + exit 1 +} + +enable_pam_mkhomedir() { + if ! grep -qE "^session.*pam_mkhomedir\.so" /etc/pam.d/common-session; then + echo "session optional pam_mkhomedir.so" >> /etc/pam.d/common-session + fi +} + +adjust_hostname() { + local myhostname="$1" + + echo "${myhostname}" > /etc/hostname + hostname "${myhostname}" + if ! grep -qE "${myhostname}" /etc/hosts; then + # just so it's resolvable + echo "127.0.1.10 ${myhostname}" >> /etc/hosts + fi +} + +generate_certs() { + local cn="$1" + local cert="/etc/ldap/server.pem" + local key="/etc/ldap/server.key" + local cnf="/etc/ldap/openssl.cnf" + + cat > "$cnf" < /etc/ldap/ldap.conf < /etc/sssd/sssd.conf < /etc/sssd/sssd.conf < /etc/krb5kdc/kdc.conf < /etc/krb5.conf < /etc/krb5kdc/kadm5.acl + + # create the realm + kdb5_util create -s -P secretpassword + + # restart services + systemctl restart krb5-kdc.service krb5-admin-server.service +} + +create_krb_principal() { + local principal="$1" + local password="$2" + + kadmin.local -q "addprinc -pw ${password} ${principal}" +} +