From b2cb3b21fe30e1431bab94b03e20b4e2b96bd706 Mon Sep 17 00:00:00 2001 From: Bob Beck Date: Mon, 22 Dec 2025 11:32:08 -0700 Subject: [PATCH] Constify the X509_STORE_CTX argument to the lookup_certs funcitons. The justification for this not being const was because of lookup_certs_sk(). The reasons this function could not have a const store, is that it set the ctx's error code when we could not allocate memory and returned NULL. However, the other lookup_certs function, X509_STORE_CTX_get1_certs, already does not set this error code when failing to allocate memory on a return. Given that you can't depend on the out of memory error code being set in the general case, and the Beyonce rule appears to indicate that nobody likes this behaviour (as nobody put a test on it) I think it's safe to say we should just not modify the ctx, and constify it. For #28654 --- crypto/x509/x509_local.h | 3 +-- crypto/x509/x509_lu.c | 2 +- crypto/x509/x509_vfy.c | 3 +-- include/crypto/x509.h | 2 +- include/openssl/x509_vfy.h.in | 4 ++-- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crypto/x509/x509_local.h b/crypto/x509/x509_local.h index 9eef700436..58f2d9f79b 100644 --- a/crypto/x509/x509_local.h +++ b/crypto/x509/x509_local.h @@ -149,9 +149,8 @@ struct x509_store_st { int (*cert_crl)(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x); /* Check policy status of the chain */ int (*check_policy)(X509_STORE_CTX *ctx); - STACK_OF(X509) *(*lookup_certs)(X509_STORE_CTX *ctx, + STACK_OF(X509) *(*lookup_certs)(const X509_STORE_CTX *ctx, const X509_NAME *nm); - /* cannot constify 'ctx' param due to lookup_certs_sk() in x509_vfy.c */ STACK_OF(X509_CRL) *(*lookup_crls)(const X509_STORE_CTX *ctx, const X509_NAME *nm); int (*cleanup)(X509_STORE_CTX *ctx); diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index 1167fcf1fc..bd32bc9b34 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -829,7 +829,7 @@ out_free: * Collect from |ctx->store| all certs with subject matching |nm|. * Returns NULL on internal/fatal error, empty stack if not found. */ -STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx, +STACK_OF(X509) *X509_STORE_CTX_get1_certs(const X509_STORE_CTX *ctx, const X509_NAME *nm) { int i, idx = -1, cnt = 0; diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index ef15131e82..a789ec6f91 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -520,7 +520,7 @@ static int get1_best_issuer_other_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x * Alternative lookup method: look from a STACK stored in other_ctx. * Returns NULL on internal/fatal error, empty stack if not found. */ -static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx, const X509_NAME *nm) +static STACK_OF(X509) *lookup_certs_sk(const X509_STORE_CTX *ctx, const X509_NAME *nm) { STACK_OF(X509) *sk = sk_X509_new_null(); X509 *x; @@ -533,7 +533,6 @@ static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx, const X509_NAME *nm) if (X509_NAME_cmp(nm, X509_get_subject_name(x)) == 0) { if (!X509_add_cert(sk, x, X509_ADD_FLAG_UP_REF)) { OSSL_STACK_OF_X509_free(sk); - ctx->error = X509_V_ERR_OUT_OF_MEM; return NULL; } } diff --git a/include/crypto/x509.h b/include/crypto/x509.h index 95367b8ff2..fd749f1bf3 100644 --- a/include/crypto/x509.h +++ b/include/crypto/x509.h @@ -244,7 +244,7 @@ struct x509_store_ctx_st { /* X509_STORE_CTX */ int (*cert_crl)(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x); /* Check policy status of the chain */ int (*check_policy)(X509_STORE_CTX *ctx); - STACK_OF(X509) *(*lookup_certs)(X509_STORE_CTX *ctx, + STACK_OF(X509) *(*lookup_certs)(const X509_STORE_CTX *ctx, const X509_NAME *nm); /* cannot constify 'ctx' param due to lookup_certs_sk() in x509_vfy.c */ STACK_OF(X509_CRL) *(*lookup_crls)(const X509_STORE_CTX *ctx, diff --git a/include/openssl/x509_vfy.h.in b/include/openssl/x509_vfy.h.in index 17faf310b1..d7c1a6a471 100644 --- a/include/openssl/x509_vfy.h.in +++ b/include/openssl/x509_vfy.h.in @@ -169,7 +169,7 @@ typedef int (*X509_STORE_CTX_cert_crl_fn)(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x); typedef int (*X509_STORE_CTX_check_policy_fn)(X509_STORE_CTX *ctx); typedef STACK_OF(X509) - *(*X509_STORE_CTX_lookup_certs_fn)(X509_STORE_CTX *ctx, + *(*X509_STORE_CTX_lookup_certs_fn)(const X509_STORE_CTX *ctx, const X509_NAME *nm); typedef STACK_OF(X509_CRL) *(*X509_STORE_CTX_lookup_crls_fn)(const X509_STORE_CTX *ctx, @@ -427,7 +427,7 @@ STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(const X509_STORE *xs); #endif STACK_OF(X509_OBJECT) *X509_STORE_get1_objects(X509_STORE *xs); STACK_OF(X509) *X509_STORE_get1_all_certs(X509_STORE *xs); -STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *xs, +STACK_OF(X509) *X509_STORE_CTX_get1_certs(const X509_STORE_CTX *xs, const X509_NAME *nm); STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(const X509_STORE_CTX *st, const X509_NAME *nm);