URL: https://github.com/freeipa/freeipa/pull/2732
Author: abbra
 Title: #2732: Dcerpc fix conflict resolution
Action: opened

PR body:
"""
This pull request attempts to fix an issue found by Red Hat QE in our trust 
code. When we try to automatically handle a topology collision during creation 
of a forest trust, we might get back a topology that includes a number of 
records of a type we don't expect (see more details in the commit message). The 
solution is to make sure we use address those structure's members by their 
proper names.

A second change is fixing a failing 'fastcheck' for me.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/2732/head:pr2732
git checkout pr2732
From 0859f8eb2d5429170d8d96c1e9b92fd67a729c82 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Mon, 7 Jan 2019 15:28:29 +0200
Subject: [PATCH 1/2] ipaserver/dcerpc: fix exclusion entry with a forest trust
 domain info returned

When looking through the topology of a trusted forest, we should support
all types of forest trust records. Since Samba Python bindings parse the
data into a typed structure, a type of the record has to be taken into
account or there will be type mismatch when accessing elements of the
union:

        typedef [switch_type(lsa_ForestTrustRecordType)] union {
                [case(LSA_FOREST_TRUST_TOP_LEVEL_NAME)] lsa_StringLarge top_level_name;
                [case(LSA_FOREST_TRUST_TOP_LEVEL_NAME_EX)] lsa_StringLarge top_level_name_ex;
                [case(LSA_FOREST_TRUST_DOMAIN_INFO)] lsa_ForestTrustDomainInfo domain_info;
                [default] lsa_ForestTrustBinaryData data;
        } lsa_ForestTrustData;

        typedef struct {
                lsa_ForestTrustRecordFlags flags;
                lsa_ForestTrustRecordType type;
                NTTIME_hyper time;
                [switch_is(type)] lsa_ForestTrustData forest_trust_data;
        } lsa_ForestTrustRecord;

        typedef [public] struct {
                [range(0,4000)] uint32 count;
                [size_is(count)] lsa_ForestTrustRecord **entries;
        } lsa_ForestTrustInformation;

Each entry in the lsa_ForestTrustInformation has forest_trust_data
member but its content depends on the value of a type member
(forest_trust_data is a union of all possible structures).

Previously we assumed only TLN or TLN exclusion record which were
of the same type (lsa_StringLarge). Access to forest_trust_data.string
fails when forest_trust_data's type is lsa_ForestTrustDomainInfo as it
has no string member.

Fix the code by properly accessing the dns_domain_name from the
lsa_ForestTrustDomainInfo structure.

Fixes: https://pagure.io/freeipa/issue/7828
---
 ipaserver/dcerpc.py | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 5e841350b8..cc80849a7d 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -1105,6 +1105,25 @@ def clear_ftinfo_conflict(self, another_domain, cinfo):
         original forest.
         """
 
+        def domain_name_from_ftinfo(ftinfo):
+            """
+            Returns a domain name string from a ForestTrustRecord
+
+            :param ftinfo: LSA ForestTrustRecord to parse
+            """
+            if ftinfo.type == lsa.LSA_FOREST_TRUST_DOMAIN_INFO:
+                return ftinfo.forest_trust_data.dns_domain_name.string
+            elif ftinfo.type == lsa.LSA_FOREST_TRUST_TOP_LEVEL_NAME:
+                return ftinfo.forest_trust_data.string
+            elif ftinfo.type == lsa.LSA_FOREST_TRUST_TOP_LEVEL_NAME_EX:
+                # We should ignore TLN exclusion record because it
+                # is already an exclusion so we aren't going to
+                # change anything here
+                return None
+            else:
+                # Ignore binary blobs we don't know about
+                return None
+
         # List of entries for unsolved conflicts
         result = []
 
@@ -1145,18 +1164,26 @@ def clear_ftinfo_conflict(self, another_domain, cinfo):
                     e1.time = e.time
                     e1.forest_trust_data = e.forest_trust_data
 
+                    # We either have a domain struct, a TLN name,
+                    # or a TLN exclusion name in the list.
+                    # The rest we should skip, those are binary blobs
+                    dns_domain_name = domain_name_from_ftinfo(e)
+
                     # Search for a match in the topology of another domain
                     # if there is a match, we have to convert a record
                     # into a TLN exclusion to allow its routing to the
                     # another domain
                     for r in another_domain.ftinfo_records:
-                        if r['rec_name'] == e.forest_trust_data.string:
+                        # r['rec_name'] cannot be None, thus we can ignore
+                        # the case when dns_domain_name is None
+                        if r['rec_name'] == dns_domain_name:
                             is_our_record = True
 
                             # Convert e1 into an exclusion record
                             e1.type = lsa.LSA_FOREST_TRUST_TOP_LEVEL_NAME_EX
                             e1.flags = 0
                             e1.time = trust_timestamp
+                            e1.forest_trust_data.string = dns_domain_name
                             break
                     entries.append(e1)
 

From 8e37c002b8877d57670fd822b15445b263f4e417 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Tue, 8 Jan 2019 09:54:07 +0200
Subject: [PATCH 2/2] make sure IPA_CONFDIR is used to check that client is
 configured

Fixes a test ipatests/test_cmdline/test_cli.py:test_cli_fs_encoding()
which sets IPA_CONFDIR and attempts to interpret the resulting error
message. However, if the test is run on an enrolled machine (a
developer's laptop, for example), check_client_configuration() will
succeed because it ignores IPA_CONFDIR and, as result, api.finalize()
will fail later with a stacktrace.

Pass an environment object and test an overridden config file existence
in this case to fail with a proper and expected message.
---
 ipalib/cli.py  | 2 +-
 ipalib/util.py | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 5d10aacae1..ecd9ef2b3b 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -1454,7 +1454,7 @@ def run(api):
         (_options, argv) = api.bootstrap_with_global_options(context='cli')
 
         try:
-            check_client_configuration()
+            check_client_configuration(env=api.env)
         except ScriptError as e:
             sys.exit(e)
 
diff --git a/ipalib/util.py b/ipalib/util.py
index e68414544c..bf46950db7 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -1123,15 +1123,16 @@ def ensure_krbcanonicalname_set(ldap, entry_attrs):
     entry_attrs.update(old_entry)
 
 
-def check_client_configuration():
+def check_client_configuration(env=None):
     """
     Check if IPA client is configured on the system.
 
     Hardcode return code to avoid recursive imports
     """
-    if (not os.path.isfile(paths.IPA_DEFAULT_CONF) or
+    if ((env is not None and not os.path.isfile(env.conf_default)) or
+       (not os.path.isfile(paths.IPA_DEFAULT_CONF) or
             not os.path.isdir(paths.IPA_CLIENT_SYSRESTORE) or
-            not os.listdir(paths.IPA_CLIENT_SYSRESTORE)):
+            not os.listdir(paths.IPA_CLIENT_SYSRESTORE))):
         raise ScriptError('IPA client is not configured on this system',
                           2)  # CLIENT_NOT_CONFIGURED
 
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org

Reply via email to