Skip to content

Commit

Permalink
Use conscrypt TrustManager on API>=24
Browse files Browse the repository at this point in the history
* Prompted by investigations on owntracks#896
* Updated CHANGELOG to notify users that SANs are currently required for
hostname verification to work
  • Loading branch information
growse committed Dec 28, 2020
1 parent 5ab4d35 commit 7c2acc7
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Version 2.2.2

### Breaking changes

* TLS hostname verification now no longer considers the CN field in the server certificate. This is in line with the deprecation of using CN for host verification as of RFC2818 (2000) and RFC6125 (2011). Instead, hostnames will be validated against the list of subjectAltNames (SANs) in the server certificate. If you were previously relying on the CN field in your certificate and didn't have any SANs listed, or the SAN list doesn't contain the hostname you're connecting to, TLS connections may now fail. The suggested course of action is to regenerate the cert ensuring that the SAN list is correct.

### Bug fixes

* Fixed a crash caused by a race condition on some devices where certain things are used before init (#890)
Expand Down
11 changes: 9 additions & 2 deletions project/app/src/main/java/org/owntracks/android/App.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.owntracks.android

import android.content.Intent
import android.os.Build
import android.os.StrictMode
import android.os.StrictMode.VmPolicy
import androidx.work.Configuration
Expand Down Expand Up @@ -41,8 +42,14 @@ class App : DaggerApplication() {
var eventBus: EventBus? = null
override fun onCreate() {
WorkManager.initialize(this, Configuration.Builder().build())
// Make sure we use Conscrypt for advanced TLS features on all devices
Security.insertProviderAt(Conscrypt.newProvider(), 1)
// Make sure we use Conscrypt for advanced TLS features on all devices.
// X509ExtendedTrustManager not available pre-24, fall back to device. https://github.com/google/conscrypt/issues/603
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
Security.insertProviderAt(Conscrypt.newProviderBuilder().provideTrustManager(true).build(), 1)
} else {
Security.insertProviderAt(Conscrypt.newProviderBuilder().provideTrustManager(false).build(), 1)
}

super.onCreate()
if (BuildConfig.DEBUG) {
Timber.plant(TimberDebugLogTree())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.security.KeyManagementException;
Expand Down Expand Up @@ -286,6 +287,8 @@ private MqttConnectOptions getMqttConnectOptions() throws MqttConnectionExceptio
connectOptions.setPassword(preferences.getPassword().toCharArray());
}
connectOptions.setMqttVersion(preferences.getMqttProtocolLevel());
InputStream clientCaInputStream = null;
InputStream clientCertInputStream = null;
try {
if (preferences.getTls()) {
String tlsCaCrt = preferences.getTlsCaCrt();
Expand All @@ -295,15 +298,17 @@ private MqttConnectOptions getMqttConnectOptions() throws MqttConnectionExceptio

if (tlsCaCrt.length() > 0) {
try {
socketFactoryOptions.withCaInputStream(applicationContext.openFileInput(tlsCaCrt));
clientCaInputStream= applicationContext.openFileInput(tlsCaCrt);
socketFactoryOptions.withCaInputStream(clientCaInputStream);
} catch (FileNotFoundException e) {
Timber.e(e);
}
}

if (tlsClientCrt.length() > 0) {
try {
socketFactoryOptions.withClientP12InputStream(applicationContext.openFileInput(tlsClientCrt)).withClientP12Password(preferences.getTlsClientCrtPassword());
clientCertInputStream = applicationContext.openFileInput(tlsClientCrt);
socketFactoryOptions.withClientP12InputStream(clientCertInputStream).withClientP12Password(preferences.getTlsClientCrtPassword());
} catch (FileNotFoundException e) {
Timber.e(e);
}
Expand All @@ -315,6 +320,18 @@ private MqttConnectOptions getMqttConnectOptions() throws MqttConnectionExceptio
} catch (CertificateException | NoSuchAlgorithmException | UnrecoverableKeyException | KeyStoreException | KeyManagementException | IOException e) {
changeState(EndpointState.ERROR.withError(e).withMessage("TLS setup failed"));
throw new MqttConnectionException(e);
} finally {
try {
if (clientCaInputStream != null) {
clientCaInputStream.close();
}
if (clientCertInputStream != null) {
clientCertInputStream.close();
}
} catch (IOException e) {
Timber.e(e);
}

}

setWill(connectOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ boolean hasClientP12Password() {
}


private TrustManagerFactory tmf;
private final TrustManagerFactory tmf;

public SocketFactory(SocketFactoryOptions options) throws KeyStoreException, NoSuchAlgorithmException, IOException, KeyManagementException, java.security.cert.CertificateException, UnrecoverableKeyException {
Timber.tag(this.toString()).v("initializing CustomSocketFactory");

tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
KeyManagerFactory kmf = KeyManagerFactory.getInstance("X509");

KeyManagerFactory kmf = KeyManagerFactory.getInstance("X509");

if(options.hasCaCrt()) {
Timber.tag(this.toString()).v("options.hasCaCrt(): true");
Expand All @@ -99,13 +99,10 @@ public SocketFactory(SocketFactoryOptions options) throws KeyStoreException, NoS
Timber.v("Certificate Version: %s", ca.getVersion());
Timber.v("Certificate OID: %s", ca.getSigAlgOID());
Enumeration<String> aliasesCA = caKeyStore.aliases();
for (; aliasesCA.hasMoreElements(); ) {
while (aliasesCA.hasMoreElements()) {
String o = aliasesCA.nextElement();
Timber.v("Alias: %s isKeyEntry:%s isCertificateEntry:%s", o, caKeyStore.isKeyEntry(o), caKeyStore.isCertificateEntry(o));
}



} else {
Timber.v("CA sideload: false, using system keystore");
KeyStore keyStore = KeyStore.getInstance("AndroidCAStore");
Expand All @@ -123,7 +120,7 @@ public SocketFactory(SocketFactoryOptions options) throws KeyStoreException, NoS

Timber.tag(this.toString()).v("Client .p12 Keystore content: ");
Enumeration<String> aliasesClientCert = clientKeyStore.aliases();
for (; aliasesClientCert.hasMoreElements(); ) {
while (aliasesClientCert.hasMoreElements()) {
String o = aliasesClientCert.nextElement();
Timber.v("Alias: %s", o);
}
Expand All @@ -133,10 +130,9 @@ public SocketFactory(SocketFactoryOptions options) throws KeyStoreException, NoS
}

// Create an SSLContext that uses our TrustManager
SSLContext context = SSLContext.getInstance("TLSv1.2");
SSLContext context = SSLContext.getInstance("TLS");
context.init(kmf.getKeyManagers(), getTrustManagers(), null);
this.factory= context.getSocketFactory();

}

public TrustManager[] getTrustManagers() {
Expand Down

0 comments on commit 7c2acc7

Please sign in to comment.