[SSSD] [PATCH] subdomains: replace dot with underscore in krb5 mapping file name

Jakub Hrozek jhrozek at redhat.com
Thu Jan 31 19:07:44 UTC 2013


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 at 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.

> ---
>  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 at 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 at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list