From 291ddaef788741fe724fec71760b90cbb6edaa2f Mon Sep 17 00:00:00 2001 From: Paul Stewart Date: Tue, 24 Jan 2017 19:36:05 -0800 Subject: [PATCH 1/3] Add a client chain to WifiEnterpriseConfig Add a list of supporting certificates to be presented in the process of presenting client credentials. Cherry-pick of e3511767169357a1409119b5666c62d50e005583 Bug: 34688653 Test: Compile, unit tests Change-Id: I6afd8baf67312e8ddaaeefd26f30dacc51aa33bb --- api/current.txt | 2 + api/system-current.txt | 2 + api/test-current.txt | 2 + .../net/wifi/WifiEnterpriseConfig.java | 90 ++++++++++++++++--- .../net/wifi/WifiEnterpriseConfigTest.java | 36 ++++++++ 5 files changed, 121 insertions(+), 11 deletions(-) diff --git a/api/current.txt b/api/current.txt index 6e52af61d24d5..1d2b2a96477a0 100644 --- a/api/current.txt +++ b/api/current.txt @@ -24696,6 +24696,7 @@ package android.net.wifi { method public java.security.cert.X509Certificate getCaCertificate(); method public java.security.cert.X509Certificate[] getCaCertificates(); method public java.security.cert.X509Certificate getClientCertificate(); + method public java.security.cert.X509Certificate[] getClientCertificateChain(); method public java.lang.String getDomainSuffixMatch(); method public int getEapMethod(); method public java.lang.String getIdentity(); @@ -24709,6 +24710,7 @@ package android.net.wifi { method public void setCaCertificate(java.security.cert.X509Certificate); method public void setCaCertificates(java.security.cert.X509Certificate[]); method public void setClientKeyEntry(java.security.PrivateKey, java.security.cert.X509Certificate); + method public void setClientKeyEntryWithCertificateChain(java.security.PrivateKey, java.security.cert.X509Certificate[]); method public void setDomainSuffixMatch(java.lang.String); method public void setEapMethod(int); method public void setIdentity(java.lang.String); diff --git a/api/system-current.txt b/api/system-current.txt index 17ec7a1a34fe2..e3d2b9411c31d 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -27068,6 +27068,7 @@ package android.net.wifi { method public java.security.cert.X509Certificate getCaCertificate(); method public java.security.cert.X509Certificate[] getCaCertificates(); method public java.security.cert.X509Certificate getClientCertificate(); + method public java.security.cert.X509Certificate[] getClientCertificateChain(); method public java.lang.String getDomainSuffixMatch(); method public int getEapMethod(); method public java.lang.String getIdentity(); @@ -27081,6 +27082,7 @@ package android.net.wifi { method public void setCaCertificate(java.security.cert.X509Certificate); method public void setCaCertificates(java.security.cert.X509Certificate[]); method public void setClientKeyEntry(java.security.PrivateKey, java.security.cert.X509Certificate); + method public void setClientKeyEntryWithCertificateChain(java.security.PrivateKey, java.security.cert.X509Certificate[]); method public void setDomainSuffixMatch(java.lang.String); method public void setEapMethod(int); method public void setIdentity(java.lang.String); diff --git a/api/test-current.txt b/api/test-current.txt index dc3ea996a4514..273088189d65e 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -24769,6 +24769,7 @@ package android.net.wifi { method public java.security.cert.X509Certificate getCaCertificate(); method public java.security.cert.X509Certificate[] getCaCertificates(); method public java.security.cert.X509Certificate getClientCertificate(); + method public java.security.cert.X509Certificate[] getClientCertificateChain(); method public java.lang.String getDomainSuffixMatch(); method public int getEapMethod(); method public java.lang.String getIdentity(); @@ -24782,6 +24783,7 @@ package android.net.wifi { method public void setCaCertificate(java.security.cert.X509Certificate); method public void setCaCertificates(java.security.cert.X509Certificate[]); method public void setClientKeyEntry(java.security.PrivateKey, java.security.cert.X509Certificate); + method public void setClientKeyEntryWithCertificateChain(java.security.PrivateKey, java.security.cert.X509Certificate[]); method public void setDomainSuffixMatch(java.lang.String); method public void setEapMethod(int); method public void setIdentity(java.lang.String); diff --git a/wifi/java/android/net/wifi/WifiEnterpriseConfig.java b/wifi/java/android/net/wifi/WifiEnterpriseConfig.java index e410a9cf917eb..5028d47481b05 100644 --- a/wifi/java/android/net/wifi/WifiEnterpriseConfig.java +++ b/wifi/java/android/net/wifi/WifiEnterpriseConfig.java @@ -142,7 +142,7 @@ public class WifiEnterpriseConfig implements Parcelable { private HashMap mFields = new HashMap(); private X509Certificate[] mCaCerts; private PrivateKey mClientPrivateKey; - private X509Certificate mClientCertificate; + private X509Certificate[] mClientCertificateChain; private int mEapMethod = Eap.NONE; private int mPhase2Method = Phase2.NONE; @@ -161,9 +161,19 @@ public class WifiEnterpriseConfig implements Parcelable { for (String key : source.mFields.keySet()) { mFields.put(key, source.mFields.get(key)); } - mCaCerts = source.mCaCerts; + if (source.mCaCerts != null) { + mCaCerts = Arrays.copyOf(source.mCaCerts, source.mCaCerts.length); + } else { + mCaCerts = null; + } mClientPrivateKey = source.mClientPrivateKey; - mClientCertificate = source.mClientCertificate; + if (source.mClientCertificateChain != null) { + mClientCertificateChain = Arrays.copyOf( + source.mClientCertificateChain, + source.mClientCertificateChain.length); + } else { + mClientCertificateChain = null; + } mEapMethod = source.mEapMethod; mPhase2Method = source.mPhase2Method; } @@ -185,7 +195,7 @@ public class WifiEnterpriseConfig implements Parcelable { dest.writeInt(mPhase2Method); ParcelUtil.writeCertificates(dest, mCaCerts); ParcelUtil.writePrivateKey(dest, mClientPrivateKey); - ParcelUtil.writeCertificate(dest, mClientCertificate); + ParcelUtil.writeCertificates(dest, mClientCertificateChain); } public static final Creator CREATOR = @@ -204,7 +214,7 @@ public class WifiEnterpriseConfig implements Parcelable { enterpriseConfig.mPhase2Method = in.readInt(); enterpriseConfig.mCaCerts = ParcelUtil.readCertificates(in); enterpriseConfig.mClientPrivateKey = ParcelUtil.readPrivateKey(in); - enterpriseConfig.mClientCertificate = ParcelUtil.readCertificate(in); + enterpriseConfig.mClientCertificateChain = ParcelUtil.readCertificates(in); return enterpriseConfig; } @@ -742,10 +752,51 @@ public class WifiEnterpriseConfig implements Parcelable { * @throws IllegalArgumentException for an invalid key or certificate. */ public void setClientKeyEntry(PrivateKey privateKey, X509Certificate clientCertificate) { - if (clientCertificate != null) { - if (clientCertificate.getBasicConstraints() != -1) { - throw new IllegalArgumentException("Cannot be a CA certificate"); + setClientKeyEntryWithCertificateChain(privateKey, + new X509Certificate[] {clientCertificate}); + } + + /** + * Specify a private key and client certificate chain for client authorization. + * + *

A default name is automatically assigned to the key entry and used + * with this configuration. The framework takes care of installing the + * key entry when the config is saved and removing the key entry when + * the config is removed. + + * @param privateKey + * @param clientCertificateChain + * @throws IllegalArgumentException for an invalid key or certificate. + */ + public void setClientKeyEntryWithCertificateChain(PrivateKey privateKey, + X509Certificate[] clientCertificateChain) { + X509Certificate[] newCerts = null; + if (clientCertificateChain != null && clientCertificateChain.length > 0) { + // We validate that this is a well formed chain that starts + // with an end-certificate and is followed by CA certificates. + // We don't validate that each following certificate verifies + // the previous. https://en.wikipedia.org/wiki/Chain_of_trust + // + // Basic constraints is an X.509 extension type that defines + // whether a given certificate is allowed to sign additional + // certificates and what path length restrictions may exist. + // We use this to judge whether the certificate is an end + // certificate or a CA certificate. + // https://cryptography.io/en/latest/x509/reference/ + if (clientCertificateChain[0].getBasicConstraints() != -1) { + throw new IllegalArgumentException( + "First certificate in the chain must be a client end certificate"); } + + for (int i = 1; i < clientCertificateChain.length; i++) { + if (clientCertificateChain[i].getBasicConstraints() == -1) { + throw new IllegalArgumentException( + "All certificates following the first must be CA certificates"); + } + } + newCerts = Arrays.copyOf(clientCertificateChain, + clientCertificateChain.length); + if (privateKey == null) { throw new IllegalArgumentException("Client cert without a private key"); } @@ -755,7 +806,7 @@ public class WifiEnterpriseConfig implements Parcelable { } mClientPrivateKey = privateKey; - mClientCertificate = clientCertificate; + mClientCertificateChain = newCerts; } /** @@ -764,7 +815,24 @@ public class WifiEnterpriseConfig implements Parcelable { * @return X.509 client certificate */ public X509Certificate getClientCertificate() { - return mClientCertificate; + if (mClientCertificateChain != null && mClientCertificateChain.length > 0) { + return mClientCertificateChain[0]; + } else { + return null; + } + } + + /** + * Get the complete client certificate chain + * + * @return X.509 client certificates + */ + @Nullable public X509Certificate[] getClientCertificateChain() { + if (mClientCertificateChain != null && mClientCertificateChain.length > 0) { + return mClientCertificateChain; + } else { + return null; + } } /** @@ -772,7 +840,7 @@ public class WifiEnterpriseConfig implements Parcelable { */ public void resetClientKeyEntry() { mClientPrivateKey = null; - mClientCertificate = null; + mClientCertificateChain = null; } /** diff --git a/wifi/tests/src/android/net/wifi/WifiEnterpriseConfigTest.java b/wifi/tests/src/android/net/wifi/WifiEnterpriseConfigTest.java index 0e503d5e71390..5a67a7e5f2851 100644 --- a/wifi/tests/src/android/net/wifi/WifiEnterpriseConfigTest.java +++ b/wifi/tests/src/android/net/wifi/WifiEnterpriseConfigTest.java @@ -86,6 +86,42 @@ public class WifiEnterpriseConfigTest { assertTrue(result[0] == cert0 && result[1] == cert1); } + @Test + public void testSetClientCertificateChain() { + PrivateKey clientKey = FakeKeys.RSA_KEY1; + X509Certificate cert0 = FakeKeys.CLIENT_CERT; + X509Certificate cert1 = FakeKeys.CA_CERT1; + X509Certificate[] clientChain = new X509Certificate[] {cert0, cert1}; + mEnterpriseConfig.setClientKeyEntryWithCertificateChain(clientKey, clientChain); + X509Certificate[] result = mEnterpriseConfig.getClientCertificateChain(); + assertEquals(result.length, 2); + assertTrue(result[0] == cert0 && result[1] == cert1); + assertTrue(mEnterpriseConfig.getClientCertificate() == cert0); + } + + private boolean isClientCertificateChainInvalid(X509Certificate[] clientChain) { + boolean exceptionThrown = false; + try { + PrivateKey clientKey = FakeKeys.RSA_KEY1; + mEnterpriseConfig.setClientKeyEntryWithCertificateChain(clientKey, clientChain); + } catch (IllegalArgumentException e) { + exceptionThrown = true; + } + return exceptionThrown; + } + + @Test + public void testSetInvalidClientCertificateChain() { + X509Certificate clientCert = FakeKeys.CLIENT_CERT; + X509Certificate caCert = FakeKeys.CA_CERT1; + assertTrue("Invalid client certificate", + isClientCertificateChainInvalid(new X509Certificate[] {caCert, caCert})); + assertTrue("Invalid CA certificate", + isClientCertificateChainInvalid(new X509Certificate[] {clientCert, clientCert})); + assertTrue("Both certificates invalid", + isClientCertificateChainInvalid(new X509Certificate[] {caCert, clientCert})); + } + @Test public void testSaveSingleCaCertificateAlias() { final String alias = "single_alias 0"; From 1ca57a1d10e56c376b94a4f4992b037f1428d853 Mon Sep 17 00:00:00 2001 From: Paul Stewart Date: Fri, 27 Jan 2017 09:37:17 -0800 Subject: [PATCH 2/3] Account for null client certificate If a null certificate is passed to setClientKeyEntry() we should not pass a non-null array with a single null element to the setClientKeyEntryWithCertificateChain helper method. Instead we should pass a null array. Cherry-pick of 410a3498ac28dccf69212d94a533040893c7ce0c Bug: 34765004 Test: cts-tradefed run cts -d --module CtsNetTestCases --test android.net.wifi.cts.WifiEnterpriseConfigTest Change-Id: I02793b4b29bc7325f98833c58bf652ba68353827 --- wifi/java/android/net/wifi/WifiEnterpriseConfig.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/wifi/java/android/net/wifi/WifiEnterpriseConfig.java b/wifi/java/android/net/wifi/WifiEnterpriseConfig.java index 5028d47481b05..0bfb95581e602 100644 --- a/wifi/java/android/net/wifi/WifiEnterpriseConfig.java +++ b/wifi/java/android/net/wifi/WifiEnterpriseConfig.java @@ -752,8 +752,11 @@ public class WifiEnterpriseConfig implements Parcelable { * @throws IllegalArgumentException for an invalid key or certificate. */ public void setClientKeyEntry(PrivateKey privateKey, X509Certificate clientCertificate) { - setClientKeyEntryWithCertificateChain(privateKey, - new X509Certificate[] {clientCertificate}); + X509Certificate[] clientCertificates = null; + if (clientCertificate != null) { + clientCertificates = new X509Certificate[] {clientCertificate}; + } + setClientKeyEntryWithCertificateChain(privateKey, clientCertificates); } /** From 88b3c589ad4c8d48bef6c252628dc59a9addd353 Mon Sep 17 00:00:00 2001 From: Paul Stewart Date: Fri, 27 Jan 2017 12:03:47 -0800 Subject: [PATCH 3/3] Test passing null cert/keys to WifiEnterpriseConfig Ensure that null certificates and keys don't crash. Bug: 34765004 Test: This is a test Change-Id: I439b4f985c1b88ad4a9b58ee6f4eb4f90bd81246 --- .../src/android/net/wifi/WifiEnterpriseConfigTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/wifi/tests/src/android/net/wifi/WifiEnterpriseConfigTest.java b/wifi/tests/src/android/net/wifi/WifiEnterpriseConfigTest.java index 5a67a7e5f2851..fa546a565c864 100644 --- a/wifi/tests/src/android/net/wifi/WifiEnterpriseConfigTest.java +++ b/wifi/tests/src/android/net/wifi/WifiEnterpriseConfigTest.java @@ -86,6 +86,16 @@ public class WifiEnterpriseConfigTest { assertTrue(result[0] == cert0 && result[1] == cert1); } + @Test + public void testSetClientKeyEntryWithNull() { + mEnterpriseConfig.setClientKeyEntry(null, null); + assertEquals(null, mEnterpriseConfig.getClientCertificateChain()); + assertEquals(null, mEnterpriseConfig.getClientCertificate()); + mEnterpriseConfig.setClientKeyEntryWithCertificateChain(null, null); + assertEquals(null, mEnterpriseConfig.getClientCertificateChain()); + assertEquals(null, mEnterpriseConfig.getClientCertificate()); + } + @Test public void testSetClientCertificateChain() { PrivateKey clientKey = FakeKeys.RSA_KEY1;