[gnutls-devel] Bug: SNI is ignored when resuming session from cache

[Thomas Klute]

Am 19.09.2017 um 11:20 schrieb Nikos Mavrogiannopoulos:
> On Mon, Sep 18, 2017 at 8:28 AM, Thomas Klute
> <thomas2.klute at uni-dortmund.de> wrote:
>> Am 18.09.2017 um 07:53 schrieb Nikos Mavrogiannopoulos:
>>> On Fri, Sep 15, 2017 at 3:41 PM, Thomas Klute
>>> <thomas2.klute at uni-dortmund.de> wrote:
>>>> Hi,
>>>> I have discovered that GnuTLS servers ignore the SNI extension when
>>>> resuming a TLS session from cache (but not when using tickets). The
>>>> function gnutls_server_name_get() always returns
>>>> GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE when called on the resumed
>>>> session, breaking virtual server systems (e.g. virtual hosts with Apache
>>>> and mod_gnutls).
>>>>
>>>> You can reproduce the issue with gnutls-serv and gnutls-cli (paths from
>>>> dev build):
>>>>
>>>> ./src/gnutls-serv --x509keyfile=doc/credentials/x509/key-rsa.pem
>>>> --x509certfile=doc/credentials/x509/cert-rsa.pem -p 4433 --sni-hostname
>>>> server.example.com --noticket
>>>>
>>>> ./src/gnutls-cli --resume --sni server.example.com -p 4433
>>>> --x509cafile=doc/credentials/x509/ca.pem localhost
>>>>
>>>> The server will warn about the missing server name during resumption:
>>>>
>>>>   Warning: client did not include SNI extension, using default host
>>>>
>>>> The proper fix appears to be to reclassify SNI as a "mandatory"
>>>> extension, because if implemented at all it must be parsed on session
>>>> resumption according to RFC 6066, Section 3:
>>>>
>>>>   "A server that implements this extension MUST NOT accept the request
>>>>   to resume the session if the server_name extension contains a
>>>>   different name."
>>>>
>>>> The attached one line patch changes the parse type of the Server Name
>>>> extension to GNUTLS_EXT_MANDATORY. For applications that include the
>>>> server name in their session cache handling this fix is sufficient, but
>>>> adding a check to the resumption code might still be reasonable.
>>>
>>> Thank you Thomas. That seems like a reasonable fix. Would you like to
>>> send it as a merge request?
>>
>> At the moment the one-line parse type change is the only fix I have,
>> please just apply that so gnutls_server_name_get works with a session
>> cache. If I have time to add server name checks for resumption I'll look
>> into Gitlab's merge requests, or maybe just send a normal Git pull request.
> 
> I will not have much time to check into that soon (tls1.3 support is
> my priority at the moment). Would it make sense to open an issue on
> gitlab to track it?

My immediate concern is getting SNI parsing during session resumption
fixed, because the bug breaks mod_gnutls virtual hosts with different
certificates or cipher suites if tickets are disabled. If you'd like I
can look into adding a reproducer script for the test suite.

Tracking the question of checking SNI values on resumption separately
makes sense in my opinion.

Regards,
Thomas