A hook for certificate verification overrides

[vthriller]

Problem Description

It seems there is currently no way to tell mitmproxy to accept individual invalid server certificates¹, --ssl-insecure/--no-ssl-insecure is as granular as it currently gets. This is a problem for people that use mitmproxy for casual browsing and occasionally stumble upon some non-critical websites (like, say, blogs) with annoyances like slightly outdated certs.

¹ Or maybe even discard valid ones? I'm not sure if anyone would find such feature useful though, so I'll speak about verification overrides for the rest of this post.

Proposal

A hook that will let addons choose Verify.VERIFY_NONE or Verify.VERIFY_PEER for each individual certificate and/or host.

Alternatives

• Extend the ssl_insecure option so that it could also accept a list of filters of some sort.
  • This will definitely look like a mess if exposed via cmdline.
• Alternatively ssl_insecure could accept path to file with a list of overrides.
  • It seems it would make addons that implement "accept the risk" page or web-UI for overrides mgmt, well, a lot more hacky.
  • As a bonus, however, this may allow users to reuse overrides stored in their Firefox profiles (cert_override.txt). (Can't say anything about WebKit/Blink-based browsers though.)

Additional context

Gecko's nsCertOverrideService.cpp describes override matching process, as well as format of cert_override.txt, which basically boils down to: host:port, SHA-256 fingerprint, error kinds (any combination of Mismatch, Untrusted, Time), and DB key, which itself is generated from serial and DN.

[mhils]

Instead of providing an additional hook, I think this is better solved by configuring data.ssl_conn in the tls_start_server hook (note: links goes to main branch, where the API is slightly better than v7.0.4). I've just commited 3fbf3cf, which makes the TlsConfig addon preserve an existing pyOpenSSL context. With that change, a custom addon like this should work:

from mitmproxy import tls
from mitmproxy.addons.tlsconfig import TlsConfig


class UserTlsConfig(TlsConfig):
    def tls_start_server(self, tls_start: tls.TlsData):
        super().tls_start_server(tls_start)  # use the default implementation to setup the pyOpenSSL context
        print(f"{tls_start.ssl_conn=}")  # do modifications here, e.g. set VERIFY_NONE for some domains


addons = [UserTlsConfig()]

---

It seems there is currently no way to tell mitmproxy to accept individual invalid server certificates

For the record, there are also the ssl_verify_upstream_trusted_ca and ssl_verify_upstream_trusted_confdir options, which do however replace the defaults entirely.

[vthriller]

I was afraid that wouldn't work because tls_start_server() already creates connection object. Turns out it kinda doesn't: pyOpenSSL doesn't expose SSL_set_verify, SSL.Connection.set_context() does not update verify_mode, so the only way to override verify_mode for addon developer is to create another SSL.Connection, and, consequently, borrow code from TlsConfig. It's not a lot of code, but still.

I'll see if I can do anything with the store, as it seems to be better fit for what I want to do (i. e. trust specific certificates for some connections, and not just disable verification completely).

[vthriller]

I'll see if I can do anything with the store, as it seems to be better fit for what I want to do

Completely forgot that it will only help with self-signed certs, not expired ones. So the point about verify_mode stands anyway.

[jsmucr]

I fell into this today. I tried to override the data.context.options.ssl_insecure option from within the tls_start_server hook and it works. The bad thing is that it overrides the global configuration. A solution to this might be replacing the ctx.options in tlsconfig.py with tls_start.context.options so that every request could work with its own set of configuration flags.

EDIT: Here's my use case: https://stackoverflow.com/questions/70515761/trusting-individual-invalid-certs-in-mitmproxy

[jsmucr]

See this: jsmucr@05118e5
I can then create a simple hook to override the setting for a specific address:

import json, copy
from mitmproxy import ctx

def tls_start_server(data: any):
    address = data.context.server.address[0] + ":" + str(data.context.server.address[1])
    if address == 'as2.pipechain.com:10443':
        data.context.options = copy.deepcopy(data.context.options)
        data.context.options.ssl_insecure = True
    ctx.log.info("ssl_insecure[" + address + "] => " + str(data.context.options.ssl_insecure))

Is this the correct way to go? Global context is IMO generally a bad thing.

[hazcod]

@jsmucr Ohh, can you create a PR with those changes? Thanks.

[jsmucr]

@hazcod #5027

EDIT: This line is no longer neccessary:

        data.context.options = copy.deepcopy(data.context.options)

[mhils]

Thank you for putting much more work into this @vthriller and @jsmucr! 🍰

@jsmucr's PR is great, but unfortunately doesn't fly for architectural reasons. Nonetheless, thank you very much! 🍰

Turns out it kinda doesn't: pyOpenSSL doesn't expose SSL_set_verify, SSL.Connection.set_context() does not update verify_mode, so the only way to override verify_mode for addon developer is to create another SSL.Connection, and, consequently, borrow code from TlsConfig. It's not a lot of code, but still.

This sounds like the most reasonable approach may just be to get SSL_set_verify exposed? Judging by pyca/pyopenssl#255 we would not be the only ones benefiting from that.

Alternatively, here's the workaround implementation where SSL.Connection is just created again:

from mitmproxy import tls
from mitmproxy.addons.tlsconfig import TlsConfig
from OpenSSL import SSL


class UserTlsConfig(TlsConfig):
    def tls_start_server(self, tls_start: tls.TlsData):
        super().tls_start_server(tls_start)
        if tls_start.conn.sni == "wrong.host.badssl.com":
            # Get existing SSL.Context context and disable verification
            ssl_ctx = tls_start.ssl_conn.get_context()
            ssl_ctx.set_verify(SSL.VERIFY_NONE, None)
            # Build a new SSL.Connection object as the existing one has copied the old verification flags.
            # (this code is adapted from the buitlin TlsConfig addon)
            tls_start.ssl_conn = SSL.Connection(ssl_ctx)
            tls_start.ssl_conn.set_tlsext_host_name(tls_start.conn.sni.encode())
            tls_start.ssl_conn.set_connect_state()

addons = [UserTlsConfig()]

[jsmucr]

@mhils I tried your workaround and it appears to work ok. Sadly -- only unless you do not plan to reconfigure the ssl_ctx from time to time. The object appears to be cached per host. It's not a single connection context.

ssl_ctx.set_options(SSL.OP_NO_SSLv3)

The effect of the line above is irreversible for a single host. I can't enable SSLv3 for that host again unless I restart mitmproxy.

This whole thing is anything but my domain, so I may be missing something important. Any ideas?

[vthriller]

This sounds like the most reasonable approach may just be to get SSL_set_verify exposed? Judging by pyca/pyopenssl#255 we would not be the only ones benefiting from that.

Surely that'd be the best solution, however relevant PR remains untouched for almost 3 years now, so I don't have much hopes in that regard.

        if tls_start.conn.sni == "wrong.host.badssl.com":

Is there any reason to prefer conn.sni over conn.address for target matching, or it doesn't really matter?

            tls_start.ssl_conn.set_tlsext_host_name(tls_start.conn.sni.encode())

Um, wouldn't that bypass literal IP check performed in the TlsConfig? (Not that I expect this to be a big deal, but I'm no expect so I'd rather cautiously copy the whole thing verbatim even for the example code.)

[mhils]

Surely that'd be the best solution, however relevant PR remains untouched for almost 3 years now, so I don't have much hopes in that regard.

The cryptography/pyOpenSSL folks are usually fantastic, this one may just have slipped through or they consider it out of scope. Let's see if we get a positive answer to pyca/pyopenssl#255 (comment). In either case, we have a relatively decent workaround anyways. If necessary we can move the context -> connection logic into a separate function that can be reused.

Is there any reason to prefer conn.sni over conn.address for target matching, or it doesn't really matter?

When dealing with non-malicious clients both are consistent. There are some edge cases where they differ:

1. In transparent mode, conn.address is only the IP address.
2. conn.sni will be empty if the client is on an ancient TLS version and does not send a SNI value.
3. Malicious clients could spoof a trustworthy SNI while connecting to a attacker-controlled IP

Um, wouldn't that bypass literal IP check performed in the TlsConfig? (Not that I expect this to be a big deal, but I'm no expect so I'd rather cautiously copy the whole thing verbatim even for the example code.)

Per the spec, Server Name Indication values must be DNS names and not IP addresses, so we don't want to set an IP as SNI. Many implementations don't care in practice, but some do refuse to serve traffic. I don't think it's ever a security issue, only a potential compatibility problem with some servers. In the example I posted we've already established that the SNI is "wrong.host.badssl.com", so the check is not needed.

[jsmucr]

Just an addition to my last post: As a workaround I have to clear the context generator cache if the SSL options change.

net_tls.create_proxy_server_context.cache_clear()

@vthriller Here's the current state of my solution if it helps: https://gist.github.com/jsmucr/24cf0859dd7c9bba8eb2817d7b0bf4b6
It's controlled by a JSON configuration file accessible from the --set confdir path.

[mhils]

Quick update: We got the underlying bindings into cryptography (pyca/cryptography@b6e7b07) and I have prepared a PR for pyOpenSSL (pyca/pyopenssl#1073), which should be ready to merge once cryptography ships a new release. So this will take some time for upstream to move, but we'll eventually get Connection.set_verify. :)