Merge "Use a builder for NetworkSecurityConfig" am: 7e98f2e697 am: 478fad3cf5
am: 54b4b8fdfc
* commit '54b4b8fdfc336b8a4902637e622c3ede879edcde':
Use a builder for NetworkSecurityConfig
diff --git a/core/java/android/security/net/config/NetworkSecurityConfig.java b/core/java/android/security/net/config/NetworkSecurityConfig.java
index 915fbef..0a4ce11 100644
--- a/core/java/android/security/net/config/NetworkSecurityConfig.java
+++ b/core/java/android/security/net/config/NetworkSecurityConfig.java
@@ -17,6 +17,9 @@
package android.security.net.config;
import android.util.ArraySet;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Set;
@@ -26,6 +29,12 @@
* @hide
*/
public final class NetworkSecurityConfig {
+ /** @hide */
+ 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;
private final PinSet mPins;
@@ -35,7 +44,7 @@
private X509TrustManager mTrustManager;
private final Object mTrustManagerLock = new Object();
- public NetworkSecurityConfig(boolean cleartextTrafficPermitted, boolean hstsEnforced,
+ private NetworkSecurityConfig(boolean cleartextTrafficPermitted, boolean hstsEnforced,
PinSet pins, List<CertificatesEntryRef> certificatesEntryRefs) {
mCleartextTrafficPermitted = cleartextTrafficPermitted;
mHstsEnforced = hstsEnforced;
@@ -83,4 +92,151 @@
mAnchors = null;
}
}
+
+ /**
+ * Return a {@link Builder} for the default {@code NetworkSecurityConfig}.
+ *
+ * <p>
+ * The default configuration has the following properties:
+ * <ol>
+ * <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>
+ * </ol>
+ *
+ * @hide
+ */
+ public static final Builder getDefaultBuilder() {
+ return 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));
+ }
+
+ /**
+ * Builder for creating {@code NetworkSecurityConfig} objects.
+ * @hide
+ */
+ public static final class Builder {
+ private List<CertificatesEntryRef> mCertificatesEntryRefs;
+ private PinSet mPinSet;
+ private boolean mCleartextTrafficPermitted = DEFAULT_CLEARTEXT_TRAFFIC_PERMITTED;
+ private boolean mHstsEnforced = DEFAULT_HSTS_ENFORCED;
+ private boolean mCleartextTrafficPermittedSet = false;
+ private boolean mHstsEnforcedSet = false;
+ private Builder mParentBuilder;
+
+ /**
+ * Sets the parent {@code Builder} for this {@code Builder}.
+ * The parent will be used to determine values not configured in this {@code Builder}
+ * in {@link Builder#build()}, recursively if needed.
+ */
+ public Builder setParent(Builder parent) {
+ // Sanity check to avoid adding loops.
+ Builder current = parent;
+ while (current != null) {
+ if (current == this) {
+ throw new IllegalArgumentException("Loops are not allowed in Builder parents");
+ }
+ current = current.getParent();
+ }
+ mParentBuilder = parent;
+ return this;
+ }
+
+ public Builder getParent() {
+ return mParentBuilder;
+ }
+
+ public Builder setPinSet(PinSet pinSet) {
+ mPinSet = pinSet;
+ return this;
+ }
+
+ private PinSet getEffectivePinSet() {
+ if (mPinSet != null) {
+ return mPinSet;
+ }
+ if (mParentBuilder != null) {
+ return mParentBuilder.getEffectivePinSet();
+ }
+ return PinSet.EMPTY_PINSET;
+ }
+
+ public Builder setCleartextTrafficPermitted(boolean cleartextTrafficPermitted) {
+ mCleartextTrafficPermitted = cleartextTrafficPermitted;
+ mCleartextTrafficPermittedSet = true;
+ return this;
+ }
+
+ private boolean getEffectiveCleartextTrafficPermitted() {
+ if (mCleartextTrafficPermittedSet) {
+ return mCleartextTrafficPermitted;
+ }
+ if (mParentBuilder != null) {
+ return mParentBuilder.getEffectiveCleartextTrafficPermitted();
+ }
+ return DEFAULT_CLEARTEXT_TRAFFIC_PERMITTED;
+ }
+
+ public Builder setHstsEnforced(boolean hstsEnforced) {
+ mHstsEnforced = hstsEnforced;
+ mHstsEnforcedSet = true;
+ return this;
+ }
+
+ private boolean getEffectiveHstsEnforced() {
+ if (mHstsEnforcedSet) {
+ return mHstsEnforced;
+ }
+ if (mParentBuilder != null) {
+ return mParentBuilder.getEffectiveHstsEnforced();
+ }
+ return DEFAULT_HSTS_ENFORCED;
+ }
+
+ public Builder addCertificatesEntryRef(CertificatesEntryRef ref) {
+ if (mCertificatesEntryRefs == null) {
+ mCertificatesEntryRefs = new ArrayList<CertificatesEntryRef>();
+ }
+ mCertificatesEntryRefs.add(ref);
+ return this;
+ }
+
+ public Builder addCertificatesEntryRefs(Collection<? extends CertificatesEntryRef> refs) {
+ if (mCertificatesEntryRefs == null) {
+ mCertificatesEntryRefs = new ArrayList<CertificatesEntryRef>();
+ }
+ mCertificatesEntryRefs.addAll(refs);
+ return this;
+ }
+
+ private List<CertificatesEntryRef> getEffectiveCertificatesEntryRefs() {
+ if (mCertificatesEntryRefs != null) {
+ return mCertificatesEntryRefs;
+ }
+ if (mParentBuilder != null) {
+ return mParentBuilder.getEffectiveCertificatesEntryRefs();
+ }
+ return Collections.<CertificatesEntryRef>emptyList();
+ }
+
+ public boolean hasCertificateEntryRefs() {
+ return mCertificatesEntryRefs != null;
+ }
+
+ public NetworkSecurityConfig build() {
+ boolean cleartextPermitted = getEffectiveCleartextTrafficPermitted();
+ boolean hstsEnforced = getEffectiveCleartextTrafficPermitted();
+ PinSet pinSet = getEffectivePinSet();
+ List<CertificatesEntryRef> entryRefs = getEffectiveCertificatesEntryRefs();
+ return new NetworkSecurityConfig(cleartextPermitted, hstsEnforced, pinSet, entryRefs);
+ }
+ }
}
diff --git a/core/java/android/security/net/config/PinSet.java b/core/java/android/security/net/config/PinSet.java
index a9ee039..d3c975e 100644
--- a/core/java/android/security/net/config/PinSet.java
+++ b/core/java/android/security/net/config/PinSet.java
@@ -17,10 +17,13 @@
package android.security.net.config;
import android.util.ArraySet;
+import java.util.Collections;
import java.util.Set;
/** @hide */
public final class PinSet {
+ public static final PinSet EMPTY_PINSET =
+ new PinSet(Collections.<Pin>emptySet(), Long.MAX_VALUE);
public final long expirationTime;
public final Set<Pin> pins;
diff --git a/core/java/android/security/net/config/SystemCertificateSource.java b/core/java/android/security/net/config/SystemCertificateSource.java
index 640ebd9..7649a97 100644
--- a/core/java/android/security/net/config/SystemCertificateSource.java
+++ b/core/java/android/security/net/config/SystemCertificateSource.java
@@ -36,18 +36,23 @@
* @hide
*/
public class SystemCertificateSource implements CertificateSource {
- private static Set<X509Certificate> sSystemCerts = null;
- private static final Object sLock = new Object();
+ private static final SystemCertificateSource INSTANCE = new SystemCertificateSource();
+ private Set<X509Certificate> mSystemCerts = null;
+ private final Object mLock = new Object();
- public SystemCertificateSource() {
+ private SystemCertificateSource() {
+ }
+
+ public static SystemCertificateSource getInstance() {
+ return INSTANCE;
}
@Override
public Set<X509Certificate> getCertificates() {
// TODO: loading all of these is wasteful, we should instead use a keystore style API.
- synchronized (sLock) {
- if (sSystemCerts != null) {
- return sSystemCerts;
+ synchronized (mLock) {
+ if (mSystemCerts != null) {
+ return mSystemCerts;
}
CertificateFactory certFactory;
try {
@@ -83,14 +88,14 @@
IoUtils.closeQuietly(is);
}
}
- sSystemCerts = systemCerts;
- return sSystemCerts;
+ mSystemCerts = systemCerts;
+ return mSystemCerts;
}
}
public void onCertificateStorageChange() {
- synchronized (sLock) {
- sSystemCerts = null;
+ synchronized (mLock) {
+ mSystemCerts = null;
}
}
}
diff --git a/core/java/android/security/net/config/UserCertificateSource.java b/core/java/android/security/net/config/UserCertificateSource.java
index 77e2c88..ca246e1 100644
--- a/core/java/android/security/net/config/UserCertificateSource.java
+++ b/core/java/android/security/net/config/UserCertificateSource.java
@@ -36,18 +36,23 @@
* @hide
*/
public class UserCertificateSource implements CertificateSource {
- private static Set<X509Certificate> sUserCerts = null;
- private static final Object sLock = new Object();
+ private static final UserCertificateSource INSTANCE = new UserCertificateSource();
+ private Set<X509Certificate> mUserCerts = null;
+ private final Object mLock = new Object();
- public UserCertificateSource() {
+ private UserCertificateSource() {
+ }
+
+ public static UserCertificateSource getInstance() {
+ return INSTANCE;
}
@Override
public Set<X509Certificate> getCertificates() {
// TODO: loading all of these is wasteful, we should instead use a keystore style API.
- synchronized (sLock) {
- if (sUserCerts != null) {
- return sUserCerts;
+ synchronized (mLock) {
+ if (mUserCerts != null) {
+ return mUserCerts;
}
CertificateFactory certFactory;
try {
@@ -75,14 +80,14 @@
IoUtils.closeQuietly(is);
}
}
- sUserCerts = userCerts;
- return sUserCerts;
+ mUserCerts = userCerts;
+ return mUserCerts;
}
}
public void onCertificateStorageChange() {
- synchronized (sLock) {
- sUserCerts = null;
+ synchronized (mLock) {
+ mUserCerts = null;
}
}
}
diff --git a/tests/NetworkSecurityConfigTest/src/android/security/net/config/NetworkSecurityConfigTests.java b/tests/NetworkSecurityConfigTest/src/android/security/net/config/NetworkSecurityConfigTests.java
index 9a1fe15..11d8136 100644
--- a/tests/NetworkSecurityConfigTest/src/android/security/net/config/NetworkSecurityConfigTests.java
+++ b/tests/NetworkSecurityConfigTest/src/android/security/net/config/NetworkSecurityConfigTests.java
@@ -100,16 +100,14 @@
* SSLHandshakeException when used for a connection.
*/
private NetworkSecurityConfig getEmptyConfig() {
- return new NetworkSecurityConfig(true, false,
- new PinSet(new ArraySet<Pin>(), -1),
- new ArrayList<CertificatesEntryRef>());
+ return new NetworkSecurityConfig.Builder().build();
}
private NetworkSecurityConfig getSystemStoreConfig() {
- ArrayList<CertificatesEntryRef> defaultSource = new ArrayList<CertificatesEntryRef>();
- defaultSource.add(new CertificatesEntryRef(new SystemCertificateSource(), false));
- return new NetworkSecurityConfig(true, false, new PinSet(new ArraySet<Pin>(),
- -1), defaultSource);
+ return new NetworkSecurityConfig.Builder()
+ .addCertificatesEntryRef(
+ new CertificatesEntryRef(SystemCertificateSource.getInstance(), false))
+ .build();
}
public void testEmptyConfig() throws Exception {
@@ -126,24 +124,20 @@
= new ArraySet<Pair<Domain, NetworkSecurityConfig>>();
domainMap.add(new Pair<Domain, NetworkSecurityConfig>(
new Domain("android.com", true), getEmptyConfig()));
- ArrayList<CertificatesEntryRef> defaultSource = new ArrayList<CertificatesEntryRef>();
- defaultSource.add(new CertificatesEntryRef(new SystemCertificateSource(), false));
- NetworkSecurityConfig defaultConfig = new NetworkSecurityConfig(true, false,
- new PinSet(new ArraySet<Pin>(), -1),
- defaultSource);
+ NetworkSecurityConfig defaultConfig = getSystemStoreConfig();
SSLContext context = getSSLContext(new TestConfigSource(domainMap, defaultConfig));
assertConnectionFails(context, "android.com", 443);
assertConnectionSucceeds(context, "google.com", 443);
}
public void testBadPin() throws Exception {
- ArrayList<CertificatesEntryRef> systemSource = new ArrayList<CertificatesEntryRef>();
- systemSource.add(new CertificatesEntryRef(new SystemCertificateSource(), false));
ArraySet<Pin> pins = new ArraySet<Pin>();
pins.add(new Pin("SHA-256", new byte[0]));
- NetworkSecurityConfig domain = new NetworkSecurityConfig(true, false,
- new PinSet(pins, Long.MAX_VALUE),
- systemSource);
+ NetworkSecurityConfig domain = new NetworkSecurityConfig.Builder()
+ .setPinSet(new PinSet(pins, Long.MAX_VALUE))
+ .addCertificatesEntryRef(
+ new CertificatesEntryRef(SystemCertificateSource.getInstance(), false))
+ .build();
ArraySet<Pair<Domain, NetworkSecurityConfig>> domainMap
= new ArraySet<Pair<Domain, NetworkSecurityConfig>>();
domainMap.add(new Pair<Domain, NetworkSecurityConfig>(
@@ -155,13 +149,13 @@
}
public void testGoodPin() throws Exception {
- ArrayList<CertificatesEntryRef> systemSource = new ArrayList<CertificatesEntryRef>();
- systemSource.add(new CertificatesEntryRef(new SystemCertificateSource(), false));
ArraySet<Pin> pins = new ArraySet<Pin>();
pins.add(new Pin("SHA-256", G2_SPKI_SHA256));
- NetworkSecurityConfig domain = new NetworkSecurityConfig(true, false,
- new PinSet(pins, Long.MAX_VALUE),
- systemSource);
+ NetworkSecurityConfig domain = new NetworkSecurityConfig.Builder()
+ .setPinSet(new PinSet(pins, Long.MAX_VALUE))
+ .addCertificatesEntryRef(
+ new CertificatesEntryRef(SystemCertificateSource.getInstance(), false))
+ .build();
ArraySet<Pair<Domain, NetworkSecurityConfig>> domainMap
= new ArraySet<Pair<Domain, NetworkSecurityConfig>>();
domainMap.add(new Pair<Domain, NetworkSecurityConfig>(
@@ -174,13 +168,13 @@
public void testOverridePins() throws Exception {
// Use a bad pin + granting the system CA store the ability to override pins.
- ArrayList<CertificatesEntryRef> systemSource = new ArrayList<CertificatesEntryRef>();
- systemSource.add(new CertificatesEntryRef(new SystemCertificateSource(), true));
ArraySet<Pin> pins = new ArraySet<Pin>();
pins.add(new Pin("SHA-256", new byte[0]));
- NetworkSecurityConfig domain = new NetworkSecurityConfig(true, false,
- new PinSet(pins, Long.MAX_VALUE),
- systemSource);
+ NetworkSecurityConfig domain = new NetworkSecurityConfig.Builder()
+ .setPinSet(new PinSet(pins, Long.MAX_VALUE))
+ .addCertificatesEntryRef(
+ new CertificatesEntryRef(SystemCertificateSource.getInstance(), true))
+ .build();
ArraySet<Pair<Domain, NetworkSecurityConfig>> domainMap
= new ArraySet<Pair<Domain, NetworkSecurityConfig>>();
domainMap.add(new Pair<Domain, NetworkSecurityConfig>(
@@ -220,14 +214,33 @@
assertConnectionFails(context, "developer.android.com", 443);
}
+ 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())
+ .build();
+ assert(!config.getTrustAnchors().isEmpty());
+ }
+
+ public void testConfigBuilderParentLoop() throws Exception {
+ NetworkSecurityConfig.Builder config1 = new NetworkSecurityConfig.Builder();
+ NetworkSecurityConfig.Builder config2 = new NetworkSecurityConfig.Builder();
+ config1.setParent(config2);
+ try {
+ config2.setParent(config1);
+ fail("Loop in NetworkSecurityConfig parents");
+ } catch (IllegalArgumentException expected) {
+ }
+ }
+
public void testWithUrlConnection() throws Exception {
- ArrayList<CertificatesEntryRef> systemSource = new ArrayList<CertificatesEntryRef>();
- systemSource.add(new CertificatesEntryRef(new SystemCertificateSource(), false));
ArraySet<Pin> pins = new ArraySet<Pin>();
pins.add(new Pin("SHA-256", G2_SPKI_SHA256));
- NetworkSecurityConfig domain = new NetworkSecurityConfig(true, false,
- new PinSet(pins, Long.MAX_VALUE),
- systemSource);
+ NetworkSecurityConfig domain = new NetworkSecurityConfig.Builder()
+ .setPinSet(new PinSet(pins, Long.MAX_VALUE))
+ .addCertificatesEntryRef(
+ new CertificatesEntryRef(SystemCertificateSource.getInstance(), false))
+ .build();
ArraySet<Pair<Domain, NetworkSecurityConfig>> domainMap
= new ArraySet<Pair<Domain, NetworkSecurityConfig>>();
domainMap.add(new Pair<Domain, NetworkSecurityConfig>(