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

support to intercept mTLS protected traffics. #6430

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fungaren
Copy link

@fungaren fungaren commented Oct 31, 2023

Description

Currently we have options.client_certs as a per-site config to enable mTLS. However, when mitmproxy is working as a reverse proxy for a single server, there is no way for us to generate client certificates for each client.

This is a very common scenario in kubernetes clusters. The kube-apiserver is a REST server wtih RBAC enabled, where mTLS is used to indicate the user/client.

Currently mitmproxy have addons/tlsconfig.py, which is a good start point. But some configs are hard coded, other addons cannot override then. This commit also add a hook for tls_start_server & tls_start_client to override any config of the ssl context.

Last but not least, to generate a valid client certificate, we must add CLIENT_AUTH to ExtendedKeyUsage.

Checklist

  • I have updated tests where applicable.
  • I have added an entry to the CHANGELOG.

Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR and the detailed explanation! This is a great contribution for contrib/, but I'm a bit hesistant to add callbacks here. See detailed comments below. 😃

mitmproxy/addons/tlsconfig.py Outdated Show resolved Hide resolved
examples/contrib/mtls.py Outdated Show resolved Hide resolved
mitmproxy/certs.py Outdated Show resolved Hide resolved
examples/contrib/mtls.py Outdated Show resolved Hide resolved
examples/contrib/mtls.py Outdated Show resolved Hide resolved
examples/contrib/mtls.py Outdated Show resolved Hide resolved
@mhils
Copy link
Member

mhils commented Nov 2, 2023

Thanks! Please ignore the Codacy check, I'm in the process of setting that up properly.

Currently we have options.client_certs as a per-site config
to enable mTLS. However, when mitmproxy is working as
a reverse proxy for a single server, there is no way for us
to generate client certificates for each client.

This is a very common scenario in kubernetes clusters.
The kube-apiserver is a REST server wtih RBAC enabled,
where mTLS is used to indicate the user/client.

Now mitmproxy have addons/tlsconfig.py, which is a
good start point. But client cert logic are embedded inside,
other addons cannot override them. This commit adds
make_certificate_builder and use_client_cert functions
so that addons can monkey patch them.

This commit introduces a new option tls_request_client_cert
as well to accept original client certificate.
@fungaren
Copy link
Author

fungaren commented Nov 13, 2023

If the client didn't provide the client certificate, the server may refuse the connection, so it would be better to abort the connection. But I can not to find a way to achieve it. Is there any way to close the connection immediately in tls_start_server?

And, I found that even I never set the client certificate, it still provided a client cert for the server, so the server accepted the request:

        if client_certs and len(client_certs) > 0:
            # do something with SSL.context to use the client cert
        else:
            logging.info("tls_start_server: no client cert")

            def monkey_no_client_cert(context: SSL.Context, server: connection.Server):
                pass
            tlsconfig.use_client_cert = monkey_no_client_cert

        super().tls_start_server(data)

After browsing the codes (takes me about two days 😢) I finally found the reason is the LRU cache applied to create_proxy_server_context leads to the SSL context reuse.

https://github.com/mitmproxy/mitmproxy/blob/v8.1.1/mitmproxy/net/tls.py#L119

Currently the only way to address the problem is to create a new context by myself:

        if client_certs and len(client_certs) > 0:
            # do something with SSL.context to use the client cert
        else:
            logging.info("tls_start_server: no client cert")

            data.ssl_conn = SSL.Connection(SSL.Context(SSL.TLS_CLIENT_METHOD))
            data.ssl_conn.set_connect_state()
            return

@fungaren
Copy link
Author

@mhils Hi, will you accept the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants