On Fri, 04 Oct 2013, Alexander Bokovoy wrote:
On Fri, 04 Oct 2013, Alexander Bokovoy wrote:
On Thu, 03 Oct 2013, Martin Kosek wrote:
On 10/03/2013 03:10 PM, Alexander Bokovoy wrote:
On Wed, 02 Oct 2013, Sumit Bose wrote:
Please note that I did not test with more than 1 subdomain, since I
do not have more ADs available.


I have done some testing as well and the patches are working as expected
except the trustdomain-disable issue Tomas mentioned. But I think it
would be sufficient to add a comment to the release notes and fix this
with the next release to not delay this release anymore.

The patches are also working for trusts which were added with older
releases. So ACK from my side for the functional part.
New patchset is attached. I've fixed all outstanding issues and
implemented proper SID filtering for subdomains. In addition, I've
added MS-PAC cache eviction when we change blacklists from IPA side
and forced removal of the domain from SID blacklist if the domain is
being removed by trustdomain-del.


1) Minor issue in 0118:

+        if keys[0].lower() == keys[1].lower():
+            raise errors.ValidationError(name='trustdomain_enable',
+                error=_("Root domain of the trust is always enabled for the
existing trust"))

The error message looks weird (double trustdomain_enable)

# ipa trustdomain-enable realm domain
ipa: ERROR: invalid 'trustdomain_enable': Root domain of the trust is always
enabled for the existing trust

I would rather do something like

+            raise errors.ValidationError(name='domain',


2) trustconfig-enable and trustconfig-disable should use standard output like
other enable/disable methods. See user-enable/user-disable for example. Current
situation puts all the authoritative information to summary which:
a) Does not look nice in terminal
# ipa trustdomain-disable very.long.long.long.realm very.long.long.long.domain
----------------------------------------------------------------------------------------------------------------------------
Domain very.long.long.long.domain of trust very.long.long.long.realm is not
allowed to access IPA resources
----------------------------------------------------------------------------------------------------------------------------
b) How am I supposed to parse an information about the result if all I get is a
text in summary? Using standard errors and output values will allow easier
consumption of the API later (like in Web UI).

I am attaching a patch (0001) how to make it consistent with other
enable/disable commands. Example:

# ipa trustdomain-disable realm domain
ipa: ERROR: This entry is already disabled

# ipa trustdomain-enable realm domain
-----------------------------
Enabled trust domain "domain"
-----------------------------

3) Let's use standard primary key for the trustdomain object. This will let us
overcome some hacks and also let us use handle_not_found method - patch
attached (0002).

0002 also changes some ValidationError errors to standard errors, just for
being consistent with the rest of the API.

Note that in order to make primary_key=True, I had to enhance trustdomain_del
command to manage multiple domains.


I think these API fixes are a must, it would be very hard to amend the API
later. If these patches are squashed to your 0118, it would be ACK from me to
the Python side. I will let C parts and a functional test to Sumit's mighty 
hands.
Thanks. I've merged these changes, along with a API.txt correction. In
my tests these worked fine.

I'll resend 0118 shortly.
New edition of 0118 attached.
... and updated 0124 to match the 0118.

--
/ Alexander Bokovoy
>From 34dc771417b247de180fe490da9fe9cb09644fee Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <[email protected]>
Date: Thu, 3 Oct 2013 12:30:44 +0200
Subject: [PATCH 8/8] ipa-kdb: Handle parent-child relationship for subdomains

When MS-PAC information is re-initialized, record also parent-child
relationship between trust root level domain and its subdomains.

Use parent incoming SID black list to check if child domain is not
allowed to access IPA realm.

We also should really use 'cn' of the entry as domain name.
ipaNTTrustPartner has different meaning on wire, it is an index
pointing to the parent domain of the domain and will be 0 for top
level domains or disjoint subdomains of the trust.

Finally, trustdomain-enable and trustdomain-disable commands should
force MS-PAC cache re-initalization in case of black list change.
Trigger that by asking for cross-realm TGT for HTTP service.
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 109 ++++++++++++++++++++++++++++++++++++----
 ipalib/plugins/trust.py         |   6 +++
 2 files changed, 105 insertions(+), 10 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index e20de36..ff67391 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -37,6 +37,8 @@ struct ipadb_adtrusts {
     int len_sid_blacklist_incoming;
     struct dom_sid *sid_blacklist_outgoing;
     int len_sid_blacklist_outgoing;
+    struct ipadb_adtrusts *parent;
+    char *parent_name;
 };
 
 struct ipadb_mspac {
@@ -1359,6 +1361,18 @@ static krb5_error_code filter_logon_info(krb5_context 
context,
         return EINVAL;
     }
 
+    /* Check if this domain has been filtered out by the trust itself*/
+    if (domain->parent != NULL) {
+        for(k = 0; k < domain->parent->len_sid_blacklist_incoming; k++) {
+            result = dom_sid_check(info->info->info3.base.domain_sid,
+                                   &domain->parent->sid_blacklist_incoming[k], 
true);
+            if (result) {
+                
filter_logon_info_log_message(info->info->info3.base.domain_sid);
+                return EINVAL;
+            }
+        }
+    }
+
     /* According to MS-KILE 25.0, info->info->info3.sids may be non zero, so 
check
      * should include different possibilities into account
      * */
@@ -2121,6 +2135,8 @@ void ipadb_mspac_struct_free(struct ipadb_mspac **mspac)
             free((*mspac)->trusts[i].domain_sid);
             free((*mspac)->trusts[i].sid_blacklist_incoming);
             free((*mspac)->trusts[i].sid_blacklist_outgoing);
+            free((*mspac)->trusts[i].parent_name);
+            (*mspac)->trusts[i].parent = NULL;
         }
         free((*mspac)->trusts);
     }
@@ -2209,18 +2225,42 @@ done:
     return ret;
 }
 
+static void ipadb_free_sid_blacklists(char ***sid_blacklist_incoming, char 
***sid_blacklist_outgoing)
+{
+    int i;
+
+    if (sid_blacklist_incoming && *sid_blacklist_incoming) {
+        for (i = 0; *sid_blacklist_incoming && (*sid_blacklist_incoming)[i]; 
i++) {
+            free((*sid_blacklist_incoming)[i]);
+        }
+        free(*sid_blacklist_incoming);
+        *sid_blacklist_incoming = NULL;
+    }
+
+    if (sid_blacklist_outgoing && *sid_blacklist_outgoing) {
+        for (i = 0; *sid_blacklist_outgoing && (*sid_blacklist_outgoing)[i]; 
i++) {
+            free((*sid_blacklist_outgoing)[i]);
+        }
+        free(*sid_blacklist_outgoing);
+        *sid_blacklist_outgoing = NULL;
+    }
+}
+
 krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx)
 {
     struct ipadb_adtrusts *t;
     LDAP *lc = ipactx->lcontext;
-    char *attrs[] = { "ipaNTTrustPartner", "ipaNTFlatName",
+    char *attrs[] = { "cn", "ipaNTTrustPartner", "ipaNTFlatName",
                       "ipaNTTrustedDomainSID", "ipaNTSIDBlacklistIncoming",
                       "ipaNTSIDBlacklistOutgoing", NULL };
     char *filter = "(objectclass=ipaNTTrustedDomain)";
     krb5_error_code kerr;
     LDAPMessage *res = NULL;
     LDAPMessage *le;
+    LDAPRDN rdn;
     char *base = NULL;
+    char *dnstr = NULL;
+    char *dnl = NULL;
     char **sid_blacklist_incoming = NULL;
     char **sid_blacklist_outgoing = NULL;
     int ret, n, i;
@@ -2243,6 +2283,13 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct 
ipadb_context *ipactx)
     }
 
     for (le = ldap_first_entry(lc, res); le; le = ldap_next_entry(lc, le)) {
+        dnstr = ldap_get_dn(lc, le);
+
+        if (dnstr == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+
         n = ipactx->mspac->num_trusts;
         ipactx->mspac->num_trusts++;
         t = realloc(ipactx->mspac->trusts,
@@ -2253,7 +2300,9 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct 
ipadb_context *ipactx)
         }
         ipactx->mspac->trusts = t;
 
-        ret = ipadb_ldap_attr_to_str(lc, le, "ipaNTTrustPartner",
+        memset(&t[n], 0, sizeof(t[n]));
+
+        ret = ipadb_ldap_attr_to_str(lc, le, "cn",
                                      &t[n].domain_name);
         if (ret) {
             ret = EINVAL;
@@ -2287,6 +2336,7 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct 
ipadb_context *ipactx)
             if (ret == ENOENT) {
                 /* This attribute is optional */
                 ret = 0;
+                sid_blacklist_incoming = NULL;
             } else {
                 ret = EINVAL;
                 goto done;
@@ -2300,6 +2350,7 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct 
ipadb_context *ipactx)
             if (ret == ENOENT) {
                 /* This attribute is optional */
                 ret = 0;
+                sid_blacklist_outgoing = NULL;
             } else {
                 ret = EINVAL;
                 goto done;
@@ -2312,6 +2363,49 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct 
ipadb_context *ipactx)
         if (ret) {
             goto done;
         }
+        ipadb_free_sid_blacklists(&sid_blacklist_incoming,
+                                  &sid_blacklist_outgoing);
+
+        /* Parse first two RDNs of the entry to find its parent */
+        dnl = strcasestr(dnstr, base);
+        if (dnl == NULL) {
+            goto done;
+        }
+
+        /* Note that after ldap_str2rdn() call dnl will point to end of one RDN
+         * which would be '\0' for trust root domain and ',' for subdomain */
+        dnl--; dnl[0] = '\0';
+        ret = ldap_str2rdn(dnstr, &rdn, &dnl, LDAP_DN_FORMAT_LDAPV3);
+        if (ret) {
+            goto done;
+        }
+
+        ldap_rdnfree(rdn);
+
+        if (dnl[0] != '\0') {
+            dnl++;
+            ret = ldap_str2rdn(dnl, &rdn, &dnl, LDAP_DN_FORMAT_LDAPV3);
+            if (ret) {
+                goto done;
+            }
+            t[n].parent_name = strndup(rdn[0]->la_value.bv_val, 
rdn[0]->la_value.bv_len);
+            ldap_rdnfree(rdn);
+        }
+
+        free(dnstr);
+        dnstr = NULL;
+    }
+
+    /* Traverse through all trusts and resolve parents */
+    t = ipactx->mspac->trusts;
+    for (i = 0; i < ipactx->mspac->num_trusts; i++) {
+        if (t[i].parent_name != NULL) {
+            for (n = 0; n < ipactx->mspac->num_trusts; n++) {
+                if (strcasecmp(t[i].parent_name, t[n].domain_name) == 0) {
+                    t[i].parent = &t[n];
+                }
+            }
+        }
     }
 
     ret = 0;
@@ -2320,15 +2414,10 @@ done:
     if (ret != 0) {
         krb5_klog_syslog(LOG_ERR, "Failed to read list of trusted domains");
     }
+    free(dnstr);
     free(base);
-    for (i = 0; sid_blacklist_incoming && sid_blacklist_incoming[i]; i++) {
-        free(sid_blacklist_incoming[i]);
-    }
-    free(sid_blacklist_incoming);
-    for (i = 0; sid_blacklist_outgoing && sid_blacklist_outgoing[i]; i++) {
-        free(sid_blacklist_outgoing[i]);
-    }
-    free(sid_blacklist_outgoing);
+    ipadb_free_sid_blacklists(&sid_blacklist_incoming,
+                              &sid_blacklist_outgoing);
     ldap_msgfree(res);
     return ret;
 }
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 5a1ec17..8aa6ed5 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -1286,6 +1286,9 @@ class trustdomain_enable(LDAPQuery):
             if sid in trust_entry['ipantsidblacklistincoming']:
                 trust_entry['ipantsidblacklistincoming'].remove(sid)
                 ldap.update_entry(trust_entry)
+                # Force MS-PAC cache re-initialization on KDC side
+                domval = ipaserver.dcerpc.DomainValidator(api)
+                (ccache_name, principal) = domval.kinit_as_http(keys[0])
             else:
                 raise errors.AlreadyActive()
         except errors.NotFound:
@@ -1323,6 +1326,9 @@ class trustdomain_disable(LDAPQuery):
             if not (sid in trust_entry['ipantsidblacklistincoming']):
                 trust_entry['ipantsidblacklistincoming'].append(sid)
                 ldap.update_entry(trust_entry)
+                # Force MS-PAC cache re-initialization on KDC side
+                domval = ipaserver.dcerpc.DomainValidator(api)
+                (ccache_name, principal) = domval.kinit_as_http(keys[0])
             else:
                 raise errors.AlreadyInactive()
         except errors.NotFound:
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to