> On 28 May 2015, at 17:24, Dirk-Willem van Gulik <[email protected]> wrote:
> 
> 
>> On 28 May 2015, at 17:03, William A Rowe Jr <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>> 
>> On May 26, 2015 10:31 AM, "Dirk-Willem van Gulik" <[email protected] 
>> <mailto:[email protected]>> wrote:
>> >
>> >
>> > > On 26 May 2015, at 17:22, Dirk-Willem van Gulik <[email protected] 
>> > > <mailto:[email protected]>> wrote:
>> > ..
>> > > So I think that what is needed are two (or three) functions
>> > ...
>> > > -     A string comparison function; where at least one string is is 
>> > > under control of the attacker.
>> >
>> > Now the issue here is that length is every easily revealed. So I can think 
>> > of 2 strategies;
>> >
>> > -       firmly declare (in the signature of the compare function) one 
>> > argument as potentially hostile.
>> >
>> >         And do the comparison largely based on that; which means we only 
>> > marginally reveal the
>> >         actual length of the string compared to. Below is an example; but 
>> > my gut feel it is not
>> >         nearly good enough when you can apply a large chunk of statistics 
>> > against it.
>> >
>> > -       treat them both as hostile; and scan for either the shortest or 
>> > longest one and accept
>> >         that you leak something about length.
>> >
>> >         Or - if needed - pad this out for strings <1024 (or similar) chars 
>> > in length by doing always
>> >         that many (which will leak less).
>> >
>> > Examples are below. Suggestions appreciated.
>> >
>> > Dw.
>> >
>> > static int or_bits(int  x) {
>> >   x |= (x >> 4);
>> >   x |= (x >> 2);
>> >   x |= (x >> 1);
>> >   return -(x & 1);
>> > }
>> >
>> > /* Quick mickey mouse version to compare the strings. XXX fixme.
>> >  */
>> > AP_DECLARE(int) ap_timingsafe_strcmp(const char * hostile_string, const 
>> > char * to_protect__string) {
>> >         const unsigned char *p1 = (const unsigned char *)hostile_string;
>> >         const unsigned char *p2 = (const unsigned char 
>> > *)to_protect__string;
>> >         size_t i = 0, i1 = 0 ,i2 = 0;
>> >         unsigned int res = 0;
>> >         unsigned int d1, d2;
>> >
>> >         do {
>> >                 res |= or_bits(p1[i1] - p2[i2]);
>> >
>> >                 d1 = -or_bits(p1[i1]);
>> >                 d2 = -or_bits(p2[i2]);
>> >
>> >                 i1 += d1;
>> >                 i2 += d2;
>> >                 i += (d1 | d2);
>> >
>> > #icase A
>> >         } while (d1 | d2); // longest one will abort
>> > #case B
>> >         } while (d1 & d2); // shortest one will abort
>> > #case C
>> >         } while (i < 1024) } while (d1 | d2); // at least 1024 or longest 
>> > one/shortest one
>> >
>> >         // include the length in the coparision; as to avoid foo v.s. 
>> > foofoofoo to match.
>> >         //
>> >         return (int) (res | ( i1 - i2));
>> > }
>> 
>> Giving this some thought for the string version, does it make sense to loop 
>> the underflow string back to offset zero on EOS?  There is a certain amount 
>> of cache avoidance that could cause, but it would dodge the optimization of 
>> that phase and ensure the longest-match comparisons are performed (measured 
>> by the untrusted input, presumably).
>> 
> So I am currently experimenting with 
> 
>       https://gist.github.com/dirkx/37c29dc5a82b6deb0bf0 
> <https://gist.github.com/dirkx/37c29dc5a82b6deb0bf0>
> 
> which seems to behave reasonably and does not get optimized out too far.  We 
> could perhaps do something where we change the i1/i2 loop indexes
> by str[i % i1] instead of i1/i2 that ‘stop’; and keep i1 one higher than i 
> until we hit the \0.

Updated the above gist with your idea (also shown below).

Dw.

/* Quick mickey mouse version to compare the strings. XXX fixme. 
 */
typedef enum ap_compareflag_t {
        AP_HOSTILE_STR1 = 1,  // if a string is ‘hostile’ - we’re fine to abort 
on it early.
        AP_HOSTILE_STR2 = 2, // otherwise we hide its length by checking it for 
the length of the other (or both).
        AP_MIN1024 = 4       // pretend that they are at least 1024 chars in 
size.
} ap_compareflag_t;
 
AP_DECLARE(int) ap_timingsafe_strcmp(const char * str1, const char * str2, 
ap_compareflag_t flags) {
        const unsigned char *p1 = (const unsigned char *)str1;
        const unsigned char *p2 = (const unsigned char *)str2;
        size_t i = 0, i1 = 1 ,i2 = 1;
        unsigned int res = 0;
        unsigned int d1, d2;
 
        if (flags == 0)
                flags = AP_HOSTILE_STR1 | AP_HOSTILE_STR2 | AP_MIN1024;
 
        // printf("\n%s=%s\n\t",str1,str2);
        do {
          do {
                res |= or_bits(p1[i % i1] - p2[i % i2]);
 
                d1 = -or_bits(p1[i1]);
                d2 = -or_bits(p2[i2]);
 
                i1 += d1;
                i2 += d2;
                i++;
                // printf("%c", ((d1 & d2) ? '=' : (d1 ? '1' : (d2 ? '2' : 
'.'))));
 
           } while(i < (flags & AP_MIN1024) * 256);
        } while (
                ((flags & AP_HOSTILE_STR1) && d1) | 
                ((flags & AP_HOSTILE_STR2) && d2) 
        );
        res |= (i1 - i2);
 
        // printf(" = %d\n", res);
 
        // include the length in the coparision; as to avoid foo v.s. foofoofoo 
to match.
        //
        return (int) res;
}
 



Reply via email to