Re: login_yubikey case-insensitive hex decoding

2012-11-23 Thread Alexander Hall 
On 11/23/12 02:17, Philip Guenther wrote:
 On Thu, Nov 22, 2012 at 5:28 PM, Alexander Hall alexan...@beard.se wrote:
 The corresponding part in yubikey_hex_decode is for consistency and,
 IMO, sanity, allowing mixed case hex strings, e.g. /var/db/yubikey/*.

 Comments? OK? (Don't mess with the from-upstream-derived yubikey.c?)
 ...
 -   if ((p1 = strchr(hex_trans, *src)) == NULL)
 +   if ((p1 = strchr(hex_trans, tolower(*src))) == NULL)
 
 The argument to tolower() must be a value in the range [EOF,
 0..UCHAR_MAX].  When taking characters from a char * string, you need
 to cast the value to (unsigned char), ala
 tolower((unsigned char)*src)
 
 
 Philip Guenther
 

Ok... Is this documented somewhere or just common knowledge?

New diff follows, with missing include added too.

OK?

/Alexander


Index: yubikey.c
===
RCS file: /data/openbsd/cvs/src/libexec/login_yubikey/yubikey.c,v
retrieving revision 1.2
diff -u -p -r1.2 yubikey.c
--- yubikey.c   31 Jan 2012 16:58:38 -  1.2
+++ yubikey.c   23 Nov 2012 15:35:29 -
@@ -32,6 +32,8 @@
  *
  */
 
+#include ctype.h
+
 #include yubikey.h
 
 static const uint8_t RC[] = {
@@ -252,7 +254,8 @@ yubikey_hex_decode(char *dst, const char
char *p1;
 
for (; *src  dstSize  0; src++) {
-   if ((p1 = strchr(hex_trans, *src)) == NULL)
+   p1 = strchr(hex_trans, tolower((unsigned char)*src));
+   if (p1 == NULL)
b = 0;
else
b = (char)(p1 - hex_trans);
@@ -278,7 +281,8 @@ yubikey_modhex_decode(char *dst, const c
char *p1;
 
for (; *src  dstSize  0; src++) {
-   if ((p1 = strchr(modhex_trans, *src)) == NULL)
+   p1 = strchr(modhex_trans, tolower((unsigned char)*src));
+   if (p1 == NULL)
b = 0;
else
b = (char)(p1 - modhex_trans);

Re: login_yubikey case-insensitive hex decoding

2012-11-23 Thread Philip Guenther 
On Fri, 23 Nov 2012, Alexander Hall wrote:
 On 11/23/12 02:17, Philip Guenther wrote:
...
  The argument to tolower() must be a value in the range [EOF,
  0..UCHAR_MAX].  When taking characters from a char * string, you need
  to cast the value to (unsigned char), ala
  tolower((unsigned char)*src)
 
 Ok... Is this documented somewhere or just common knowledge?

It's a property of all the ctype.h functions, which we've documented for 
the is* functions in the CAVEATS section of their pages; here's the diff 
to add the matching statement to tolower(3) and toupper(3).


Philip


Index: gen/tolower.3
===
RCS file: /cvs/src/lib/libc/gen/tolower.3,v
retrieving revision 1.11
diff -u -p -r1.11 tolower.3
--- gen/tolower.3   31 May 2007 19:19:29 -  1.11
+++ gen/tolower.3   23 Nov 2012 22:12:21 -
@@ -99,3 +99,11 @@ and
 .Fn _tolower
 functions conform to
 .St -ansiC .
+.Sh CAVEATS
+The argument to
+.Nm
+must be
+.Dv EOF
+or representable as an
+.Li unsigned char ;
+otherwise, the result is undefined.
Index: gen/toupper.3
===
RCS file: /cvs/src/lib/libc/gen/toupper.3,v
retrieving revision 1.13
diff -u -p -r1.13 toupper.3
--- gen/toupper.3   31 May 2007 19:19:29 -  1.13
+++ gen/toupper.3   23 Nov 2012 22:12:21 -
@@ -97,3 +97,11 @@ The
 .Fn toupper
 function conforms to
 .St -ansiC .
+.Sh CAVEATS
+The argument to
+.Nm
+must be
+.Dv EOF
+or representable as an
+.Li unsigned char ;
+otherwise, the result is undefined.

login_yubikey case-insensitive hex decoding

2012-11-22 Thread Alexander Hall 
Cheers,

SHIFT or CAPS LOCK (on any keyboard) screws up the parsing of the
yubikey OTP. So make that parsing case-insensitive.

The corresponding part in yubikey_hex_decode is for consistency and,
IMO, sanity, allowing mixed case hex strings, e.g. /var/db/yubikey/*.

Comments? OK? (Don't mess with the from-upstream-derived yubikey.c?)

/Alexander


Index: yubikey.c
===
RCS file: /data/openbsd/cvs/src/libexec/login_yubikey/yubikey.c,v
retrieving revision 1.2
diff -u -p -r1.2 yubikey.c
--- yubikey.c   31 Jan 2012 16:58:38 -  1.2
+++ yubikey.c   23 Nov 2012 00:16:28 -
@@ -252,7 +252,7 @@ yubikey_hex_decode(char *dst, const char
char *p1;
 
for (; *src  dstSize  0; src++) {
-   if ((p1 = strchr(hex_trans, *src)) == NULL)
+   if ((p1 = strchr(hex_trans, tolower(*src))) == NULL)
b = 0;
else
b = (char)(p1 - hex_trans);
@@ -278,7 +278,7 @@ yubikey_modhex_decode(char *dst, const c
char *p1;
 
for (; *src  dstSize  0; src++) {
-   if ((p1 = strchr(modhex_trans, *src)) == NULL)
+   if ((p1 = strchr(modhex_trans, tolower(*src))) == NULL)
b = 0;
else
b = (char)(p1 - modhex_trans);

Re: login_yubikey case-insensitive hex decoding

2012-11-22 Thread Philip Guenther 
On Thu, Nov 22, 2012 at 5:28 PM, Alexander Hall alexan...@beard.se wrote:
 The corresponding part in yubikey_hex_decode is for consistency and,
 IMO, sanity, allowing mixed case hex strings, e.g. /var/db/yubikey/*.

 Comments? OK? (Don't mess with the from-upstream-derived yubikey.c?)
...
 -   if ((p1 = strchr(hex_trans, *src)) == NULL)
 +   if ((p1 = strchr(hex_trans, tolower(*src))) == NULL)

The argument to tolower() must be a value in the range [EOF,
0..UCHAR_MAX].  When taking characters from a char * string, you need
to cast the value to (unsigned char), ala
   tolower((unsigned char)*src)


Philip Guenther