Blind tunneling breaks with Google Chrome and TLS Kyber cipher

[fsecure-kilkanen]

Executive summary

When TLS Client Hello is split into two TCP segments and segments arrive to TrafficServer in different parts, blind tunneling flow breaks.

Details

Use case

Trafficserver runs as a transparent forward proxy for TLS traffic. A custom plugin uses SSL_CERT_HOOK to determine destination hostname of the TLS connection.

If the destination is accepted, connection is converted into a blind tunnel by calling TSVConnTunnel(sslvc) and TSVConnReenable(sslvc) in the hook callback function.

Affected versions

Reproduced with TrafficServer 8.x branch and TrafficServer 9.2.4. Issue should also exist in 10.x branch because affected code paths have not changed.

The issue

When a connection is started with Kyber cipher, the TLS Client Hello packet contains more key material than previous ciphers. With Kyber cipher, the TLS Client Hello packet is 1700-2100 bytes in size.

Since the network MTU is usually 1500, it means that the TLS Client Hello is split into two TCP segments. In some conditions, clients' network connectivity is slow such that the gap between the first and second segment of Client Hello is big enough to trigger the bug.

https://github.com/apache/trafficserver/blob/master/src/iocore/net/SSLNetVConnection.cc#L423 is the place where TLS Client Hello is read. This is a call to read data from socket that is for the incoming TCP connection. There is no guarantee that this call will return both segments of the Client Hello. If data for complete Client Hello is received here, the blind tunnel operation is succesful.

However, if only first segment of Client Hello is received, then the following sequence of events happen:

https://github.com/apache/trafficserver/blob/master/src/iocore/net/SSLNetVConnection.cc#L1365 OpenSSL ssl_accept() is called. It returns SSL_ERROR_WANT_READ, which is returned as SSL_HANDSHAKE_WANT_READ.

https://github.com/apache/trafficserver/blob/master/src/iocore/net/SSLNetVConnection.cc#L661 is the branch taken in this case. Processing goes to update_rbio at https://github.com/apache/trafficserver/blob/master/src/iocore/net/SSLNetVConnection.cc#L529 with move_to_socket set to true.

In update_rbio, OpenSSL BIO is set to the file descriptor of the connection, and existing handshake buffers are deallocated.

Now, when the second segment of Client Hello arrives, the blind tunnel branch https://github.com/apache/trafficserver/blob/master/src/iocore/net/SSLNetVConnection.cc#L624 is not taken, because this->handShakeReader is nullptr after it was deallocated previously in update_rbio.

To fix the issue, the case of SSL_HANDSHAKE_WANT_READ should be handled so that the second TCP segment of handshake is read into the existing SSL handshake buffer, and then ssl_accept() is called with that buffer.

I have tried a couple of workarounds to do this. However, since I don't fully understand the context why current implementation is like this, I am not confident of making big changes, because they might affect other use cases.

[fsecure-kilkanen]

I will add more detailed reproduction instructions.

[fsecure-kilkanen]

These are relevant debug logs that show what happens.

caesars-with-kyber-fail.txt caesars-without-kyber-slow.txt

In the caesars-with-kyber-fail.txt one can see how 1291 bytes of client handshake are first received, and then OpenSSL ssl_accept() returns WANT_READ. Second part is received by OpenSSL and processed properly. However, then processing gets stuck with probe needs data, read.. step.

In caesars-without-kyber-slow.txt, the Kyber cipher was disabled. There, 655 bytes of handshake data is read, and OpenSSL ssl_accept() processes it properly.

Here one can see that after probe needs data, read.. step the action moves to HttpSessionAccept correcly, and operation finishes succesfully.

[fsecure-kilkanen]

https://github.com/fsecure-kilkanen/kyber-handshaketest/blob/main/kyber-handshaketest.py is a simple Python script that sends a previously recorded TLS Client Hello packet with Kyber cipher to a chosen destination.

It sends the client hello packet in two ways:

1. One call to send(). This means TCP stack will do TCP segmentation.
2. Client Hello is split into two parts, and the parts are sent separately with slight delay. This simulates what happens in the error case above.

Both cases succeed when running directly against a web server.

With ATS, first case succeeds depending on network conditions. Second case breaks always, as ATS receives the ClientHello always in two separate socket read calls.

[fsecure-kilkanen]

https://tldr.fail/ is a website that covers these classes of bugs.

[fsecure-kilkanen]

@bryancall Are there any plans to fix this bug?

[masaori335]

Hello, Tero. Thanks for report.

I recently start taking a look at PQTLS and tested ATS with tldr_fail_test.py. If I run ATS as a reverse proxy, there're no issue with large Client Hello nor separated Client Hello. However, as you pointed out, the Blind Tunnel case is not working with separated Client Hello.

To fix the issue, the case of SSL_HANDSHAKE_WANT_READ should be handled so that the second TCP segment of handshake is read into the existing SSL handshake buffer, and then ssl_accept() is called with that buffer.

I agree with we should read all packets that has Client Hello and forward them to origin server as tunnel.

I have tried a couple of workarounds to do this. However, since I don't fully understand the context why current implementation is like this, I am not confident of making big changes, because they might affect other use cases.

There're several test cases around this code. If you open a PR, they should run automatically :)

[fsecure-kilkanen]

I have some questions about the implementation.

What is the reason for switching OpenSSL to read from the socket after reading first packet?

My plan is to check the length of handshake from TLS client hello header, and then compare that to amount of data that was read. If the whole handshake was not read, then further read of handshake needs to be triggered.

How do I properly trigger a second read at this point using the async event system in ATS?

The handshake data is read into IOBufferBlock:

IOBufferBlock *b = this->handShakeBuffer->first_write_block();

If I do a second read to this IOBufferBlock, will the data be stored in continuous memory block? So that when I get this->handShakeBuffer->start() and this->handShakeBuffer->end(), it will have the full handshake data without any extra data?

Thanks for the assistance.

[fsecure-kilkanen]

Hi @shinrich I see that you have worked with this code path, could you give some input on my questions above?

[bneradt]

Thank you @fsecure-kilkanen for the thorough description. Using a slightly modified version of your split CLIENT_HELLO python script, I have written an autest that reproduces the issue. It is on this branch: https://github.com/bneradt/trafficserver/tree/fix_client_hello_split_blind_tunnel

Delaying the call to free_handshake_buffers helps ATS to internally process the CLIENT_HELLO correctly, but it does not tunnel both packets to the origin correctly, only the first. I have verified that the origin will handle both packets corretly if ATS does the right thing, but I haven't made it that far yet.

Just following up on your investigation: have you made further progress in finding a patch that addresses this issue?

[bneradt]

Digging into things a bit, it seems like when the HttpSM sets up the tunnel for blind tunneling, the ua VC only has the first 1,100 bytes instead of all the 1993 bytes, even though all those bytes have arrived by that point. I'd like to investigate whether we can populate things to have all the bytes available? I'm not sure how easy that is in practice to do.

[bneradt]

I think this is the key here: https://github.com/apache/trafficserver/blob/8fbd40122fc4818bf646d282d665da0cd1eebd87/src/iocore/net/SSLNetVConnection.cc#L538

When that vio buffer is written with handShakeHolder, it is only the 1,100 bytes from the first packet despite being after the bytes were received from the second packet. I suspect that we need to figure out how to append any bytes from subquent packets to handShakeHolder before this point.

[bneradt]

@fsecure-kilkanen : I now have a tentative PR up which addresses the test case I added for this based off of your python script (#12290). Thank you again for the detailed description of the problem and reproduction steps. I plan to test this in production next week.

[fsecure-kilkanen]

Hi @bneradt .

Thanks for looking into this issue. I gave up on trying to patch this issue. Instead I resorted to using a workaround where I am delaying the first part of TLS Client Hello packet with Linux tc framework, if the Client Hello consists of two parts.

This delay ensures that the socket read gets the full Client Hello packet, and the problematic code path is not triggered in TrafficServer.

It is nice that you have been able to work on this.