use the vhost config check for generating the passphrase when not set

Signed-off-by: Hans Zandbelt <[email protected]>

Author: zandbelt

src/cache/common.c

@@ -314,7 +314,7 @@ apr_byte_t oidc_cache_get(request_rec *r, const char *section, const char *key,
 
 	oidc_debug(r, "enter: %s (section=%s, decrypt=%d, type=%s)", key, s_section, encrypted, cfg->cache.impl->name);
 
-	s_secret = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	s_secret = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	if (oidc_cache_get_key(r, key, s_secret, encrypted, &s_key) == FALSE)
 		goto end;
 
@@ -385,12 +385,12 @@ apr_byte_t oidc_cache_set(request_rec *r, const char *section, const char *key,
 		   value ? (int)_oidc_strlen(value) : 0, encrypted, apr_time_sec(expiry - apr_time_now()),
 		   cfg->cache.impl->name);
 
-	if (oidc_cache_get_key(r, key, oidc_cfg_crypto_passphrase_secret1_get(cfg, r), encrypted, &s_key) == FALSE)
+	if (oidc_cache_get_key(r, key, oidc_cfg_crypto_passphrase_secret1_get(cfg), encrypted, &s_key) == FALSE)
 		goto end;
 
 	/* see if we need to encrypt */
 	if ((encrypted == 1) && (value != NULL)) {
-		if (oidc_cache_crypto_encrypt(r, value, oidc_cfg_crypto_passphrase_get(cfg, r), &encoded) == FALSE)
+		if (oidc_cache_crypto_encrypt(r, value, oidc_cfg_crypto_passphrase_get(cfg), &encoded) == FALSE)
 			goto end;
 		value = encoded;
 	}

src/cfg/cfg.c

@@ -108,18 +108,18 @@ const char *oidc_cmd_crypto_passphrase_set(cmd_parms *cmd, void *struct_ptr, con
 	return rv;
 }
 
-const oidc_crypto_passphrase_t *oidc_cfg_crypto_passphrase_get(oidc_cfg_t *cfg, request_rec *r) {
-	// make sure secret1 is set
-	oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+const oidc_crypto_passphrase_t *oidc_cfg_crypto_passphrase_get(oidc_cfg_t *cfg) {
 	return &cfg->crypto_passphrase;
 }
 
-const char *oidc_cfg_crypto_passphrase_secret1_get(oidc_cfg_t *cfg, request_rec *r) {
-	if (cfg->crypto_passphrase.secret1 == NULL)
-		oidc_util_rand_str(r, (char **)&cfg->crypto_passphrase.secret1, 32);
+const char *oidc_cfg_crypto_passphrase_secret1_get(oidc_cfg_t *cfg) {
 	return cfg->crypto_passphrase.secret1;
 }
 
+void oidc_cfg_crypto_passphrase_secret1_set(oidc_cfg_t *cfg, const char *secret) {
+	cfg->crypto_passphrase.secret1 = secret;
+}
+
 const char *oidc_cfg_crypto_passphrase_secret2_get(oidc_cfg_t *cfg) {
 	return cfg->crypto_passphrase.secret2;
 }

src/cfg/cfg.h

@@ -195,6 +195,7 @@ void oidc_cfg_child_init(apr_pool_t *pool, oidc_cfg_t *cfg, server_rec *s);
 void oidc_cfg_cleanup_child(oidc_cfg_t *cfg, server_rec *s);
 const char *oidc_cfg_string_list_add(apr_pool_t *pool, apr_array_header_t **list, const char *arg);
 const char *oidc_cfg_endpoint_auth_set(apr_pool_t *pool, oidc_cfg_t *cfg, const char *arg, char **auth, char **alg);
+void oidc_cfg_crypto_passphrase_secret1_set(oidc_cfg_t *cfg, const char *secret);
 
 #define OIDC_CFG_MEMBER_FUNC_NAME(member, type, method) oidc_##type##_##member##_##method
 
@@ -209,9 +210,8 @@ const char *oidc_cfg_endpoint_auth_set(apr_pool_t *pool, oidc_cfg_t *cfg, const
 
 OIDC_CFG_MEMBER_FUNCS_DECL(delete_oldest_state_cookies, int)
 OIDC_CFG_MEMBER_FUNCS_DECL(action_on_userinfo_error, oidc_on_error_action_t)
-OIDC_CMD_MEMBER_FUNC_DECL(crypto_passphrase_secret1, const char *);
-const char *oidc_cfg_crypto_passphrase_secret1_get(oidc_cfg_t *cfg, request_rec *r);
-OIDC_CFG_MEMBER_FUNC_GET_DECL(crypto_passphrase_secret2, const char *)
+OIDC_CFG_MEMBER_FUNCS_DECL(crypto_passphrase_secret1, const char *)
+OIDC_CFG_MEMBER_FUNCS_DECL(crypto_passphrase_secret2, const char *)
 OIDC_CFG_MEMBER_FUNCS_DECL(refresh_mutex, oidc_cache_mutex_t *)
 OIDC_CFG_MEMBER_FUNCS_DECL(store_id_token, int)
 OIDC_CFG_MEMBER_FUNCS_DECL(post_preserve_template, const char *)
@@ -249,11 +249,9 @@ OIDC_CFG_MEMBER_FUNCS_DECL(dpop_api_enabled, int)
 
 // 2 args
 OIDC_CFG_MEMBER_FUNCS_DECL(post_preserve_templates, const char *, const char *)
+OIDC_CFG_MEMBER_FUNCS_DECL(crypto_passphrase, const oidc_crypto_passphrase_t *, const char *)
 OIDC_CFG_MEMBER_FUNCS_DECL(max_number_of_state_cookies, int, const char *)
 
-const char *oidc_cmd_crypto_passphrase_set(cmd_parms *, void *, const char *, const char *);
-const oidc_crypto_passphrase_t *oidc_cfg_crypto_passphrase_get(oidc_cfg_t *cfg, request_rec *r);
-
 // 3 args
 OIDC_CFG_MEMBER_FUNCS_DECL(cookie_same_site_session, oidc_samesite_cookie_t, const char *, const char *)
 OIDC_CFG_MEMBER_FUNC_GET_DECL(cookie_same_site_state, oidc_samesite_cookie_t)

src/mod_auth_openidc.c

@@ -1564,6 +1564,9 @@ static int oidc_config_check_vhost_config(apr_pool_t *pool, server_rec *s) {
 
 	oidc_sdebug(s, "enter");
 
+	if (oidc_cfg_crypto_passphrase_secret1_get(cfg) == NULL)
+		oidc_cfg_crypto_passphrase_secret1_set(cfg, oidc_util_rand_hex_str(NULL, s->process->pconf, 32));
+
 	if ((oidc_cfg_metadata_dir_get(cfg) != NULL) ||
 	    (oidc_cfg_provider_issuer_get(oidc_cfg_provider_get(cfg)) != NULL) ||
 	    (oidc_cfg_provider_metadata_url_get(oidc_cfg_provider_get(cfg)) != NULL)) {

src/proto/state.c

@@ -237,7 +237,7 @@ void oidc_proto_state_set_timestamp_now(oidc_proto_state_t *proto_state) {
 oidc_proto_state_t *oidc_proto_state_from_cookie(request_rec *r, oidc_cfg_t *c, const char *cookieValue) {
 	char *s_payload = NULL;
 	json_t *result = NULL;
-	oidc_util_jwt_verify(r, oidc_cfg_crypto_passphrase_get(c, r), cookieValue, &s_payload);
+	oidc_util_jwt_verify(r, oidc_cfg_crypto_passphrase_get(c), cookieValue, &s_payload);
 	oidc_util_json_decode_object(r, s_payload, &result);
 	return result;
 }
@@ -247,7 +247,7 @@ oidc_proto_state_t *oidc_proto_state_from_cookie(request_rec *r, oidc_cfg_t *c,
  */
 char *oidc_proto_state_to_cookie(request_rec *r, oidc_cfg_t *c, oidc_proto_state_t *proto_state) {
 	char *cookieValue = NULL;
-	oidc_util_jwt_create(r, oidc_cfg_crypto_passphrase_get(c, r),
+	oidc_util_jwt_create(r, oidc_cfg_crypto_passphrase_get(c),
 			     oidc_util_json_encode(r->pool, proto_state, JSON_COMPACT), &cookieValue);
 	return cookieValue;
 }

src/session.c

@@ -71,12 +71,12 @@ static apr_byte_t oidc_session_encode(request_rec *r, oidc_cfg_t *c, oidc_sessio
 	if (encrypt == FALSE) {
 		*s_value = oidc_util_json_encode(r->pool, z->state, JSON_COMPACT);
 		return (*s_value != NULL);
-	} else if (oidc_cfg_crypto_passphrase_secret1_get(c, r) == NULL) {
+	} else if (oidc_cfg_crypto_passphrase_secret1_get(c) == NULL) {
 		oidc_error(r, "cannot encrypt session state because " OIDCCryptoPassphrase " is not set");
 		return FALSE;
 	}
 
-	if (oidc_util_jwt_create(r, oidc_cfg_crypto_passphrase_get(c, r),
+	if (oidc_util_jwt_create(r, oidc_cfg_crypto_passphrase_get(c),
 				 oidc_util_json_encode(r->pool, z->state, JSON_COMPACT), s_value) == FALSE)
 		return FALSE;
 
@@ -92,12 +92,12 @@ static apr_byte_t oidc_session_decode(request_rec *r, oidc_cfg_t *c, oidc_sessio
 
 	if (encrypt == FALSE) {
 		return oidc_util_json_decode_object(r, s_json, &z->state);
-	} else if (oidc_cfg_crypto_passphrase_secret1_get(c, r) == NULL) {
+	} else if (oidc_cfg_crypto_passphrase_secret1_get(c) == NULL) {
 		oidc_error(r, "cannot decrypt session state because " OIDCCryptoPassphrase " is not set");
 		return FALSE;
 	}
 
-	if (oidc_util_jwt_verify(r, oidc_cfg_crypto_passphrase_get(c, r), s_json, &s_payload) == FALSE) {
+	if (oidc_util_jwt_verify(r, oidc_cfg_crypto_passphrase_get(c), s_json, &s_payload) == FALSE) {
 		oidc_error(r, "could not verify secure JWT: cache value possibly corrupted");
 		return FALSE;
 	}
@@ -111,7 +111,7 @@ static apr_byte_t oidc_session_decode(request_rec *r, oidc_cfg_t *c, oidc_sessio
  * generate a unique identifier for a session
  */
 void oidc_session_id_new(request_rec *r, oidc_session_t *z) {
-	z->uuid = oidc_util_rand_hex_str(r, OIDC_SESSION_ID_LEN);
+	z->uuid = oidc_util_rand_hex_str(r, r->pool, OIDC_SESSION_ID_LEN);
 }
 
 /*

src/util/random.c

@@ -111,9 +111,11 @@ static apr_byte_t _oidc_util_rand_bytes(request_rec *r, unsigned char *buf, apr_
 #else
 	const char *gen = DEV_RANDOM;
 #endif
-	oidc_debug(r, "use [%s] for generating %" APR_SIZE_T_FMT " random bytes", gen, len);
+	if (r)
+		oidc_debug(r, "use [%s] for generating %" APR_SIZE_T_FMT " random bytes", gen, len);
 	rv = _oidc_util_rand(buf, len);
-	oidc_debug(r, "return: %d", rv);
+	if (r)
+		oidc_debug(r, "return: %d", rv);
 	return rv;
 }
 
@@ -145,11 +147,12 @@ apr_byte_t oidc_util_rand_str(request_rec *r, char **str, int len) {
 /*
  * generate a random string of (lowercase) hexadecimal characters, representing len bytes
  */
-char *oidc_util_rand_hex_str(request_rec *r, int len) {
-	unsigned char *bytes = apr_pcalloc(r->pool, len);
+char *oidc_util_rand_hex_str(request_rec *r, apr_pool_t *pool, int len) {
+	unsigned char *bytes = apr_pcalloc(pool, len);
 	if (_oidc_util_rand_bytes(r, bytes, len) != TRUE) {
-		oidc_error(r, "_oidc_util_rand_bytes returned an error");
+		if (r)
+			oidc_error(r, "_oidc_util_rand_bytes returned an error");
 		return NULL;
 	}
-	return oidc_util_hex_encode(r, bytes, len);
+	return oidc_util_hex_encode(pool, bytes, len);
 }

src/util/util.c

@@ -406,10 +406,10 @@ void oidc_util_table_add_query_encoded_params(apr_pool_t *pool, apr_table_t *tab
 	}
 }
 
-char *oidc_util_hex_encode(request_rec *r, const unsigned char *bytes, unsigned int len) {
+char *oidc_util_hex_encode(apr_pool_t *pool, const unsigned char *bytes, unsigned int len) {
 	char *s = "";
 	for (int i = 0; i < len; i++)
-		s = apr_psprintf(r->pool, "%s%02x", s, bytes[i]);
+		s = apr_psprintf(pool, "%s%02x", s, bytes[i]);
 	return s;
 }
 

src/util/util.h

@@ -117,7 +117,7 @@ oidc_jwk_t *oidc_util_key_list_first(const apr_array_header_t *key_list, int kty
 // random.c
 unsigned int oidc_util_rand_int(unsigned int mod);
 apr_byte_t oidc_util_rand_str(request_rec *r, char **output, int len);
-char *oidc_util_rand_hex_str(request_rec *r, int len);
+char *oidc_util_rand_hex_str(request_rec *r, apr_pool_t *pool, int len);
 
 // url.c
 const char *oidc_util_url_cur_host(request_rec *r, oidc_hdr_x_forwarded_t x_forwarded_headers);
@@ -131,7 +131,7 @@ apr_byte_t oidc_util_url_has_parameter(request_rec *r, const char *param);
 apr_byte_t oidc_util_url_parameter_get(request_rec *r, char *name, char **value);
 
 // util.c
-char *oidc_util_hex_encode(request_rec *r, const unsigned char *bytes, unsigned int len);
+char *oidc_util_hex_encode(apr_pool_t *pool, const unsigned char *bytes, unsigned int len);
 apr_byte_t oidc_util_hash_string_and_base64url_encode(request_rec *r, const char *openssl_hash_algo, const char *input,
 						      char **output);
 int oidc_util_strnenvcmp(const char *a, const char *b, int len);

test/test_cache.c

@@ -169,20 +169,20 @@ START_TEST(test_cache_encrypt_no_secret) {
 
 	/* get cfg and temporarily remove the secret to simulate missing passphrase */
 	oidc_cfg_t *cfg = oidc_test_cfg_get();
-	const char *old_secret = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	const char *old_secret = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	cfg->crypto_passphrase.secret1 = NULL;
 	cfg->cache.encrypt = 1;
 
 	/* fail when value is too short and compression fails */
 	ck_assert_int_eq(oidc_cache_set(r, OIDC_CACHE_SECTION_SESSION, "nokey", "v", expiry), FALSE);
 
-	/* do not fail when encryption is on but no secret is set (long enough value to compress) */
+	/* fail when encryption is on but no secret is set (long enough value to compress) */
 	ck_assert_int_eq(oidc_cache_set(r, OIDC_CACHE_SECTION_SESSION, "nokey",
 					"vadadfsssssssssssssssssssssssssssssssssssssssssssssssssssssssss", expiry),
-			 TRUE);
+			 FALSE);
 
-	/* get should not fail because a secret should be generated */
-	ck_assert_int_eq(oidc_cache_get(r, OIDC_CACHE_SECTION_SESSION, "nokey", &value), TRUE);
+	/* fail because no secret is set */
+	ck_assert_int_eq(oidc_cache_get(r, OIDC_CACHE_SECTION_SESSION, "nokey", &value), FALSE);
 
 	/* restore secret */
 	cfg->crypto_passphrase.secret1 = (char *)old_secret;
@@ -204,7 +204,7 @@ START_TEST(test_cache_second_passphrase_retry) {
 
 	/* prepare cfg and secrets */
 	oidc_cfg_t *cfg = oidc_test_cfg_get();
-	const char *old_s1 = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	const char *old_s1 = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	const char *old_s2 = oidc_cfg_crypto_passphrase_secret2_get(cfg);
 	int old_encrypt = cfg->cache.encrypt;
 
@@ -275,7 +275,7 @@ START_TEST(test_cache_secret1_empty_secret2_fallback) {
 
 	/* prepare cfg and secrets */
 	oidc_cfg_t *cfg = oidc_test_cfg_get();
-	const char *old_s1 = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	const char *old_s1 = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	const char *old_s2 = oidc_cfg_crypto_passphrase_secret2_get(cfg);
 	int old_encrypt = cfg->cache.encrypt;
 
@@ -342,7 +342,7 @@ START_TEST(test_cache_compression_enabled_set_get) {
 	/* verify encryption+compression works by trying to create a JWT with the current cfg secret */
 	oidc_cfg_t *cfg = oidc_test_cfg_get();
 	oidc_crypto_passphrase_t passphrase;
-	passphrase.secret1 = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	passphrase.secret1 = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	passphrase.secret2 = oidc_cfg_crypto_passphrase_secret2_get(cfg);
 	char *encoded = NULL;
 	apr_byte_t forced_no_compress = FALSE;
@@ -373,7 +373,7 @@ START_TEST(test_cache_compression_enabled_second_passphrase) {
 
 	/* prepare cfg and secrets */
 	oidc_cfg_t *cfg = oidc_test_cfg_get();
-	const char *old_s1 = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	const char *old_s1 = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	const char *old_s2 = oidc_cfg_crypto_passphrase_secret2_get(cfg);
 	int old_encrypt = cfg->cache.encrypt;
 
@@ -384,7 +384,7 @@ START_TEST(test_cache_compression_enabled_second_passphrase) {
 
 	/* verify encryption+compression works for this cfg; fall back to no-compress if not */
 	oidc_crypto_passphrase_t passphrase;
-	passphrase.secret1 = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	passphrase.secret1 = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	passphrase.secret2 = oidc_cfg_crypto_passphrase_secret2_get(cfg);
 	char *encoded = NULL;
 	apr_byte_t forced_no_compress = FALSE;
@@ -433,7 +433,7 @@ START_TEST(test_cache_compression_enabled_empty_secret2_fallback) {
 
 	/* prepare cfg and secrets */
 	oidc_cfg_t *cfg = oidc_test_cfg_get();
-	const char *old_s1 = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	const char *old_s1 = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	const char *old_s2 = oidc_cfg_crypto_passphrase_secret2_get(cfg);
 	int old_encrypt = cfg->cache.encrypt;
 
@@ -443,7 +443,7 @@ START_TEST(test_cache_compression_enabled_empty_secret2_fallback) {
 
 	/* verify encryption+compression works; fall back to no-compress if not */
 	oidc_crypto_passphrase_t passphrase2;
-	passphrase2.secret1 = oidc_cfg_crypto_passphrase_secret1_get(cfg, r);
+	passphrase2.secret1 = oidc_cfg_crypto_passphrase_secret1_get(cfg);
 	passphrase2.secret2 = oidc_cfg_crypto_passphrase_secret2_get(cfg);
 	char *encoded2 = NULL;
 	apr_byte_t forced_no_compress = FALSE;