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