Skip to content

Commit 802768d

Browse files
krutonkruton
committed
Add ability to replace chain for PrivateKeyEntry
For the AndroidKeyStore API, allow entries to have their certificate chain replaced without destroying the underlying PrivateKey. Since entries are backed by unexportable private keys, requiring them to be supplied again doesn't make sense and is impossible. Change-Id: I629ce2a625315c8d8020a082892650ac5eba22ae
1 parent 6479ecd commit 802768d

File tree

4 files changed

+242
-24
lines changed

4 files changed

+242
-24
lines changed

‎keystore/java/android/security/AndroidKeyStore.java‎

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616

1717
package android.security;
1818

19+
import org.apache.harmony.xnet.provider.jsse.OpenSSLDSAPrivateKey;
1920
import org.apache.harmony.xnet.provider.jsse.OpenSSLEngine;
21+
import org.apache.harmony.xnet.provider.jsse.OpenSSLRSAPrivateKey;
2022

2123
import android.util.Log;
2224

@@ -193,17 +195,41 @@ public void engineSetKeyEntry(String alias, Key key, char[] password, Certificat
193195

194196
private void setPrivateKeyEntry(String alias, PrivateKey key, Certificate[] chain)
195197
throws KeyStoreException {
196-
// Make sure the PrivateKey format is the one we support.
197-
final String keyFormat = key.getFormat();
198-
if ((keyFormat == null) || (!"PKCS#8".equals(keyFormat))) {
199-
throw new KeyStoreException(
200-
"Only PrivateKeys that can be encoded into PKCS#8 are supported");
198+
byte[] keyBytes = null;
199+
200+
final String pkeyAlias;
201+
if (key instanceof OpenSSLRSAPrivateKey) {
202+
pkeyAlias = ((OpenSSLRSAPrivateKey) key).getPkeyAlias();
203+
} else if (key instanceof OpenSSLDSAPrivateKey) {
204+
pkeyAlias = ((OpenSSLDSAPrivateKey) key).getPkeyAlias();
205+
} else {
206+
pkeyAlias = null;
201207
}
202208

203-
// Make sure we can actually encode the key.
204-
final byte[] keyBytes = key.getEncoded();
205-
if (keyBytes == null) {
206-
throw new KeyStoreException("PrivateKey has no encoding");
209+
final boolean shouldReplacePrivateKey;
210+
if (pkeyAlias != null && pkeyAlias.startsWith(Credentials.USER_PRIVATE_KEY)) {
211+
final String keySubalias = pkeyAlias.substring(Credentials.USER_PRIVATE_KEY.length());
212+
if (!alias.equals(keySubalias)) {
213+
throw new KeyStoreException("Can only replace keys with same alias: " + alias
214+
+ " != " + keySubalias);
215+
}
216+
217+
shouldReplacePrivateKey = false;
218+
} else {
219+
// Make sure the PrivateKey format is the one we support.
220+
final String keyFormat = key.getFormat();
221+
if ((keyFormat == null) || (!"PKCS#8".equals(keyFormat))) {
222+
throw new KeyStoreException(
223+
"Only PrivateKeys that can be encoded into PKCS#8 are supported");
224+
}
225+
226+
// Make sure we can actually encode the key.
227+
keyBytes = key.getEncoded();
228+
if (keyBytes == null) {
229+
throw new KeyStoreException("PrivateKey has no encoding");
230+
}
231+
232+
shouldReplacePrivateKey = true;
207233
}
208234

209235
// Make sure the chain exists since this is a PrivateKey
@@ -273,17 +299,25 @@ private void setPrivateKeyEntry(String alias, PrivateKey key, Certificate[] chai
273299
}
274300

275301
/*
276-
* Make sure we clear out all the types we know about before trying to
302+
* Make sure we clear out all the appropriate types before trying to
277303
* write.
278304
*/
279-
Credentials.deleteAllTypesForAlias(mKeyStore, alias);
305+
if (shouldReplacePrivateKey) {
306+
Credentials.deleteAllTypesForAlias(mKeyStore, alias);
307+
} else {
308+
Credentials.deleteCertificateTypesForAlias(mKeyStore, alias);
309+
}
280310

281-
if (!mKeyStore.importKey(Credentials.USER_PRIVATE_KEY + alias, keyBytes)) {
311+
if (shouldReplacePrivateKey
312+
&& !mKeyStore.importKey(Credentials.USER_PRIVATE_KEY + alias, keyBytes)) {
313+
Credentials.deleteAllTypesForAlias(mKeyStore, alias);
282314
throw new KeyStoreException("Couldn't put private key in keystore");
283315
} else if (!mKeyStore.put(Credentials.USER_CERTIFICATE + alias, userCertBytes)) {
316+
Credentials.deleteAllTypesForAlias(mKeyStore, alias);
284317
throw new KeyStoreException("Couldn't put certificate #1 in keystore");
285318
} else if (chainBytes != null
286319
&& !mKeyStore.put(Credentials.CA_CERTIFICATE + alias, chainBytes)) {
320+
Credentials.deleteAllTypesForAlias(mKeyStore, alias);
287321
throw new KeyStoreException("Couldn't put certificate chain in keystore");
288322
}
289323
}

‎keystore/java/android/security/Credentials.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,20 @@ static boolean deleteAllTypesForAlias(KeyStore keystore, String alias) {
197197
* don't use a conditional here.
198198
*/
199199
return keystore.delKey(Credentials.USER_PRIVATE_KEY + alias)
200-
| keystore.delete(Credentials.USER_CERTIFICATE + alias)
200+
| deleteCertificateTypesForAlias(keystore, alias);
201+
}
202+
203+
/**
204+
* Delete all types (private key, certificate, CA certificate) for a
205+
* particular {@code alias}. All three can exist for any given alias.
206+
* Returns {@code true} if there was at least one of those types.
207+
*/
208+
static boolean deleteCertificateTypesForAlias(KeyStore keystore, String alias) {
209+
/*
210+
* Make sure every certificate type is deleted. There can be two types,
211+
* so don't use a conditional here.
212+
*/
213+
return keystore.delete(Credentials.USER_CERTIFICATE + alias)
201214
| keystore.delete(Credentials.CA_CERTIFICATE + alias);
202215
}
203216
}

‎keystore/tests/Android.mk‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ include $(CLEAR_VARS)
55
LOCAL_MODULE_TAGS := tests
66
LOCAL_CERTIFICATE := platform
77

8-
LOCAL_JAVA_LIBRARIES := android.test.runner
8+
LOCAL_JAVA_LIBRARIES := android.test.runner bouncycastle
99

1010
# Include all test java files.
1111
LOCAL_SRC_FILES := $(call all-java-files-under, src)

‎keystore/tests/src/android/security/AndroidKeyStoreTest.java‎

Lines changed: 181 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,17 @@
1616

1717
package android.security;
1818

19+
import com.android.org.bouncycastle.x509.X509V3CertificateGenerator;
20+
21+
import org.apache.harmony.xnet.provider.jsse.OpenSSLEngine;
22+
1923
import android.test.AndroidTestCase;
2024

2125
import java.io.ByteArrayInputStream;
2226
import java.io.ByteArrayOutputStream;
23-
import java.io.IOException;
2427
import java.io.OutputStream;
28+
import java.math.BigInteger;
29+
import java.security.InvalidKeyException;
2530
import java.security.Key;
2631
import java.security.KeyFactory;
2732
import java.security.KeyStore.Entry;
@@ -30,12 +35,14 @@
3035
import java.security.KeyStoreException;
3136
import java.security.NoSuchAlgorithmException;
3237
import java.security.PrivateKey;
38+
import java.security.PublicKey;
3339
import java.security.cert.Certificate;
34-
import java.security.cert.CertificateException;
3540
import java.security.cert.CertificateFactory;
41+
import java.security.cert.X509Certificate;
3642
import java.security.interfaces.RSAPrivateKey;
3743
import java.security.spec.InvalidKeySpecException;
3844
import java.security.spec.PKCS8EncodedKeySpec;
45+
import java.security.spec.X509EncodedKeySpec;
3946
import java.util.Arrays;
4047
import java.util.Collection;
4148
import java.util.Date;
@@ -44,6 +51,8 @@
4451
import java.util.Iterator;
4552
import java.util.Set;
4653

54+
import javax.security.auth.x500.X500Principal;
55+
4756
public class AndroidKeyStoreTest extends AndroidTestCase {
4857
private android.security.KeyStore mAndroidKeyStore;
4958

@@ -55,6 +64,22 @@ public class AndroidKeyStoreTest extends AndroidTestCase {
5564

5665
private static final String TEST_ALIAS_3 = "test3";
5766

67+
private static final X500Principal TEST_DN_1 = new X500Principal("CN=test1");
68+
69+
private static final X500Principal TEST_DN_2 = new X500Principal("CN=test2");
70+
71+
private static final BigInteger TEST_SERIAL_1 = BigInteger.ONE;
72+
73+
private static final BigInteger TEST_SERIAL_2 = BigInteger.valueOf(2L);
74+
75+
private static final long NOW_MILLIS = System.currentTimeMillis();
76+
77+
/* We have to round this off because X509v3 doesn't store milliseconds. */
78+
private static final Date NOW = new Date(NOW_MILLIS - (NOW_MILLIS % 1000L));
79+
80+
@SuppressWarnings("deprecation")
81+
private static final Date NOW_PLUS_10_YEARS = new Date(NOW.getYear() + 10, 0, 1);
82+
5883
/*
5984
* The keys and certificates below are generated with:
6085
*
@@ -759,31 +784,41 @@ public void testKeyStore_GetEntry_NullParams_Success() throws Exception {
759784
assertPrivateKeyEntryEquals(keyEntry, FAKE_KEY_1, FAKE_USER_1, FAKE_CA_1);
760785
}
761786

787+
@SuppressWarnings("unchecked")
762788
private void assertPrivateKeyEntryEquals(PrivateKeyEntry keyEntry, byte[] key, byte[] cert,
763789
byte[] ca) throws Exception {
764790
KeyFactory keyFact = KeyFactory.getInstance("RSA");
765791
PrivateKey expectedKey = keyFact.generatePrivate(new PKCS8EncodedKeySpec(key));
766792

767-
assertEquals("Returned PrivateKey should be what we inserted", expectedKey,
768-
keyEntry.getPrivateKey());
769-
770793
CertificateFactory certFact = CertificateFactory.getInstance("X.509");
771794
Certificate expectedCert = certFact.generateCertificate(new ByteArrayInputStream(cert));
772795

796+
final Collection<Certificate> expectedChain;
797+
if (ca != null) {
798+
expectedChain = (Collection<Certificate>) certFact
799+
.generateCertificates(new ByteArrayInputStream(ca));
800+
} else {
801+
expectedChain = null;
802+
}
803+
804+
assertPrivateKeyEntryEquals(keyEntry, expectedKey, expectedCert, expectedChain);
805+
}
806+
807+
private void assertPrivateKeyEntryEquals(PrivateKeyEntry keyEntry, PrivateKey expectedKey,
808+
Certificate expectedCert, Collection<Certificate> expectedChain) throws Exception {
809+
assertEquals("Returned PrivateKey should be what we inserted", expectedKey,
810+
keyEntry.getPrivateKey());
811+
773812
assertEquals("Returned Certificate should be what we inserted", expectedCert,
774813
keyEntry.getCertificate());
775814

776815
Certificate[] actualChain = keyEntry.getCertificateChain();
777816

778817
assertEquals("First certificate in chain should be user cert", expectedCert, actualChain[0]);
779818

780-
if (ca == null) {
819+
if (expectedChain == null) {
781820
assertEquals("Certificate chain should not include CAs", 1, actualChain.length);
782821
} else {
783-
@SuppressWarnings("unchecked")
784-
Collection<Certificate> expectedChain = (Collection<Certificate>) certFact
785-
.generateCertificates(new ByteArrayInputStream(ca));
786-
787822
int i = 1;
788823
final Iterator<Certificate> it = expectedChain.iterator();
789824
while (it.hasNext()) {
@@ -1306,6 +1341,142 @@ public void testKeyStore_SetKeyEntry_Replaced_Success() throws Exception {
13061341
}
13071342
}
13081343

1344+
@SuppressWarnings("deprecation")
1345+
private static X509Certificate generateCertificate(android.security.KeyStore keyStore,
1346+
String alias, BigInteger serialNumber, X500Principal subjectDN, Date notBefore,
1347+
Date notAfter) throws Exception {
1348+
final String privateKeyAlias = Credentials.USER_PRIVATE_KEY + alias;
1349+
1350+
final PrivateKey privKey;
1351+
final OpenSSLEngine engine = OpenSSLEngine.getInstance("keystore");
1352+
try {
1353+
privKey = engine.getPrivateKeyById(privateKeyAlias);
1354+
} catch (InvalidKeyException e) {
1355+
throw new RuntimeException("Can't get key", e);
1356+
}
1357+
1358+
final byte[] pubKeyBytes = keyStore.getPubkey(privateKeyAlias);
1359+
1360+
final PublicKey pubKey;
1361+
try {
1362+
final KeyFactory keyFact = KeyFactory.getInstance("RSA");
1363+
pubKey = keyFact.generatePublic(new X509EncodedKeySpec(pubKeyBytes));
1364+
} catch (NoSuchAlgorithmException e) {
1365+
throw new IllegalStateException("Can't instantiate RSA key generator", e);
1366+
} catch (InvalidKeySpecException e) {
1367+
throw new IllegalStateException("keystore returned invalid key encoding", e);
1368+
}
1369+
1370+
final X509V3CertificateGenerator certGen = new X509V3CertificateGenerator();
1371+
certGen.setPublicKey(pubKey);
1372+
certGen.setSerialNumber(serialNumber);
1373+
certGen.setSubjectDN(subjectDN);
1374+
certGen.setIssuerDN(subjectDN);
1375+
certGen.setNotBefore(notBefore);
1376+
certGen.setNotAfter(notAfter);
1377+
certGen.setSignatureAlgorithm("sha1WithRSA");
1378+
1379+
final X509Certificate cert = certGen.generate(privKey);
1380+
1381+
return cert;
1382+
}
1383+
1384+
public void testKeyStore_SetKeyEntry_ReplacedChain_Success() throws Exception {
1385+
mKeyStore.load(null, null);
1386+
1387+
// Create key #1
1388+
{
1389+
final String privateKeyAlias = Credentials.USER_PRIVATE_KEY + TEST_ALIAS_1;
1390+
assertTrue(mAndroidKeyStore.generate(privateKeyAlias));
1391+
1392+
Key key = mKeyStore.getKey(TEST_ALIAS_1, null);
1393+
1394+
assertTrue(key instanceof PrivateKey);
1395+
1396+
PrivateKey expectedKey = (PrivateKey) key;
1397+
1398+
X509Certificate expectedCert = generateCertificate(mAndroidKeyStore, TEST_ALIAS_1,
1399+
TEST_SERIAL_1, TEST_DN_1, NOW, NOW_PLUS_10_YEARS);
1400+
1401+
assertTrue(mAndroidKeyStore.put(Credentials.USER_CERTIFICATE + TEST_ALIAS_1,
1402+
expectedCert.getEncoded()));
1403+
1404+
Entry entry = mKeyStore.getEntry(TEST_ALIAS_1, null);
1405+
1406+
assertTrue(entry instanceof PrivateKeyEntry);
1407+
1408+
PrivateKeyEntry keyEntry = (PrivateKeyEntry) entry;
1409+
1410+
assertPrivateKeyEntryEquals(keyEntry, expectedKey, expectedCert, null);
1411+
}
1412+
1413+
// Replace key #1 with new chain
1414+
{
1415+
Key key = mKeyStore.getKey(TEST_ALIAS_1, null);
1416+
1417+
assertTrue(key instanceof PrivateKey);
1418+
1419+
PrivateKey expectedKey = (PrivateKey) key;
1420+
1421+
X509Certificate expectedCert = generateCertificate(mAndroidKeyStore, TEST_ALIAS_1,
1422+
TEST_SERIAL_2, TEST_DN_2, NOW, NOW_PLUS_10_YEARS);
1423+
1424+
mKeyStore.setKeyEntry(TEST_ALIAS_1, expectedKey, null,
1425+
new Certificate[] { expectedCert });
1426+
1427+
Entry entry = mKeyStore.getEntry(TEST_ALIAS_1, null);
1428+
1429+
assertTrue(entry instanceof PrivateKeyEntry);
1430+
1431+
PrivateKeyEntry keyEntry = (PrivateKeyEntry) entry;
1432+
1433+
assertPrivateKeyEntryEquals(keyEntry, expectedKey, expectedCert, null);
1434+
}
1435+
}
1436+
1437+
public void testKeyStore_SetKeyEntry_ReplacedChain_DifferentPrivateKey_Failure()
1438+
throws Exception {
1439+
mKeyStore.load(null, null);
1440+
1441+
// Create key #1
1442+
{
1443+
final String privateKeyAlias = Credentials.USER_PRIVATE_KEY + TEST_ALIAS_1;
1444+
assertTrue(mAndroidKeyStore.generate(privateKeyAlias));
1445+
1446+
X509Certificate cert = generateCertificate(mAndroidKeyStore, TEST_ALIAS_1,
1447+
TEST_SERIAL_1, TEST_DN_1, NOW, NOW_PLUS_10_YEARS);
1448+
1449+
assertTrue(mAndroidKeyStore.put(Credentials.USER_CERTIFICATE + TEST_ALIAS_1,
1450+
cert.getEncoded()));
1451+
}
1452+
1453+
// Create key #2
1454+
{
1455+
final String privateKeyAlias = Credentials.USER_PRIVATE_KEY + TEST_ALIAS_2;
1456+
assertTrue(mAndroidKeyStore.generate(privateKeyAlias));
1457+
1458+
X509Certificate cert = generateCertificate(mAndroidKeyStore, TEST_ALIAS_2,
1459+
TEST_SERIAL_2, TEST_DN_2, NOW, NOW_PLUS_10_YEARS);
1460+
1461+
assertTrue(mAndroidKeyStore.put(Credentials.USER_CERTIFICATE + TEST_ALIAS_2,
1462+
cert.getEncoded()));
1463+
}
1464+
1465+
// Replace key #1 with key #2
1466+
{
1467+
Key key1 = mKeyStore.getKey(TEST_ALIAS_2, null);
1468+
1469+
X509Certificate cert = generateCertificate(mAndroidKeyStore, TEST_ALIAS_2,
1470+
TEST_SERIAL_2, TEST_DN_2, NOW, NOW_PLUS_10_YEARS);
1471+
1472+
try {
1473+
mKeyStore.setKeyEntry(TEST_ALIAS_1, key1, null, new Certificate[] { cert });
1474+
fail("Should not allow setting of KeyEntry with wrong PrivaetKey");
1475+
} catch (KeyStoreException success) {
1476+
}
1477+
}
1478+
}
1479+
13091480
public void testKeyStore_Size_Success() throws Exception {
13101481
mKeyStore.load(null, null);
13111482

0 commit comments

Comments
 (0)