[SSSD] [PATCH] util: sss_get_domain_name regex mismatch not fatal

Lukas Slebodnik lslebodn at redhat.com
Tue Nov 25 08:18:39 UTC 2014


On (24/11/14 22:43), Lukas Slebodnik wrote:
>On (24/11/14 22:25), Michal Židek wrote:
>>On 11/24/2014 10:13 PM, Lukas Slebodnik wrote:
>>>On (24/11/14 20:50), Michal Židek wrote:
>>>>On 11/22/2014 05:48 PM, Jakub Hrozek wrote:
>>>>>On Fri, Nov 21, 2014 at 10:26:18PM +0100, Michal Židek wrote:
>>>>>>Hi,
>>>>>>
>>>>>>please see attached patch for ticket
>>>>>>https://fedorahosted.org/sssd/ticket/2487
>>>>>>
>>>>>>Probably in some part of the code we call the
>>>>>>function sss_get_domain_name with FQDN. We use
>>>>>>regular expression to check if it is the case and
>>>>>>if not (and regex is configured not to accept
>>>>>>non FQDN) than this function fails to construct the
>>>>>>requested name in the output.
>>>>>>
>>>>>>Since the sss_parse_name is here only used to detect
>>>>>>if the name contains domain portion, it is safe
>>>>>>to assume it does not contain it if regular
>>>>>>expression has failed to match anything.
>>>>>>
>>>>>>Michal
>>>>>
>>>>>> From 21dc9d402501f6728f743d082f1046ac558195bd Mon Sep 17 00:00:00 2001
>>>>>>From: Michal Zidek <mzidek at redhat.com>
>>>>>>Date: Fri, 21 Nov 2014 20:06:32 +0100
>>>>>>Subject: [PATCH 1/2] util: sss_get_domain_name regex mismatch not fatal
>>>>>>
>>>>>>Assume name is not FQDN if sss_parse_name fails to
>>>>>>match domain with regular expression.
>>>>>>
>>>>>>Fixes:
>>>>>>https://fedorahosted.org/sssd/ticket/2487
>>>>>>---
>>>>>>  src/util/usertools.c | 8 +++++++-
>>>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>
>>>>>>diff --git a/src/util/usertools.c b/src/util/usertools.c
>>>>>>index a0b914e..4168421 100644
>>>>>>--- a/src/util/usertools.c
>>>>>>+++ b/src/util/usertools.c
>>>>>>@@ -646,7 +646,13 @@ sss_get_domain_name(TALLOC_CTX *mem_ctx,
>>>>>>      /* check if the name already contains domain part */
>>>>>>      if (dom->names != NULL) {
>>>>>>          ret = sss_parse_name(mem_ctx, dom->names, orig_name, &domain, NULL);
>>>>>>-        if (ret != EOK) {
>>>>>>+        if (ret == EINVAL) {
>>>>>>+            DEBUG(SSSDBG_TRACE_FUNC,
>>>>>>+                  "sss_parse_name could not parse domain from [%s]. "
>>>>>>+                  "Assuming it is not FQDN.\n", orig_name);
>>>>>
>>>>>What do you think about returning a specific return code from
>>>>>sss_parse_name instead of EINVAL in all cases?
>>>>
>>>>Ok, I added specific error code for the case when regular expression
>>>>does not match anything. In other cases EINVAL is still used.
>>>>
>>>>>
>>>>>Did you check other callers of sss_parse_name()/sss_get_domain_name() ?
>>>>
>>>>Yes. We do not look explicitly for EINVAL being returned in the
>>>>code after these functions are called, so creating the new ERR
>>>>code should not affect anything.
>>>>
>>>>Michal
>>>
>>>I don't think problem was in function sss_parse_name.
>>>The problem is how it this function used (indirectly)
>>>
>>>sss_parse_name was called twice.
>>>the 1st time with fully qualified user name user at domain.
>>>and the 2nd time with extracted name(which was not fqdn) from 1st call.
>>>
>>>LS
>>
>>I disagree. You are right that the function was called twice,
>>once with the FQDN and once with the extracted name. But the
>>function sss_get_domain_name should not fail on simple check if
>>the name has/has not domain part (and it uses sss_parse_name
>>for this check). So the problem indeed was in the way
>>sss_get_domain_name was behaving.
>>
>>I do not know when this check was added to this function,
>If a name cannot be parsed and domain has enabled option
>use_fully_qualified_names then it should fail.
>Your patch broke this behaviour.
>
>LS
>
>BTW bug from ticket #2487 was caused
>by commit 09a36be00ddcf1d7bd5b8a368143d5b2e2f4fb68
>"sss_get_domain_name: check for fq name first"
Next diff fixed regression as well. I am not really sure what was a purpose
of change introduced in comit 09a36be00ddcf1d7bd5b8a368143d5b2e2f4fb68.

diff --git a/src/util/usertools.c b/src/util/usertools.c
index 809b42d..f2f97b5 100644
--- a/src/util/usertools.c
+++ b/src/util/usertools.c
@@ -644,7 +644,7 @@ sss_get_domain_name(TALLOC_CTX *mem_ctx,
     if (dom->names != NULL) {
         ret = sss_parse_name(mem_ctx, dom->names, orig_name, &domain, NULL);
         if (ret != EOK) {
-            return NULL;
+            /* we cannot extract domain name from orig_name */
+            domain = NULL;
         }
     }


LS



More information about the sssd-devel mailing list