Fix ECDH bug

Verify that the public key is in the curve defined by the private key
and explicitly construct public key using the private key's curve.

Bug: 24082558
Change-Id: I146c58ba543d829565e1b199879003a662e57f19

3 files changed, 88 insertions, 7 deletions

diff --git a/bcprov/src/main/java/org/bouncycastle/crypto/agreement/ECDHBasicAgreement.java b/bcprov/src/main/java/org/bouncycastle/crypto/agreement/ECDHBasicAgreement.java
index a491b9df..5aa2f2bb 100644
--- a/bcprov/src/main/java/org/bouncycastle/crypto/agreement/ECDHBasicAgreement.java
+++ b/bcprov/src/main/java/org/bouncycastle/crypto/agreement/ECDHBasicAgreement.java
@@ -6,6 +6,9 @@ import org.bouncycastle.crypto.BasicAgreement;
 import org.bouncycastle.crypto.CipherParameters;
 import org.bouncycastle.crypto.params.ECPrivateKeyParameters;
 import org.bouncycastle.crypto.params.ECPublicKeyParameters;
+// BEGIN android-added
+import org.bouncycastle.math.ec.ECCurve;
+// END android-added
 import org.bouncycastle.math.ec.ECPoint;
 
 /**
@@ -41,8 +44,23 @@ public class ECDHBasicAgreement
     public BigInteger calculateAgreement(
         CipherParameters pubKey)
     {
-        ECPublicKeyParameters pub = (ECPublicKeyParameters)pubKey;
-        ECPoint P = pub.getQ().multiply(key.getD()).normalize();
+        // BEGIN android-changed
+        ECPoint peerPoint = ((ECPublicKeyParameters) pubKey).getQ();
+        ECCurve myCurve = key.getParameters().getCurve();
+        if (peerPoint.isInfinity()) {
+          throw new IllegalStateException("Infinity is not a valid public key for ECDH");
+        }
+        try {
+          myCurve.validatePoint(peerPoint.getXCoord().toBigInteger(),
+              peerPoint.getYCoord().toBigInteger());
+        } catch (IllegalArgumentException ex) {
+          throw new IllegalStateException("The peer public key must be on the curve for ECDH");
+        }
+        // Explicitly construct a public key using the private key's curve.
+        ECPoint pubPoint = myCurve.createPoint(peerPoint.getXCoord().toBigInteger(),
+            peerPoint.getYCoord().toBigInteger());
+        ECPoint P = pubPoint.multiply(key.getD()).normalize();
+        // END android-changed
 
         if (P.isInfinity())
         {
diff --git a/bcprov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java b/bcprov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java
index 3dbe0046..754b4cae 100644
--- a/bcprov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java
+++ b/bcprov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java
@@ -166,7 +166,15 @@ public class KeyAgreementSpi
             // TODO Validate that all the keys are using the same parameters?
         }
 
+        // BEGIN android-added
+        try {
+        // END android-added
         result = agreement.calculateAgreement(pubKey);
+        // BEGIN android-added
+        } catch (IllegalStateException e) {
+          throw new InvalidKeyException("Invalid public key");
+        }
+        // END android-added
 
         return null;
     }
diff --git a/patches/bcprov.patch b/patches/bcprov.patch
index 047d43bc..35329ad1 100644
--- a/patches/bcprov.patch
+++ b/patches/bcprov.patch
@@ -380,6 +380,45 @@ diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/asn1/x9/ECNamedCurveTable.ja
  
          return v.elements();
      }
+diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/crypto/agreement/ECDHBasicAgreement.java bcprov-jdk15on-152/org/bouncycastle/crypto/agreement/ECDHBasicAgreement.java
+--- bcprov-jdk15on-152.orig/org/bouncycastle/crypto/agreement/ECDHBasicAgreement.java	2015-03-01 12:03:02.000000000 +0000
++++ bcprov-jdk15on-152/org/bouncycastle/crypto/agreement/ECDHBasicAgreement.java	2015-09-22 23:11:43.000000000 +0000
+@@ -6,6 +6,9 @@
+ import org.bouncycastle.crypto.CipherParameters;
+ import org.bouncycastle.crypto.params.ECPrivateKeyParameters;
+ import org.bouncycastle.crypto.params.ECPublicKeyParameters;
++// BEGIN android-added
++import org.bouncycastle.math.ec.ECCurve;
++// END android-added
+ import org.bouncycastle.math.ec.ECPoint;
+ 
+ /**
+@@ -41,8 +44,23 @@
+     public BigInteger calculateAgreement(
+         CipherParameters pubKey)
+     {
+-        ECPublicKeyParameters pub = (ECPublicKeyParameters)pubKey;
+-        ECPoint P = pub.getQ().multiply(key.getD()).normalize();
++        // BEGIN android-changed
++        ECPoint peerPoint = ((ECPublicKeyParameters) pubKey).getQ();
++        ECCurve myCurve = key.getParameters().getCurve();
++        if (peerPoint.isInfinity()) {
++          throw new IllegalStateException("Infinity is not a valid public key for ECDH");
++        }
++        try {
++          myCurve.validatePoint(peerPoint.getXCoord().toBigInteger(),
++              peerPoint.getYCoord().toBigInteger());
++        } catch (IllegalArgumentException ex) {
++          throw new IllegalStateException("The peer public key must be on the curve for ECDH");
++        }
++        // Explicitly construct a public key using the private key's curve.
++        ECPoint pubPoint = myCurve.createPoint(peerPoint.getXCoord().toBigInteger(),
++            peerPoint.getYCoord().toBigInteger());
++        ECPoint P = pubPoint.multiply(key.getD()).normalize();
++        // END android-changed
+ 
+         if (P.isInfinity())
+         {
 diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/crypto/digests/AndroidDigestFactory.java bcprov-jdk15on-152/org/bouncycastle/crypto/digests/AndroidDigestFactory.java
 --- bcprov-jdk15on-152.orig/org/bouncycastle/crypto/digests/AndroidDigestFactory.java	1970-01-01 00:00:00.000000000 +0000
 +++ bcprov-jdk15on-152/org/bouncycastle/crypto/digests/AndroidDigestFactory.java	2013-09-26 18:06:21.000000000 +0000
@@ -2067,7 +2106,7 @@ diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/jcajce/provider/asymmetric/d
  
 diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java bcprov-jdk15on-152/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java
 --- bcprov-jdk15on-152.orig/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java	2015-03-01 12:03:02.000000000 +0000
-+++ bcprov-jdk15on-152/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java	2014-07-28 19:51:54.000000000 +0000
++++ bcprov-jdk15on-152/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java	2015-09-22 23:11:43.000000000 +0000
 @@ -24,22 +24,28 @@
  import org.bouncycastle.crypto.CipherParameters;
  import org.bouncycastle.crypto.DerivationFunction;
@@ -2174,7 +2213,23 @@ diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/jcajce/provider/asymmetric/e
          {
              if (!(key instanceof PublicKey))
              {
-@@ -162,11 +174,13 @@
+@@ -154,7 +166,15 @@
+             // TODO Validate that all the keys are using the same parameters?
+         }
+ 
++        // BEGIN android-added
++        try {
++        // END android-added
+         result = agreement.calculateAgreement(pubKey);
++        // BEGIN android-added
++        } catch (IllegalStateException e) {
++          throw new InvalidKeyException("Invalid public key");
++        }
++        // END android-added
+ 
+         return null;
+     }
+@@ -162,11 +182,13 @@
      protected byte[] engineGenerateSecret()
          throws IllegalStateException
      {
@@ -2193,7 +2248,7 @@ diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/jcajce/provider/asymmetric/e
  
          return bigIntToBytes(result);
      }
-@@ -201,23 +215,25 @@
+@@ -201,23 +223,25 @@
              oidAlgorithm = ((ASN1ObjectIdentifier)oids.get(algKey)).getId();
          }
  
@@ -2236,7 +2291,7 @@ diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/jcajce/provider/asymmetric/e
          {
              if (algorithms.containsKey(oidAlgorithm))
              {
-@@ -264,35 +280,37 @@
+@@ -264,35 +288,37 @@
      private void initFromKey(Key key)
          throws InvalidKeyException
      {
@@ -2303,7 +2358,7 @@ diff -Naur bcprov-jdk15on-152.orig/org/bouncycastle/jcajce/provider/asymmetric/e
          {
              if (!(key instanceof PrivateKey))
              {
-@@ -323,39 +341,41 @@
+@@ -323,39 +349,41 @@
          }
      }