On 11/11/2014 01:37 PM, Jakub Hrozek wrote:
On Thu, Nov 06, 2014 at 07:48:20PM +0100, Michal Židek wrote:
On 11/06/2014 07:43 PM, Michal Židek wrote:
On 11/05/2014 04:53 PM, Michal Židek wrote:
I found this bug while working on
https://fedorahosted.org/sssd/ticket/2461
It turned out that not only case_sensitive = preserving,
but also case_sensitive = false did not work
with proxy provider.
But this patch does not solve the issue in the ticket,
so I did not link them.
After some investigation I actually think this was
the main issue that caused the bug mentioned in
the ticket so I added the "Fixes" label to it.
I also added second patch that preservers the
service name with proxy provider.
Michal
And now with patches :)
Coverity found a warning in the patches:
Error: UNUSED_VALUE (CWE-563):
sssd-1.12.3/src/providers/proxy/proxy_id.c:609: value_overwrite: Value from
"sysdb_attrs_add_string(attrs, "nameAlias", lc_gr_name)" is overwritten with value
"12".
sssd-1.12.3/src/providers/proxy/proxy_id.c:614: value_overwrite: Value from "sysdb_attrs_add_string(attrs,
"nameAlias", lc_gr_name)" is overwritten with value from "sysdb_attrs_add_string(attrs,
"nameAlias", cased_alias)".
sssd-1.12.3/src/providers/proxy/proxy_id.c:623: value_overwrite: Value from "sysdb_attrs_add_string(attrs,
"nameAlias", lc_gr_name)" is overwritten with value from "sysdb_store_group(dom, real_name,
grp->gr_gid, attrs, cache_timeout, now)".
sssd-1.12.3/src/providers/proxy/proxy_id.c:603: returned_value: Value from
"sysdb_attrs_add_string(attrs, "nameAlias", lc_gr_name)" is assigned to "ret"
here, but that stored value is not used before it is overwritten.
Looks like an unchecked call. Apart from that, the code looks good.
Thank you Jakub and Pavel for the review. New patches are attached.
Michal
>From fbd49da512a60d18dad11a5f1751b45db7ded168 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Fri, 31 Oct 2014 16:39:25 +0100
Subject: [PATCH 1/2] proxy: Do not try to store same alias twice
LDB does not store attributes if they have the
same name and value and errors out instead.
Fixes:
https://fedorahosted.org/sssd/ticket/2461
---
src/providers/proxy/proxy_id.c | 76 ++++++++++++++++++++++++++----------------
1 file changed, 48 insertions(+), 28 deletions(-)
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index d867ec4..a4684c3 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -222,6 +222,7 @@ static int save_user(struct sss_domain_info *domain,
struct sysdb_attrs *attrs = NULL;
errno_t ret;
const char *cased_alias;
+ const char *lc_pw_name = NULL;
if (pwd->pw_shell && pwd->pw_shell[0] != '\0') {
shell = pwd->pw_shell;
@@ -239,31 +240,41 @@ static int save_user(struct sss_domain_info *domain,
attrs = sysdb_new_attrs(NULL);
if (!attrs) {
DEBUG(SSSDBG_CRIT_FAILURE, "Allocation error ?!\n");
- return ENOMEM;
+ ret = ENOMEM;
+ goto done;
}
}
if (lowercase) {
- ret = sysdb_attrs_add_lc_name_alias(attrs, pwd->pw_name);
+ lc_pw_name = sss_tc_utf8_str_tolower(attrs, pwd->pw_name);
+ if (lc_pw_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "Cannot convert name to lowercase.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lc_pw_name);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
- talloc_zfree(attrs);
- return ret;
+ ret = ENOMEM;
+ goto done;
}
+
}
if (alias) {
cased_alias = sss_get_cased_name(attrs, alias, !lowercase);
if (!cased_alias) {
- talloc_zfree(attrs);
- return ENOMEM;
+ goto done;
}
- ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, cased_alias);
- if (ret) {
- DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
- talloc_zfree(attrs);
- return ret;
+ /* Add the alias only if it differs from lowercased pw_name */
+ if (lc_pw_name == NULL || strcmp(cased_alias, lc_pw_name) != 0) {
+ ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, cased_alias);
+ if (ret) {
+ DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
+ goto done;
+ }
}
}
@@ -280,13 +291,14 @@ static int save_user(struct sss_domain_info *domain,
NULL,
cache_timeout,
0);
- talloc_zfree(attrs);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, "Could not add user to cache\n");
- return ret;
+ goto done;
}
- return EOK;
+done:
+ talloc_zfree(attrs);
+ return ret;
}
/* =Getpwuid-wrapper======================================================*/
@@ -527,6 +539,7 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
errno_t ret, sret;
struct sysdb_attrs *attrs = NULL;
const char *cased_alias;
+ const char *lc_gr_name = NULL;
TALLOC_CTX *tmp_ctx;
time_t now = time(NULL);
bool in_transaction = false;
@@ -578,27 +591,34 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
goto done;
}
}
+ }
- if (dom->case_sensitive == false) {
- ret = sysdb_attrs_add_lc_name_alias(attrs, grp->gr_name);
- if (ret) {
- DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
- ret = ENOMEM;
- goto done;
- }
+ if (dom->case_sensitive == false) {
+ lc_gr_name = sss_tc_utf8_str_tolower(attrs, grp->gr_name);
+ if (lc_gr_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "Cannot convert name to lowercase.\n");
+ ret = ENOMEM;
+ goto done;
}
- if (alias) {
- cased_alias = sss_get_cased_name(attrs, alias, dom->case_sensitive);
- if (!cased_alias) {
- talloc_zfree(attrs);
- return ENOMEM;
- }
+ ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lc_gr_name);
+ if (ret != EOK) {
+ goto done;
+ }
+ }
+ if (alias) {
+ cased_alias = sss_get_cased_name(attrs, alias, dom->case_sensitive);
+ if (!cased_alias) {
+ ret = ENOMEM;
+ DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
+ goto done;
+ }
+
+ if (lc_gr_name == NULL || strcmp(cased_alias, lc_gr_name)) {
ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, cased_alias);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
- ret = ENOMEM;
goto done;
}
}
--
1.9.3
>From a72282515096f11542a8a0693a5e4ba6a00c2bbc Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Thu, 6 Nov 2014 19:25:59 +0100
Subject: [PATCH 2/2] PROXY: Preserve service name in proxy provider
Fixes:
https://fedorahosted.org/sssd/ticket/2461
---
src/providers/proxy/proxy_services.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/providers/proxy/proxy_services.c b/src/providers/proxy/proxy_services.c
index 16e90b0..2aa44db 100644
--- a/src/providers/proxy/proxy_services.c
+++ b/src/providers/proxy/proxy_services.c
@@ -38,12 +38,14 @@ proxy_save_service(struct sss_domain_info *domain,
const char **protocols;
const char **cased_aliases;
TALLOC_CTX *tmp_ctx;
+ char *lc_alias = NULL;
time_t now = time(NULL);
tmp_ctx = talloc_new(NULL);
if (!tmp_ctx) return ENOMEM;
- cased_name = sss_get_cased_name(tmp_ctx, svc->s_name, !lowercase);
+ cased_name = sss_get_cased_name(tmp_ctx, svc->s_name,
+ domain->case_preserve);
if (!cased_name) {
ret = ENOMEM;
goto done;
@@ -71,6 +73,24 @@ proxy_save_service(struct sss_domain_info *domain,
goto done;
}
+ if (domain->case_preserve) {
+ /* Add lowercased alias to allow case-insensitive lookup */
+ lc_alias = sss_tc_utf8_str_tolower(tmp_ctx, svc->s_name);
+ if (lc_alias == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "Cannot convert name to lowercase.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = add_string_to_list(tmp_ctx, lc_alias,
+ discard_const_p(char **, &cased_aliases));
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Failed to add lowercased name alias.\n");
+ goto done;
+ }
+ }
+
ret = sysdb_store_service(domain,
cased_name,
ntohs(svc->s_port),
--
1.9.3
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
