Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history 
* chore:Fix a meson error.

* Fix potential security issue due to unknown key types.

HS256, HS384 & HS512 are symmetric key based algorithms that use an
array of bytes as the key.  The array of bytes could be the public key
of a RSxxx, ECxxx or PSxxx algorithm.  This means that unless the user
of this library validated that the jwt algorithm matched the expected
algorithm type(s), the user could be using a compromised JWT and not
realize it.

This fix requires the caller to declare if they expect a symmetric
cypher or not to prevent this attack and maintain the interface.  With
this change pub/private algorithms and symmetric algorithms are
segmented to protect the user from this kind of attack.
  • Loading branch information
@schmidtw
schmidtw authored Dec 19, 2024
1 parent 72f74ea commit 096ab3e
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 150 deletions.
6 changes: 5 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ int main( int argc, char *argv[] )

const char *hs_key = "hs256-secret";

rv = cjwt_decode( hs_text, strlen(hs_text), 0, (uint8_t*) hs_key, strlen(hs_key), 0, 0, &jwt );
rv = cjwt_decode( hs_text,
strlen(hs_text),
OPT_ALLOW_ONLY_HS_ALG,
(uint8_t*) hs_key,
strlen(hs_key), 0, 0, &jwt );
if( CJWTE_OK != rv ) {
printf( "There was an error processing the text: %d\n", rv );
return -1;
Expand Down
19 changes: 19 additions & 0 deletions include/cjwt/cjwt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@
*/
#define OPT_ALLOW_ANY_TYP (1 << 2)

/* If you specify OPT_ALLOW_ONLY_HS_ALG as part of the options bitmask you
* are allowing only the use of HMAC-SHA algorithms (HS256, HS384, HS512).
* Otherwise only public key algorithms are allowed.
*
* Symmetric algorithms and the unknown key type passed in pose a security
* risk since an attacker can treate the provided public key as the secret
* key and sign their own JWTs with the public key as a symmetric key.
*/
#define OPT_ALLOW_ONLY_HS_ALG (1 << 3)

/*----------------------------------------------------------------------------*/
/* Data Structures */
Expand Down Expand Up @@ -167,12 +176,22 @@ typedef struct {
* are accepted unless OPT_ALLOW_ANY_TIME is specified as an option.
*
* @note The key for HS signed JWTs is the plain text secret.
*
* @note To accept HS signed JWTs, OPT_ALLOW_ONLY_HS_ALG must be specified as
* an option. This option disables the use of public key algorithms.
* This is done to prevent an attacker from using a public key as a
* symmetric key and signing their own JWTs.
*
* @note The key for PS, RS and EC signed JWTs expect the text from the PEM
* file including the -----BEGIN PUBLIC KEY----- and
* -----END PUBLIC KEY----- lines.
*
* @note The 'time' parameter is seconds since Jan 1, 1970.
*
* @note It is strongly encouraged to validate the 'alg' header to ensure
* that the JWT is signed with the expected algorithm. This can be
* done by checking the 'alg' header against a known value in the cjwt_t
* object passed out via the jwt parameter.
*
* @param text [IN] the original JWT text
* @param text_len [IN] length of the original text
Expand Down
45 changes: 28 additions & 17 deletions src/cjwt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

#include <cjson/cJSON.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
Expand All @@ -22,26 +23,27 @@
/*----------------------------------------------------------------------------*/
struct alg_map {
cjwt_alg_t alg;
bool symmetric;
const char *text;
};

/*----------------------------------------------------------------------------*/
/* File Scoped Variables */
/*----------------------------------------------------------------------------*/
const struct alg_map the_alg_map[] = {
{ .alg = alg_none, .text = "none"},
{.alg = alg_es256, .text = "ES256"},
{.alg = alg_es384, .text = "ES384"},
{.alg = alg_es512, .text = "ES512"},
{.alg = alg_hs256, .text = "HS256"},
{.alg = alg_hs384, .text = "HS384"},
{.alg = alg_hs512, .text = "HS512"},
{.alg = alg_ps256, .text = "PS256"},
{.alg = alg_ps384, .text = "PS384"},
{.alg = alg_ps512, .text = "PS512"},
{.alg = alg_rs256, .text = "RS256"},
{.alg = alg_rs384, .text = "RS384"},
{.alg = alg_rs512, .text = "RS512"}
{.alg = alg_none, .symmetric = false, .text = "none" },
{.alg = alg_es256, .symmetric = false, .text = "ES256"},
{.alg = alg_es384, .symmetric = false, .text = "ES384"},
{.alg = alg_es512, .symmetric = false, .text = "ES512"},
{.alg = alg_hs256, .symmetric = true, .text = "HS256"},
{.alg = alg_hs384, .symmetric = true, .text = "HS384"},
{.alg = alg_hs512, .symmetric = true, .text = "HS512"},
{.alg = alg_ps256, .symmetric = false, .text = "PS256"},
{.alg = alg_ps384, .symmetric = false, .text = "PS384"},
{.alg = alg_ps512, .symmetric = false, .text = "PS512"},
{.alg = alg_rs256, .symmetric = false, .text = "RS256"},
{.alg = alg_rs384, .symmetric = false, .text = "RS384"},
{.alg = alg_rs512, .symmetric = false, .text = "RS512"}
};

/*----------------------------------------------------------------------------*/
Expand Down Expand Up @@ -243,12 +245,21 @@ static cjwt_code_t process_header_json(cjwt_t *cjwt, uint32_t options,
return CJWTE_HEADER_UNSUPPORTED_ALG;
}

if ((alg_none == cjwt->header.alg)
&& (0 == (OPT_ALLOW_ALG_NONE & options)))
{
return CJWTE_HEADER_UNSUPPORTED_ALG;
if (alg_none == cjwt->header.alg) {
if (0 == (OPT_ALLOW_ALG_NONE & options)) {
return CJWTE_HEADER_UNSUPPORTED_ALG;
}
}

if (true == the_alg_map[cjwt->header.alg].symmetric) {
if (!(OPT_ALLOW_ONLY_HS_ALG & options)) {
return CJWTE_HEADER_UNSUPPORTED_ALG;
}
} else {
if (OPT_ALLOW_ONLY_HS_ALG & options) {
return CJWTE_HEADER_UNSUPPORTED_ALG;
}
}

typ = cJSON_GetObjectItemCaseSensitive(json, "typ");
if (typ && (0 == (OPT_ALLOW_ANY_TYP & options))) {
Expand Down
2 changes: 1 addition & 1 deletion subprojects/trower-base64.wrap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ source_filename = trower-base64-1.2.7.tar.gz
source_url = https://github.com/xmidt-org/trower-base64/releases/download/v1.2.7/trower-base64-1.2.7.tar.gz
source_hash = 9921ea1eca2328c6cadc058df805e6ac58cca06c0737ac21b987ba13b8697ad9

[provides]
[provide]
libtrower_base64 = libtrower_base64_dep
140 changes: 73 additions & 67 deletions tests/test_cjwt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,77 +17,78 @@ typedef struct {
int expected;
const char *jwt_file_name;
bool is_key_in_file;
bool expect_symmetric;
const char *key;
const char *decode_test_name;
} test_case_t;

test_case_t test_list[] = {
{ 0, "jwtn.txt", false, "", "No Alg claims on on"},
{ 0, "jwtnx.txt", false, "", "No Alg claims off on"},
{ 0, "jwtny.txt", false, "", "No Alg claims off off"},
{ EINVAL, "jwtia.txt", false, "test_passwd1", "HS256 invalid jwt"},
{ EINVAL, "jwtib.txt", false, "test_passwd1", "HS256 invalid jwt"},
// { EINVAL, "jwtic.txt", false, "test_passwd1", "HS256 invalid jwt"}, /*TBD */ //FAILED test after modifying verify_signature logic
{ EINVAL, "jwtid.txt", false, "test_passwd1", "HS256 invalid jwt"},
{ EINVAL, "jwtie.txt", false, "test_passwd1", "HS256 invalid jwt"},
{ EINVAL, "jwtif.txt", false, "test_passwd1", "HS256 invalid jwt"},
{ 0, "jwt1.txt", false, "test_passwd1", "HS256 claims on on"},
{ EINVAL, "jwt1.txt", false, "test_passbad", "HS256 claims on on"},
{ 0, "jwt2.txt", false, "test_passwd2", "HS384 claims on on"},
{ EINVAL, "jwt2.txt", false, "test_passbad", "HS384 claims on on"},
{ 0, "jwt3.txt", false, "test_passwd3", "HS512 claims on on"},
{ EINVAL, "jwt3.txt", false, "test_passbad", "HS512 claims on on"},
{ 0, "jwt5.txt", true, "pubkey5.pem", "RS384 claims on on"},
{ EINVAL, "jwt5.txt", true, "badkey4.pem", "RS384 claims on on"},
{ 0, "jwt4.txt", true, "pubkey4.pem", "RS256 claims on on"},
{ EINVAL, "jwt4.txt", true, "badkey4.pem", "RS256 claims on on"},
{ 0, "jwt6.txt", true, "pubkey6.pem", "RS512 claims on on"},
{ EINVAL, "jwt6.txt", true, "badkey6.pem", "RS512 claims on on"},
{ 0, "jwt1x.txt", false, "test_passwd1", "HS256 claims off on"},
{ EINVAL, "jwt1x.txt", false, "test_prasswd1", "HS256 claims off on"},
{ 0, "jwt2x.txt", false, "test_passwd2", "HS384 claims off on"},
{ EINVAL, "jwt2x.txt", false, "twest_passwd2", "HS384 claims off on"},
{ 0, "jwt3x.txt", false, "test_passwd3", "HS512 claims off on"},
{ EINVAL, "jwt3x.txt", false, "test_passwd3...", "HS512 claims off on"},
{ 0, "jwt4x.txt", true, "pubkey4.pem", "RS256 claims off on"},
{ EINVAL, "jwt4x.txt", true, "pubkey5.pem", "RS256 claims off on"},
{ 0, "jwt5x.txt", true, "pubkey5.pem", "RS384 claims off on"},
{ EINVAL, "jwt5x.txt", true, "badkey5.pem", "RS384 claims off on"},
{ 0, "jwt6x.txt", true, "pubkey6.pem", "RS512 claims off on"},
{ EINVAL, "jwt6x.txt", true, "badkey6.pem", "RS512 claims off on"},
{ 0, "jwt1y.txt", false, "test_passwd1", "HS256 claims off off"},
{ EINVAL, "jwt1y.txt", false, "tast_passwd1", "HS256 claims off off"},
{ 0, "jwt2y.txt", false, "test_passwd2", "HS384 claims off off"},
{ EINVAL, "jwt2y.txt", false, "test..passwd2", "HS384 claims off off"},
{ 0, "jwt3y.txt", false, "test_passwd3", "HS512 claims off off"},
{ EINVAL, "jwt3y.txt", false, "tteesstt_passwd3", "HS512 claims off off"},
{ 0, "jwt4y.txt", true, "pubkey4.pem", "RS256 claims off off"},
{ EINVAL, "jwt4y.txt", true, "badkey4.pem", "RS256 claims off off"},
{ 0, "jwt5y.txt", true, "pubkey5.pem", "RS384 claims off off"},
{ EINVAL, "jwt5y.txt", true, "pubkey6.pem", "RS384 claims off off"},
{ 0, "jwt6y.txt", true, "pubkey6.pem", "RS512 claims off off"},
{ EINVAL, "jwt6y.txt", true, "pubkey5.pem", "RS512 claims off off"},
{ 0, "jwt1l.txt", false, "test_passwd1", "HS256 claims long"},
{ EINVAL, "jwt1l.txt", false, "test_keyword1", "HS256 claims long"},
{ 0, "jwt2l.txt", false, "test_passwd2", "HS384 claims long"},
{ EINVAL, "jwt2l.txt", false, "test_passwd1", "HS384 claims long"},
{ 0, "jwt3l.txt", false, "test_passwd3", "HS512 claims long"},
{ EINVAL, "jwt3l.txt", false, "passwd3", "HS512 claims long"},
{ 0, "jwt4l.txt", true, "pubkey4.pem", "RS256 claims long"},
{ EINVAL, "jwt4l.txt", true, "badkey4.pem", "RS256 claims long"},
{ 0, "jwt5l.txt", true, "pubkey5.pem", "RS384 claims long"},
{ EINVAL, "jwt5l.txt", true, "badkey5.pem", "RS384 claims long"},
{ 0, "jwt6l.txt", true, "pubkey6.pem", "RS512 claims long"},
{ EINVAL, "jwt6l.txt", true, "badkey6.pem", "RS512 claims long"},
{ 0, "jwt2.txt", false, "test_passwd2", "HS384 claims on on"},
{ 0, "jwt3.txt", false, "test_passwd3", "HS512 claims on on"},
{ 0, "jwt8_hs256.txt", true, "key8_hs256.pem", "HS256 claims on on"},
{ 0, "jwt9_hs384.txt", true, "key9_hs384.pem", "HS384 claims on on"},
{ 0, "jwt10_hs512.txt", true, "key10_hs512.pem", "HS512 claims on on"},
{ EINVAL, "jwt11.txt", false, "incorrect_key", "RS256 claims all"},
{ EINVAL, "jwt12.txt", false, "incorrect_key", "RS256 claims all"},
{ EINVAL, "jwt13.txt", false, "incorrect_key", "RS256 claims all"},
{ENOTSUP, "jwtbadalg.txt", false, "incorrect_key", "Invalid/unsupported alg."}
{ 0, "jwtn.txt", false, false, "", "No Alg claims on on"},
{ 0, "jwtnx.txt", false, false, "", "No Alg claims off on"},
{ 0, "jwtny.txt", false, false, "", "No Alg claims off off"},
{ EINVAL, "jwtia.txt", false, true, "test_passwd1", "HS256 invalid jwt"},
{ EINVAL, "jwtib.txt", false, true, "test_passwd1", "HS256 invalid jwt"},
// { EINVAL, "jwtic.txt", false, true, "test_passwd1", "HS256 invalid jwt"}, /*TBD */ //FAILED test after modifying verify_signature logic
{ EINVAL, "jwtid.txt", false, true, "test_passwd1", "HS256 invalid jwt"},
{ EINVAL, "jwtie.txt", false, true, "test_passwd1", "HS256 invalid jwt"},
{ EINVAL, "jwtif.txt", false, true, "test_passwd1", "HS256 invalid jwt"},
{ 0, "jwt1.txt", false, true, "test_passwd1", "HS256 claims on on"},
{ EINVAL, "jwt1.txt", false, true, "test_passbad", "HS256 claims on on"},
{ 0, "jwt2.txt", false, true, "test_passwd2", "HS384 claims on on"},
{ EINVAL, "jwt2.txt", false, true, "test_passbad", "HS384 claims on on"},
{ 0, "jwt3.txt", false, true, "test_passwd3", "HS512 claims on on"},
{ EINVAL, "jwt3.txt", false, true, "test_passbad", "HS512 claims on on"},
{ 0, "jwt5.txt", true, false, "pubkey5.pem", "RS384 claims on on"},
{ EINVAL, "jwt5.txt", true, false, "badkey4.pem", "RS384 claims on on"},
{ 0, "jwt4.txt", true, false, "pubkey4.pem", "RS256 claims on on"},
{ EINVAL, "jwt4.txt", true, false, "badkey4.pem", "RS256 claims on on"},
{ 0, "jwt6.txt", true, false, "pubkey6.pem", "RS512 claims on on"},
{ EINVAL, "jwt6.txt", true, false, "badkey6.pem", "RS512 claims on on"},
{ 0, "jwt1x.txt", false, true, "test_passwd1", "HS256 claims off on"},
{ EINVAL, "jwt1x.txt", false, true, "test_prasswd1", "HS256 claims off on"},
{ 0, "jwt2x.txt", false, true, "test_passwd2", "HS384 claims off on"},
{ EINVAL, "jwt2x.txt", false, true, "twest_passwd2", "HS384 claims off on"},
{ 0, "jwt3x.txt", false, true, "test_passwd3", "HS512 claims off on"},
{ EINVAL, "jwt3x.txt", false, true, "test_passwd3...", "HS512 claims off on"},
{ 0, "jwt4x.txt", true, false, "pubkey4.pem", "RS256 claims off on"},
{ EINVAL, "jwt4x.txt", true, false, "pubkey5.pem", "RS256 claims off on"},
{ 0, "jwt5x.txt", true, false, "pubkey5.pem", "RS384 claims off on"},
{ EINVAL, "jwt5x.txt", true, false, "badkey5.pem", "RS384 claims off on"},
{ 0, "jwt6x.txt", true, false, "pubkey6.pem", "RS512 claims off on"},
{ EINVAL, "jwt6x.txt", true, false, "badkey6.pem", "RS512 claims off on"},
{ 0, "jwt1y.txt", false, true, "test_passwd1", "HS256 claims off off"},
{ EINVAL, "jwt1y.txt", false, true, "tast_passwd1", "HS256 claims off off"},
{ 0, "jwt2y.txt", false, true, "test_passwd2", "HS384 claims off off"},
{ EINVAL, "jwt2y.txt", false, true, "test..passwd2", "HS384 claims off off"},
{ 0, "jwt3y.txt", false, true, "test_passwd3", "HS512 claims off off"},
{ EINVAL, "jwt3y.txt", false, true, "tteesstt_passwd3", "HS512 claims off off"},
{ 0, "jwt4y.txt", true, false, "pubkey4.pem", "RS256 claims off off"},
{ EINVAL, "jwt4y.txt", true, false, "badkey4.pem", "RS256 claims off off"},
{ 0, "jwt5y.txt", true, false, "pubkey5.pem", "RS384 claims off off"},
{ EINVAL, "jwt5y.txt", true, false, "pubkey6.pem", "RS384 claims off off"},
{ 0, "jwt6y.txt", true, false, "pubkey6.pem", "RS512 claims off off"},
{ EINVAL, "jwt6y.txt", true, false, "pubkey5.pem", "RS512 claims off off"},
{ 0, "jwt1l.txt", false, true, "test_passwd1", "HS256 claims long"},
{ EINVAL, "jwt1l.txt", false, true, "test_keyword1", "HS256 claims long"},
{ 0, "jwt2l.txt", false, true, "test_passwd2", "HS384 claims long"},
{ EINVAL, "jwt2l.txt", false, true, "test_passwd1", "HS384 claims long"},
{ 0, "jwt3l.txt", false, true, "test_passwd3", "HS512 claims long"},
{ EINVAL, "jwt3l.txt", false, true, "passwd3", "HS512 claims long"},
{ 0, "jwt4l.txt", true, false, "pubkey4.pem", "RS256 claims long"},
{ EINVAL, "jwt4l.txt", true, false, "badkey4.pem", "RS256 claims long"},
{ 0, "jwt5l.txt", true, false, "pubkey5.pem", "RS384 claims long"},
{ EINVAL, "jwt5l.txt", true, false, "badkey5.pem", "RS384 claims long"},
{ 0, "jwt6l.txt", true, false, "pubkey6.pem", "RS512 claims long"},
{ EINVAL, "jwt6l.txt", true, false, "badkey6.pem", "RS512 claims long"},
{ 0, "jwt2.txt", false, true, "test_passwd2", "HS384 claims on on"},
{ 0, "jwt3.txt", false, true, "test_passwd3", "HS512 claims on on"},
{ 0, "jwt8_hs256.txt", true, true, "key8_hs256.pem", "HS256 claims on on"},
{ 0, "jwt9_hs384.txt", true, true, "key9_hs384.pem", "HS384 claims on on"},
{ 0, "jwt10_hs512.txt", true, true, "key10_hs512.pem", "HS512 claims on on"},
{ EINVAL, "jwt11.txt", false, false, "incorrect_key", "RS256 claims all"},
{ EINVAL, "jwt12.txt", false, false, "incorrect_key", "RS256 claims all"},
{ EINVAL, "jwt13.txt", false, false, "incorrect_key", "RS256 claims all"},
{ENOTSUP, "jwtbadalg.txt", false, false, "incorrect_key", "Invalid/unsupported alg."}
};

#define _NUM_TEST_CASES (sizeof(test_list) / sizeof(test_case_t))
Expand Down Expand Up @@ -154,11 +155,13 @@ void test_case(unsigned _i)
cjwt_t *jwt = NULL;
char jwt_buf[65535];
char pem_buf[8192];
int options;
jwt_fname = test_list[_i].jwt_file_name;
key_str = test_list[_i].key;
key_len = strlen(key_str);
expected = test_list[_i].expected;
decode_test_name = test_list[_i].decode_test_name;
options = OPT_ALLOW_ALG_NONE | OPT_ALLOW_ANY_TIME;

if (key_len == 0) {
key_str = NULL;
Expand Down Expand Up @@ -188,8 +191,11 @@ void test_case(unsigned _i)
jwt_bytes = read_file(jwt_fname, jwt_buf, sizeof(jwt_buf));

if (jwt_bytes > 0) {
if (test_list[_i].expect_symmetric) {
options |= OPT_ALLOW_ONLY_HS_ALG;
}
result = cjwt_decode(jwt_buf, strlen(jwt_buf),
OPT_ALLOW_ALG_NONE | OPT_ALLOW_ANY_TIME,
options,
(const uint8_t *) key_str, key_len,
0, 0, &jwt);
} else {
Expand Down
Loading

0 comments on commit 096ab3e

Please sign in to comment.