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 <[email protected]> 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 <[email protected]> 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 -- [email protected] To unsubscribe send an email to [email protected] 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/[email protected]
