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

Review impact of let's encrypt CA change #6403

Closed
yschimke opened this issue Nov 8, 2020 · 13 comments
Closed

Review impact of let's encrypt CA change #6403

yschimke opened this issue Nov 8, 2020 · 13 comments
Labels
bug Bug in existing code

Comments

@yschimke
Copy link
Collaborator

yschimke commented Nov 8, 2020

Effective: As of January 11, 2021

https://letsencrypt.org/2020/11/06/own-two-feet.html

However, this does introduce some compatibility woes. Some software that hasn’t been updated since 2016 (approximately when our root was accepted to many root programs) still doesn’t trust our root certificate, ISRG Root X1. Most notably, this includes versions of Android prior to 7.1.1. That means those older versions of Android will no longer trust certificates issued by Let’s Encrypt.

Potentially we could promote adoption of Conscrypt for older clients and confirm they are shipping updated CA certs outside of the App?

@yschimke yschimke added the bug Bug in existing code label Nov 8, 2020
@yschimke
Copy link
Collaborator Author

yschimke commented Nov 8, 2020

@yschimke
Copy link
Collaborator Author

yschimke commented Nov 8, 2020

test server https://valid-isrgrootx1.letsencrypt.org

@swankjesse
Copy link
Member

We should also check that our cert pinning code does something reasonable for cross-signed certs. I suspect that might be a common mitigation?

@yschimke
Copy link
Collaborator Author

yschimke commented Nov 8, 2020

My current suggestion is a stackoverflow Question and self answer with sample code like the following (with correct certificate of course)

  val cert: X509Certificate = """
   -----BEGIN CERTIFICATE-----
   MIIBFzCBwgIJAIVAqagcVN7/MA0GCSqGSIb3DQEBBAUAMBMxETAPBgNVBAMMCGNh
   c2guYXBwMB4XDTE5MDkwNzAyMjg0NFoXDTE5MDkwODAyMjg0NFowEzERMA8GA1UE
   AwwIY2FzaC5hcHAwXDANBgkqhkiG9w0BAQEFAANLADBIAkEA8qAeoubm4mBTD9/J
   ujLQkfk/fuJt/T5pVQ1vUEqxfcMw0zYgszQ5C2MiIl7M6JkTRKU01q9hVFCR83wX
   zIdrLQIDAQABMA0GCSqGSIb3DQEBBAUAA0EAO1UpwhrkW3Ho1nZK/taoUQOoqz/n
   HFVMtyEkm5gBDgz8nJXwb3zbegclQyH+kVou02S8zC5WWzEtd0R8S0LsTA==
   -----END CERTIFICATE----- 
  """.trimIndent().parsePemCertificate().toX509Certificate()

  val handshakeCertificates = HandshakeCertificates.Builder()
      .addPlatformTrustedCertificates()
      .addTrustedCertificate(cert)
      .build()

  val client = OkHttpClient.Builder()
      .sslSocketFactory(handshakeCertificates.sslSocketFactory(), handshakeCertificates.trustManager)
      .build()

@swankjesse
Copy link
Member

I like that!

@swankjesse
Copy link
Member

Should we post this in our changelog? Or on our website?

@yschimke
Copy link
Collaborator Author

yschimke commented Nov 8, 2020

Should we post this in our changelog? Or on our website?

I'm working on a confirmed fix now. Then yes, but stackoverflow is the right forum for the canonical answer.

@yschimke
Copy link
Collaborator Author

yschimke commented Nov 8, 2020

The fix works, but is complicated by Junit5 requirements on Android...

@yschimke
Copy link
Collaborator Author

Closing this out. Think the two test cases and stackoverflow + forum post are better than the samples.

@shubham1g5
Copy link

@yschimke Is there advice on how to port this solution with enforcing TLS 1.2 on Android 4.x devices. More specifically how can I provide a particular TLS protocol to the SSLContext of the handshakeCertificates.

@yschimke
Copy link
Collaborator Author

Not really you could copy https://github.com/square/okhttp/blob/parent-3.12.12/okhttp-tls/src/main/java/okhttp3/tls/HandshakeCertificates.java and adjust to what you need.

Also do you really need to change the call to SSLContext.getInstance? AndroidPlatform tries to do the optimal thing here and 3.12.12 should support these older devices. I think I always get mixed up at whether you are trying to conditionally enable TLSv1.2 or the opposite and keep TLSv1.1 which was dropped later in 3.13. Sorry for the confusion.

https://github.com/square/okhttp/blob/parent-3.12.12/okhttp/src/main/java/okhttp3/internal/platform/AndroidPlatform.java#L442-L465

@shubham1g5
Copy link

@yschimke Thanks, My issue turned out to be something else. I was adding the call to .sslSocketFactory(handshakeCertificates.sslSocketFactory(), handshakeCertificates.trustManager) before adding TLSv1.2 (using conscrypt) which resulted into HTTP calls not using TLSv1.2 on Android 4.

@yschimke
Copy link
Collaborator Author

yschimke commented Dec 2, 2020

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

No branches or pull requests

3 participants