2.4.16.4rc2: fix oidc_jwk_copy wrt. "x5t"

which broke private_key_jwt authentication to Azure AD since 2.4.13; see #1269; thanks @uoe-pjackson Signed-off-by: Hans Zandbelt <[email protected]>
OpenIDC · Sep 26, 2024 · 0da9545 · 0da9545
1 parent 93bdf9d
commit 0da9545
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 21 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,8 @@
+09/26/2024
+- fix oidc_jwk_copy wrt. "x5t", which broke private_key_jwt authentication to Azure AD since 2.4.13
+  see #1269; thanks @uoe-pjackson
+- bump to 2.4.16.4rc2
+
 09/21/2024
 - refactor state and userinfo
 

diff --git a/configure.ac b/configure.ac
@@ -1,4 +1,4 @@
-AC_INIT([mod_auth_openidc],[2.4.16.4rc1],[[email protected]])
+AC_INIT([mod_auth_openidc],[2.4.16.4rc2],[[email protected]])
 
 AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())
 

diff --git a/src/jose.c b/src/jose.c
@@ -332,17 +332,17 @@ static oidc_jwk_t *oidc_jwk_from_cjose(apr_pool_t *pool, cjose_jwk_t *cjose_jwk,
 /*
  * parse a JSON string to a JWK struct
  */
-oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, const char *s_json, oidc_jose_error_t *err) {
+oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, json_t *json, oidc_jose_error_t *err) {
 	oidc_jwk_t *result = NULL;
 	cjose_jwk_t *cjose_jwk = NULL;
 	cjose_err cjose_err;
-	json_error_t json_error;
 	oidc_jose_error_t x5c_err;
 	char *use = NULL;
+	json_t *v = NULL;
 
-	json_t *json = json_loads(s_json, 0, &json_error);
-	if (json == NULL) {
-		oidc_jose_error(err, "could not parse JWK: %s (%s)", json_error.text, s_json);
+	char *s_json = json_dumps(json, 0);
+	if (s_json == NULL) {
+		oidc_jose_error(err, "could not serialize JWK");
 		goto end;
 	}
 
@@ -362,10 +362,22 @@ oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, const char *s_json, oidc_jose_error
 
 	result = oidc_jwk_from_cjose(pool, cjose_jwk, use);
 
+	// TODO: parse the optional x5c array
+
+	// set x5t#256
+	v = json_object_get(json, OIDC_JOSE_JWK_X5T256_STR);
+	if (v)
+		result->x5t_S256 = apr_pstrdup(pool, json_string_value(v));
+
+	// set x5t
+	v = json_object_get(json, OIDC_JOSE_JWK_X5T_STR);
+	if (v)
+		result->x5t = apr_pstrdup(pool, json_string_value(v));
+
 end:
 
-	if (json)
-		json_decref(json);
+	if (s_json)
+		free(s_json);
 
 	return result;
 }
@@ -374,11 +386,22 @@ oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, const char *s_json, oidc_jose_error
  * copy a JWK by converting oidc_jwk_t to JSON and parsing it back
  */
 oidc_jwk_t *oidc_jwk_copy(apr_pool_t *pool, const oidc_jwk_t *src) {
-	char *s_json = NULL;
-	oidc_jose_error_t err;
-	if (oidc_jwk_to_json(pool, src, &s_json, &err) == FALSE)
-		return NULL;
-	return oidc_jwk_parse(pool, s_json, &err);
+	int i = 0;
+	cjose_err err;
+	oidc_jwk_t *dst = oidc_jwk_new(pool);
+	dst->cjose_jwk = cjose_jwk_retain(src->cjose_jwk, &err);
+	dst->kid = apr_pstrdup(pool, src->kid);
+	dst->kty = src->kty;
+	dst->use = apr_pstrdup(pool, src->use);
+	dst->x5c = NULL;
+	if (src->x5c) {
+		dst->x5c = apr_array_make(pool, src->x5c->nelts, sizeof(char *));
+		for (i = 0; i < src->x5c->nelts; i++)
+			APR_ARRAY_PUSH(dst->x5c, char *) = APR_ARRAY_IDX(src->x5c, i, char *);
+	}
+	dst->x5t = apr_pstrdup(pool, src->x5t);
+	dst->x5t_S256 = apr_pstrdup(pool, src->x5t_S256);
+	return dst;
 }
 
 /*
@@ -443,9 +466,7 @@ void oidc_jwk_list_destroy(apr_array_header_t *keys_list) {
  * parse a JSON object in to a JWK struct
  */
 apr_byte_t oidc_jwk_parse_json(apr_pool_t *pool, json_t *json, oidc_jwk_t **jwk, oidc_jose_error_t *err) {
-	char *s_json = json_dumps(json, 0);
-	*jwk = oidc_jwk_parse(pool, s_json, err);
-	cjose_get_dealloc()(s_json);
+	*jwk = oidc_jwk_parse(pool, json, err);
 	return (*jwk != NULL);
 }
 

diff --git a/src/jose.h b/src/jose.h
@@ -174,7 +174,7 @@ typedef struct oidc_jwk_t {
 apr_byte_t oidc_jwe_decrypt(apr_pool_t *pool, const char *input_json, apr_hash_t *keys, char **plaintext,
 			    int *plaintext_len, oidc_jose_error_t *err, apr_byte_t import_must_succeed);
 /* parse a JSON string (JWK) to a JWK struct */
-oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, const char *s_json, oidc_jose_error_t *err);
+oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, json_t *json, oidc_jose_error_t *err);
 oidc_jwk_t *oidc_jwk_copy(apr_pool_t *pool, const oidc_jwk_t *jwk);
 /* parse a JSON object (JWK) in to a JWK struct */
 apr_byte_t oidc_jwk_parse_json(apr_pool_t *pool, json_t *json, oidc_jwk_t **jwk, oidc_jose_error_t *err);

diff --git a/test/test-cmd.c b/test/test-cmd.c
@@ -163,8 +163,11 @@ int verify(int argc, char **argv, apr_pool_t *pool) {
 		return -1;
 	}
 
+	json_error_t json_error;
+	json_t *json = json_loads(s_jwk, 0, &json_error);
+
 	oidc_jose_error_t oidc_err;
-	oidc_jwk_t *jwk = oidc_jwk_parse(pool, s_jwk, &oidc_err);
+	oidc_jwk_t *jwk = oidc_jwk_parse(pool, json, &oidc_err);
 	if (jwk == NULL) {
 		fprintf(stderr, "could not import JWK: %s [file: %s, function: %s, line: %d]\n", oidc_err.text,
 			oidc_err.source, oidc_err.function, oidc_err.line);
@@ -189,6 +192,7 @@ int verify(int argc, char **argv, apr_pool_t *pool) {
 
 	cjose_jws_release(jws);
 	oidc_jwk_destroy(jwk);
+	json_decref(json);
 
 	return 0;
 }
@@ -206,9 +210,12 @@ int decrypt(int argc, char **argv, apr_pool_t *pool) {
 		return -1;
 
 	apr_hash_t *keys = apr_hash_make(pool);
-	oidc_jose_error_t oidc_err;
 
-	oidc_jwk_t *jwk = oidc_jwk_parse(pool, s_jwk, &oidc_err);
+	json_error_t json_error;
+	json_t *json = json_loads(s_jwk, 0, &json_error);
+
+	oidc_jose_error_t oidc_err;
+	oidc_jwk_t *jwk = oidc_jwk_parse(pool, json, &oidc_err);
 	if (jwk == NULL) {
 		fprintf(stderr, "could not import JWK: %s [file: %s, function: %s, line: %d]\n", oidc_err.text,
 			oidc_err.source, oidc_err.function, oidc_err.line);
@@ -226,6 +233,7 @@ int decrypt(int argc, char **argv, apr_pool_t *pool) {
 
 	fprintf(stdout, "%s", plaintext);
 	oidc_jwk_destroy(jwk);
+	json_decref(json);
 
 	return 0;
 }

diff --git a/test/test.c b/test/test.c
@@ -119,9 +119,12 @@ static int TST_RC;
 		return message;
 
 static char *_jwk_parse(apr_pool_t *pool, const char *s, oidc_jwk_t **jwk, oidc_jose_error_t *err) {
-	oidc_jwk_t *k = oidc_jwk_parse(pool, s, err);
+	json_error_t json_err;
+	json_t *json = json_loads(s, 0, &json_err);
+	oidc_jwk_t *k = oidc_jwk_parse(pool, json, err);
 	TST_ASSERT_ERR("oidc_jwk_parse", k != NULL, pool, (*err));
 	*jwk = k;
+	json_decref(json);
 	return 0;
 }
 
@@ -777,6 +780,42 @@ static char *test_jwk_parse_json(apr_pool_t *pool) {
 	return 0;
 }
 
+static char *test_jwk_copy(apr_pool_t *pool) {
+	oidc_jose_error_t err;
+	char *s = NULL;
+	oidc_jwk_t *jwk1 = NULL;
+	oidc_jwk_t *jwk2 = NULL;
+
+	s = "{"
+	    "\"kty\": \"RSA\","
+	    "\"kid\": \"[email protected]\","
+	    "\"use\": \"sig\","
+	    "\"n\": \"n4EPtAOCc9AlkeQHPzHStgAbgs7bTZLwUBZdR8_KuKPEHLd4rHVTeT"
+	    "-O-XV2jRojdNhxJWTDvNd7nqQ0VEiZQHz_AJmSCpMaJMRBSFKrKb2wqV"
+	    "wGU_NsYOYL-QtiWN2lbzcEe6XC0dApr5ydQLrHqkHHig3RBordaZ6Aj-"
+	    "oBHqFEHYpPe7Tpe-OfVfHd1E6cS6M1FZcD1NNLYD5lFHpPI9bTwJlsde"
+	    "3uhGqC0ZCuEHg8lhzwOHrtIQbS0FVbb9k3-tVTU4fg_3L_vniUFAKwuC"
+	    "LqKnS2BYwdq_mzSnbLY7h_qixoR7jig3__kRhuaxwUkRz5iaiQkqgc5g"
+	    "HdrNP5zw\","
+	    "\"e\": \"AQAB\","
+	    "\"x5t\": \"myx5t\","
+	    "\"x5t#S256\": \"myx5t#S256\""
+	    "}";
+
+	jwk1 = NULL;
+	TST_ASSERT_ERR("oidc_jwk_parse", _jwk_parse(pool, s, &jwk1, &err) == 0, pool, err);
+	jwk2 = oidc_jwk_copy(pool, jwk1);
+
+	TST_ASSERT_STR("oidc_jwk_parse (x5t)", jwk1->x5t, "myx5t");
+	TST_ASSERT_STR("oidc_jwk_parse (x5t#S256)", jwk1->x5t_S256, "myx5t#S256");
+	TST_ASSERT_STR("oidc_jwk_copy (x5t)", jwk2->x5t, "myx5t");
+	TST_ASSERT_STR("oidc_jwk_copy (x5t#S256)", jwk2->x5t_S256, "myx5t#S256");
+
+	oidc_jwk_destroy(jwk2);
+	oidc_jwk_destroy(jwk1);
+
+	return 0;
+}
 static char *test_plaintext_decrypt_symmetric(apr_pool_t *pool) {
 	oidc_jose_error_t err;
 	apr_hash_t *keys = apr_hash_make(pool);
@@ -1703,6 +1742,7 @@ static char *all_tests(apr_pool_t *pool, request_rec *r) {
 	TST_RUN(test_jwt_get_string, pool);
 
 	TST_RUN(test_jwk_parse_json, pool);
+	TST_RUN(test_jwk_copy, pool);
 	TST_RUN(test_plaintext_decrypt_symmetric, pool);
 
 #if (OPENSSL_VERSION_NUMBER >= 0x1000100f)