search

google / nogotofail

An on-path blackbox network traffic security testing tool
Apache License 2.0
2.94k stars 418 forks source link

Add peek_request and peek_response #86

Closed chadbrubaker closed 8 years ago

chadbrubaker commented 9 years ago

These handler methods get a first pass at socket data before the consuming recv. These can be used to, for example, hook into libraries that read from sockets themselves or otherwise consume the data in the handler.

This is the start of moving all the SSL MiTM code out of Connection and into SSL specific handlers as well as adding handler dynamically.

This change is pretty straightfoward except for the work to be done to support peek on mitm'd connections. pyOpenSSL does not support peek so we need to read into a buffer and read from that when peeking. This requires some pretty hacky code to keep select working correct on a connection where there is data remaning in the peek buffer as the underlying socket is no longer ready to be selected for reading (as it would be with real MSG_PEEK) so in that case we use a different fd for select that is always ready for reading.

chadbrubaker commented 9 years ago

@klyubin can you take a look at this? I've been testing this since blackhat and its looking pretty good at least locally, even helped me find some bugs we never hit when using the VPN.

It has a pretty awful hack it in because of pyOpenSSL, which you'll enjoy/hate.

chadbrubaker commented 8 years ago

Alex PTAL I updated some of the comments to make it clearer what this does. Sadly Github eated your comments :(.

Its still a pretty power user feature, but I want to land it so I can land my connection refactor.

chadbrubaker commented 8 years ago

peek_request/peek_response is consistent with all the other data methods (on_request/on_request/inject_request/inject_response). I think its clear given that its consistent with everything else in the handle as far as naming goes.

Refactoring those names is on my todo list, but not right now.

klyubin commented 8 years ago

I'm not talking about renaming methods. I'm talking about update the documentation of the methods to remove ambiguity.

chadbrubaker commented 8 years ago

I don't think there is any ambiguity in the context of that file, _response and _request are always on data received, it just states the direction of traffic.

klyubin commented 8 years ago

OK