Skip to content

Commit

Permalink
hash the cache key if it is larger than 512 bytes
Browse files Browse the repository at this point in the history
so large cache key entries (i.e. for JWT tokens) are no longer a problem
in unencrypted SHM cache configs, i.e. the default shared memory cache
setup; see issues/discussions on "could not construct cache key since
key size is too large"

this is relevant for OAuth 2.0 RS configs with JWT access tokens and
OpenID Connect RP configs where
OIDCRefreshAccessTokenBeforeExpiry/OIDCUserInfoRefreshInterval is set
and a JWT refresh token is returned

also fix debug printout of cache key in oidc_cache_get

Signed-off-by: Hans Zandbelt <hans.zandbelt@openidc.com>
  • Loading branch information
zandbelt committed Feb 29, 2024
1 parent b2f933e commit d2b36e5
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 10 deletions.
9 changes: 9 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
02/29/2024
- hash the cache key if it is larger than 512 bytes so large cache key entries (i.e. for JWT tokens)
are no longer a problem in unencrypted SHM cache configs, i.e. the default shared memory cache setup;
see issues/discussions on "could not construct cache key since key size is too large"
this is relevant for OAuth 2.0 RS configs with JWT access tokens and OpenID Connect RP
configs where OIDCRefreshAccessTokenBeforeExpiry/OIDCUserInfoRefreshInterval is set and
a JWT refresh token is returned
- fix debug printout of cache key in oidc_cache_get

02/28/2024
- optimize performance by skipping JSON processing when `OIDCPassClaimsAs none` is set

Expand Down
7 changes: 7 additions & 0 deletions src/cache/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@
#include <apr_shm.h>
#include <httpd.h>

/*
* The maximum cache key size:
* must be greater than a base64url encoded sha256 hash result (32 bytes) and must
* be a multiple of 8 since OIDC_CACHE_SHM_KEY_MAX is derived from it (memory alignment)
*/
#define OIDC_CACHE_KEY_SIZE_MAX 512

typedef void *(*oidc_cache_cfg_create)(apr_pool_t *pool);
typedef int (*oidc_cache_post_config_function)(server_rec *s);
typedef int (*oidc_cache_child_init_function)(apr_pool_t *p, server_rec *s);
Expand Down
27 changes: 18 additions & 9 deletions src/cache/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,18 +244,24 @@ static apr_byte_t oidc_cache_crypto_decrypt(request_rec *r, const char *cache_va
}

/*
* hash a cache key and a crypto passphrase so the result is suitable as an randomized cache key
* hash a cache key, useful for large keys e.g. JWT access/refresh tokens
*/
static char *oidc_cache_get_hashed_key(request_rec *r, const char *passphrase, const char *key) {
const char *input = apr_psprintf(r->pool, "%s:%s", passphrase, key);
static char *oidc_cache_get_hashed_key(request_rec *r, const char *key) {
char *output = NULL;
if (oidc_util_hash_string_and_base64url_encode(r, OIDC_JOSE_ALG_SHA256, input, &output) == FALSE) {
if (oidc_util_hash_string_and_base64url_encode(r, OIDC_JOSE_ALG_SHA256, key, &output) == FALSE) {
oidc_error(r, "oidc_util_hash_string_and_base64url_encode returned an error");
return NULL;
}
return output;
}

/*
* hash a cache key plus a crypto passphrase so the result is suitable as an randomized cache key
*/
static char *oidc_cache_get_hashed_key_secret(request_rec *r, const char *passphrase, const char *key) {
return oidc_cache_get_hashed_key(r, apr_psprintf(r->pool, "%s:%s", passphrase, key));
}

/*
* get a key/value string pair from the cache, possibly decrypting it
*/
Expand All @@ -268,7 +274,7 @@ apr_byte_t oidc_cache_get(request_rec *r, const char *section, const char *key,
const char *s_key = NULL;
char *cache_value = NULL, *s_secret = NULL;

oidc_debug(r, "enter: %s (section=%s, decrypt=%d, type=%s)", section, section, encrypted, cfg->cache->name);
oidc_debug(r, "enter: %s (section=%s, decrypt=%d, type=%s)", key, section, encrypted, cfg->cache->name);

s_key = key;
/* see if encryption is turned on */
Expand All @@ -278,7 +284,9 @@ apr_byte_t oidc_cache_get(request_rec *r, const char *section, const char *key,
goto out;
}
s_secret = cfg->crypto_passphrase.secret1;
s_key = oidc_cache_get_hashed_key(r, s_secret, key);
s_key = oidc_cache_get_hashed_key_secret(r, s_secret, key);
} else if (_oidc_strlen(key) >= OIDC_CACHE_KEY_SIZE_MAX) {
s_key = oidc_cache_get_hashed_key(r, key);
}

OIDC_METRICS_TIMING_START(r, cfg);
Expand All @@ -295,7 +303,7 @@ apr_byte_t oidc_cache_get(request_rec *r, const char *section, const char *key,
if ((cache_value == NULL) && (encrypted == 1) && (cfg->crypto_passphrase.secret2 != NULL)) {
oidc_debug(r, "2nd try with previous passphrase");
s_secret = cfg->crypto_passphrase.secret2;
s_key = oidc_cache_get_hashed_key(r, s_secret, key);
s_key = oidc_cache_get_hashed_key_secret(r, s_secret, key);
if (cfg->cache->get(r, section, s_key, &cache_value) == FALSE) {
rc = FALSE;
goto out;
Expand Down Expand Up @@ -346,13 +354,12 @@ apr_byte_t oidc_cache_set(request_rec *r, const char *section, const char *key,

/* see if we need to encrypt */
if (encrypted == 1) {

if (cfg->crypto_passphrase.secret1 == NULL) {
oidc_error(r, "could not encrypt cache entry because " OIDCCryptoPassphrase " is not set");
goto out;
}

key = oidc_cache_get_hashed_key(r, cfg->crypto_passphrase.secret1, key);
key = oidc_cache_get_hashed_key_secret(r, cfg->crypto_passphrase.secret1, key);
if (key == NULL)
goto out;

Expand All @@ -361,6 +368,8 @@ apr_byte_t oidc_cache_set(request_rec *r, const char *section, const char *key,
goto out;
value = encoded;
}
} else if (_oidc_strlen(key) >= OIDC_CACHE_KEY_SIZE_MAX) {
key = oidc_cache_get_hashed_key(r, key);
}

OIDC_METRICS_TIMING_START(r, cfg);
Expand Down
2 changes: 1 addition & 1 deletion src/cache/shm.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ typedef struct oidc_cache_cfg_shm_t {
} oidc_cache_cfg_shm_t;

/* size of key in cached key/value pairs */
#define OIDC_CACHE_SHM_KEY_MAX 512
#define OIDC_CACHE_SHM_KEY_MAX OIDC_CACHE_KEY_SIZE_MAX

/* represents one (fixed size) cache entry, cq. name/value string pair */
typedef __attribute__((aligned(64))) struct oidc_cache_shm_entry_t {
Expand Down

0 comments on commit d2b36e5

Please sign in to comment.