From: Remi Tricot-Le Breton Date: Thu, 2 Oct 2025 13:32:40 +0000 (+0200) Subject: MEDIUM: jwt: Remove certificate support in jwt_verify converter X-Git-Tag: v3.3-dev10~30 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c3c0597a34c685ac92efca910790422efa3c6348;p=thirdparty%2Fhaproxy.git MEDIUM: jwt: Remove certificate support in jwt_verify converter The jwt_verify converter will not take full-on certificates anymore in favor of a new soon to come jwt_verify_cert. We might end up with a new jwt_verify_hmac in the future as well which would allow to deprecate the jwt_verify converter and remove the need for a specific internal tree for public keys. The logic to always look into the internal jwt tree by default and resolve to locking the ckch tree as little as possible will also be removed. This allows to get rid of the duplicated reference to EVP_PKEYs, the one in the jwt tree entry and the one in the ckch_store. --- diff --git a/include/haproxy/jwt-t.h b/include/haproxy/jwt-t.h index 1372fddac..d5fda19cd 100644 --- a/include/haproxy/jwt-t.h +++ b/include/haproxy/jwt-t.h @@ -64,17 +64,8 @@ enum jwt_elt { JWT_ELT_MAX }; -enum jwt_entry_type { - JWT_ENTRY_DFLT, - JWT_ENTRY_STORE, - JWT_ENTRY_PKEY, - JWT_ENTRY_INVALID, /* already tried looking into ckch_store tree (unsuccessful) */ -}; - struct jwt_cert_tree_entry { EVP_PKEY *pubkey; - struct ckch_store *ckch_store; - int type; /* jwt_entry_type */ struct ebmb_node node; char path[VAR_ARRAY]; }; diff --git a/include/haproxy/jwt.h b/include/haproxy/jwt.h index 9e2df709a..41b5bcaa9 100644 --- a/include/haproxy/jwt.h +++ b/include/haproxy/jwt.h @@ -33,8 +33,6 @@ int jwt_tree_load_cert(char *path, int pathlen, const char *file, int line, char enum jwt_vrfy_status jwt_verify(const struct buffer *token, const struct buffer *alg, const struct buffer *key); -void jwt_replace_ckch_store(struct ckch_store *old_ckchs, struct ckch_store *new_ckchs); - #endif /* USE_OPENSSL */ #endif /* _HAPROXY_JWT_H */ diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h index 12739d501..31705faaf 100644 --- a/include/haproxy/ssl_ckch-t.h +++ b/include/haproxy/ssl_ckch-t.h @@ -73,8 +73,6 @@ struct ckch_conf { } acme; }; -struct jwt_cert_tree_entry; - /* * this is used to store 1 to SSL_SOCK_NUM_KEYTYPES cert_key_and_chain and * metadata. @@ -90,7 +88,6 @@ struct ckch_store { struct list crtlist_entry; /* list of entries which use this store */ struct ckch_conf conf; struct task *acme_task; - struct jwt_cert_tree_entry *jwt_entry; struct ebmb_node node; char path[VAR_ARRAY]; }; diff --git a/include/haproxy/thread-t.h b/include/haproxy/thread-t.h index c9683542f..2c99a4cb9 100644 --- a/include/haproxy/thread-t.h +++ b/include/haproxy/thread-t.h @@ -217,7 +217,6 @@ enum lock_label { QC_CID_LOCK, CACHE_LOCK, GUID_LOCK, - JWT_LOCK, OTHER_LOCK, /* WT: make sure never to use these ones outside of development, * we need them for lock profiling! diff --git a/src/jwt.c b/src/jwt.c index 9b0320991..f3e3385e9 100644 --- a/src/jwt.c +++ b/src/jwt.c @@ -28,8 +28,7 @@ #ifdef USE_OPENSSL /* Tree into which the public certificates used to validate JWTs will be stored. */ -struct eb_root jwt_cert_tree = EB_ROOT_UNIQUE; -__decl_rwlock(jwt_tree_lock); +static struct eb_root jwt_cert_tree = EB_ROOT_UNIQUE; /* @@ -132,8 +131,6 @@ int jwt_tokenize(const struct buffer *jwt, struct jwt_item *items, unsigned int /* * Parse a public certificate and insert it into the jwt_cert_tree. - * This function can only be called during configuration parsing so we do not - * need to lock the jwt certificate tree. * Returns 0 in case of success. */ int jwt_tree_load_cert(char *path, int pathlen, const char *file, int line, char **err) @@ -144,29 +141,12 @@ int jwt_tree_load_cert(char *path, int pathlen, const char *file, int line, char BIO *bio = NULL; struct stat buf; struct ebmb_node *eb = NULL; - struct ckch_store *store = NULL; eb = ebst_lookup(&jwt_cert_tree, path); if (eb) return 0; /* Entry already in the tree, nothing to do. */ - entry = calloc(1, sizeof(*entry) + pathlen + 1); - if (!entry) { - memprintf(err, "%sunable to allocate memory (jwt_cert_tree_entry).\n", err && *err ? *err : ""); - return -1; - } - memcpy(entry->path, path, pathlen + 1); - entry->type = JWT_ENTRY_DFLT; - - if (ebst_insert(&jwt_cert_tree, &entry->node) != &entry->node) { - /* Should never happen since we checked if the entry already - * existed previously. - */ - free(entry); - return 0; - } - if (stat(path, &buf) == 0) { bio = BIO_new(BIO_s_file()); if (!bio) { @@ -182,71 +162,31 @@ int jwt_tree_load_cert(char *path, int pathlen, const char *file, int line, char * named crt-store). */ if (pubkey) { - entry->type = JWT_ENTRY_PKEY; + entry = calloc(1, sizeof(*entry) + pathlen + 1); + if (!entry) { + memprintf(err, "%sunable to allocate memory (jwt_cert_tree_entry).\n", err && *err ? *err : ""); + goto end; + } + memcpy(entry->path, path, pathlen + 1); + entry->pubkey = pubkey; + + /* The entry insertion can't fail, we already + * checked if the entry existed. */ + ebst_insert(&jwt_cert_tree, &entry->node); + retval = 0; goto end; } } } - /* Look for an actual certificate or crt-store with the given name. - * If the path corresponds to an actual certificate that was not loaded - * yet we will create the corresponding ckch_store. */ - if (HA_SPIN_TRYLOCK(CKCH_LOCK, &ckch_lock)) - goto end; - - store = ckchs_lookup(path); - if (!store) { - struct ckch_conf conf = {}; - int err_code = 0; - - /* Create a new store with the given path */ - store = ckch_store_new(path); - if (!store) { - HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - goto end; - } - - conf.crt = path; - - err_code = ckch_store_load_files(&conf, store, 0, file, line, err); - if (err_code & ERR_FATAL) { - ckch_store_free(store); - - /* If we are in this case we are in the conf - * parsing phase and this case might happen if - * we were provided an HMAC secret or a variable - * name. - */ - retval = 0; - ha_free(err); - HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - goto end; - } - - if (ebst_insert(&ckchs_tree, &store->node) != &store->node) { - ckch_store_free(store); - HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - goto end; - } - } - - retval = 0; - - BUG_ON(store->jwt_entry != NULL); - entry->type = JWT_ENTRY_STORE; - entry->ckch_store = store; - entry->pubkey = X509_get_pubkey(store->data->cert); - store->jwt_entry = entry; - HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - end: if (retval) { /* Some error happened during pubkey parsing, remove the already * inserted node from the tree and free it. */ - ebmb_delete(&entry->node); + EVP_PKEY_free(pubkey); free(entry); } BIO_free(bio); @@ -255,62 +195,6 @@ end: } -/* Try to look for an already existing ckch_store in the store tree with the - * path found in the jwt_entry. Keep a reference to its pubkey if it exists. - * Return 0 in case of success. - */ -static int jwt_tree_tryload_store(struct jwt_cert_tree_entry *jwt_entry) -{ - struct ckch_store *store = NULL; - int retval = 1; - - if (!jwt_entry) - return 1; - - /* We might have been given a 'crt-store' name */ - if (HA_SPIN_TRYLOCK(CKCH_LOCK, &ckch_lock)) - return 1; - - store = ckchs_lookup(jwt_entry->path); - if (!store || store->jwt_entry) - goto end; - - store->jwt_entry = jwt_entry; - - BUG_ON(jwt_entry->pubkey != NULL); - jwt_entry->pubkey = X509_get_pubkey(store->data->cert); - - retval = 0; - -end: - HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - return retval; - -} - -/* Update the ckch_store and public key reference of a jwt_entry. This is only - * useful when updating a certificate from the CLI if it was being used for JWT - * validation. - */ -void jwt_replace_ckch_store(struct ckch_store *old_ckchs, struct ckch_store *new_ckchs) -{ - struct jwt_cert_tree_entry *entry = old_ckchs->jwt_entry; - - HA_RWLOCK_WRLOCK(JWT_LOCK, &jwt_tree_lock); - - if (entry == NULL) - goto end; - - old_ckchs->jwt_entry->ckch_store = new_ckchs; - new_ckchs->jwt_entry = old_ckchs->jwt_entry; - - EVP_PKEY_free(entry->pubkey); - entry->pubkey = X509_get_pubkey(new_ckchs->data->cert); - -end: - HA_RWLOCK_WRUNLOCK(JWT_LOCK, &jwt_tree_lock); -} - /* * Calculate the HMAC signature of a specific JWT and check that it matches the * one included in the token. @@ -416,7 +300,6 @@ jwt_jwsverify_rsa_ecdsa(const struct jwt_ctx *ctx, struct buffer *decoded_signat int is_ecdsa = 0; int padding = RSA_PKCS1_PADDING; EVP_PKEY *pubkey = NULL; - int lock_iswrlock = 0; switch(ctx->alg) { case JWS_ALG_RS256: @@ -461,71 +344,21 @@ jwt_jwsverify_rsa_ecdsa(const struct jwt_ctx *ctx, struct buffer *decoded_signat if (!evp_md_ctx) return JWT_VRFY_OUT_OF_MEMORY; - HA_RWLOCK_RDLOCK(JWT_LOCK, &jwt_tree_lock); - + /* Look for a public key in the JWT tree */ eb = ebst_lookup(&jwt_cert_tree, ctx->key); - if (!eb) { - HA_RWLOCK_RDUNLOCK(JWT_LOCK, &jwt_tree_lock); - - /* Create new entry and insert it in the jwt cert tree if we - * could find a corresponding ckch_store. - */ - entry = calloc(1, sizeof(*entry) + ctx->key_length + 1); - if (!entry) { - retval = JWT_VRFY_OUT_OF_MEMORY; - goto end; - } - memcpy(entry->path, ctx->key, ctx->key_length); - - /* The ckch_lock will be taken in jwt_tree_tryload_store so we - * can't hold the lock on the jwt_cert_tree here because the lock - * order is different when updating a certificate from the CLI, - * where the ckch_lock is taken first and then the JWT one is - * taken in jwt_replace_ckch_store. - * If no corresponding ckch_store was found, we still try to - * insert the entry in the tree so that next calls to jwt_verify - * with the same 'key' path do not perform the lookup in the - * ckch_store anymore. */ - entry->type = (jwt_tree_tryload_store(entry) == 0) ? JWT_ENTRY_STORE : JWT_ENTRY_INVALID; - - HA_RWLOCK_WRLOCK(JWT_LOCK, &jwt_tree_lock); - if (ebst_insert(&jwt_cert_tree, &entry->node) != &entry->node) { - /* This rather unlikely case can only happen if the tree was - * modified between the previous read unlock and here. - */ - retval = JWT_VRFY_INTERNAL_ERR; - free(entry); - entry = NULL; - HA_RWLOCK_WRUNLOCK(JWT_LOCK, &jwt_tree_lock); - goto end; - } - lock_iswrlock = 1; - } else { + if (eb) { entry = ebmb_entry(eb, struct jwt_cert_tree_entry, node); - } - /* We tried looking for a ckch_store but could not find it */ - switch (entry->type) { - case JWT_ENTRY_PKEY: - case JWT_ENTRY_STORE: pubkey = entry->pubkey; if (pubkey) EVP_PKEY_up_ref(pubkey); - break; - case JWT_ENTRY_DFLT: - case JWT_ENTRY_INVALID: - retval = JWT_VRFY_UNKNOWN_CERT; - break; } - if (lock_iswrlock) - HA_RWLOCK_WRUNLOCK(JWT_LOCK, &jwt_tree_lock); - else - HA_RWLOCK_RDUNLOCK(JWT_LOCK, &jwt_tree_lock); - - if (!pubkey) + if (!pubkey) { + retval = JWT_VRFY_UNKNOWN_CERT; goto end; + } /* * ECXXX signatures are a direct concatenation of the (R, S) pair and @@ -654,13 +487,12 @@ end: return retval; } + static void jwt_deinit(void) { struct ebmb_node *node = NULL; struct jwt_cert_tree_entry *entry = NULL; - HA_RWLOCK_WRLOCK(JWT_LOCK, &jwt_tree_lock); - node = ebmb_first(&jwt_cert_tree); while (node) { entry = ebmb_entry(node, struct jwt_cert_tree_entry, node); @@ -669,8 +501,6 @@ static void jwt_deinit(void) ha_free(&entry); node = ebmb_first(&jwt_cert_tree); } - - HA_RWLOCK_WRUNLOCK(JWT_LOCK, &jwt_tree_lock); } REGISTER_POST_DEINIT(jwt_deinit); diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index 72241297c..9b720e597 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -41,7 +41,6 @@ #include #include #include -#include /* Uncommitted CKCH transaction */ @@ -2709,8 +2708,6 @@ void ckch_store_replace(struct ckch_store *old_ckchs, struct ckch_store *new_ckc __ckch_inst_free_locked(ckchi); } - jwt_replace_ckch_store(old_ckchs, new_ckchs); - ckch_store_free(old_ckchs); ebst_insert(&ckchs_tree, &new_ckchs->node); } @@ -3171,9 +3168,6 @@ static int cli_parse_del_cert(char **args, char *payload, struct appctx *appctx, if (!LIST_ISEMPTY(&store->ckch_inst)) { memprintf(&err, "certificate '%s' in use, can't be deleted!\n", filename); goto error; - } else if (store->jwt_entry) { - memprintf(&err, "certificate '%s' in use for JWT validation, can't be deleted!\n", filename); - goto error; } ebmb_delete(&store->node); diff --git a/src/thread.c b/src/thread.c index a451ad95e..02929916c 100644 --- a/src/thread.c +++ b/src/thread.c @@ -440,7 +440,6 @@ const char *lock_label(enum lock_label label) case QC_CID_LOCK: return "QC_CID"; case CACHE_LOCK: return "CACHE"; case GUID_LOCK: return "GUID"; - case JWT_LOCK: return "JWT"; case OTHER_LOCK: return "OTHER"; case DEBUG1_LOCK: return "DEBUG1"; case DEBUG2_LOCK: return "DEBUG2";