[Cronet] Public key pinning for Java API

components/cronet/android/test/javatests/src/org/chromium/net/HpkpTest.java

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.net;
+
+import android.test.suitebuilder.annotation.SmallTest;
+
+import org.chromium.base.test.util.Feature;
+import org.chromium.net.test.util.CertTestUtil;
+
+import java.io.ByteArrayInputStream;
+import java.security.cert.CertificateFactory;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+
+/**
+ * Public-Key-Pinning tests of Cronet Java API.

[xunjieli, 2015/11/06 15:47:54]

Should we say **dynamic** public key pinning here? to separate it from the case
of static public key pinning which we do not do.
Also in the CL description.

[kapishnikov, 2015/11/06 16:18:13]

I would like to clarify the terminology here. I was under impression that we
call the dynamic pins the ones that are added from HTTP response headers during
runtime. The static ones are the ones that are added during configuration time
and cannot be changed. I also learned that we have other static pins that are
hardcoded. So, is it correctly to call the pins that we supply during
configuration dynamic?

[xunjieli, 2015/11/06 16:43:50]

I got the impression that we do not want to do static key pinning, so I assumed
what we have here is considered dynamic key pinning. Don't really know much
about the terminology. On closer look, these new pins do look static to me. I
haven't followed the discussion closely. I guess the security folks have already
given green light? TransportSecurityState::AddHPKP's comment says that it is
used for tests and net-internals, we probably need to change the comment there
to reflect that the method is also used in production code. 

[davidben, 2015/11/06 16:53:13]

"static" vs "dynamic" is whatever we want to call it. This is different from
both use cases in //net today (which is why I think the right answer is to tidy
up TransportSecurityState a little because calling AddHPKP is somewhat a hack.
It's a hack that works for now, but nonetheless it's not what that API is used
for. See also persistence discussion).

Today, Chrome has two kinds of pins:

1. "static" or "preloaded" pins. These are hardcoded into Chrome. These *cannot*
be enabled in Cronet.

2. "dynamic" pins. These are discovered by HPKP headers and persisted when
persistence is hooked up. That's what AddHPKP hooks into.

This is adding an API to configure pins in code. These should NOT be persisted.
What words you want to call them, I dunno. Probably the words "static" and
"dynamic" are not appropriate at this point, given that you're adding a third
kind of thing.

+ */
+public class HpkpTest extends CronetTestBase {
+    private static final String CERT_USED = "quic_test.example.com.crt";
+    private static final String[] CERTS_USED = {CERT_USED};
+
+    private CronetTestFramework mTestFramework;
+    private CronetEngine.Builder mBuilder;
+    private TestUrlRequestCallback mListener;
+    private String mServerUrl; // https://test.example.com:6121
+    private String mServerHost; // test.example.com
+    private String mDomain; // example.com
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        // Start QUIC Test Server
+        System.loadLibrary("cronet_tests");
+        QuicTestServer.startQuicTestServer(getContext());
+        mServerUrl = QuicTestServer.getServerURL();
+        mServerHost = QuicTestServer.getServerHost();
+        mDomain = mServerHost.substring(mServerHost.indexOf('.') + 1, mServerHost.length());
+
+        // Set common CronetEngine parameters
+        mBuilder = new CronetEngine.Builder(getContext());
+        mBuilder.enableQUIC(true);
+        mBuilder.addQuicHint(QuicTestServer.getServerHost(), QuicTestServer.getServerPort(),
+                QuicTestServer.getServerPort());
+        mBuilder.setStoragePath(CronetTestFramework.getTestStorage(getContext()));
+        mBuilder.enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISK_NO_HTTP, 1000 * 1024);
+        mBuilder.setMockCertVerifierForTesting(MockCertVerifier.createMockCertVerifier(CERTS_USED));
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        QuicTestServer.shutdownQuicTestServer();
+        super.tearDown();
+    }
+
+    /**
+     * Tests the case when the pin hash does not match. The client is expected to
+     * receive the error response.
+     *
+     * @throws Exception
+     */
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testErrorCodeIfPinDoesNotMatch() throws Exception {
+        byte[] nonMatchingHash = generateSomeSha256();
+        addPkpSha256(mServerHost, nonMatchingHash, false);
+        startCronetFramework();
+        registerHostResolver();
+        sendRequestAndWaitForResult();
+
+        assertErrorResponse();
+    }
+
+    /**
+     * Tests the case when the pin hash matches. The client is expected to
+     * receive the successful response with the response code 200.
+     *
+     * @throws Exception
+     */
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testSuccessIfPinMatches() throws Exception {
+        // Get PKP hash of the real certificate
+        X509Certificate cert = readCertFromFileInPemFormat(CERT_USED);
+        byte[] matchingHash = CertTestUtil.getPublicKeySha256(cert);
+
+        addPkpSha256(mServerHost, matchingHash, false);
+        startCronetFramework();
+        registerHostResolver();
+        sendRequestAndWaitForResult();
+
+        assertSuccessfulResponse();
+    }
+
+    /**
+     * Tests the case when the pin hash does not match and the client accesses the subdomain of
+     * the configured HPKP host with includeSubdomains flag set to true. The client is
+     * expected to receive the error response.
+     *
+     * @throws Exception
+     */
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testIncludeSubdomainsFlagEqualTrue() throws Exception {
+        addPkpSha256(mDomain, generateSomeSha256(), true);
+        startCronetFramework();
+        registerHostResolver();
+        sendRequestAndWaitForResult();
+
+        assertErrorResponse();
+    }
+
+    /**
+     * Tests the case when the pin hash does not match and the client accesses the subdomain of
+     * the configured HPKP host with includeSubdomains flag set to false. The client is expected to
+     * receive the successful response with the response code 200.
+     *
+     * @throws Exception
+     */
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testIncludeSubdomainsFlagEqualFalse() throws Exception {
+        addPkpSha256(mDomain, generateSomeSha256(), false);
+        startCronetFramework();
+        registerHostResolver();
+        sendRequestAndWaitForResult();
+
+        assertSuccessfulResponse();
+    }
+
+    /**
+     * Tests the case when the mismatching pin is set for some host that is different from the one
+     * the client wants to access. In that case the other host pinning policy should not be applied
+     * and the client is expected to receive the successful response with the response code 200.
+     *
+     * @throws Exception
+     */
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testSuccessIfNoPinSpecified() throws Exception {
+        addPkpSha256("somerandomhost.com", generateSomeSha256(), true);
+        startCronetFramework();
+        registerHostResolver();
+        sendRequestAndWaitForResult();
+
+        assertSuccessfulResponse();
+    }
+
+    /**
+     * Asserts that the response from the server contains an HPKP error.
+     * TODO(kapishnikov) currently QUIC returns ERR_QUIC_PROTOCOL_ERROR instead of expected

[xunjieli, 2015/11/06 15:47:54]

nit: Probably only one TODO is needed. There should be a colon after the closing
paren.

[kapishnikov, 2015/11/06 16:18:13]

Done.

+     * TODO(kapishnikov) ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN error code when the pin doesn't match.
+     * TODO(kapishnikov) This method should be changed when the bug is resolved.
+     * TODO(kapishnikov) See http://crbug.com/548378
+     */
+    private void assertErrorResponse() {
+        assertNotNull("Expected an error", mListener.mError);
+        int errorCode = mListener.mError.netError();
+        boolean correctErrorCode = errorCode == NetError.ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN
+                || errorCode == NetError.ERR_QUIC_PROTOCOL_ERROR;
+        assertTrue(String.format("Incorrect error code. Expected %s or %s but received %s",
+                           NetError.ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN,
+                           NetError.ERR_QUIC_PROTOCOL_ERROR, errorCode),
+                correctErrorCode);
+    }
+
+    /**
+     * Asserts a successful response with response code 200.
+     */
+    private void assertSuccessfulResponse() {
+        if (mListener.mError != null) {
+            fail("Did not expect an error but got error code " + mListener.mError.mNetError);
+        }
+        assertNotNull("Expected non-null response from the server", mListener.mResponseInfo);
+        assertEquals(200, mListener.mResponseInfo.getHttpStatusCode());
+    }
+
+    private void startCronetFramework() {
+        mTestFramework = startCronetTestFrameworkWithUrlAndCronetEngineBuilder(null, mBuilder);
+    }
+
+    private void registerHostResolver() {
+        long urlRequestContextAdapter = ((CronetUrlRequestContext) mTestFramework.mCronetEngine)
+                                                .getUrlRequestContextAdapter();
+        NativeTestServer.registerHostResolverProc(urlRequestContextAdapter, false);
+    }
+
+    private byte[] generateSomeSha256() {
+        byte[] sha256 = new byte[32];
+        Arrays.fill(sha256, (byte) 58);
+        return sha256;
+    }
+
+    private void addPkpSha256(String host, byte[] pinHashValue, boolean includeSubdomain) {
+        Collection<byte[]> hashes = new ArrayList<>();
+        hashes.add(pinHashValue);
+        mBuilder.addPublicKeyPins(host, hashes, includeSubdomain);
+    }
+
+    private void sendRequestAndWaitForResult() {
+        mListener = new TestUrlRequestCallback();
+
+        String quicURL = mServerUrl + "/simple.txt";
+        UrlRequest.Builder requestBuilder = new UrlRequest.Builder(
+                quicURL, mListener, mListener.getExecutor(), mTestFramework.mCronetEngine);
+        requestBuilder.build().start();
+        mListener.blockForDone();
+    }
+
+    private X509Certificate readCertFromFileInPemFormat(String certFileName) throws Exception {
+        byte[] certDer = CertTestUtil.pemToDer(CertTestUtil.CERTS_DIRECTORY + certFileName);
+        CertificateFactory certFactory = CertificateFactory.getInstance("X.509");
+        return (X509Certificate) certFactory.generateCertificate(new ByteArrayInputStream(certDer));
+    }
+}