Merge pull request #95 from google/acz

Move closer to validating whole android cert chain
google · Feb 9, 2016 · abfa519 · abfa519
2 parents a91f044 + d2c220f
commit abfa519
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 17 deletions.
diff --git a/...e/java/src/com/google/u2f/server/impl/attestation/android/AndroidKeyStoreAttestation.java b/...e/java/src/com/google/u2f/server/impl/attestation/android/AndroidKeyStoreAttestation.java
@@ -49,7 +49,12 @@ private AndroidKeyStoreAttestation(Integer keymasterVersion, byte[] attestationC
   }
 
   /**
-   * Parses the key description extension.  Expected format is:
+   * Parses the key description extension.  Note that this method only parses the description
+   * extension in the leaf cert.  It *does not* validate the certificate (or any chain).
+   *
+   * TODO(aczeskis): Add chain validation and remove/clarify the above comment.
+   *
+   * Expected format of the description extension is:
    *   KeyDescription ::= SEQUENCE {
    *       keymasterVersion     INTEGER,
    *       attestationChallenge OCTET_STRING,

diff --git a/u2f-ref-code/java/tests/com/google/u2f/TestUtils.java b/u2f-ref-code/java/tests/com/google/u2f/TestUtils.java
@@ -6,6 +6,17 @@
 
 package com.google.u2f;
 
+import org.apache.commons.codec.DecoderException;
+import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.codec.binary.Hex;
+import org.bouncycastle.asn1.sec.SECNamedCurves;
+import org.bouncycastle.asn1.x9.X9ECParameters;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.jce.spec.ECParameterSpec;
+import org.bouncycastle.jce.spec.ECPrivateKeySpec;
+import org.bouncycastle.jce.spec.ECPublicKeySpec;
+import org.bouncycastle.math.ec.ECPoint;
+
 import java.io.ByteArrayInputStream;
 import java.math.BigInteger;
 import java.security.KeyFactory;
@@ -14,21 +25,12 @@
 import java.security.PrivateKey;
 import java.security.PublicKey;
 import java.security.Security;
+import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.security.spec.InvalidKeySpecException;
-
-import org.apache.commons.codec.DecoderException;
-import org.apache.commons.codec.binary.Hex;
-import org.apache.commons.codec.binary.Base64;
-import org.bouncycastle.asn1.sec.SECNamedCurves;
-import org.bouncycastle.asn1.x9.X9ECParameters;
-import org.bouncycastle.jce.provider.BouncyCastleProvider;
-import org.bouncycastle.jce.spec.ECParameterSpec;
-import org.bouncycastle.jce.spec.ECPrivateKeySpec;
-import org.bouncycastle.jce.spec.ECPublicKeySpec;
-import org.bouncycastle.math.ec.ECPoint;
+import java.util.Collection;
 
 public class TestUtils {
 
@@ -65,6 +67,17 @@ public static X509Certificate parseCertificateBase64(String encodedDerCertificat
     return parseCertificate(parseBase64(encodedDerCertificate));
   }
 
+  public static X509Certificate[] parseCertificateChainBase64(String encodedDerCertificates) {
+    try {
+      Collection<? extends Certificate> certCollection =
+          CertificateFactory.getInstance("X.509").generateCertificates(
+              new ByteArrayInputStream(parseBase64(encodedDerCertificates)));
+      return certCollection.toArray(new X509Certificate[0]);
+    } catch (CertificateException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
   public static PrivateKey parsePrivateKey(String keyBytesHex) {
     try {
       KeyFactory fac = KeyFactory.getInstance("ECDSA");

diff --git a/u2f-ref-code/java/tests/com/google/u2f/TestVectors.java b/u2f-ref-code/java/tests/com/google/u2f/TestVectors.java
@@ -9,6 +9,7 @@
 import static com.google.u2f.TestUtils.computeSha256;
 import static com.google.u2f.TestUtils.parseCertificate;
 import static com.google.u2f.TestUtils.parseCertificateBase64;
+import static com.google.u2f.TestUtils.parseCertificateChainBase64;
 import static com.google.u2f.TestUtils.parseHex;
 import static com.google.u2f.TestUtils.parsePrivateKey;
 import static com.google.u2f.TestUtils.parsePublicKey;
@@ -315,17 +316,36 @@ public class TestVectors {
   protected static final X509Certificate ANDROID_KEYSTORE_ATTESTATION_CERT_NO_VERSION =
       parseCertificateBase64(ANDROID_KEYSTORE_ATTESTATION_CERT_NO_VERSION_BASE64);
 
-  private static final String ANDROID_KEYSTORE_ATTESTATION_CERT_BASE64 =
+  /**
+   * Contains a chain where:
+   *   cert[0] = attestation certificate describing some new key
+   *   cert[1] = batch certificate
+   *
+   * Note that cert[1] is signed by another cert that should be known to RPs.
+   */
+  private static final String ANDROID_KEYSTORE_ATTESTATION_CERT_CHAIN_BASE64 =
       "MIIBjTCCATKgAwIBAgICJxAwCgYIKoZIzj0EAwIwHDEaMBgGA1UEAwwRQW5kcm9pZCBLZXltYXN0"
       + "ZXIwIBcNNzAwMTAxMDAwMDAwWhgPMjEwNjAyMDcwNjI4MTVaMBoxGDAWBgNVBAMMD0EgS2V5bWFz"
       + "dGVyIEtleTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABJiTI/rSw9N1NYV3FGxgeJSj1NWyyb61"
       + "/gbdEefKuM3dYOeUZhciSigDY/u9Y3gBKm0wmXsd7DxXibDk/VvGIVWjZDBiMGAGCisGAQQB1nkC"
       + "AREEUjBQAgECBAljaGFsbGVuZ2UwPqEIMQYCAQICAQOiAwIBA6MEAgIBAKUFMQMCAQS/g3gDAgEB"
       + "v4N5BAICASy/hT0IAgYBUqi8MmC/hT4DAgEAMAAwCgYIKoZIzj0EAwIDSQAwRgIhANnmsSeWsnVH"
       + "aF5zII50tkiA7fRhIMNeZZBcPvSV2BN5AiEAwUZm63OxMZEHTIFL50ASKVN/sCLs8+gMY6uEVZRy"
-      + "61Q=";
-  protected static final X509Certificate ANDROID_KEYSTORE_ATTESTATION_CERT =
-      parseCertificateBase64(ANDROID_KEYSTORE_ATTESTATION_CERT_BASE64);
+      + "61QwggK2MIICH6ADAgECAgIQADANBgkqhkiG9w0BAQsFADBjMQswCQYDVQQGEwJVUzETMBEGA1UE"
+      + "CAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNTW91bnRhaW4gVmlldzEVMBMGA1UECgwMR29vZ2xlLCBJ"
+      + "bmMuMRAwDgYDVQQLDAdBbmRyb2lkMB4XDTE2MDEwNDEyNDA1M1oXDTM1MTIzMDEyNDA1M1owdjEL"
+      + "MAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFTATBgNVBAoMDEdvb2dsZSwgSW5jLjEQ"
+      + "MA4GA1UECwwHQW5kcm9pZDEpMCcGA1UEAwwgQW5kcm9pZCBTb2Z0d2FyZSBBdHRlc3RhdGlvbiBL"
+      + "ZXkwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMCDI9xWiBu4MCBp9bCFYcbuvn8F4vWoQgSK"
+      + "votHvnb+rvJc8psq+jIAFBYBQpmJoV/PxoFes2NYPC/S8gvkmDKD3YFLFtfhhUF65Uq8KWo6bbXA"
+      + "BAg7aMVWwfAjOZFkGYZNULdNQK7KSEx3NWyJWgwnWr+sSZ1dfSNi8pxeAuhxAgMBAAGjZjBkMB0G"
+      + "A1UdDgQWBBTUDBAb+M1jufc5UrUOE1ym15mThjAfBgNVHSMEGDAWgBQp+vGszE3STJZAJ3W2sOky"
+      + "5Qf+LjASBgNVHRMBAf8ECDAGAQH/AgEAMA4GA1UdDwEB/wQEAwIChDANBgkqhkiG9w0BAQsFAAOB"
+      + "gQCeLUhfjGcz3BqFrZnXUCPqFOxDsOGd6sIjRh5ytRncYCLkpWgxbAtVxOacoi2fOk+TazGLFngW"
+      + "DYjL2YvMgJ2E8MIn42s48f3R5xdyMVk1fZbzxX+rnY+WYSZPsr6Buw1JBCKKzp/39UIuJUT6IQcS"
+      + "WoO1Va0YgvhAFJucIGMEfw==";
+  protected static final X509Certificate[] ANDROID_KEYSTORE_ATTESTATION_CERT_CHAIN =
+      parseCertificateChainBase64(ANDROID_KEYSTORE_ATTESTATION_CERT_CHAIN_BASE64);
 
   protected static final byte[] REGISTRATION_DATA_2 =
       parseHex("0504478E16BBDBBB741A660A000314A8B6BD63095196ED704C52EEBC0FA02A61"

diff --git a/.../tests/com/google/u2f/server/impl/attestation/android/AndroidKeyStoreAttestationTest.java b/.../tests/com/google/u2f/server/impl/attestation/android/AndroidKeyStoreAttestationTest.java
@@ -21,7 +21,7 @@ public class AndroidKeyStoreAttestationTest extends TestVectors {
   @Test
   public void testValidCert() throws Exception {
     AndroidKeyStoreAttestation attestation =
-        AndroidKeyStoreAttestation.Parse(ANDROID_KEYSTORE_ATTESTATION_CERT);
+        AndroidKeyStoreAttestation.Parse(ANDROID_KEYSTORE_ATTESTATION_CERT_CHAIN[0]);
 
     assertNotNull("Not expecting null attestation", attestation);