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

Jakub Hrozek jhrozek at redhat.com
Tue Nov 25 13:01:40 UTC 2014


On Tue, Nov 25, 2014 at 01:29:03PM +0100, 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
> >>
> 
> I tried to find another simple and nice solution
> but it's not possible at the moment.
> The situation might change if users are stored as fqdn in sysdb.
> 
> So it will better to have your solution rather than regression.
> 
> >From 6880eddbc45a02985178b622849898d2634502da Mon Sep 17 00:00:00 2001
> >From: Michal Zidek <mzidek at redhat.com>
> >Date: Mon, 24 Nov 2014 19:10:01 +0100
> >Subject: [PATCH 1/3] test: Wrong parameter type in sss_parse_name_check
> >
> >This caused aritmetic overflow when SSSD specific error
> >codes where used.
> >---
> ACK
> 
> >From 960500afa69f9b80c4812e5c0675c4f6be211fd6 Mon Sep 17 00:00:00 2001
> >From: Michal Zidek <mzidek at redhat.com>
> >Date: Mon, 24 Nov 2014 19:50:14 +0100
> >Subject: [PATCH 2/3] util: Special-case PCRE_ERROR_NOMATCH in sss_parse_name
> >
> >Add new SSSD specific error code for the case when
> >pcre_exec returns PCRE_ERROR_NOMATCH.
> >---
> > src/tests/cmocka/test_fqnames.c | 14 +++++++-------
> > src/util/usertools.c            |  2 +-
> > src/util/util_errors.c          |  1 +
> > src/util/util_errors.h          |  1 +
> > 4 files changed, 10 insertions(+), 8 deletions(-)
> >
> >diff --git a/src/tests/cmocka/test_fqnames.c b/src/tests/cmocka/test_fqnames.c
> >index de20843..b9b6230 100644
> >--- a/src/tests/cmocka/test_fqnames.c
> >+++ b/src/tests/cmocka/test_fqnames.c
> >@@ -471,13 +471,13 @@ void sss_parse_name_fail(void **state)
> >     struct parse_name_test_ctx *test_ctx = talloc_get_type(*state,
> >                                                            struct parse_name_test_ctx);
> > 
> >-    sss_parse_name_check(test_ctx, "", EINVAL, NULL, NULL);
> >-    sss_parse_name_check(test_ctx, "@", EINVAL, NULL, NULL);
> >-    sss_parse_name_check(test_ctx, "\\", EINVAL, NULL, NULL);
> >-    sss_parse_name_check(test_ctx, "\\"NAME, EINVAL, NULL, NULL);
> >-    sss_parse_name_check(test_ctx, "@"NAME, EINVAL, NULL, NULL);
> >-    sss_parse_name_check(test_ctx, NAME"@", EINVAL, NULL, NULL);
> >-    sss_parse_name_check(test_ctx, NAME"\\", EINVAL, NULL, NULL);
> >+    sss_parse_name_check(test_ctx, "", ERR_REGEX_NOMATCH, NULL, NULL);
> >+    sss_parse_name_check(test_ctx, "@", ERR_REGEX_NOMATCH, NULL, NULL);
> >+    sss_parse_name_check(test_ctx, "\\", ERR_REGEX_NOMATCH, NULL, NULL);
> >+    sss_parse_name_check(test_ctx, "\\"NAME, ERR_REGEX_NOMATCH, NULL, NULL);
> >+    sss_parse_name_check(test_ctx, "@"NAME, ERR_REGEX_NOMATCH, NULL, NULL);
> >+    sss_parse_name_check(test_ctx, NAME"@", ERR_REGEX_NOMATCH, NULL, NULL);
> >+    sss_parse_name_check(test_ctx, NAME"\\", ERR_REGEX_NOMATCH, NULL, NULL);
> > }
> > 
> > void test_sss_get_domain_name(void **state)
> >diff --git a/src/util/usertools.c b/src/util/usertools.c
> >index a0b914e..09cfd6b 100644
> >--- a/src/util/usertools.c
> >+++ b/src/util/usertools.c
> >@@ -309,7 +309,7 @@ int sss_parse_name(TALLOC_CTX *memctx,
> > 
> >     ret = pcre_exec(re, NULL, orig, origlen, 0, PCRE_NOTEMPTY, ovec, 30);
> >     if (ret == PCRE_ERROR_NOMATCH) {
> >-        return EINVAL;
> >+        return ERR_REGEX_NOMATCH;
> >     } else if (ret < 0) {
> >         DEBUG(SSSDBG_MINOR_FAILURE, "PCRE Matching error, %d\n", ret);
> >         return EINVAL;
> >diff --git a/src/util/util_errors.c b/src/util/util_errors.c
> >index c1ed0fb..7ecc59e 100644
> >--- a/src/util/util_errors.c
> >+++ b/src/util/util_errors.c
> >@@ -64,6 +64,7 @@ struct err_string error_to_str[] = {
> >     { "Cannot connect to system bus" }, /* ERR_NO_SYSBUS */
> >     { "LDAP search returned a referral" }, /* ERR_REFERRAL */
> >     { "Error setting SELinux user context" }, /* ERR_SELINUX_CONTEXT */
> >+    { "Username format not allowed by re_expression" }, /* ERR_REGEX_MISMATCH */
>                                                              ^^^^^^^^^^^^^^^^^^
>                                                    wrong error code in comment.
>                                                  It should be ERR_REGEX_NOMATCH
> 
> 
> >From 73aefab45fd0b6ff962c2f697f698a60e5de03de 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 3/3] 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
> >---
> ACK
> 
> LS

I fixed the comment and pushed the patches to master:
    * e894a127a9979dea667408b0cced59fedc3bcd0a
    * 8394eddba54b5d3e3fda868145e3751247bdbdb2
    * 5777a98940bd742d213fba8ff55c070c5a058fc6



More information about the sssd-devel mailing list