On Thu, Jan 31, 2013 at 11:52:53AM +0100, Pavel Březina wrote:
>
https://fedorahosted.org/sssd/ticket/1795
> From aba6be77012e2c4f741d78e887f9524b869b78ae Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
> Date: Thu, 31 Jan 2013 11:21:06 +0100
> Subject: [PATCH 1/2] util: add replace_char() function
>
> Replace all occurrences of character in string with other character.
Please add a unit test for new utility functions. I know that this is
a trivial C function, but in general I think we should start to be far
stricter especially with new code now that we are not rushing for a deadline.
I would also say that we should be even more defensive because currently
the libkrb5 code only allows these file names (see valid_name() function
in src/util/profile/prof_parse.c):
if (!isalnum((unsigned char)*p) && *p != '-' && *p !=
'_')
return invalid;
So in my opinion we should replace any character that doesn't match the
above. Dot would be the typical case, but I don't think we guarantee
that other weird characters don't appear in the domain name. Debugging
why the file didn't load would then be very hard.
Replacing more than one character would mean you can end up with collisions.
I am not against it just noting that the problem immediately becomes
less than trivial.
Example:
domain@foo
domain.foo
will translate into the same if you replace both weird characters with '_' .
> ---
> src/util/util.c | 19 +++++++++++++++++++
> src/util/util.h | 3 +++
> 2 files changed, 22 insertions(+)
>
> diff --git a/src/util/util.c b/src/util/util.c
> index
ba85e0da2a2e03784c5bb9abbbd038ddaeb3455c..45fea9dd1c0f85e8279ff758e4764fde5203426b 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -688,3 +688,22 @@ void safezero(void *data, size_t size)
> *p++ = 0;
> }
> }
> +
> +char *replace_char(TALLOC_CTX *mem_ctx, char needle, char new,
> + const char *haystack)
Maybe make it clear with the function name that the replace is not done
in-place?
> +{
> + char *str = NULL;
> + char *pos = NULL;
> +
You don't need to initialize the pointers here as they are assigned on
the next line..
> + str = talloc_strdup(mem_ctx, haystack);
> + if (str == NULL) {
> + return NULL;
> + }
> +
> + pos = str;
> + while ((pos = strchr(pos, needle)) != NULL) {
> + *pos = new;
> + }
> +
> + return str;
> +}
> diff --git a/src/util/util.h b/src/util/util.h
> index
df1ee3b08df71c18bd5201ce12ac6c0fbae1251d..d76f281707aa208f3a3d3ff9e3b2e7d3719c0178 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -550,6 +550,9 @@ bool string_in_list(const char *string, char **list, bool
case_sensitive);
> */
> void safezero(void *data, size_t size);
>
> +char *replace_char(TALLOC_CTX *mem_ctx, char needle, char new,
> + const char *haystack);
> +
> /* from sss_tc_utf8.c */
> char *
> sss_tc_utf8_str_tolower(TALLOC_CTX *mem_ctx, const char *s);
> --
> 1.7.11.7
>
> From 1117a2b3e103b61d0d7be029ee41b7f2874056ae Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
> Date: Thu, 31 Jan 2013 11:22:12 +0100
> Subject: [PATCH 2/2] subdomains: replace dot with underscore in krb5 mapping
> file name
>
>
https://fedorahosted.org/sssd/ticket/1795
>
> krb5 ignores files that contain dot in their name
> ---
> src/providers/ipa/ipa_subdomains.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
> index
ef6195d19de72be7fd2b12a309b33fcf20e0e3a1..30ebfd7d2b6c13f878a4ac8542379c7492710d89 100644
> --- a/src/providers/ipa/ipa_subdomains.c
> +++ b/src/providers/ipa/ipa_subdomains.c
> @@ -25,6 +25,7 @@
> #include "providers/ldap/sdap_async.h"
> #include "providers/ipa/ipa_subdomains.h"
> #include "providers/ipa/ipa_common.h"
> +#include "util/util.h"
> #include <ctype.h>
>
> #define SUBDOMAINS_FILTER "objectclass=ipaNTTrustedDomain"
> @@ -286,6 +287,7 @@ ipa_subdomains_write_mappings(struct sss_domain_info *domain,
> errno_t ret;
> errno_t err;
> TALLOC_CTX *tmp_ctx;
> + const char *sanitized_domain;
> const char *mapping_file;
> char *tmp_file = NULL;
> int fd = -1;
> @@ -296,8 +298,16 @@ ipa_subdomains_write_mappings(struct sss_domain_info *domain,
> tmp_ctx = talloc_new(NULL);
> if (!tmp_ctx) return ENOMEM;
>
> + /* replace dot with underscore, because files that contains dot in name
> + * are ignored by krb5 */
As said in the previous comment, it's not just dots. Also the
requirement is only valid for included files in an includedir, not files
read by krb5 in general.
> + sanitized_domain = replace_char(tmp_ctx, '.', '_',
domain->name);
> + if (sanitized_domain == NULL) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> mapping_file = talloc_asprintf(tmp_ctx, "%s/domain_realm_%s",
> - IPA_SUBDOMAIN_MAPPING_DIR, domain->name);
> + IPA_SUBDOMAIN_MAPPING_DIR, sanitized_domain);
> if (!mapping_file) {
> ret = ENOMEM;
> goto done;
> --
> 1.7.11.7
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Sr. Engineering Manager for IdM portfolio
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?