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

[Nikos Mavrogiannopoulos]

On Mon, Jul 20, 2015 at 2:30 PM, Rick van Rein <rick at openfortress.nl> wrote:
> 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.

Most likely we can live with a fixed version as even if gnutls_prf()
is used, the context is not typically set. However, I'm pretty sure if
I do that there will be a bug report for incompatibility at some
point.

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

But you should have checked the error code :) Nevertheless, I modified
it to return error only when there are any data available.

Reading again the RFC though, it makes me not agree with the following.
>    Ample warning about the distinction between "extra == NULL" (in
>    which case no context or length is added) and "extra_size == 0"
>    (zero bytes of context added, with a zero length preceding it) is
>    missing in the function documentation.

I don't think that the case of non-null context with zero size is
intended to be handled. What is my understanding of RFC5705 is that if
no context is provided no length is put there. The case of having a
zero length seems to be outside the scope.

regards,
Nikos