fix Elliptic Curve signature verification; release 1.8.10.1

ChangeLog

@@ -1,3 +1,7 @@
+7/11/2016
+- fix Elliptic Curve signature verification for corrupted input
+- release 1.8.10.1
+
 6/27/2016
 - use EVP_CIPHER_CTX_new to avoid compilation errors with OpenSSL 1.1.0
 - release 1.8.10

configure.ac

@@ -1,4 +1,4 @@
-AC_INIT([mod_auth_openidc],[1.8.10],[[email protected]])
+AC_INIT([mod_auth_openidc],[1.8.10.1],[[email protected]])
 
 AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())
 

debian/changelog

@@ -1,3 +1,9 @@
+libapache2-mod-auth-openidc (1.8.10.1-1) unstable; urgency=medium
+
+  * fix Elliptic Curve signature verification
+
+ -- Hans Zandbelt <[email protected]>  Mon, 11 Jul 2016 15:12:50 +0200
+
 libapache2-mod-auth-openidc (1.8.10-1) unstable; urgency=medium
 
   * build with OpenSSL 1.1.0

src/jose/apr_jws.c

@@ -57,6 +57,8 @@
 #include <openssl/hmac.h>
 #include <openssl/err.h>
 
+#include <openssl/ecdsa.h>
+
 #include <apr_base64.h>
 
 #include "apr_jose.h"
@@ -483,9 +485,15 @@ static apr_byte_t apr_jws_verify_rsa(apr_pool_t *pool, apr_jwt_t *jwt,
 			apr_jwt_error_openssl(err, "EVP_VerifyUpdate");
 			goto end;
 		}
-		if (!EVP_VerifyFinal(&ctx, (const unsigned char *) jwt->signature.bytes,
-				jwt->signature.length, pRsaKey)) {
-			apr_jwt_error_openssl(err, "wrong key? EVP_VerifyFinal");
+
+		int rv = EVP_VerifyFinal(&ctx, (const unsigned char *) jwt->signature.bytes,
+				jwt->signature.length, pRsaKey);
+
+		if (rv < 0) {
+			apr_jwt_error_openssl(err, "EVP_VerifyFinal");
+			goto end;
+		} else if (rv == 0) {
+			apr_jwt_error(err, "wrong key");
 			goto end;
 		}
 
@@ -545,12 +553,8 @@ static apr_byte_t apr_jws_verify_ec(apr_pool_t *pool, apr_jwt_t *jwt,
 	apr_byte_t rc = FALSE;
 
 	/* get the OpenSSL digest function */
-	const EVP_MD *digest = NULL;
-	if ((digest = apr_jws_crypto_alg_to_evp(pool, jwt->header.alg, err)) == NULL)
-		return FALSE;
-
-	EVP_MD_CTX ctx;
-	EVP_MD_CTX_init(&ctx);
+	const char *digest = apr_jws_alg_to_openssl_digest(jwt->header.alg);
+	if (digest == NULL) return FALSE;
 
 	EC_KEY * pubkey = EC_KEY_new();
 	EC_KEY_set_group(pubkey, curve);
@@ -566,43 +570,50 @@ static apr_byte_t apr_jws_verify_ec(apr_pool_t *pool, apr_jwt_t *jwt,
 		return FALSE;
 	}
 
-	EVP_PKEY* pEcKey = EVP_PKEY_new();
-	if (!EVP_PKEY_assign_EC_KEY(pEcKey, pubkey)) {
-		pEcKey = NULL;
-		apr_jwt_error_openssl(err, "EVP_PKEY_assign_EC_KEY");
-		goto end;
-	}
+	//P-256:  64 byte signature
+	//P-384:  96 byte signature
+	//P-521: 132 byte signature
+	int key_len = jwt->signature.length / 2;
 
-	ctx.pctx = EVP_PKEY_CTX_new(pEcKey, NULL);
+	ECDSA_SIG *ecdsa_sig = NULL;
+	ecdsa_sig = ECDSA_SIG_new();
 
-	if (!EVP_PKEY_verify_init(ctx.pctx)) {
-		apr_jwt_error_openssl(err, "EVP_PKEY_verify_init");
-		goto end;
-	}
-	if (!EVP_VerifyInit_ex(&ctx, digest, NULL)) {
-		apr_jwt_error_openssl(err, "EVP_VerifyInit_ex");
+#if OPENSSL_VERSION_NUMBER >= 0x10100005L
+	BIGNUM *r = BN_new();
+	BIGNUM *s = BN_new();
+	BN_bin2bn(jwt->signature.bytes, key_len, r);
+	BN_bin2bn(jwt->signature.bytes + key_len, key_len, s);
+	ECDSA_SIG_set0(ecdsa_sig, r, s);
+#else
+	BN_bin2bn(jwt->signature.bytes, key_len, ecdsa_sig->r);
+	BN_bin2bn(jwt->signature.bytes + key_len, key_len, ecdsa_sig->s);
+#endif
+
+	char *hash = NULL;
+	int hash_len = 0;
+	if (apr_jws_hash_bytes(pool, digest, (const unsigned char *)jwt->message, (unsigned int)strlen(jwt->message), (unsigned char **)&hash, (unsigned int *)&hash_len, err) == FALSE) {
+ 		apr_jwt_error(err, "apr_jws_hash_bytes");
 		goto end;
 	}
-	if (!EVP_VerifyUpdate(&ctx, jwt->message, strlen(jwt->message))) {
-		apr_jwt_error_openssl(err, "EVP_VerifyUpdate");
+
+    int rv = ECDSA_do_verify((const unsigned char *)hash, hash_len, ecdsa_sig, pubkey);
+ 	if (rv < 0) {
+ 		apr_jwt_error_openssl(err, "ECDSA_do_verify");
 		goto end;
-	}
-	if (!EVP_VerifyFinal(&ctx, (const unsigned char *) jwt->signature.bytes,
-			jwt->signature.length, pEcKey)) {
-		apr_jwt_error_openssl(err, "wrong key? EVP_VerifyFinal");
+ 	} else if (rv == 0) {
+ 		apr_jwt_error(err, "wrong key");
 		goto end;
-	}
+ 	}
 
 	rc = TRUE;
 
 end:
 
-	if (pEcKey) {
-		EVP_PKEY_free(pEcKey);
-	} else if (pubkey) {
+	if (ecdsa_sig)
+		ECDSA_SIG_free(ecdsa_sig);
+	if (pubkey) {
 		EC_KEY_free(pubkey);
 	}
-	EVP_MD_CTX_cleanup(&ctx);
 
 	return rc;
 }

test/test.c

@@ -203,6 +203,72 @@ static char *test_jwt_parse(apr_pool_t *pool) {
 	return 0;
 }
 
+#if (APR_JWS_EC_SUPPORT)
+
+static char *test_jwt_verify_ec(apr_pool_t *pool) {
+
+	/*
+	 * {
+	 *   "sub": "joe",
+	 *   "aud": "ac_oic_client",
+	 *   "jti": "oDWivWPJB47zkjOm2cygDv",
+	 *   "iss": "https://localhost:9031",
+	 *   "iat": 1467997207,
+	 *   "exp": 1467997507,
+	 *   "nonce": "WLxmv5StYyUk9JlWI8SaXTLPkGZ0Vs8aSTdj_VQ6rao"
+	 * }
+	 */
+	char *s_jwt = apr_pstrdup(pool, "eyJhbGciOiJFUzI1NiIsImtpZCI6ImY2cXRqIn0.eyJzdWIiOiJqb2UiLCJhdWQiOiJhY19vaWNfY2xpZW50IiwianRpIjoib0RXaXZXUEpCNDd6a2pPbTJjeWdEdiIsImlzcyI6Imh0dHBzOlwvXC9sb2NhbGhvc3Q6OTAzMSIsImlhdCI6MTQ2Nzk5NzIwNywiZXhwIjoxNDY3OTk3NTA3LCJub25jZSI6IldMeG12NVN0WXlVazlKbFdJOFNhWFRMUGtHWjBWczhhU1Rkal9WUTZyYW8ifQ.2kqX56QNow37gOlnfLn0SIzwie4mLLIUx_p9OSQa0hiUXKQWQLmMYBjIp5qGh2-R-KPHwNEBxqXwuPgXG4Y7Eg");
+	apr_jwt_t *jwt = NULL;
+	apr_jwt_error_t err;
+	TST_ASSERT_ERR("apr_jwt_parse",
+			apr_jwt_parse(pool, s_jwt, &jwt, NULL, &err), pool, err);
+
+	char *s_key = "{"
+		      "\"kty\": \"EC\","
+		      "\"kid\": \"f6qtj\","
+		      "\"use\": \"sig\","
+		      "\"x\": \"iARwFlN3B3xa8Zn_O-CVfqry68tXIhO9DckKo1yrNg0\","
+		      "\"y\": \"583S_mPS7YVZtLCjx2O69G_JzQPnMxjieOli-9cc_6Q\","
+		      "\"crv\": \"P-256\""
+		    "}";
+
+	apr_hash_t *keys = apr_hash_make(pool);
+	apr_jwk_t *jwk = NULL;
+	TST_ASSERT_ERR("apr_jwk_parse_json",
+			_jwk_parse(pool, s_key, &jwk, &err) == 0, pool, err);
+	apr_hash_set(keys, "f6qtj", APR_HASH_KEY_STRING, jwk);
+
+	TST_ASSERT_ERR("apr_jws_verify (ec0)", apr_jws_verify(pool, jwt, keys, &err),
+			pool, err);
+
+//	apr_jwt_destroy(jwt);
+//	jwt = NULL;
+//	TST_ASSERT_ERR("apr_jwt_parse (ec1)",
+//			apr_jwt_parse(pool, s_jwt, &jwt, NULL, &err), pool, err);
+
+	jwt->message[5] = 0xFF;
+
+	TST_ASSERT_ERR("apr_jws_verify (ec1)", apr_jws_verify(pool, jwt, keys, &err) == FALSE,
+			pool, err);
+
+	apr_jwt_destroy(jwt);
+	jwt = NULL;
+	TST_ASSERT_ERR("apr_jwt_parse (ec2)",
+			apr_jwt_parse(pool, s_jwt, &jwt, NULL, &err), pool, err);
+
+	jwt->signature.bytes[5] = 0xFF;
+
+	TST_ASSERT_ERR("apr_jws_verify (ec2)", apr_jws_verify(pool, jwt, keys, &err) == FALSE,
+			pool, err);
+
+	apr_jwt_destroy(jwt);
+
+	return 0;
+}
+
+#endif
+
 static char *test_jwt_verify_rsa(apr_pool_t *pool) {
 	/*
 	 * {
@@ -253,6 +319,26 @@ static char *test_jwt_verify_rsa(apr_pool_t *pool) {
 	TST_ASSERT_ERR("apr_jws_verify", apr_jws_verify(pool, jwt, keys, &err),
 			pool, err);
 
+	apr_jwt_destroy(jwt);
+	jwt = NULL;
+	TST_ASSERT_ERR("apr_jwt_parse (rsa1)",
+			apr_jwt_parse(pool, s_jwt, &jwt, NULL, &err), pool, err);
+
+	jwt->message[5] = 0xFF;
+
+	TST_ASSERT_ERR("apr_jws_verify (rsa1)", apr_jws_verify(pool, jwt, keys, &err) == FALSE,
+			pool, err);
+
+	apr_jwt_destroy(jwt);
+	jwt = NULL;
+	TST_ASSERT_ERR("apr_jwt_parse (rsa2)",
+			apr_jwt_parse(pool, s_jwt, &jwt, NULL, &err), pool, err);
+
+	jwt->signature.bytes[5] = 0xFF;
+
+	TST_ASSERT_ERR("apr_jws_verify (rsa2)", apr_jws_verify(pool, jwt, keys, &err) == FALSE,
+			pool, err);
+
 	apr_jwt_destroy(jwt);
 
 	return 0;
@@ -1137,6 +1223,10 @@ static char * all_tests(apr_pool_t *pool, request_rec *r) {
 #if (OPENSSL_VERSION_NUMBER >= 0x1000100f)	
 	TST_RUN(test_jwt_decrypt_gcm, pool);
 #endif
+#if (APR_JWS_EC_SUPPORT)
+	TST_RUN(test_jwt_verify_ec, pool);
+#endif
+
 	TST_RUN(test_jwt_verify_rsa, pool);
 	TST_RUN(test_jwt_sign_verify, pool);