Skip to content
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

Log ignored requests (--ignore-hosts) that are forwarded #6421

Open
segevfiner opened this issue Oct 27, 2023 · 7 comments · May be fixed by #6720
Open

Log ignored requests (--ignore-hosts) that are forwarded #6421

segevfiner opened this issue Oct 27, 2023 · 7 comments · May be fixed by #6720
Labels
kind/feature New features / enhancements

Comments

@segevfiner
Copy link

Problem Description

Requests that go through --ignore-hosts are not logged as a request. This makes it inconvenient to know if any have happened even if you can't see their contents.

Proposal

Make it possible to see ignored requests in the request log of mitmproxy.

Alternatives

Additional context

It's sometimes necessary to use that with stuff that you can't make ignore SSL verification errors, but still want to go through the mitmproxy for any reason.

@segevfiner segevfiner added the kind/feature New features / enhancements label Oct 27, 2023
@mhils
Copy link
Member

mhils commented Oct 27, 2023

What do you mean by "see ignored request"? Are you looking for the entire HTTP request line, or just the hostname?

@segevfiner
Copy link
Author

segevfiner commented Oct 27, 2023

Like, we get a CONNECT request that gets forwarded off to somewhere, it would be nice to know that, even if the underlying connection is encrypted and ignored by ignore-hosts. Of course, this could be a different flag or something. Currently it seems I can only see some info about those in the events log, but they are invisible in the flows.

@nneonneo
Copy link
Contributor

nneonneo commented Nov 7, 2023

I would love to see this as an option. Right now ignore_hosts is hardcoded to set ignore=True on TCPLayer/UDPLayer, which sets flow = None and makes the connection completely invisible to logging etc.

Meanwhile, it's possible to implement using an addon:

from mitmproxy.addons.next_layer import NextLayer, NeedsMoreData
from mitmproxy.proxy import layers

class MyNextLayer(NextLayer):
    def next_layer(self, nextlayer):
        if nextlayer.layer:
            return

        context = nextlayer.context
        tcp_based = context.client.transport_protocol == "tcp"

        try:
            if self._ignore_connection(context, nextlayer.data_client()):
                nextlayer.layer = (
                    layers.TCPLayer(context)
                    if tcp_based
                    else layers.UDPLayer(context)
                )
        except NeedsMoreData:
            return

addons = [MyNextLayer()]

@mhils
Copy link
Member

mhils commented Nov 7, 2023

@nneonneo: if an option would make your life easier, I'd be more than happy to merge a PR for now without asking any further questions. Very grateful for your help with issue triage and PRs, so that's the least I can do. :)

Thinking a bit more long term, I suppose we should figure out if we want a) an option to enable more or less regular TCP/UDP flows, or b) a dedicated Flow type that just records the presence of a flow without data. The idea with the latter would be that we could also add "TlsHandshakeError" flows, and the surface that properly in the UI. The latter is obviously a bit more work, but that feels like a useful addition in terms of UX.

@nneonneo
Copy link
Contributor

nneonneo commented Nov 8, 2023

The new Wireguard mode enables practically arbitrary TCP/UDP interception, and is sufficiently powerful that we probably do want new functionality to enable new use cases. In principle, it would be nice to have mitmproxy record literally every flow that is coming through (kind of like wireshark), including TLS handshake errors.

In general, for live requests there are actually three distinct actions that could be subject to filtering:

  • TLS interception: intercepting and decrypting TLS traffic; disabled by ignore_hosts but enabled by tcp_hosts
  • Higher-level protocol layers: parsing HTTP, etc. and invoking higher-level protocol processing; disabled by ignore_hosts and by tcp_hosts
  • Flow recording: keeping track of a flow in memory for viewing, hooking, saving; disabled by ignore_hosts but enabled by tcp_hosts

I can imagine use-cases for separating these things: disabling TLS interception for a host but leaving recording enabled is a useful way to snoop on traffic purely passively, to be able to see things like TLS fingerprints and SNI information; specifically disabling higher-level protocols is useful to disable HTTP parsing for non-HTTP flows (e.g. SIP) or when you don't actually care about parsing even an unencrypted HTTP connection (right now allow_hosts will still record plaintext HTTP even from non-allowed hosts), and so on.

@nneonneo
Copy link
Contributor

nneonneo commented Nov 8, 2023

As for my use-case, for the time being I am happy with just having a slightly monkey-patched proxy, because I really only want the ignored host list for logging purposes (discovering hosts that I should be intercepting but am not currently intercepting).

@segevfiner
Copy link
Author

It seems mitmproxy also doesn't log requests that failed SSL/TLS as flows. Could be nice as well. Both ignored requests and SSL/TLS failures are only logged to events.

NicolaiSoeborg added a commit to NicolaiSoeborg/mitmproxy that referenced this issue Mar 7, 2024
Maybe a bit counterintuitive, but mitmproxy is very nice even without
the MITM part.  When doing `--ignore-hosts '.*'` it is not possible to
see SNI's, so add new flag to show the raw TCP/UDP streams.

Fixes mitmproxy#6421
@NicolaiSoeborg NicolaiSoeborg linked a pull request Mar 7, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New features / enhancements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants