Okhttp: Provide a way to support dev servers

Created on 4 Mar 2020 · 9Comments · Source: square/okhttp

As seen in https://github.com/square/okhttp/issues/5845 and a metric shedload of stackoverflow posts

    private final static X509TrustManager TRUST_MANAGER = new X509TrustManager() {
        @Override
        public void checkClientTrusted(X509Certificate[] x509Certificates, String s) {
        }

        @Override
        public void checkServerTrusted(X509Certificate[] x509Certificates, String s) {
        }

        @Override
        public X509Certificate[] getAcceptedIssuers() {
            return new X509Certificate[0];
        }
    };

As it is it's creating work for users and for us.

Is pretty much the 2nd most typed code in history after Hello World. We should either make an Intellij plugin to create it, or find a nicer and safer pattern to support dev servers?

Thoughts

cc @swankjesse

enhancement

Source

yschimke

All 9 comments

I was thinking similarly, but I also really don't want to make this too easy. Bad unsafe code should look bad and unsafe.

swankjesse on 5 Mar 2020

Maybe the right API makes it safe. E.g. checks it's a Dev build on Android and uses a strict allowlist of patterns.

yschimke on 5 Mar 2020

This is absolutely part of the normal developer workflow if your devserver has self signed or corp signed certs.

yschimke on 5 Mar 2020

Yep. I just don't want this accidentally shipping to production.

Potential ways to do that:

Name the method 'SABOTAGE_HTTPS()`
On Android use a Toast, “☠️ SABOTAGE_HTTPS”
Log this as an error “☠️ SABOTAGE_HTTPS” with a stacktrace
Send “☠️ SABOTAGE_HTTPS” to the uncaught exception handler
Refuse to connect to hosts with public-internet IP addresses (somehow)
Detect if we're in a debug build, and crash if we're not
Detect if a debugger is attached and crash if it isn’t
Hit every target host with an extra request for the server's logs /.well-known/CLIENT_SABOTAGED_TLS/☠️
Refuse to connect to hosts with valid HTTP

Are there other examples of mitigations against debug-only settings?

swankjesse on 5 Mar 2020

👍1

A very early draft PR for discussion

https://github.com/square/okhttp/pull/5854

yschimke on 6 Mar 2020

Yep. I just don't want this accidentally shipping to production.

Agree 100%, but I think by not making this easier, we have effectively made it more likely that apps are out there in the Play Store with security turned off. But at least it isn't our fault currently.

The PR attempts to make it safe, correct and optimal.

yschimke on 6 Mar 2020

For information in some cases we have no choices than disable those security when connecting to badly configured servers with self signed certificates. From apps that connect to some devices and not public secure web servers.

We do this with tons of warning to user and a few confirmation clicks, but currently it's fully disabled for that connection and as such is probably leaking that security removal when the server redirect to other things. Would be nice to have this debug thing allowed in prod and runtime configurable under a tons of opt in warning / lint checks whatever, so that we can lift those checks properly only for 1 IP and prevent bad implementation that goes out of that IP, because by lack of knowledge on how to do this right, I do use the OP code snippet for this. Or some better recipe about this use case.