Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

Commit

Permalink
Dont trust the user added CA store by default for apps targeting N
Browse files Browse the repository at this point in the history 
Android's security model is such that the applications data is secure by
default unless the application specifically grants access to it.
Application data in transit should have similar security properties.

Bug: 27301579
Change-Id: I72f106aefecccd6edfcc1d3ae10131ad2f69a559
  • Loading branch information
@chadbrubaker
chadbrubaker committed Feb 24, 2016
1 parent d78bf97 commit 32d2a10
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 16 deletions.
12 changes: 7 additions & 5 deletions core/java/android/security/net/config/ManifestConfigSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ private ConfigSource getConfigSource() {
} catch (PackageManager.NameNotFoundException e) {
throw new RuntimeException("Failed to look up ApplicationInfo", e);
}
int targetSdkVersion = info.targetSdkVersion;
int configResourceId = 0;
if (info != null && info.metaData != null) {
configResourceId = info.metaData.getInt(META_DATA_NETWORK_SECURITY_CONFIG);
Expand All @@ -74,14 +75,15 @@ private ConfigSource getConfigSource() {
+ mContext.getResources().getResourceEntryName(configResourceId)
+ " debugBuild: " + debugBuild);
}
source = new XmlConfigSource(mContext, configResourceId, debugBuild);
source = new XmlConfigSource(mContext, configResourceId, debugBuild,
targetSdkVersion);
} else {
if (DBG) {
Log.d(LOG_TAG, "No Network Security Config specified, using platform default");
}
boolean usesCleartextTraffic =
(info.flags & ApplicationInfo.FLAG_USES_CLEARTEXT_TRAFFIC) != 0;
source = new DefaultConfigSource(usesCleartextTraffic);
source = new DefaultConfigSource(usesCleartextTraffic, targetSdkVersion);
}
mConfigSource = source;
return mConfigSource;
Expand All @@ -92,11 +94,11 @@ private static final class DefaultConfigSource implements ConfigSource {

private final NetworkSecurityConfig mDefaultConfig;

public DefaultConfigSource(boolean usesCleartextTraffic) {
mDefaultConfig = NetworkSecurityConfig.getDefaultBuilder()
public DefaultConfigSource(boolean usesCleartextTraffic, int targetSdkVersion) {
mDefaultConfig = NetworkSecurityConfig.getDefaultBuilder(targetSdkVersion)
.setCleartextTrafficPermitted(usesCleartextTraffic)
.build();
}
}

@Override
public NetworkSecurityConfig getDefaultConfig() {
Expand Down
23 changes: 15 additions & 8 deletions core/java/android/security/net/config/NetworkSecurityConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package android.security.net.config;

import android.os.Build;
import android.util.ArrayMap;
import android.util.ArraySet;
import java.security.cert.X509Certificate;
Expand All @@ -37,7 +38,6 @@ public final class NetworkSecurityConfig {
public static final boolean DEFAULT_CLEARTEXT_TRAFFIC_PERMITTED = true;
/** @hide */
public static final boolean DEFAULT_HSTS_ENFORCED = false;
public static final NetworkSecurityConfig DEFAULT = getDefaultBuilder().build();

private final boolean mCleartextTrafficPermitted;
private final boolean mHstsEnforced;
Expand Down Expand Up @@ -163,21 +163,28 @@ public Set<X509Certificate> findAllCertificatesByIssuerAndSignature(X509Certific
* <li>Cleartext traffic is permitted.</li>
* <li>HSTS is not enforced.</li>
* <li>No certificate pinning is used.</li>
* <li>The system and user added trusted certificate stores are trusted for connections.</li>
* <li>The system certificate store is trusted for connections.</li>
* <li>If the application targets API level 23 (Android M) or lower then the user certificate
* store is trusted by default as well.</li>
* </ol>
*
* @hide
*/
public static final Builder getDefaultBuilder() {
return new Builder()
public static final Builder getDefaultBuilder(int targetSdkVersion) {
Builder builder = new Builder()
.setCleartextTrafficPermitted(DEFAULT_CLEARTEXT_TRAFFIC_PERMITTED)
.setHstsEnforced(DEFAULT_HSTS_ENFORCED)
// System certificate store, does not bypass static pins.
.addCertificatesEntryRef(
new CertificatesEntryRef(SystemCertificateSource.getInstance(), false))
// User certificate store, does not bypass static pins.
.addCertificatesEntryRef(
new CertificatesEntryRef(UserCertificateSource.getInstance(), false));
new CertificatesEntryRef(SystemCertificateSource.getInstance(), false));
// Applications targeting N and above must opt in into trusting the user added certificate
// store.
if (targetSdkVersion <= Build.VERSION_CODES.M) {
// User certificate store, does not bypass static pins.
builder.addCertificatesEntryRef(
new CertificatesEntryRef(UserCertificateSource.getInstance(), false));
}
return builder;
}

/**
Expand Down
13 changes: 12 additions & 1 deletion core/java/android/security/net/config/XmlConfigSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
import android.content.Context;
import android.content.res.Resources;
import android.content.res.XmlResourceParser;
import android.os.Build;
import android.util.ArraySet;
import android.util.Base64;
import android.util.Pair;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.XmlUtils;

import org.xmlpull.v1.XmlPullParser;
Expand Down Expand Up @@ -34,20 +36,29 @@ public class XmlConfigSource implements ConfigSource {
private final Object mLock = new Object();
private final int mResourceId;
private final boolean mDebugBuild;
private final int mTargetSdkVersion;

private boolean mInitialized;
private NetworkSecurityConfig mDefaultConfig;
private Set<Pair<Domain, NetworkSecurityConfig>> mDomainMap;
private Context mContext;

@VisibleForTesting
public XmlConfigSource(Context context, int resourceId) {
this(context, resourceId, false);
}

@VisibleForTesting
public XmlConfigSource(Context context, int resourceId, boolean debugBuild) {
this(context, resourceId, debugBuild, Build.VERSION_CODES.CUR_DEVELOPMENT);
}

public XmlConfigSource(Context context, int resourceId, boolean debugBuild,
int targetSdkVersion) {
mResourceId = resourceId;
mContext = context;
mDebugBuild = debugBuild;
mTargetSdkVersion = targetSdkVersion;
}

public Set<Pair<Domain, NetworkSecurityConfig>> getPerDomainConfigs() {
Expand Down Expand Up @@ -341,7 +352,7 @@ private void parseNetworkSecurityConfig(XmlResourceParser parser)
// Use the platform default as the parent of the base config for any values not provided
// there. If there is no base config use the platform default.
NetworkSecurityConfig.Builder platformDefaultBuilder =
NetworkSecurityConfig.getDefaultBuilder();
NetworkSecurityConfig.getDefaultBuilder(mTargetSdkVersion);
addDebugAnchorsIfNeeded(debugConfigBuilder, platformDefaultBuilder);
if (baseConfigBuilder != null) {
baseConfigBuilder.setParent(platformDefaultBuilder);
Expand Down
91 changes: 89 additions & 2 deletions ...NetworkSecurityConfigTest/src/android/security/net/config/NetworkSecurityConfigTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,28 @@
package android.security.net.config;

import android.app.Activity;
import android.os.Build;
import android.test.ActivityUnitTestCase;
import android.util.ArraySet;
import android.util.Pair;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.net.Socket;
import java.net.URL;
import java.security.cert.Certificate;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.TrustManager;

import com.android.org.conscrypt.TrustedCertificateStore;

public class NetworkSecurityConfigTests extends ActivityUnitTestCase<Activity> {

public NetworkSecurityConfigTests() {
Expand All @@ -40,6 +49,51 @@ public NetworkSecurityConfigTests() {
private static final byte[] G2_SPKI_SHA256
= hexToBytes("ec722969cb64200ab6638f68ac538e40abab5b19a6485661042a1061c4612776");

private static final byte[] TEST_CA_BYTES
= hexToBytes(
"3082036130820249a003020102020900bd54597d6750ea62300d06092a86"
+ "4886f70d01010b05003047310b3009060355040613025553310b30090603"
+ "5504080c0243413110300e060355040a0c07416e64726f69643119301706"
+ "035504030c104e53436f6e6669672054657374204341301e170d31363032"
+ "32343030313130325a170d3136303332353030313130325a3047310b3009"
+ "060355040613025553310b300906035504080c0243413110300e06035504"
+ "0a0c07416e64726f69643119301706035504030c104e53436f6e66696720"
+ "5465737420434130820122300d06092a864886f70d01010105000382010f"
+ "003082010a0282010100e15ce8fd5794029841e760d68d6e0159c9c67630"
+ "089775bc728d83dae7e29e23fe5f6e113b789f4c5b22f052300ec6d5faa5"
+ "724432e7bac96682792ef6e9617c939c4329dce8788cbdf3a11b621fac9e"
+ "2edbec2d7e5e07296bbb544b89263137a6a31573a2362e05ca8ff9c886bf"
+ "52df4ff93c45475145a40a83f2670e23669220a5a4bf2c6860edb78d3022"
+ "192fb5dc5e8c118f70870f89da292dfe522751462f020ed556653c8b07f8"
+ "89712a6e8196c457a637439e3073d7d917ab55aa51a146826367f7b5922a"
+ "64fb2f95099de21eb98341fa76faa79ffbda123fe5b8adc614b16174e8b0"
+ "dfdac2bbc4d526d2487ad2b009d53996ec23ffbd732112efa66b02030100"
+ "01a350304e301d0603551d0e04160414f66e1a95486c879edd60a5756bc2"
+ "f1f4677e128e301f0603551d23041830168014f66e1a95486c879edd60a5"
+ "756bc2f1f4677e128e300c0603551d13040530030101ff300d06092a8648"
+ "86f70d01010b05000382010100d2856130dccae24e5f8901900d94bc642f"
+ "85466ab7cfa1066399077a168cd4b56603a9e2af9d2e58aec13101e338a4"
+ "8e95e9c7a84d7991f0d381d4965eaada1b80fbbd8277445f449babe64f53"
+ "ba625387460b592a1a97b14b8251115e6610350021a6e716ae22b905f8d4"
+ "eae24e668e71b12ab51fd2f2bb600e074487dec720c3db14dbca504844b6"
+ "933bb0248283ea95464747689c37d706d4839c7d0e9bd86abf98ddce5d36"
+ "8b38bfe5062353e28d5be378827fade1caa6bba3df9cd9ebf83d839eae52"
+ "780181f31973f15f982686ba6d899f7b644fd1f26c8ebb99f4c986faaf4c"
+ "1b9e3d9d391943ce3fb9fa2e631bd66b8ef3d47fd85acf09ea3a30f15f");

private static final X509Certificate TEST_CA_CERT;

static {
try {
CertificateFactory factory = CertificateFactory.getInstance("X.509");
Certificate cert = factory.generateCertificate(new ByteArrayInputStream(TEST_CA_BYTES));
TEST_CA_CERT = (X509Certificate) cert;
} catch (Exception e) {
throw new RuntimeException(e);
}
}


private static byte[] hexToBytes(String s) {
int len = s.length();
byte[] data = new byte[len / 2];
Expand All @@ -51,7 +105,6 @@ private static byte[] hexToBytes(String s) {
}



/**
* Return a NetworkSecurityConfig that has an empty TrustAnchor set. This should always cause a
* SSLHandshakeException when used for a connection.
Expand Down Expand Up @@ -174,7 +227,7 @@ public void testSubdomainIncluded() throws Exception {
public void testConfigBuilderUsesParents() throws Exception {
// Check that a builder with a parent uses the parent's values when non is set.
NetworkSecurityConfig config = new NetworkSecurityConfig.Builder()
.setParent(NetworkSecurityConfig.getDefaultBuilder())
.setParent(NetworkSecurityConfig.getDefaultBuilder(Build.VERSION_CODES.N))
.build();
assert(!config.getTrustAnchors().isEmpty());
}
Expand Down Expand Up @@ -208,4 +261,38 @@ public void testWithUrlConnection() throws Exception {
TestUtils.assertUrlConnectionSucceeds(context, "developer.android.com", 443);
TestUtils.assertUrlConnectionFails(context, "google.com", 443);
}

public void testUserAddedCaOptIn() throws Exception {
TrustedCertificateStore store = new TrustedCertificateStore();
try {
// Install the test CA.
store.installCertificate(TEST_CA_CERT);
NetworkSecurityConfig preNConfig =
NetworkSecurityConfig.getDefaultBuilder(Build.VERSION_CODES.M).build();
NetworkSecurityConfig nConfig =
NetworkSecurityConfig.getDefaultBuilder(Build.VERSION_CODES.N).build();
Set<TrustAnchor> preNAnchors = preNConfig.getTrustAnchors();
Set<TrustAnchor> nAnchors = nConfig.getTrustAnchors();
Set<X509Certificate> preNCerts = new HashSet<X509Certificate>();
for (TrustAnchor anchor : preNAnchors) {
preNCerts.add(anchor.certificate);
}
Set<X509Certificate> nCerts = new HashSet<X509Certificate>();
for (TrustAnchor anchor : nAnchors) {
nCerts.add(anchor.certificate);
}
assertTrue(preNCerts.contains(TEST_CA_CERT));
assertFalse(nCerts.contains(TEST_CA_CERT));
} finally {
// Delete the user added CA. We don't know the alias so just delete them all.
for (String alias : store.aliases()) {
if (store.isUser(alias)) {
try {
store.deleteCertificateEntry(alias);
} catch (Exception ignored) {
}
}
}
}
}
}

0 comments on commit 32d2a10

Please sign in to comment.