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

Jakub Hrozek jhrozek at redhat.com
Thu Feb 14 13:20:03 UTC 2013


On Tue, Feb 12, 2013 at 11:44:25AM +0100, Pavel Březina wrote:
> On 02/01/2013 12:32 PM, Pavel Březina wrote:
> >On 01/31/2013 08:07 PM, Jakub Hrozek wrote:
> >>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.
> >
> >No, thank you.
> >
> >
> >Just kidding :-) I switched to the condition below, which is not generic
> >enough to create a separate utility function.
> >
> >>
> >>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.
> >
> >I tried $ and @ characters in addition to dot and we fail to initialize
> >D-Bus with those characters present. They must be escaped to be used in
> >D-Bus.
> >
> >>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..
> >
> >I know. It is my habit to always initialize variables (except ret).
> >
> >New patch is attached.
> 
> Rebased patch is attached.
> 

This patch looks good to me, Ack.

I also filed https://fedorahosted.org/sssd/ticket/1809 to track a safer
method.



More information about the sssd-devel mailing list