You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by se...@apache.org on 2017/06/20 14:06:29 UTC
cxf git commit: [CXF-6728] Adding an invalid curve test
Repository: cxf
Updated Branches:
refs/heads/master 8fe542285 -> 998ce1e5d
[CXF-6728] Adding an invalid curve test
Project: http://git-wip-us.apache.org/repos/asf/cxf/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/998ce1e5
Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/998ce1e5
Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/998ce1e5
Branch: refs/heads/master
Commit: 998ce1e5df26773dc96d130b9b55c7d6eec746e7
Parents: 8fe5422
Author: Sergey Beryozkin <sb...@gmail.com>
Authored: Tue Jun 20 15:06:15 2017 +0100
Committer: Sergey Beryozkin <sb...@gmail.com>
Committed: Tue Jun 20 15:06:15 2017 +0100
----------------------------------------------------------------------
.../cxf/rs/security/jose/jwe/JweUtils.java | 45 ++++++++++++++++
.../jose/jwe/JweCompactReaderWriterTest.java | 57 ++++++++++++++++++++
2 files changed, 102 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cxf/blob/998ce1e5/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweUtils.java
----------------------------------------------------------------------
diff --git a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweUtils.java b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweUtils.java
index 3d851f8..f4d5842 100644
--- a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweUtils.java
+++ b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweUtils.java
@@ -18,6 +18,7 @@
*/
package org.apache.cxf.rs.security.jose.jwe;
+import java.math.BigInteger;
import java.nio.ByteBuffer;
import java.security.Key;
import java.security.PrivateKey;
@@ -28,6 +29,9 @@ import java.security.interfaces.ECPublicKey;
import java.security.interfaces.RSAKey;
import java.security.interfaces.RSAPrivateKey;
import java.security.interfaces.RSAPublicKey;
+import java.security.spec.ECFieldFp;
+import java.security.spec.ECPoint;
+import java.security.spec.EllipticCurve;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -667,12 +671,53 @@ public final class JweUtils {
JwkUtils.toECPublicKey(peerPublicKey),
partyUInfo, partyVInfo, algoName, algoKeyBitLen);
}
+
public static byte[] getECDHKey(ECPrivateKey privateKey,
ECPublicKey peerPublicKey,
byte[] partyUInfo,
byte[] partyVInfo,
String algoName,
int algoKeyBitLen) {
+ // Validate the peerPublicKey first
+
+ // Credits:
+ // https://neilmadden.wordpress.com/2017/05/17/so-how-do-you-validate-nist-ecdh-public-keys/
+ // https://blogs.adobe.com/security/2017/03/critical-vulnerability-uncovered-in-json-encryption.html
+
+ // Step 1: Verify public key is not point at infinity.
+ if (ECPoint.POINT_INFINITY.equals(peerPublicKey.getW())) {
+ throw new JweException(JweException.Error.KEY_ENCRYPTION_FAILURE);
+ }
+ EllipticCurve curve = peerPublicKey.getParams().getCurve();
+
+ final BigInteger x = peerPublicKey.getW().getAffineX();
+ final BigInteger y = peerPublicKey.getW().getAffineY();
+ final BigInteger p = ((ECFieldFp) curve.getField()).getP();
+
+ // Step 2: Verify x and y are in range [0,p-1]
+ if (x.compareTo(BigInteger.ZERO) < 0 || x.compareTo(p) >= 0
+ || y.compareTo(BigInteger.ZERO) < 0 || y.compareTo(p) >= 0) {
+ throw new JweException(JweException.Error.KEY_ENCRYPTION_FAILURE);
+ }
+ final BigInteger a = curve.getA();
+ final BigInteger b = curve.getB();
+
+ // Step 3: Verify that y^2 == x^3 + ax + b (mod p)
+ final BigInteger ySquared = y.modPow(BigInteger.valueOf(2), p);
+ final BigInteger xCubedPlusAXPlusB = x.modPow(BigInteger.valueOf(3), p).add(a.multiply(x)).add(b).mod(p);
+ if (!ySquared.equals(xCubedPlusAXPlusB)) {
+ throw new JweException(JweException.Error.KEY_ENCRYPTION_FAILURE);
+ }
+
+ // Step 4: Verify that nQ = 0, where n is the order of the curve and Q is the public key.
+ // As per http://www.secg.org/sec1-v2.pdf section 3.2.2:
+ // "In Step 4, it may not be necessary to compute the point nQ. For example, if h = 1, then nQ = O is implied
+ // by the checks in Steps 2 and 3, because this property holds for all points Q ∈ E"
+ // All the NIST curves used here define h = 1.
+ assert peerPublicKey.getParams().getCofactor() == 1;
+
+ // Finally calculate the derived key
+
byte[] keyZ = generateKeyZ(privateKey, peerPublicKey);
return calculateDerivedKey(keyZ, algoName, partyUInfo, partyVInfo, algoKeyBitLen);
}
http://git-wip-us.apache.org/repos/asf/cxf/blob/998ce1e5/rt/rs/security/jose-parent/jose/src/test/java/org/apache/cxf/rs/security/jose/jwe/JweCompactReaderWriterTest.java
----------------------------------------------------------------------
diff --git a/rt/rs/security/jose-parent/jose/src/test/java/org/apache/cxf/rs/security/jose/jwe/JweCompactReaderWriterTest.java b/rt/rs/security/jose-parent/jose/src/test/java/org/apache/cxf/rs/security/jose/jwe/JweCompactReaderWriterTest.java
index 6cf4e31..8c47e6d 100644
--- a/rt/rs/security/jose-parent/jose/src/test/java/org/apache/cxf/rs/security/jose/jwe/JweCompactReaderWriterTest.java
+++ b/rt/rs/security/jose-parent/jose/src/test/java/org/apache/cxf/rs/security/jose/jwe/JweCompactReaderWriterTest.java
@@ -33,9 +33,11 @@ import org.apache.cxf.rs.security.jose.jwa.AlgorithmUtils;
import org.apache.cxf.rs.security.jose.jwa.ContentAlgorithm;
import org.apache.cxf.rs.security.jose.jwa.KeyAlgorithm;
import org.apache.cxf.rs.security.jose.jwk.JsonWebKey;
+import org.apache.cxf.rs.security.jose.jwk.JwkUtils;
import org.apache.cxf.rs.security.jose.jws.JwsCompactReaderWriterTest;
import org.apache.cxf.rt.security.crypto.CryptoUtils;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
+
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
@@ -139,6 +141,61 @@ public class JweCompactReaderWriterTest extends Assert {
new EcdhDirectKeyJweDecryption(bobPrivateKey, ContentAlgorithm.A128GCM);
assertEquals("Hello", jweIn.decrypt(jweOutput).getContentText());
}
+
+ @Test
+ public void testRejectInvalidCurve() throws Exception {
+ // Test vectors are provided by Antonio Sanso, test follows a pattern based
+ // on a similar contribution from Antonio to jose4j.
+ String receiverJwkJson = "\n{\"kty\":\"EC\",\n"
+ + " \"crv\":\"P-256\",\n"
+ + " \"x\":\"weNJy2HscCSM6AEDTDg04biOvhFhyyWvOHQfeF_PxMQ\",\n"
+ + " \"y\":\"e8lnCO-AlStT-NJVX-crhB7QRYhiix03illJOVAOyck\",\n"
+ + " \"d\":\"VEmDZpDXXK8p8N0Cndsxs924q6nS1RXFASRl6BfUqdw\"\n"
+ + "}";
+ JsonWebKey receiverJwk = JwkUtils.readJwkKey(receiverJwkJson);
+ ECPrivateKey privateKey = JwkUtils.toECPrivateKey(receiverJwk);
+
+ //========================= attacking point #1 with order 113 ======================
+ //The malicious JWE contains a public key with order 113
+ String maliciousJWE1 = "eyJhbGciOiJFQ0RILUVTK0ExMjhLVyIsImVuYyI6IkExMjhDQkMtSFMyNTYiLCJlcGsiOnsia3R5IjoiRU"
+ + "MiLCJ4IjoiZ1Rsa"
+ + "TY1ZVRRN3otQmgxNDdmZjhLM203azJVaURpRzJMcFlrV0FhRkpDYyIsInkiOiJjTEFuakthNGJ6akQ3REpWUHdhOUVQclJ6TUc3"
+ + "ck9OZ3NpVUQta"
+ + "2YzMEZzIiwiY3J2IjoiUC0yNTYifX0.qGAdxtEnrV_3zbIxU2ZKrMWcejNltjA_dtefBFnRh9A2z9cNIqYRWg.pEA5kX304PMCOm"
+ + "FSKX_cEg.a9f"
+ + "wUrx2JXi1OnWEMOmZhXd94-bEGCH9xxRwqcGuG2AMo-AwHoljdsH5C_kcTqlXS5p51OB1tvgQcMwB5rpTxg.72CHiYFecyDvuUa4"
+ + "3KKT6w";
+
+
+ JweDecryptionProvider jweIn = JweUtils.createJweDecryptionProvider(privateKey,
+ KeyAlgorithm.ECDH_ES_A128KW,
+ ContentAlgorithm.A128CBC_HS256);
+ try {
+ jweIn.decrypt(maliciousJWE1);
+ fail("Decryption should have failed due to invalid curve");
+ } catch (JweException e) {
+ // continue
+ }
+
+ //========================= attacking point #2 with order 2447 ======================
+ //The malicious JWE contains a public key with order 2447
+ String maliciousJWE2 = "eyJhbGciOiJFQ0RILUVTK0ExMjhLVyIsImVuYyI6IkExMjhDQkMtSFMyNTYiLCJlcGsiOnsia3R5IjoiRU"
+ + "MiLCJ4IjoiWE9YR1"
+ + "E5XzZRQ3ZCZzN1OHZDSS1VZEJ2SUNBRWNOTkJyZnFkN3RHN29RNCIsInkiOiJoUW9XTm90bk56S2x3aUNuZUprTElxRG5UTnc3SXNkQ"
+ + "kM1M1ZVcVZ"
+ + "qVkpjIiwiY3J2IjoiUC0yNTYifX0.UGb3hX3ePAvtFB9TCdWsNkFTv9QWxSr3MpYNiSBdW630uRXRBT3sxw.6VpU84oMob16DxOR98Y"
+ + "TRw.y1Uslv"
+ + "tkoWdl9HpugfP0rSAkTw1xhm_LbK1iRXzGdpYqNwIG5VU33UBpKAtKFBoA1Kk_sYtfnHYAvn-aes4FTg.UZPN8h7FcvA5MIOq-Pkj8A";
+ JweDecryptionProvider jweIn2 = JweUtils.createJweDecryptionProvider(privateKey,
+ KeyAlgorithm.ECDH_ES_A128KW,
+ ContentAlgorithm.A128CBC_HS256);
+ try {
+ jweIn2.decrypt(maliciousJWE2);
+ fail("Decryption should have failed due to invalid curve");
+ } catch (JweException e) {
+ // expected
+ }
+ }
@Test
public void testEncryptDecryptRSA15WrapA128CBCHS256() throws Exception {
final String specPlainText = "Live long and prosper.";