diff --git a/ChangeLog b/ChangeLog index e762a848..6c13e16b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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 diff --git a/src/cache/cache.h b/src/cache/cache.h index cca706ab..400ba3ee 100644 --- a/src/cache/cache.h +++ b/src/cache/cache.h @@ -50,6 +50,13 @@ #include #include +/* + * 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); diff --git a/src/cache/common.c b/src/cache/common.c index 7dda11ce..8e170b4e 100644 --- a/src/cache/common.c +++ b/src/cache/common.c @@ -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 */ @@ -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 */ @@ -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); @@ -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; @@ -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; @@ -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); diff --git a/src/cache/shm.c b/src/cache/shm.c index db67267f..1002bf5d 100644 --- a/src/cache/shm.c +++ b/src/cache/shm.c @@ -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 {