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(a)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@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"