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";