eduardsui / tlse

Single C file TLS 1.2/1.3 implementation, using tomcrypt as crypto library
Other
535 stars 87 forks source link

github.com TLS 1.3 transmission fails in the middle with NEED DATA #57

Open classilla opened 3 years ago

classilla commented 3 years ago

Although TLS 1.3 works well on many sites (thank you!), github.com itself does not and the stream aborts at around the same place each time with

Consumed 1375 bytes
NEED DATA: 1392/44
Message type: 17, length: 1387
encrypted (1387):  /* elided */
aad (5): 17 03 03 05 6B 
aad iv (12): CC D5 A1 35 46 37 A8 B6 AE 1B CB 09 
PT SIZE: 1371
decrypted (1371): /* elided */
tag (16): 43 DA 1D C6 CD 60 D2 BE ED DF 92 14 D6 EA B8 38 
APPLICATION DATA MESSAGE (TLS VERSION: 304):
="expected-hostname" content="github.com"> /* elided */  <link rel="icon" c
Consumed 1375 bytes
NEED DATA: 1392/88

A final pulse of data occurs, but recv() then returns EOF in the middle of the transmission.

classilla commented 3 years ago

(It works fine with TLS 1.2.)

eduardsui commented 3 years ago

Just tested it with github.com, and it works fine. Can you post a minimal client that reproduces this error?

classilla commented 3 years ago

I checked this against trunk just to make sure my app wasn't failing, and it still doesn't work (macOS 10.14, x86_64). I attached the diff. github_example.diff.txt

classilla commented 3 years ago

It also doesn't work on Linux ppc64le either, for the record (same basic failure).

eduardsui commented 3 years ago

It seems that is something to do with SSL* API. github.com works find with tls* API. I will check it.

ghost commented 3 years ago

@eduardsui: Could you share the code that you have working with TLS 1.3 for GitHub? I tried slightly modifying the tlsclienthello.c example, and I actually had the same experiences as when using the SSL_ APIs. That is: A portion of the data is read, but not the whole page.

I have been trying to figure out how to solve this issue myself, but I have been unable to. I think if the issue is indeed with the SSL_ APIs, then that issue also propagates to the adaptation for the tlsclienthello.c example too, so an example of working code could help me try to troubleshoot it more effectively.

ghost commented 3 years ago

A short update on this: It seems the buggy behavior is only observed with the modified tlsclienthello.c example when kTLS is enabled. If I remove the make_ktls call, the data is read fully, even with TLS 1.3!

So that gives me a lot more ability to investigate the issue, which I will do! Sorry for the noise.

Edit: I’ll avoid pinging people by sending yet another message, but it seems it doesn’t work even without kTLS, and I had just misassessed. If anyone can find any code that works well with TLS 1.3 and GitHub in any way, I’d really appreciate trying to investigate this again!

classilla commented 2 years ago

I'm still trying to figure this out. I agree it's not the SSL_* APIs because I'm not using those either.

The most recent tip doesn't fix it.

I do note that the cut-off point is non-deterministic. It doesn't halt at the same point each time.

classilla commented 2 years ago

Okay, with some hacking, I get the full TLS 1.3 page from Github. The problem looks lower level. If I repeatedly recv() over and over until there ain't no more, tls_consume_stream it right then and there into the buffer until there's no more to receive, and then start tls_read()ing on that once the entirety is in the buffer, I get a full reply.

I don't like loading the entire thing, though, so this isn't a solution for large files. But this suggests tls_read() is somehow at fault.

classilla commented 2 years ago

This feels like a race condition, by the way.

eduardsui commented 2 years ago

Thank you for the info. I'm planing to take a look next week.