-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TLS/SNI parsing #322
base: master
Are you sure you want to change the base?
TLS/SNI parsing #322
Conversation
Thanks for the contribution, however I'm not sure whether it's a good idea to parse TLS record per se: there's no benefit for doing that (GoodbyeDPI doesn't use any data from ClientHello, at least at this point), it's slower, it's more error-prone.
You can recompute SEQ of every packet in userspace, or more interesting, we can add this into WinDivert (kernelspace) driver if you're still interesting in that function. Modern WinDivert versions support TCP flows. |
I agree: as long as other parts of the ClientHello are unnecessary your implementation is easier and faster.
What do you mean by computing the SEQ in userspace? I haven't worked with WinDivert yet but that sounds good and I'd be happy to help! |
Done: 4c846c7
Look: to perform TLS record fragmentation, we need to add additional headers, right? And we know the size of the resulting packets. What we could do is:
That probably would not work in all cases, but if we always have TLS and always do record fragmentation, it should work. |
You want to decrease the SEQ number of the initial SYN-ACK packets, right? As you said we always need TLS for that and I imagine the seq number changes for the SYN-ACK packets to be a hassle, we would need to map the right ACK to the right SYN. Is it possible to override a socket's internal SEQ counter? That should solve everything |
Yes, this is correct, as well as
I'll think on what could be done, maybe there's simpler solution. |
This PR updates the TLS/SNI parsing to do actual protocol-parsing. It is more flexible on the TLS legacy version numbers and case of the hostname (see TODO). Feel free to take it or leave it.
This is actually a left-over from when I tried to implement TLS record fragmentation into Goodbye DPI. I had to abandon that as it invalidates the TCP SQN.