Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 80a73f59 authored by Chad Brubaker's avatar Chad Brubaker
Browse files

Use a builder for NetworkSecurityConfig 

The builder supports all the standard builder set* methods as well as
setting a parent builder to use when values are not set (recursively).
This allows us to have a level of inheretence in configurations without
complicating the lookup and trust checking logic by doing inheretence
when building the configs.

Change-Id: I054af83451e52761227479eadf9cb9803437505f
parent 6bc1e396

core/java/android/security/net/config/NetworkSecurityConfig.java

+157 −1
Original line number Diff line number Diff line
@@ -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 @@ import javax.net.ssl.X509TrustManager; 
 * @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 @@ public final class NetworkSecurityConfig { 
    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 @@ public final class NetworkSecurityConfig { 
            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);

        }

    }

}

core/java/android/security/net/config/PinSet.java

+3 −0
Original line number Diff line number Diff line
@@ -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;

core/java/android/security/net/config/SystemCertificateSource.java

+15 −10
Original line number Diff line number Diff line
@@ -36,18 +36,23 @@ import libcore.io.IoUtils; 
 * @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 @@ public class SystemCertificateSource implements CertificateSource { 
                    IoUtils.closeQuietly(is);

                }

            }

            sSystemCerts = systemCerts;

            return sSystemCerts;

            mSystemCerts = systemCerts;

            return mSystemCerts;

        }

    }



    public void onCertificateStorageChange() {

        synchronized (sLock) {

            sSystemCerts = null;

        synchronized (mLock) {

            mSystemCerts = null;

        }

    }

}

core/java/android/security/net/config/UserCertificateSource.java

+15 −10
Original line number Diff line number Diff line
@@ -36,18 +36,23 @@ import libcore.io.IoUtils; 
 * @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 @@ public class UserCertificateSource implements CertificateSource { 
                    IoUtils.closeQuietly(is);

                }

            }

            sUserCerts = userCerts;

            return sUserCerts;

            mUserCerts = userCerts;

            return mUserCerts;

        }

    }



    public void onCertificateStorageChange() {

        synchronized (sLock) {

            sUserCerts = null;

        synchronized (mLock) {

            mUserCerts = null;

        }

    }

}

tests/NetworkSecurityConfigTest/src/android/security/net/config/NetworkSecurityConfigTests.java

+45 −32
Original line number Diff line number Diff line
@@ -100,16 +100,14 @@ public class NetworkSecurityConfigTests extends ActivityUnitTestCase<Activity> { 
     * 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 @@ public class NetworkSecurityConfigTests extends ActivityUnitTestCase<Activity> { 
                = 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 class NetworkSecurityConfigTests extends ActivityUnitTestCase<Activity> { 
    }



    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 class NetworkSecurityConfigTests extends ActivityUnitTestCase<Activity> { 


    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 @@ public class NetworkSecurityConfigTests extends ActivityUnitTestCase<Activity> { 
        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>(