Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request #29 from xmidt-org/correct-return-codes-and-elimin…
…iate-silent-failure

Correct return codes and eliminiate silent failure
  • Loading branch information
shilpa24balaji committed Nov 25, 2019
2 parents c68ceb4 + ec13808 commit 9304d3e
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 92 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -7,10 +7,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
## [Unreleased]
### Added
- Valgrind checking and fixes.
- Algorithms that are unsupported now are not mapped to alg=none to prevent untrusted
accidental acceptance of JWT.

### Changed
- Fixed memory leaks.
- Updated the CONTRIBUTION document.
- Updated the cjwt_decode() documentation to be accurate and consistent.

## [1.0.1]
### Added
Expand Down
35 changes: 18 additions & 17 deletions src/cjwt.c
Expand Up @@ -117,16 +117,6 @@ int cjwt_alg_str_to_enum( const char *alg_str )
return -1;
}

static cjwt_alg_t __cjwt_alg_str_to_enum( const char *alg_str )
{
int alg = cjwt_alg_str_to_enum (alg_str);

if (alg >= 0)
return alg;
else
return alg_none;
}


inline static void cjwt_delete_child_json( cJSON* j, const char* s )
{
Expand Down Expand Up @@ -357,12 +347,14 @@ static int cjwt_verify_signature( cjwt_t *p_jwt, char *p_in, const char *p_sign
if( sz_signed != out_size ) {
cjwt_info( "Signature length mismatch: enc %d, signature %d\n",
( int )sz_signed, ( int )out_size );
ret = -1;
ret = EINVAL;
goto err_match;
}

ret = CRYPTO_memcmp(
( unsigned char* )signed_out, ( unsigned char* )signed_dec, out_size );
if( 0 != CRYPTO_memcmp(signed_out, signed_dec, out_size) ) {
ret = EINVAL;
}

err_match:
free( signed_dec );
err_decode:
Expand All @@ -384,7 +376,8 @@ static int cjwt_update_payload( cjwt_t *p_cjwt, char *p_decpl )
cJSON *j_payload = cJSON_Parse( ( char* )p_decpl );

if( !j_payload ) {
return ENOMEM;
// The data is probably not json vs. memory allocation error.
return EINVAL;
}

//extract data
Expand Down Expand Up @@ -562,6 +555,7 @@ static int cjwt_update_payload( cjwt_t *p_cjwt, char *p_decpl )
}

static int cjwt_update_header( cjwt_t *p_cjwt, char *p_dechead )
// The data is probably not json vs. memory allocation error.
{
if( !p_cjwt || !p_dechead ) {
return EINVAL;
Expand All @@ -571,7 +565,8 @@ static int cjwt_update_header( cjwt_t *p_cjwt, char *p_dechead )
cJSON *j_header = cJSON_Parse( ( char* )p_dechead );

if( !j_header ) {
return ENOMEM;
// The data is probably not json vs. memory allocation error.
return EINVAL;
}

cjwt_info( "Json = %s\n", cJSON_Print( j_header ) );
Expand All @@ -586,7 +581,14 @@ static int cjwt_update_header( cjwt_t *p_cjwt, char *p_dechead )
cJSON* j_alg = cJSON_GetObjectItem( j_header, "alg" );

if( j_alg ) {
p_cjwt->header.alg = __cjwt_alg_str_to_enum( j_alg->valuestring );
int alg;

alg = cjwt_alg_str_to_enum( j_alg->valuestring );
if( -1 == alg ) {
cJSON_Delete( j_header );
return ENOTSUP;
}
p_cjwt->header.alg = alg;
}

//destroy cJSON object
Expand Down Expand Up @@ -711,7 +713,6 @@ int cjwt_decode( const char *encoded, unsigned int options, cjwt_t **jwt,
int ret = 0;
char *payload, *signature;
( void )options; //suppressing unused parameter warning
( void ) options;

//validate inputs
if( !encoded || !jwt ) {
Expand Down
10 changes: 5 additions & 5 deletions src/cjwt.h
Expand Up @@ -93,7 +93,7 @@ typedef struct {
* The function to use to decode and validate a JWT.
*
* @note This function allocates memory associated with the output jwt that
* must be freed. cjwt_destroy() must be called to destry the object
* must be freed. cjwt_destroy() must be called to destroy the object
* when we are done with it.
*
* @note This function does not
Expand All @@ -105,10 +105,10 @@ typedef struct {
* @param key [IN] the public key to use for validating the signature
* @param key_len [IN] the length of the key in bytes
*
* @retval 0 successful
* @retval -1 invalid jwt format
* @retval -2 mismatched key
* ... etc
* @retval 0 successful
* @retval EINVAL invalid jwt format or mismatched key
* @retval ENOMEM unable to allocate needed memory
* @retval ENOTSUP unsupported algorithm
*/
int cjwt_decode( const char *encoded, unsigned int options, cjwt_t **jwt,
const uint8_t *key, size_t key_len );
Expand Down
1 change: 1 addition & 0 deletions tests/inputs/jwtbadalg.txt
@@ -0,0 +1 @@
eyAiYWxnIjogImludmFsaWQiLCAidHlwIjogIkpXVCIgfQ.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c
141 changes: 71 additions & 70 deletions tests/test_cjwt.c
Expand Up @@ -29,79 +29,80 @@
#include "../src/cjwt.h"

typedef struct {
bool expected;
int expected;
const char *jwt_file_name;
bool is_key_in_file;
const char *key;
const char *decode_test_name;
} test_case_t;

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

#define _NUM_TEST_CASES ( sizeof(test_list) / sizeof(test_case_t) )
Expand Down Expand Up @@ -161,7 +162,7 @@ void test_case (unsigned _i )
const char *jwt_fname;
const char *key_str;
const char *decode_test_name;
bool expected;
int expected;
int key_len;
ssize_t jwt_bytes;
int result = 0;
Expand Down Expand Up @@ -207,16 +208,16 @@ void test_case (unsigned _i )
result = jwt_bytes;
}

if( expected == ( result == 0 ) ) {
if( expected == result ) {
printf( "--- PASSED: %s\n", decode_test_name );
pass_cnt += 1;
} else {
printf( "--- FAILED: %s\n", decode_test_name );
printf( "\e[01;31m--- FAILED: %s (%d != %d)\e[00m\n", decode_test_name, expected, result );
fail_cnt += 1;
}

cjwt_destroy( &jwt );
CU_ASSERT_EQUAL ( expected, ( result == 0 ) );
CU_ASSERT ( expected == result );
}


Expand Down

0 comments on commit 9304d3e

Please sign in to comment.