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

Lukas Slebodnik lslebodn at redhat.com
Mon Nov 24 21:35:42 UTC 2014


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,
>but it makes sense that the function should accept both
>FQDN and non-FQDN names (so the check makes sense).
>
How would you distiguish between two situations?
1st - the name is not fqdn
2nd - sss_parse_name was called twice

LS



More information about the sssd-devel mailing list