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