Skip to content

Commit

Permalink
support for seamless rollover of OIDCCryptoPassphrase
Browse files Browse the repository at this point in the history
using a (temporary) 2nd value that holds the old one; bump to 2.4.15rc1

Signed-off-by: Hans Zandbelt <[email protected]>
  • Loading branch information
zandbelt committed Oct 31, 2023
1 parent 8c8ee13 commit 167acb2
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 86 deletions.
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
10/31/2023
- add capability to seamlessly rollover OIDCCryptoPassphrase using a (temporary) 2nd value that holds the old one
- bump to 2.4.15rc1

10/30/2023
- do not apply logout_on_error and authenticate_on_error when a parallel refresh token request is detected
see https://github.com/OpenIDC/mod_auth_openidc/discussions/1132; thanks @esunke
Expand Down
7 changes: 6 additions & 1 deletion auth_openidc.conf
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@
# OIDCCryptoPassphrase "exec:/bin/bash -c \"head /dev/urandom | tr -dc A-Za-z0-9 | head -c 32\""
# (notice that the above typically only works in non-clustered environments)
# The command may be absolute or relative to the web server root.
#OIDCCryptoPassphrase [ <passphrase> | "exec:/path/to/otherProgram arg1" ]
#
# A second value can be used temporarily in case of passphrase rollover: the first (i.e. new) passphrase
# will be used for encryption of new values (including a "kid" in the JWEs during the time 2 values are defined),
# both values will be used for verification (leveraging the "kid" if present); for seamless rollover one should
# (at minimum) wait for OIDCSessionInActivityTimeout seconds before removing the 2nd (i.e. old) passprase again.
#OIDCCryptoPassphrase [ <passphrase> | "exec:/path/to/otherProgram arg1" ] [ <previous-passphrase> | "exec:/path/to/otherProgram arg2" ]

#
# All other entries below this are optional though some may be required in a
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
AC_INIT([mod_auth_openidc],[2.4.15rc0],[[email protected]])
AC_INIT([mod_auth_openidc],[2.4.15rc1],[[email protected]])

AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())

Expand Down
71 changes: 44 additions & 27 deletions src/cache/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,16 +244,20 @@ apr_byte_t oidc_cache_mutex_destroy(server_rec *s, oidc_cache_mutex_t *m) {
* AES GCM encrypt using the crypto passphrase as symmetric key
*/
static apr_byte_t oidc_cache_crypto_encrypt(request_rec *r,
const char *plaintext, const char *key, char **result) {
return oidc_util_jwt_create(r, key, plaintext, result);
const char *plaintext, const oidc_crypto_passphrase_t *passphrase,
char **result) {
return oidc_util_jwt_create(r, passphrase, plaintext, result);
}

/*
* AES GCM decrypt using the crypto passphrase as symmetric key
*/
static apr_byte_t oidc_cache_crypto_decrypt(request_rec *r,
const char *cache_value, const char *key, char **plaintext) {
return oidc_util_jwt_verify(r, key, cache_value, plaintext);
const char *cache_value, char *secret, char **plaintext) {
oidc_crypto_passphrase_t passphrase;
passphrase.secret1 = secret;
passphrase.secret2 = NULL;
return oidc_util_jwt_verify(r, &passphrase, cache_value, plaintext);
}

/*
Expand Down Expand Up @@ -283,39 +287,51 @@ apr_byte_t oidc_cache_get(request_rec *r, const char *section, const char *key,
int encrypted = oidc_cfg_cache_encrypt(r);
apr_byte_t rc = TRUE;
char *msg = NULL;
char *cache_value = NULL;
const char *s_key = NULL;
char *cache_value = NULL, *s_secret = NULL;

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

s_key = key;
/* see if encryption is turned on */
if (encrypted == 1)
key = oidc_cache_get_hashed_key(r, cfg->crypto_passphrase, key);
if (encrypted == 1) {
if (cfg->crypto_passphrase.secret1 == NULL) {
oidc_error(r,
"could not decrypt cache entry because " OIDCCryptoPassphrase " is not set");
goto out;
}
s_secret = cfg->crypto_passphrase.secret1;
s_key = oidc_cache_get_hashed_key(r, s_secret, key);
}

/* get the value from the cache */
if (cfg->cache->get(r, section, key, &cache_value) == FALSE) {
if (cfg->cache->get(r, section, s_key, &cache_value) == FALSE) {
rc = FALSE;
goto out;
}

/* see if it is any good */
if (cache_value == NULL)
goto out;
if (cache_value == NULL) {
if ((encrypted != 1) || (cfg->crypto_passphrase.secret2 == NULL)) {
rc = FALSE;
goto out;
}
s_secret = cfg->crypto_passphrase.secret2;
s_key = oidc_cache_get_hashed_key(r, s_secret, key);
if (cfg->cache->get(r, section, s_key, &cache_value) == FALSE) {
rc = FALSE;
goto out;
}
}

/* see if encryption is turned on */
if (encrypted == 0) {
*value = apr_pstrdup(r->pool, cache_value);
goto out;
}

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

rc = oidc_cache_crypto_decrypt(r, cache_value, cfg->crypto_passphrase,
value);
rc = oidc_cache_crypto_decrypt(r, cache_value, s_secret, value);

out:
/* log the result */
Expand Down Expand Up @@ -354,17 +370,18 @@ apr_byte_t oidc_cache_set(request_rec *r, const char *section, const char *key,
/* see if we need to encrypt */
if (encrypted == 1) {

key = oidc_cache_get_hashed_key(r, cfg->crypto_passphrase, key);
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);
if (key == NULL)
goto out;

if (value != NULL) {
if (cfg->crypto_passphrase == NULL) {
oidc_error(r,
"could not encrypt cache entry because " OIDCCryptoPassphrase " is not set");
goto out;
}
if (oidc_cache_crypto_encrypt(r, value, cfg->crypto_passphrase,
if (oidc_cache_crypto_encrypt(r, value, &cfg->crypto_passphrase,
&encoded) == FALSE)
goto out;
value = encoded;
Expand Down
58 changes: 41 additions & 17 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -598,14 +598,11 @@ static const char* oidc_set_outgoing_proxy_slot(cmd_parms *cmd, void *ptr,
/*
* set a string value in the server config with exec support
*/
static const char* oidc_set_passphrase_slot(cmd_parms *cmd, void *struct_ptr,
const char *arg) {
oidc_cfg *cfg = (oidc_cfg*) ap_get_module_config(cmd->server->module_config,
&auth_openidc_module);
const char *passphrase = NULL;
int arglen = _oidc_strlen(arg);
static const char* oidc_parse_passphrase(cmd_parms *cmd, const char *arg,
char **passphrase) {
char **argv = NULL;
char *result = NULL;
int arglen = _oidc_strlen(arg);
/* Based on code from mod_session_crypto. */
if (arglen > 5 && _oidc_strncmp(arg, "exec:", 5) == 0) {
if (apr_tokenize_to_argv(arg + 5, &argv, cmd->temp_pool) != APR_SUCCESS) {
Expand All @@ -626,12 +623,35 @@ static const char* oidc_set_passphrase_slot(cmd_parms *cmd, void *struct_ptr,
if (_oidc_strlen(result) == 0)
return apr_pstrdup(cmd->pool,
"the output of the crypto passphrase generation command is empty (perhaps you need to pass it to bash -c \"<cmd>\"?)");
passphrase = result;
*passphrase = apr_pstrdup(cmd->pool, result);
} else {
passphrase = arg;
*passphrase = apr_pstrdup(cmd->pool, arg);
}
return NULL;
}

static const char* oidc_set_crypto_passphrase_slot(cmd_parms *cmd,
void *struct_ptr, const char *arg1, const char *arg2) {
oidc_cfg *cfg = (oidc_cfg*) ap_get_module_config(cmd->server->module_config,
&auth_openidc_module);
const char *rv = NULL;
if (arg1)
rv = oidc_parse_passphrase(cmd, arg1, &cfg->crypto_passphrase.secret1);
if ((rv == NULL) && (arg2 != NULL))
rv = oidc_parse_passphrase(cmd, arg2, &cfg->crypto_passphrase.secret2);
return NULL;
}

return ap_set_string_slot(cmd, cfg, passphrase);
static const char* oidc_set_passphrase_slot(cmd_parms *cmd, void *struct_ptr,
const char *arg) {
oidc_cfg *cfg = (oidc_cfg*) ap_get_module_config(cmd->server->module_config,
&auth_openidc_module);
const char *rv = NULL;
char *secret = NULL;
rv = oidc_parse_passphrase(cmd, arg, &secret);
if (rv == NULL)
rv = ap_set_string_slot(cmd, cfg, secret);
return rv;
}

/*
Expand Down Expand Up @@ -1891,7 +1911,8 @@ void* oidc_create_server_config(apr_pool_t *pool, server_rec *svr) {
c->outgoing_proxy.username_password = NULL;
c->outgoing_proxy.auth_type = OIDC_CONFIG_POS_INT_UNSET;

c->crypto_passphrase = NULL;
c->crypto_passphrase.secret1 = NULL;
c->crypto_passphrase.secret2 = NULL;

c->error_template = NULL;
c->post_preserve_template = NULL;
Expand Down Expand Up @@ -2234,9 +2255,12 @@ void* oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) {
add->outgoing_proxy.auth_type :
base->outgoing_proxy.auth_type;

c->crypto_passphrase =
add->crypto_passphrase != NULL ?
add->crypto_passphrase : base->crypto_passphrase;
c->crypto_passphrase.secret1 =
add->crypto_passphrase.secret1 != NULL ?
add->crypto_passphrase.secret1 : base->crypto_passphrase.secret1;
c->crypto_passphrase.secret2 =
add->crypto_passphrase.secret2 != NULL ?
add->crypto_passphrase.secret1 : base->crypto_passphrase.secret2;

c->error_template =
add->error_template != NULL ?
Expand Down Expand Up @@ -2693,7 +2717,7 @@ static int oidc_check_config_openid_openidc(server_rec *s, oidc_cfg *c) {
return oidc_check_config_error(s, OIDCRedirectURI);
redirect_uri_is_relative = (c->redirect_uri[0] == OIDC_CHAR_FORWARD_SLASH);

if (c->crypto_passphrase == NULL)
if (c->crypto_passphrase.secret1 == NULL)
return oidc_check_config_error(s, OIDCCryptoPassphrase);

if (c->metadata_dir == NULL) {
Expand Down Expand Up @@ -2787,7 +2811,7 @@ static int oidc_check_config_oauth(server_rec *s, oidc_cfg *c) {

}

if ((c->cache_encrypt == 1) && (c->crypto_passphrase == NULL))
if ((c->cache_encrypt == 1) && (c->crypto_passphrase.secret1 == NULL))
return oidc_check_config_error(s, OIDCCryptoPassphrase);

return OK;
Expand Down Expand Up @@ -3506,8 +3530,8 @@ const command_rec oidc_config_cmds[] = {
(void*)APR_OFFSETOF(oidc_cfg, outgoing_proxy),
RSRC_CONF,
"Specify an outgoing proxy for your network (<host>[:<port>]."),
AP_INIT_TAKE1(OIDCCryptoPassphrase,
oidc_set_passphrase_slot,
AP_INIT_TAKE12(OIDCCryptoPassphrase,
oidc_set_crypto_passphrase_slot,
(void*)APR_OFFSETOF(oidc_cfg, crypto_passphrase),
RSRC_CONF,
"Passphrase used for AES crypto on cookies and state."),
Expand Down
13 changes: 9 additions & 4 deletions src/mod_auth_openidc.h
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,11 @@ typedef struct oidc_http_timeout_t {
apr_time_t retry_interval;
} oidc_http_timeout_t;

typedef struct oidc_crypto_passphrase_t {
char *secret1;
char *secret2;
} oidc_crypto_passphrase_t;

typedef struct oidc_cfg {
/* indicates whether this is a derived config, merged from a base one */
unsigned int merged;
Expand Down Expand Up @@ -496,7 +501,7 @@ typedef struct oidc_cfg {

oidc_outgoing_proxy_t outgoing_proxy;

char *crypto_passphrase;
oidc_crypto_passphrase_t crypto_passphrase;

int provider_metadata_refresh_interval;

Expand Down Expand Up @@ -728,7 +733,7 @@ void oidc_proto_state_set_timestamp_now(oidc_proto_state_t *proto_state);
apr_byte_t oidc_proto_token_endpoint_auth(request_rec *r, oidc_cfg *cfg, const char *token_endpoint_auth, const char *client_id, const char *client_secret, const apr_array_header_t *client_keys, const char *audience, apr_table_t *params, const char *bearer_access_token, char **basic_auth_str, char **bearer_auth_str);
char *oidc_proto_peek_jwt_header(request_rec *r, const char *jwt, char **alg, char **enc);
char *oidc_proto_peek_jwt_header(request_rec *r, const char *jwt, char **alg, char **enc, char **kid);
int oidc_proto_authorization_request(request_rec *r, struct oidc_provider_t *provider, const char *login_hint, const char *redirect_uri, const char *state, oidc_proto_state_t *proto_state, const char *id_token_hint, const char *code_challenge, const char *auth_request_params, const char *path_scope);
apr_byte_t oidc_proto_is_post_authorization_response(request_rec *r, oidc_cfg *cfg);
apr_byte_t oidc_proto_is_redirect_authorization_response(request_rec *r, oidc_cfg *cfg);
Expand Down Expand Up @@ -892,8 +897,8 @@ apr_byte_t oidc_util_regexp_first_match(apr_pool_t *pool, const char *input, con
apr_byte_t oidc_util_json_merge(request_rec *r, json_t *src, json_t *dst);
int oidc_util_cookie_domain_valid(const char *hostname, char *cookie_domain);
apr_byte_t oidc_util_hash_string_and_base64url_encode(request_rec *r, const char *openssl_hash_algo, const char *input, char **output);
apr_byte_t oidc_util_jwt_create(request_rec *r, const char *secret, const char *s_payload, char **compact_encoded_jwt);
apr_byte_t oidc_util_jwt_verify(request_rec *r, const char *secret, const char *compact_encoded_jwt, char **s_payload);
apr_byte_t oidc_util_jwt_create(request_rec *r, const oidc_crypto_passphrase_t *passphrase, const char *s_payload, char **compact_encoded_jwt);
apr_byte_t oidc_util_jwt_verify(request_rec *r, const oidc_crypto_passphrase_t *passphrase, const char *compact_encoded_jwt, char **s_payload);
char *oidc_util_get_chunked_cookie(request_rec *r, const char *cookieName, int cookie_chunk_size);
void oidc_util_set_chunked_cookie(request_rec *r, const char *cookieName, const char *cookieValue, apr_time_t expires, int chunkSize, const char *ext);
apr_byte_t oidc_util_create_symmetric_key(request_rec *r, const char *client_secret, unsigned int r_key_len, const char *hash_algo, apr_byte_t set_kid, oidc_jwk_t **jwk);
Expand Down
2 changes: 1 addition & 1 deletion src/oauth.c
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ static apr_byte_t oidc_oauth_validate_jwt_access_token(request_rec *r,
oidc_cfg *c, const char *access_token, json_t **token, char **response) {

oidc_debug(r, "enter: JWT access_token header=%s",
oidc_proto_peek_jwt_header(r, access_token, NULL, NULL));
oidc_proto_peek_jwt_header(r, access_token, NULL, NULL, NULL));

oidc_jose_error_t err;
oidc_jwk_t *jwk = NULL;
Expand Down
18 changes: 11 additions & 7 deletions src/proto.c
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ char* oidc_proto_create_request_object(request_rec *r,
json_decref(request_object_config);

oidc_debug(r, "serialized request object JWT header = \"%s\"",
oidc_proto_peek_jwt_header(r, serialized_request_object, NULL, NULL));
oidc_proto_peek_jwt_header(r, serialized_request_object, NULL, NULL, NULL));
oidc_debug(r, "serialized request object = \"%s\"",
serialized_request_object);

Expand Down Expand Up @@ -967,7 +967,7 @@ void oidc_proto_state_destroy(oidc_proto_state_t *proto_state) {

apr_byte_t oidc_proto_check_crypto_passphrase(request_rec *r, oidc_cfg *c,
const char *action) {
if (c->crypto_passphrase == NULL) {
if (c->crypto_passphrase.secret1 == NULL) {
oidc_error(r,
"cannot %s state cookie because " OIDCCryptoPassphrase " is not set; please check your OIDC Provider configuration as well or avoid using AuthType openid-connect",
action);
Expand All @@ -982,7 +982,7 @@ oidc_proto_state_t* oidc_proto_state_from_cookie(request_rec *r, oidc_cfg *c,
json_t *result = NULL;
if (oidc_proto_check_crypto_passphrase(r, c, "parse") == FALSE)
return NULL;
oidc_util_jwt_verify(r, c->crypto_passphrase, cookieValue, &s_payload);
oidc_util_jwt_verify(r, &c->crypto_passphrase, cookieValue, &s_payload);
oidc_util_decode_json_object(r, s_payload, &result);
return result;
}
Expand All @@ -992,7 +992,7 @@ char* oidc_proto_state_to_cookie(request_rec *r, oidc_cfg *c,
char *cookieValue = NULL;
if (oidc_proto_check_crypto_passphrase(r, c, "create") == FALSE)
return NULL;
oidc_util_jwt_create(r, c->crypto_passphrase,
oidc_util_jwt_create(r, &c->crypto_passphrase,
oidc_util_encode_json_object(r, proto_state, JSON_COMPACT),
&cookieValue);
return cookieValue;
Expand Down Expand Up @@ -1637,7 +1637,7 @@ apr_byte_t oidc_proto_jwt_verify(request_rec *r, oidc_cfg *cfg, oidc_jwt_t *jwt,
* return the compact-encoded JWT header contents
*/
char* oidc_proto_peek_jwt_header(request_rec *r,
const char *compact_encoded_jwt, char **alg, char **enc) {
const char *compact_encoded_jwt, char **alg, char **enc, char **kid) {
char *input = NULL, *result = NULL;
char *p = strstr(compact_encoded_jwt ? compact_encoded_jwt : "", ".");
if (p == NULL) {
Expand All @@ -1663,6 +1663,10 @@ char* oidc_proto_peek_jwt_header(request_rec *r,
*enc = apr_pstrdup(r->pool,
json_string_value(
json_object_get(json, CJOSE_HDR_ENC)));
if (kid)
*kid = apr_pstrdup(r->pool,
json_string_value(
json_object_get(json, CJOSE_HDR_KID)));
}
json_decref(json);
}
Expand All @@ -1678,7 +1682,7 @@ apr_byte_t oidc_proto_parse_idtoken(request_rec *r, oidc_cfg *cfg,

char *alg = NULL;
oidc_debug(r, "enter: id_token header=%s",
oidc_proto_peek_jwt_header(r, id_token, &alg, NULL));
oidc_proto_peek_jwt_header(r, id_token, &alg, NULL, NULL));
apr_hash_t *decryption_keys = NULL;

char buf[APR_RFC822_DATE_LEN + 1];
Expand Down Expand Up @@ -2179,7 +2183,7 @@ static apr_byte_t oidc_user_info_response_validate(request_rec *r,
|| (provider->userinfo_encrypted_response_alg != NULL)
|| (provider->userinfo_encrypted_response_enc != NULL)) {
oidc_debug(r, "JWT header=%s",
oidc_proto_peek_jwt_header(r, *response, &alg, NULL));
oidc_proto_peek_jwt_header(r, *response, &alg, NULL, NULL));
}

oidc_jose_error_t err;
Expand Down
Loading

0 comments on commit 167acb2

Please sign in to comment.