Verify upstream certificates by default. #1111

[s4chin]

This adds the --insecure option and enables certificate verification by default. The tests fail and I need help in knowing how to fix them.

[s4chin]

Adds the basic features as per #1111

[mhils]

This looks great, but we're still failing on some systems. @Kriechi, any idea?

@s4chin: I'd love to have a toggle for this in the mitmproxy options (press o in mitmproxy). Do you think you could get that going? 😃

[s4chin]

Which key do I assign for the toggle insecure option? I'm using K.

[mhils]

The options seems to be the wrong way around at the moment?

[cortesi]

This fails, because ProxyConfig doesn't have an ssl_insecure attribute

[cortesi]

Sachin, thanks for working on this. It's a valuable feature we definitely need. Please take a look at the comments I've added inline in the code - there's more work needed before we can merge.

[s4chin]

I made changes according to all your comments. Should I also squash all the commits?

[mhils]

Looks way better now! 😃

The ProxyConfig.openssl_verification_mode_server part looks like it's going to fail at some point where somebody forgets to update one of the attributes. Can we implement openssl_verification_mode_server as a @property ?

We should also add a test (class) that overrides get_proxy_config to remove the insecure=True default in the test suite and verifies that we actually fail for untrusted CAs.

[mhils]

What about the remaining failing tests? @Kriechi, any idea?

[tdickers]

I may be late to the party, but you may find that this site helps with testing: https://badssl.com/

[Kriechi]

I think this changes the default for all HTTP/2 tests - maybe we can only set this to true in the tests we actually want it to change?

[mhils]

It's fine if all tests are run using --insecure. We should add a tests that assures that we actually verify the certificate if ssl_insecure is not passed.

[Kriechi]

Actually, HTTP/2 misses this value - because it has its own TestBase.
@s4chin please add the same option here:

mitmproxy/test/mitmproxy/test_protocol_http2.py

Lines 97 to 101 in 5d0de16

	return dict(
	no_upstream_cert = False,
	cadir = cls.cadir,
	authenticator = None,
	)

[Kriechi]

About the failing HTTP/2 tests, the error message is:

TlsException('Cannot validate certificate hostname without SNI',)",)

[Kriechi]

I just pushed an improved version of the HTTP/2 tests in 5d0de16. This should make it easier to debug and track down the underlying cause.

[mhils]

Thanks for fixing the tests, I will take a look over the weekend! 😃