[gnutls-devel] gnutls_prf not compliant to RFC 5705 (or confusingly so)
nmav at gnutls.org
Mon Jul 20 10:48:09 CEST 2015
On Sun, Jul 19, 2015 at 8:47 PM, Rick van Rein <rick at openfortress.nl> wrote:
> The gnutls_prf() function takes arguments “extra” and “extra_size”,
> which are meant to conform to what RFC 5705 calls a “context”.
> The function docs state that "extra" is optional, but passing NULL
> in it causes a memcpy() with NULL source, which causes SEGFAULT (on
> Linux) even when copying 0 bytes.
That's quite strange. I don't seem to be able to reproduce but it
could be libc-version specific.
> The RFC states that the context is preceded with its length in 16
> bits, if a context is provided. But gnutls_prf() does not check the
> length to fit in 16 bits, and it does not prefix the length. The
> function documentation also does not state that the caller must
> prefix the length -- which is probably not the idea anyway.
That's quite unfortunate. It seems that gnutls_prf() was not made to
follow rfc5705 but rather an earlier draft and was not updated in the
final version. I'm not sure whether gnutls_prf() should be updated to
fix that issue. One reason is that there may be users of gnutls_prf()
which already prefix extra properly to get the expected output, and
the other is that gnutls_prf() is quite complicated. It has this
"server_random_first" option, which while it may have been useful in
pre-rfc5705 times, now it is plainly confusing.
Hence, I think it would be cleaner to introduce another API to follow RFC5705.
> The different naming of “extra” from “context” was confusing me when
> I was trying to use the function from the API spec plus the RFC;
> perhaps the RFC-name “context” is a good name to preserve.
> If you agree that these are potential places for improvement, then
> please find patches to resolve the above attached.
Thanks for bringing that up. I'll document that gnutls_prf() does not
provide the expected for RFC5705, and add gnutls_prf_rfc5705() which
will be easier to use. I'd appreciate if you could check the new
function; it is uses gnutls_prf(), so you could use something similar
> * The 3.2.21 patch was made against the version in which I encountered
> the problem, and against which I patched it.
> * The 3.3.16 patch is virtually the same, but against stable head; it
> has only been tested to compile well.
> * Note that I am not aware of an easy way to compare the generated PRF
> with a known-correct value.
It is also unfortunate that the RFC doesn't include any known values.
I've included a known answer test in tests/prf.c in the repository.
More information about the Gnutls-devel