Move conscrypt NONE checks internal to Handshake

[yschimke]

Based on #3973, looks like this can still happen

[yschimke]

@JakeWharton @swankjesse @dave-r12 If this is happening in 8.1.0, it would be good to get the right fix in for 3.11.0 as adoption increases. I'm not sure if this is it, or there is something else we should be doing to address #3973

[dave-r12]

Do you know if they documented this anywhere? What's the difference between null and NONE?

[yschimke]

Here I guess - https://github.com/google/conscrypt/blob/82dc6605fb3d06bf51be2e2f6097f3eee1f32d22/common/src/main/java/org/conscrypt/SSLNullSession.java

I don't think this part is really documented at all. And since it's now baked into 8.1.0, there is no fix that makes our workaround not needed.

[swankjesse]

I’d prefer to remove RealConnection.isValid() completely and moving both the cipher suite check and the version check into Handshake.get(). That way we only check once!

[swankjesse]

Adding a checked exception isn’t source-compatible. I’m okay with that but we’ll need to call it out in the changelog.

[swankjesse]

This is a nice improvement.

[liufsd]

Hi, i found this issue: when got this issue, the host is null. that mean the SSLSession is error ?..?

@Aspect
public class OkhttpHandshakeTLSAspect {
    @Pointcut("execution(* okhttp3.Handshake.get(..))")
    public void handshakePoint() {
    }

    @Around("handshakePoint()")
    public Object onHandshakePointExc(final ProceedingJoinPoint joinPoint) throws Throwable {
        try {
//            throw new IllegalArgumentException("Unexpected TLS version: NONE");
            return joinPoint.proceed();
        } catch (IllegalArgumentException throwable) {
            if (TextUtils.equals(throwable.getMessage(), "Unexpected TLS version: NONE")){
                submitBIEvent(joinPoint);
                throw new IOException("Unexpected TLS version: NONE");
            } else {
                throw throwable;
            }
        }
    }

    private static void submitBIEvent(ProceedingJoinPoint joinPoint) {
        try {
            SSLSession sslSession = (SSLSession) joinPoint.getArgs()[0];
            Log.e("HandshakeTLSAspect", "PeerHost: " + sslSession.getPeerHost());
            BIReportHammer.create().buildEvent(RunTimeManager.instance().getApplicationContext(), "OKHTTP_TLS_ERROR")
                    .addExtData("host", sslSession.getPeerHost())  // host: null
                    .addExtData("port", sslSession.getPeerPort()) //port: -1
                    .commit();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}