On Sun, Feb 03, 2013 at 07:18:51PM -0500, Simo Sorce wrote:
----- Original Message -----
> 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(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.
>
> 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.
Pavel, this code does not handle domain names that contain unicode
characters outside of the ASCII space.
Neither does the INI parser apparently, so you can't have non-ASCII
characters in the domain name anyway. I tried to name my domain "foó":
[sssd] [confdb_init_db] (0x0010): Parse error reading configuration file
[/etc/sssd/sssd.conf]
[sssd] [ldb] (0x4000): cancel ldb transaction (nesting: 0)
[sssd] [load_configuration] (0x0010): ConfDB initialization has failed [Invalid argument]
[sssd] [main] (0x0020): Error loading configuration database: [22]: Invalid argument
The "Parse error" message comes from calling config_from_fd which is INI
parser's function.
Mayb an idea could be to reaplce only . with _ for the common case,
and if any other 'illegal' char is used, just mangle the name using a
hash function or similar to make it unique.
Simo.
Given that, according to my testing, you can't really put many illegal
characters into the domain name we could stick to the current implementation
and file a ticket to use something like ToASCII defined in RFC3490:
http://tools.ietf.org/html/rfc3490#page-10