[gnutls-devel] gnutls 3.2.3 segfault in _gnutls_epoch_set_keys

[Stefan Bühler]

On Sat, 03 Aug 2013 19:52:58 +0200
Nikos Mavrogiannopoulos <nmav at gnutls.org> wrote:

> On 08/02/2013 04:20 PM, Stefan Bühler wrote:
> > Hi,
> > 
> > I think I found the problem:
> > 
> > _gnutls_epoch_set_cipher_suite fails, because my normal priority
> > string has RC4 disabled; the hello_cb is supposed to enable RC4 for
> > TLS1.0
> > 
> > Although the callback gets called (and sees the correct version and
> > sets the RC4 priority), _gnutls_epoch_set_cipher_suite still doesn't
> > accept the RC4 cipher from the ticket state.
> > The return value of _gnutls_epoch_set_cipher_suite isn't checked by
> > any of the calling functions... this should probably be fixed (same
> > for _gnutls_epoch_set_compression and so on).

Looks like you fixed that part, thx :)

> > Also it'd be nice if the hello callback could change the priority :)
> 
> Ah, ok I understand what you're trying to do. I don't think it's worth
> the effort though. Both RC4 and AES-CBC are severely broken in so many
> ways it doesn't make sense to try to mitigate the issues (a more
> advanced ciphersuite selection process would have been nicer though).
> 
> Using the "normal" priority string and adding the server precedence
> option would help as the best possible option will be used depending
> on the client.

Afaik AES-CBC is ok to use with TLS1.1+ and better than RC4, but for
TLS1.0 and before you really want RC4. server preference can't fix this.
And afaics in my ssllabs analysis no client supports AES-GCM yet, so I
really need AES-CBC.

But even without that usecase I still think it'd be nice to support
different priority settings from the hello callback; for example I
could allow less secure ciphers for some SNI names, and require better
ciphers for others.

I'm not sure why you check in the epoch setters whether the cipher and
the mac are present in the priority; if a session gets resumed the
values were good not long ago, and otherwise you selected good values
right before.
Or perhaps the check could be delayed until the hello callback is done?


Whatever your decision is, it would be nice if it would be documented
and perhaps even be enforced; if you don't want the priority changed in
the hello callback, then it probably should fail somehow, instead of
setting up sesions that can't be resumed (or trigger segfaults :D).

regards,
Stefan