Skip to content

Commit 618641f

Browse files
krutonAndroid (Google) Code Review
kruton
authored and
Android (Google) Code Review
committed
Merge "Add ability to replace chain for PrivateKeyEntry" into jb-mr1-dev
2 parents 12850a9 + 802768d commit 618641f

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)