On Thu, Jun 17, 2010 at 5:51 PM, Emilien Mantel
<emilien.man...@businessdecision.com> wrote:
> 1) Done
>
> 2) Done
>
> 3) "sizeof(common_name)" is useless... Line 745: char
> common_name[TLS_USERNAME_LEN]; we can use directly TLS_USERNAME_LEN.

Usually sizeof(XXX) should be used so if XXX is modified there is no
overrun (Single point of change).

>
> 4)
> I note "common_name" is used everwhere in OpenVPN code... I can rename it
> with a big sed :)
> But substitute all "common_name" is very heavy :
> emilienm@emilienm:~/dev/openvpn-testing$ grep -i common_name * | wc -l
> 165
> Are you sure it's a good idea? If I do that, are you agree to rename it
> "username"?

Oh... this is not for me to decide, I of course think it should be
done... Maybe as separate patch... it is very confusing to leave it
this way...

>
> --
> Emilien Mantel
>
> Le 17/06/2010 15:57, Alon Bar-Lev a écrit :
>>
>> Great.
>>
>> Few more:
>>
>> 1. To upper:
>> char *s = p[1];
>> while ((*s = toupper(*s)) != '\0') s++;
>> 2. Remove compound {} at this place, move the char *s before the
>> VERIFY_PERMISSION.
>> 3. I think:
>> """
>> extract_x509_field_ssl (X509_get_subject_name (ctx->current_cert),
>> x509_username_field, common_name, TLS_USERNAME_LEN)
>> """
>> TLS_USERNAME_LEN ->  sizeof(common_name)
>> 4. Maybe rename common_name ->  x509_field or something to make it
>> clearer that it isn't actually common_name.
>>
>> Thanks.
>>
>> On Thu, Jun 17, 2010 at 3:53 PM, Emilien Mantel
>> <emilien.man...@businessdecision.com>  wrote:
>>
>>>
>>> I added toupper() + #include<ctype.h>  in options.c
>>>
>>> See attached.
>>>
>>> --
>>> Emilien Mantel
>>>
>>> Le 17/06/2010 14:02, Alon Bar-Lev a écrit :
>>>
>>>>
>>>> This is good idea.
>>>>
>>>> In order to upper case toupper() should be used and not manual guessing.
>>>>
>>>> +  else if (streq (p[0], "x509-username-field")&&    p[1])
>>>> +    {
>>>> +      VERIFY_PERMISSION (OPT_P_GENERAL);
>>>> +      /* Uppercase if necessary */
>>>> +      {
>>>> +       char *s = p[1];
>>>> +       int c, flag = 0;
>>>> +
>>>> +       while ((c = *s) != '\0')
>>>> +          {
>>>> +           if (c>= 'a'&&    c<= 'z')
>>>> +              {
>>>> +                c = c + 'A' - 'a';
>>>> +               flag++;
>>>> +              }
>>>> +            *s = (char) c;
>>>> +            s++;
>>>> +         }
>>>> +      }
>>>> +      options->x509_username_field = p[1];
>>>> +    }
>>>>
>>>> 2010/6/17 Samuli Seppänen<sam...@openvpn.net>:
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> For my company, we use a PKI (linked to a LDAP) with OpenVPN. We can't
>>>>>> use "CN" to be username (few people can have the same "CN"). In our
>>>>>> case, we only use the UID.
>>>>>>
>>>>>> With my patch, you can choose another field to be username with a new
>>>>>> option called "x509-username-field", the default value is "CN".
>>>>>>
>>>>>> Best regards
>>>>>>
>>>>>> --
>>>>>> Emilien Mantel
>>>>>>
>>>>>>
>>>>>
>>>>> Hi Emilien,
>>>>>
>>>>> Thanks for the patch! Could somebody with better C skills take a look
>>>>> and see if it needs modifications?
>>>>>
>>>>> --
>>>>> Samuli Seppänen
>>>>> Community Manager
>>>>> OpenVPN Technologies, Inc
>>>>>
>>>>> irc freenode net: mattock
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> ThinkGeek and WIRED's GeekDad team up for the Ultimate
>>>>> GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
>>>>> lucky parental unit.  See the prize list and enter to win:
>>>>> http://p.sf.net/sfu/thinkgeek-promo
>>>>> _______________________________________________
>>>>> Openvpn-devel mailing list
>>>>> Openvpn-devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> ThinkGeek and WIRED's GeekDad team up for the Ultimate
>>>> GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
>>>> lucky parental unit.  See the prize list and enter to win:
>>>> http://p.sf.net/sfu/thinkgeek-promo
>>>> _______________________________________________
>>>> Openvpn-devel mailing list
>>>> Openvpn-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>>>>
>>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> ThinkGeek and WIRED's GeekDad team up for the Ultimate
>>> GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
>>> lucky parental unit.  See the prize list and enter to win:
>>> http://p.sf.net/sfu/thinkgeek-promo
>>> _______________________________________________
>>> Openvpn-devel mailing list
>>> Openvpn-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>>>
>>>
>>>
>
>

Reply via email to