NullPointerException when the curve is not supported

[Paweł Żuk]

When we try to use curve that is not supported, we got:

Caused by: java.lang.NullPointerException: params is null
at java.security.spec.ECPublicKeySpec.<init>(ECPublicKeySpec.java:60)
at org.jose4j.keys.EcKeyUtil.publicKey(EcKeyUtil.java:52)
at org.jose4j.jwk.EllipticCurveJsonWebKey.<init>(EllipticCurveJsonWebKey.java:72)
at org.jose4j.jwk.EllipticCurveJsonWebKey.<init>(EllipticCurveJsonWebKey.java:57)
at org.jose4j.jwk.JsonWebKey$Factory.newJwk(JsonWebKey.java:245)
at org.jose4j.jwk.JsonWebKey$Factory.newJwk(JsonWebKey.java:276)

It will be nice to get JoseException instead.

[Brian Campbell]

I’m curious - can you share the JWK/JWKS JSON where this occurred? secp256k1 curve maybe?

A JoseException indeed might be more better/nicer. There are a couple different API layers involved here and I’m not sure where best to check for it and throw.

‌

[Paweł Żuk]

Yes secp256k1. We tested Connect2id server.

  {
     "kty":"EC",
     "use":"sig",
     "crv":"secp256k1",
     "kid":"iRTU",
     "x":"tcjSy7nIVZ2DVz-RKjqHIJqr5WDqZLS6fq4rEN6pfGY",
     "y":"2oqx5jvggJKy-LXFjpDOpL0g_SbiLylu_8xx-dBMQeQ"
  }, 

In my opinion the check could be in EllipticCurveJsonWebKey constructor, when the curve is null:

public EllipticCurveJsonWebKey(Map<String, Object> params, String jcaProvider) throws JoseException
{
    super(params, jcaProvider);

    curveName = getString(params, CURVE_MEMBER_NAME, true);
    ECParameterSpec curve = EllipticCurves.getSpec(curveName);

or even inside the getSpec method, if the result is null, the the curve is not supported.

[Paweł Żuk]

Or maybe instead of JoseException maybe just check if the implementation is supported?

[Brian Campbell]

But then do what, if it’s not?

I’ve got this, which throws out of the EllipticCurveJsonWebKey constructor, that I was going to commit soonish.

$ git diff
diff --git a/src/main/java/org/jose4j/jwk/EllipticCurveJsonWebKey.java b/src/main/java/org/jose4j/jwk/EllipticCurveJsonWebKey.java
index 6a9466a..3843842 100644
--- a/src/main/java/org/jose4j/jwk/EllipticCurveJsonWebKey.java
+++ b/src/main/java/org/jose4j/jwk/EllipticCurveJsonWebKey.java
@@ -18,6 +18,7 @@ package org.jose4j.jwk;

 import org.jose4j.keys.EcKeyUtil;
 import org.jose4j.keys.EllipticCurves;
+import org.jose4j.lang.InvalidKeyException;
 import org.jose4j.lang.JoseException;

 import java.math.BigInteger;
@@ -63,6 +64,10 @@ public class EllipticCurveJsonWebKey extends PublicJsonWebKey

         curveName = getString(params, CURVE_MEMBER_NAME, true);
         ECParameterSpec curve = EllipticCurves.getSpec(curveName);
+        if (curve == null)
+        {
+            throw new InvalidKeyException("\"" + curveName + "\" is an unknown or unsupported value for the \"crv\" parameter.");
+        }

         BigInteger x = getBigIntFromBase64UrlEncodedParam(params, X_MEMBER_NAME, true);

diff --git a/src/test/java/org/jose4j/jwk/EllipticCurveJsonWebKeyTest.java b/src/test/java/org/jose4j/jwk/EllipticCurveJsonWebKeyTest.java
index 2e286c0..9ef0221 100644
--- a/src/test/java/org/jose4j/jwk/EllipticCurveJsonWebKeyTest.java
+++ b/src/test/java/org/jose4j/jwk/EllipticCurveJsonWebKeyTest.java
@@ -20,6 +20,7 @@ import org.jose4j.base64url.Base64Url;
 import org.jose4j.json.JsonUtil;
 import org.jose4j.keys.EllipticCurves;
 import org.jose4j.keys.ExampleEcKeysFromJws;
+import org.jose4j.lang.InvalidKeyException;
 import org.jose4j.lang.JoseException;
 import org.junit.Test;

@@ -160,4 +161,20 @@ public class EllipticCurveJsonWebKeyTest
         assertEquals(jwkWithNoZeroLeftPaddingBytes.getPublicKey(), jwkWithZeroLeftPaddingBytes.getPublicKey());

     }
+
+    @Test (expected = InvalidKeyException.class)
+    public void newCurveWhoDis() throws JoseException
+    {
+        // from https://bitbucket.org/b_c/jose4j/issues/198/nullpointerexception-when-the-curve-is-not
+        // throw a bit more meaningful Exception rather than NPE when the curve isn't supported
+            String jwkJson =
+                    "{     \"kty\":\"EC\",\n" +
+                    "     \"use\":\"sig\",\n" +
+                    "     \"crv\":\"secp256k1\",\n" +
+                    "     \"kid\":\"iRTU\",\n" +
+                    "     \"x\":\"tcjSy7nIVZ2DVz-RKjqHIJqr5WDqZLS6fq4rEN6pfGY\",\n" +
+                    "     \"y\":\"2oqx5jvggJKy-LXFjpDOpL0g_SbiLylu_8xx-dBMQeQ\"}";
+
+        JsonWebKey jsonWebKey = JsonWebKey.Factory.newJwk(jwkJson);
+    }
 }
diff --git a/src/test/java/org/jose4j/jwk/JsonWebKeySetTest.java b/src/test/java/org/jose4j/jwk/JsonWebKeySetTest.java
index 4b96756..fd1f8ff 100644
--- a/src/test/java/org/jose4j/jwk/JsonWebKeySetTest.java
+++ b/src/test/java/org/jose4j/jwk/JsonWebKeySetTest.java
@@ -313,6 +313,7 @@ public class JsonWebKeySetTest
     public void testParseSetContainingInvalid() throws Exception
     {
         String json = "{\"keys\":[" +
+            "{\"kty\":\"EC\",\"crv\":\"NOPERS\",\"kid\":\"iRTU\",\"x\":\"tcjSy7nIVZ2DVz-RKjqHIJqr5WDqZLS6fq4rEN6pfGY\",\"y\":\"2oqx5jvggJKy-LXFjpDOpL0g_SbiLylu_8xx-dBMQeQ\"}, " +
             "{\"kty\":\"EC\",\"x\":\"riwTtQeRjmlDsR4PUQELhejpPkZkQstb0_Lf08qeBzM\",\"y\":\"izN8y6z-8j8bB_Lj10gX9mnaE_E0ZK5fl0hJVyLWMKA\",\"crv\":\"P-256\"}," +
             "{\"kty\":false,\"x\":\"GS2tEeCRf0CFHzI_y68XiLzqa9-RpG4Xn-dq2lPtShY\",\"y\":\"Rq6ybA7IbjhDTfvP2GSzxEql8II7RvRPb3mJ6tzZUgI\",\"crv\":\"P-256\"}," +
             "{\"kty\":\"EC\",\"x\":\"IiIIM4W-HDen_11XiGlFXh1kOxKcX1YB5gqMrCM-hMM\",\"y\":\"57-3xqdddSBBarwwXcWu4hIG4dAlIiEYdy4aaFGb57s\",\"crv\":\"P-256\"}," +
@@ -324,6 +325,8 @@ public class JsonWebKeySetTest
             "{\"kty\":null,\"x\":\"riwTtQeRjmlDsR4PUQELhejpPkZkQstb0_Lf08qeBzM\",\"y\":\"izN8y6z-8j8bB_Lj10gX9mnaE_E0ZK5fl0hJVyLWMKA\",\"crv\":\"P-256\"}," +
             "]}";

+
+
         JsonWebKeySet jwks = new JsonWebKeySet(json);
         assertEquals(3, jwks.getJsonWebKeys().size());
     }

‌

‌

‌

[Brian Campbell]

c350a58 throws a checked exception from new EllipticCurveJsonWebKey(...) with a semi-meaningful message when the curve isn't known/supported rather than letting ECPublicKeySpec NPE shortly after

[Brian Campbell]

released with jose4j-0.8.0