Closed slaff closed 6 years ago
Thanks @slaff@! Just one question — don't we also need to check if the fragment length extension is accepted by the server, and fragment outgoing data based on the fragment length agreed upon?
don't we also need to check if the fragment length extension is accepted by the server,
Hm... in the current implementation the client just asks the server to use that fragment size. But, as you know, most of the servers silently ignore this extension. I am not sure that there is a confirmation meta-data given back from the server. Is there something mentioned in the RFC about the server response?
That is described in the paragraphs 3-5 of https://tools.ietf.org/html/rfc6066#section-4:
Servers that receive an extended client hello containing a "max_fragment_length" extension MAY accept the requested maximum fragment length by including an extension of type "max_fragment_length" in the (extended) server hello. The "extension_data" field of this extension SHALL contain a "MaxFragmentLength" whose value is the same as the requested maximum fragment length.
If a server receives a maximum fragment length negotiation request for a value other than the allowed values, it MUST abort the handshake with an "illegal_parameter" alert. Similarly, if a client receives a maximum fragment length negotiation response that differs from the length it requested, it MUST also abort the handshake with an "illegal_parameter" alert.
Once a maximum fragment length other than 2^14 has been successfully negotiated, the client and server MUST immediately begin fragmenting messages (including handshake messages) to ensure that no fragment larger than the negotiated length is sent. Note that TLS already requires clients and servers to support fragmentation of handshake messages.
"max_fragment_length" in the (extended) server hello
That is not processed at the moment. If there is a sample of such response I could figure it out. Or is there at least one popular server with which I can test max fragment size? Is NGINX or apache support it?
The client and server MUST immediately begin fragmenting messages (including handshake messages) to ensure that no fragment larger than the negotiated length is sent.
And that is not implemented either. But maybe you can give me hints how to set it if I know the negotiated size?
Server hello is processed in https://github.com/igrr/axtls-8266/blob/eea12b59a88c71e00f7b447d25a074fb4da83927/ssl/tls1_clnt.c#L372, currently all extensions being sent are ignored.
Regarding fragmentation, currently ssl_write fragments data in max_plain_length
sized chunks, so it might be sufficient to set max_plain_length
if the fragment size extension is accepted, and make sure the fragment buffer (bm_all_data
) is re-allocated accordingly.
https://github.com/igrr/axtls-8266/blob/eea12b59a88c71e00f7b447d25a074fb4da83927/ssl/tls1.c#L344
https://github.com/igrr/axtls-8266/blob/eea12b59a88c71e00f7b447d25a074fb4da83927/ssl/tls1.c#L660
Note that the current code initially sets max_plain_length
to a value lower than 16kB in an attempt to optimize memory consumption during handshake, however it later grows the fragment buffer if necessary (if the server sends larger fragments).
currently all extensions being sent are ignored.
Thanks. I am looking into it...
@igrr Ok, with the second commit (6030e56) the extensions in the server hello response should be processed and the size should be set accordingly. At the moment that code is not thoroughly tested.
@igrr The code is ready for review and tests from your side. Tried it against gnutls and Nginx and was working as expected.
@igrr How about merging this PR?
Done, thanks @slaff!
Fixed also the comments to reflect that.