[gnutls-devel] gnutls_prf not compliant to RFC 5705 (or confusingly so)

[Rick van Rein]

Hi Nikos,

You're quick :)

I encountered the SEGFAULT on:
 - Linux 2.6.32-5-openvz-amd64 SMP
 - libc6 2.11.3-4


Like you, I was hesitant about breaking the gnutls_prf() and agree you
might split the function calls.  I just couldn't find any code (or
reason to have such code) for the current implementation.


I backported your patch to the version that was troubled, and got the
expected result:
 - SEGFAULT without my patch
 - No crash with your patch and use of gnutls_prf() using extra==NULL
 - No crash with your patch and use of gnutls_prf_rfc5705() using
context==NULL

One thing though; with your patch, gnutls_prf_rfc5705() responds to
context==NULL and context_size=-1 with an error due to the unsigned
check on > 65535.  This does not seem helpful but it can be confusing --
or lead to unnoticed weak keys (I got AAAAAAAAAAAAAAAAAAAAAA== but who
prints session keys??)  It leads to extra code too,
    gnutls_prf_rfc5705 ( ..., (in2len >= 0)? in2len: 0, (in2len >= 0) ?
in2: NULL, ...);
so I'd propose changing the check to
        if (context && (context_size > 65535))  {
                gnutls_assert();
                return GNUTLS_E_INVALID_REQUEST;
        }
I tested this, and as expected it is more robust as it also works with
the much simpler call
    gnutls_prf_rfc5705 ( ..., in2len, (in2len >= 0) ? in2: NULL, ...);
 


I hope this is helpful.  Your patch surely is, thanks!

Cheers,
 -Rick