Keystore:Expected exception for invalid Keys

If EC curves of Public and Private keys are different, an
InvalidKeyException is expected.

But the current implementation does not throw exception from doPhase method
and fails in generateSecret method.

The fix is in AndroidKeyStoreECPublicKey to provide
correct ECParameterSpec while creating a PrivateKey object.

Bug: 215175472
Test: run cts -m CtsKeystoreWycheproofTestCases -t com.google.security.wycheproof.JsonEcdhTest#testSecp224r1
Test: run cts -m CtsKeystoreWycheproofTestCases -t com.google.security.wycheproof.JsonEcdhTest#testSecp256r1
Test: run cts -m CtsKeystoreWycheproofTestCases -t com.google.security.wycheproof.JsonEcdhTest#testSecp384r1
Test: run cts -m CtsKeystoreWycheproofTestCases -t com.google.security.wycheproof.JsonEcdhTest#testSecp521r1
Test: run cts -m CtsKeystoreTestCases -t android.keystore.cts.KeyAgreementTest#testDoPhase_withDifferentCurveKey_fails
Change-Id: Ie221926d8a3be3fe6679e723575c5021cafba98e

4 files changed, 66 insertions, 4 deletions

diff --git a/core/java/android/security/keymaster/KeymasterDefs.java b/core/java/android/security/keymaster/KeymasterDefs.java
index 8efc5eb6b6ff..e720f1ab1523 100644
--- a/core/java/android/security/keymaster/KeymasterDefs.java
+++ b/core/java/android/security/keymaster/KeymasterDefs.java
@@ -65,6 +65,7 @@ public final class KeymasterDefs {
     public static final int KM_TAG_PADDING = Tag.PADDING; // KM_ENUM_REP | 6;
     public static final int KM_TAG_CALLER_NONCE = Tag.CALLER_NONCE; // KM_BOOL | 7;
     public static final int KM_TAG_MIN_MAC_LENGTH = Tag.MIN_MAC_LENGTH; // KM_UINT | 8;
+    public static final int KM_TAG_EC_CURVE = Tag.EC_CURVE; // KM_ENUM | 10;
 
     public static final int KM_TAG_RSA_PUBLIC_EXPONENT = Tag.RSA_PUBLIC_EXPONENT; // KM_ULONG | 200;
     public static final int KM_TAG_INCLUDE_UNIQUE_ID = Tag.INCLUDE_UNIQUE_ID; // KM_BOOL | 202;
diff --git a/keystore/java/android/security/keystore2/AndroidKeyStoreECPublicKey.java b/keystore/java/android/security/keystore2/AndroidKeyStoreECPublicKey.java
index b631999c2c54..4e73bd9d3c82 100644
--- a/keystore/java/android/security/keystore2/AndroidKeyStoreECPublicKey.java
+++ b/keystore/java/android/security/keystore2/AndroidKeyStoreECPublicKey.java
@@ -18,13 +18,19 @@ package android.security.keystore2;
 
 import android.annotation.NonNull;
 import android.security.KeyStoreSecurityLevel;
+import android.security.keymaster.KeymasterDefs;
 import android.security.keystore.KeyProperties;
+import android.system.keystore2.Authorization;
 import android.system.keystore2.KeyDescriptor;
 import android.system.keystore2.KeyMetadata;
 
+import java.security.AlgorithmParameters;
+import java.security.NoSuchAlgorithmException;
 import java.security.interfaces.ECPublicKey;
+import java.security.spec.ECGenParameterSpec;
 import java.security.spec.ECParameterSpec;
 import java.security.spec.ECPoint;
+import java.security.spec.InvalidParameterSpecException;
 
 /**
  * {@link ECPublicKey} backed by keystore.
@@ -56,11 +62,45 @@ public class AndroidKeyStoreECPublicKey extends AndroidKeyStorePublicKey impleme
         }
     }
 
+    private static String getEcCurveFromKeymaster(int ecCurve) {
+        switch (ecCurve) {
+            case android.hardware.security.keymint.EcCurve.P_224:
+                return "secp224r1";
+            case android.hardware.security.keymint.EcCurve.P_256:
+                return "secp256r1";
+            case android.hardware.security.keymint.EcCurve.P_384:
+                return "secp384r1";
+            case android.hardware.security.keymint.EcCurve.P_521:
+                return "secp521r1";
+        }
+        return "";
+    }
+
+    private ECParameterSpec getCurveSpec(String name)
+            throws NoSuchAlgorithmException, InvalidParameterSpecException {
+        AlgorithmParameters parameters = AlgorithmParameters.getInstance("EC");
+        parameters.init(new ECGenParameterSpec(name));
+        return parameters.getParameterSpec(ECParameterSpec.class);
+    }
+
     @Override
     public AndroidKeyStorePrivateKey getPrivateKey() {
+        ECParameterSpec params = mParams;
+        for (Authorization a : getAuthorizations()) {
+            try {
+                if (a.keyParameter.tag == KeymasterDefs.KM_TAG_EC_CURVE) {
+                    params = getCurveSpec(getEcCurveFromKeymaster(
+                            a.keyParameter.value.getEcCurve()));
+                    break;
+                }
+            } catch (Exception e) {
+                throw new RuntimeException("Unable to parse EC curve "
+                        + a.keyParameter.value.getEcCurve());
+            }
+        }
         return new AndroidKeyStoreECPrivateKey(
                 getUserKeyDescriptor(), getKeyIdDescriptor().nspace, getAuthorizations(),
-                getSecurityLevel(), mParams);
+                getSecurityLevel(), params);
     }
 
     @Override
diff --git a/keystore/java/android/security/keystore2/AndroidKeyStoreKeyAgreementSpi.java b/keystore/java/android/security/keystore2/AndroidKeyStoreKeyAgreementSpi.java
index b1338d164055..4caa47f2078b 100644
--- a/keystore/java/android/security/keystore2/AndroidKeyStoreKeyAgreementSpi.java
+++ b/keystore/java/android/security/keystore2/AndroidKeyStoreKeyAgreementSpi.java
@@ -31,6 +31,8 @@ import java.security.NoSuchAlgorithmException;
 import java.security.ProviderException;
 import java.security.PublicKey;
 import java.security.SecureRandom;
+import java.security.interfaces.ECKey;
+import java.security.interfaces.XECKey;
 import java.security.spec.AlgorithmParameterSpec;
 import java.util.ArrayList;
 import java.util.List;
@@ -132,6 +134,15 @@ public class AndroidKeyStoreKeyAgreementSpi extends KeyAgreementSpi
             throw new InvalidKeyException("key == null");
         } else if (!(key instanceof PublicKey)) {
             throw new InvalidKeyException("Only public keys supported. Key: " + key);
+        } else if (!(mKey instanceof ECKey && key instanceof ECKey)
+                && !(mKey instanceof XECKey && key instanceof XECKey)) {
+            throw new InvalidKeyException(
+                    "Public and Private key should be of the same type:");
+        } else if (mKey instanceof ECKey
+                && !((ECKey) key).getParams().getCurve()
+                .equals(((ECKey) mKey).getParams().getCurve())) {
+            throw new InvalidKeyException(
+                    "Public and Private key parameters should be same.");
         } else if (!lastPhase) {
             throw new IllegalStateException(
                     "Only one other party supported. lastPhase must be set to true.");
diff --git a/keystore/java/android/security/keystore2/AndroidKeyStoreXDHPrivateKey.java b/keystore/java/android/security/keystore2/AndroidKeyStoreXDHPrivateKey.java
index 42589640d2b7..e392c8dcca93 100644
--- a/keystore/java/android/security/keystore2/AndroidKeyStoreXDHPrivateKey.java
+++ b/keystore/java/android/security/keystore2/AndroidKeyStoreXDHPrivateKey.java
@@ -22,16 +22,18 @@ import android.system.keystore2.Authorization;
 import android.system.keystore2.KeyDescriptor;
 
 import java.security.PrivateKey;
-import java.security.interfaces.EdECKey;
+import java.security.interfaces.XECPrivateKey;
 import java.security.spec.NamedParameterSpec;
+import java.util.Optional;
 
 /**
  * X25519 Private Key backed by Keystore.
- * instance of {@link PrivateKey} and {@link EdECKey}
+ * instance of {@link PrivateKey} and {@link XECPrivateKey}
  *
  * @hide
  */
-public class AndroidKeyStoreXDHPrivateKey extends AndroidKeyStorePrivateKey implements EdECKey {
+public class AndroidKeyStoreXDHPrivateKey extends AndroidKeyStorePrivateKey
+        implements XECPrivateKey {
     public AndroidKeyStoreXDHPrivateKey(
             @NonNull KeyDescriptor descriptor, long keyId,
             @NonNull Authorization[] authorizations,
@@ -44,4 +46,12 @@ public class AndroidKeyStoreXDHPrivateKey extends AndroidKeyStorePrivateKey impl
     public NamedParameterSpec getParams() {
         return NamedParameterSpec.X25519;
     }
+
+    @Override
+    public Optional<byte[]> getScalar() {
+        /* An empty Optional if the scalar cannot be extracted (e.g. if the provider is a hardware
+         * token and the private key is not allowed to leave the crypto boundary).
+         */
+        return Optional.empty();
+    }
 }