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

New option: Add server certs to client chain #1014

Merged
merged 9 commits into from Mar 17, 2016
6 changes: 6 additions & 0 deletions mitmproxy/cmdline.py
Expand Up @@ -434,6 +434,12 @@ def proxy_ssl_options(parser):
action="store_true", dest="no_upstream_cert",
help="Don't connect to upstream server to look up certificate details."
)
group.add_argument(
"--add-server-certs-to-client-chain", default=False,
action="store_true", dest="add_server_certs_to_client_chain",
help="Add all the certificates of the server to the certificate chain "
"that will be served to the client, as extras."
)
group.add_argument(
"--verify-upstream-cert", default=False,
action="store_true", dest="ssl_verify_upstream_cert",
Expand Down
6 changes: 6 additions & 0 deletions mitmproxy/protocol/tls.py
Expand Up @@ -432,6 +432,11 @@ def _establish_tls_with_client(self):
self.log("Establish TLS with client", "debug")
cert, key, chain_file = self._find_cert()

if self.config.add_server_certs_to_client_chain:
extra_certs = self.server_conn.server_certs
else:
extra_certs = None

try:
self.client_conn.convert_to_ssl(
cert, key,
Expand All @@ -441,6 +446,7 @@ def _establish_tls_with_client(self):
dhparams=self.config.certstore.dhparams,
chain_file=chain_file,
alpn_select_callback=self.__alpn_select_callback,
extra_chain_certs = extra_certs,
)
# Some TLS clients will not fail the handshake,
# but will immediately throw an "unexpected eof" error on the first read.
Expand Down
5 changes: 4 additions & 1 deletion mitmproxy/proxy/config.py
Expand Up @@ -67,6 +67,7 @@ def __init__(
ssl_verify_upstream_cert=False,
ssl_verify_upstream_trusted_cadir=None,
ssl_verify_upstream_trusted_ca=None,
add_server_certs_to_client_chain=False,
):
self.host = host
self.port = port
Expand Down Expand Up @@ -107,6 +108,7 @@ def __init__(
self.openssl_verification_mode_server = SSL.VERIFY_NONE
self.openssl_trusted_cadir_server = ssl_verify_upstream_trusted_cadir
self.openssl_trusted_ca_server = ssl_verify_upstream_trusted_ca
self.add_server_certs_to_client_chain = add_server_certs_to_client_chain


def process_proxy_options(parser, options):
Expand Down Expand Up @@ -206,5 +208,6 @@ def process_proxy_options(parser, options):
ssl_version_server=options.ssl_version_server,
ssl_verify_upstream_cert=options.ssl_verify_upstream_cert,
ssl_verify_upstream_trusted_cadir=options.ssl_verify_upstream_trusted_cadir,
ssl_verify_upstream_trusted_ca=options.ssl_verify_upstream_trusted_ca
ssl_verify_upstream_trusted_ca=options.ssl_verify_upstream_trusted_ca,
add_server_certs_to_client_chain=options.add_server_certs_to_client_chain,
)
10 changes: 10 additions & 0 deletions netlib/tcp.py
Expand Up @@ -584,6 +584,7 @@ def __init__(self, address, source_address=None):
self.address = address
self.source_address = source_address
self.cert = None
self.server_certs = []
self.ssl_verification_error = None
self.sni = None

Expand Down Expand Up @@ -668,6 +669,10 @@ def convert_to_ssl(self, sni=None, alpn_protos=None, **sslctx_kwargs):

self.cert = certutils.SSLCert(self.connection.get_peer_certificate())

# Keep all server certificates in a list
for i in self.connection.get_peer_cert_chain():
self.server_certs.append(certutils.SSLCert(i))

# Validate TLS Hostname
try:
crt = dict(
Expand Down Expand Up @@ -734,6 +739,7 @@ def create_ssl_context(self,
request_client_cert=None,
chain_file=None,
dhparams=None,
extra_chain_certs=None,
**sslctx_kwargs):
"""
cert: A certutils.SSLCert object or the path to a certificate
Expand Down Expand Up @@ -769,6 +775,10 @@ def create_ssl_context(self,
else:
context.use_certificate_chain_file(cert)

if extra_chain_certs:
for i in extra_chain_certs:
context.add_extra_chain_cert(i.x509)

if handle_sni:
# SNI callback happens during do_handshake()
context.set_tlsext_servername_callback(handle_sni)
Expand Down
5 changes: 3 additions & 2 deletions pathod/pathoc.py
Expand Up @@ -42,7 +42,8 @@ def __str__(self):
"Cipher: %s, %s bit, %s" % self.cipher,
"SSL certificate chain:"
]
for i in self.certchain:
for n,i in enumerate(self.certchain):
parts.append(" Certificate [%s]" % n)
parts.append("\tSubject: ")
for cn in i.get_subject().get_components():
parts.append("\t\t%s=%s" % cn)
Expand All @@ -69,7 +70,7 @@ def __str__(self):
s = certutils.SSLCert(i)
if s.altnames:
parts.append("\tSANs: %s" % " ".join(s.altnames))
return "\n".join(parts)
return "\n".join(parts)
Copy link
Member

Choose a reason for hiding this comment

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

Very good find, thanks! 😃




Expand Down
47 changes: 47 additions & 0 deletions test/mitmproxy/test_server.py
Expand Up @@ -999,3 +999,50 @@ def handler(f):
# (both terminated)
# nothing happened here
assert self.chain[1].tmaster.state.flow_count() == 2


class AddServerCertsToClientChainMixin:

def test_add_server_certs_to_client_chain(self):
with open(self.servercert, "rb") as f:
d = f.read()
c1 = SSLCert.from_pem(d)
p = self.pathoc()
server_cert_found_in_client_chain = False
for cert in p.server_certs:
if cert.digest('sha256') == c1.digest('sha256'):
server_cert_found_in_client_chain = True
break
assert(server_cert_found_in_client_chain == self.add_server_certs_to_client_chain)


class TestHTTPSAddServerCertsToClientChainTrue(tservers.HTTPProxyTest, AddServerCertsToClientChainMixin):

"""
If --add-server-certs-to-client-chain is True, then the client should receive the server's certificates
"""
add_server_certs_to_client_chain = True
ssl = True
servercert = tutils.test_data.path("data/trusted-server.crt")
ssloptions = pathod.SSLOptions(
cn="trusted-cert",
certs=[
("trusted-cert", servercert)
]
)


class TestHTTPSAddServerCertsToClientChainFalse(tservers.HTTPProxyTest, AddServerCertsToClientChainMixin):

"""
If --add-server-certs-to-client-chain is False, then the client should not receive the server's certificates
"""
add_server_certs_to_client_chain = False
ssl = True
servercert = tutils.test_data.path("data/trusted-server.crt")
ssloptions = pathod.SSLOptions(
cn="trusted-cert",
certs=[
("trusted-cert", servercert)
]
)
Copy link
Member

Choose a reason for hiding this comment

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

Excellent. I think we can even move ssl, servercert and ssloptions into AddServerCertsToClientChainMixin ? 😃

2 changes: 2 additions & 0 deletions test/mitmproxy/tservers.py
Expand Up @@ -86,6 +86,7 @@ class ProxyTestBase(object):
no_upstream_cert = False
authenticator = None
masterclass = TestMaster
add_server_certs_to_client_chain = False

@classmethod
def setup_class(cls):
Expand Down Expand Up @@ -129,6 +130,7 @@ def get_proxy_config(cls):
no_upstream_cert = cls.no_upstream_cert,
cadir = cls.cadir,
authenticator = cls.authenticator,
add_server_certs_to_client_chain = cls.add_server_certs_to_client_chain,
)


Expand Down