Skip to content

Commit 884ad00

Browse files
davidbensamuel40791765
authored andcommitted
Remove X509_STORE_set_get_issuer
This is unused. Removing it removes a codepath where callers may inadvertently break internal invariants of the verifier. It also removes an attractive nuisance: pyca/cryptograpy at one point intended to use this callback for AIA fetching. They are lucky they never did because that would have been a security bug. Certificates returned by this callback are "trusted" which means, if they satisfy the X509_TRUST criteria (e.g. are self-signed), they would become trust anchors! Also remove the getters for the callbacks, as no one is using them. Not much good can be done by extracting callbacks. Either it is your X509_STORE, in which case you know your own callbacks, or it is someone else's, in which case it probably depends on some application-specific state that you don't know about. Update-Note: Removed a handful of unused functions. Change-Id: Ic95db40186a9107e2a3f44028aa28a335653c25a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64987 Commit-Queue: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit b083a69e5b9a38fcbae710b0c2081de0235d509b)
1 parent 22d6f7a commit 884ad00

File tree

4 files changed

+5
-39
lines changed

4 files changed

+5
-39
lines changed

crypto/x509/internal.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -308,14 +308,15 @@ struct x509_store_st {
308308

309309
// Callbacks for various operations
310310
X509_STORE_CTX_verify_cb verify_cb; // error callback
311-
X509_STORE_CTX_get_issuer_fn get_issuer; // get issuers cert from ctx
312311
X509_STORE_CTX_get_crl_fn get_crl; // retrieve CRL
313312
X509_STORE_CTX_check_crl_fn check_crl; // Check CRL validity
314313

315314
CRYPTO_refcount_t references;
316315
CRYPTO_EX_DATA ex_data;
317316
} /* X509_STORE */;
318317

318+
typedef int (*X509_STORE_CTX_get_issuer_fn)(X509 **issuer, X509_STORE_CTX *ctx,
319+
X509 *x);
319320

320321
// This is the functions plus an instance of the local variables.
321322
struct x509_lookup_st {

crypto/x509/x509_lu.c

-21
Original file line numberDiff line numberDiff line change
@@ -642,37 +642,16 @@ void X509_STORE_set_verify_cb(X509_STORE *ctx,
642642
ctx->verify_cb = verify_cb;
643643
}
644644

645-
X509_STORE_CTX_verify_cb X509_STORE_get_verify_cb(X509_STORE *ctx) {
646-
return ctx->verify_cb;
647-
}
648-
649-
void X509_STORE_set_get_issuer(X509_STORE *ctx,
650-
X509_STORE_CTX_get_issuer_fn get_issuer) {
651-
ctx->get_issuer = get_issuer;
652-
}
653-
654-
X509_STORE_CTX_get_issuer_fn X509_STORE_get_get_issuer(X509_STORE *ctx) {
655-
return ctx->get_issuer;
656-
}
657-
658645
void X509_STORE_set_get_crl(X509_STORE *ctx,
659646
X509_STORE_CTX_get_crl_fn get_crl) {
660647
ctx->get_crl = get_crl;
661648
}
662649

663-
X509_STORE_CTX_get_crl_fn X509_STORE_get_get_crl(X509_STORE *ctx) {
664-
return ctx->get_crl;
665-
}
666-
667650
void X509_STORE_set_check_crl(X509_STORE *ctx,
668651
X509_STORE_CTX_check_crl_fn check_crl) {
669652
ctx->check_crl = check_crl;
670653
}
671654

672-
X509_STORE_CTX_check_crl_fn X509_STORE_get_check_crl(X509_STORE *ctx) {
673-
return ctx->check_crl;
674-
}
675-
676655
X509_STORE *X509_STORE_CTX_get0_store(X509_STORE_CTX *ctx) { return ctx->ctx; }
677656

678657
int X509_STORE_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,

crypto/x509/x509_vfy.c

+3-5
Original file line numberDiff line numberDiff line change
@@ -1691,11 +1691,9 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
16911691
goto err;
16921692
}
16931693

1694-
if (store->get_issuer) {
1695-
ctx->get_issuer = store->get_issuer;
1696-
} else {
1697-
ctx->get_issuer = X509_STORE_CTX_get1_issuer;
1698-
}
1694+
// TODO(davidben): Remove this pointer. It only exists to be overwritten by
1695+
// X509_STORE_CTX_set0_trusted_stack.
1696+
ctx->get_issuer = X509_STORE_CTX_get1_issuer;
16991697

17001698
if (store->verify_cb) {
17011699
ctx->verify_cb = store->verify_cb;

include/openssl/x509.h

-12
Original file line numberDiff line numberDiff line change
@@ -3665,8 +3665,6 @@ certificate chain.
36653665
DEFINE_STACK_OF(X509_OBJECT)
36663666

36673667
typedef int (*X509_STORE_CTX_verify_cb)(int, X509_STORE_CTX *);
3668-
typedef int (*X509_STORE_CTX_get_issuer_fn)(X509 **issuer, X509_STORE_CTX *ctx,
3669-
X509 *x);
36703668
typedef int (*X509_STORE_CTX_get_crl_fn)(X509_STORE_CTX *ctx, X509_CRL **crl,
36713669
X509 *x);
36723670
typedef int (*X509_STORE_CTX_check_crl_fn)(X509_STORE_CTX *ctx, X509_CRL *crl);
@@ -3944,20 +3942,10 @@ OPENSSL_EXPORT void X509_STORE_set_verify_cb(
39443942
X509_STORE *ctx, X509_STORE_CTX_verify_cb verify_cb);
39453943
#define X509_STORE_set_verify_cb_func(ctx, func) \
39463944
X509_STORE_set_verify_cb((ctx), (func))
3947-
OPENSSL_EXPORT X509_STORE_CTX_verify_cb
3948-
X509_STORE_get_verify_cb(X509_STORE *ctx);
3949-
OPENSSL_EXPORT void X509_STORE_set_get_issuer(
3950-
X509_STORE *ctx, X509_STORE_CTX_get_issuer_fn get_issuer);
3951-
OPENSSL_EXPORT X509_STORE_CTX_get_issuer_fn
3952-
X509_STORE_get_get_issuer(X509_STORE *ctx);
39533945
OPENSSL_EXPORT void X509_STORE_set_get_crl(X509_STORE *ctx,
39543946
X509_STORE_CTX_get_crl_fn get_crl);
3955-
OPENSSL_EXPORT X509_STORE_CTX_get_crl_fn
3956-
X509_STORE_get_get_crl(X509_STORE *ctx);
39573947
OPENSSL_EXPORT void X509_STORE_set_check_crl(
39583948
X509_STORE *ctx, X509_STORE_CTX_check_crl_fn check_crl);
3959-
OPENSSL_EXPORT X509_STORE_CTX_check_crl_fn
3960-
X509_STORE_get_check_crl(X509_STORE *ctx);
39613949

39623950
// X509_STORE_CTX_new returns a newly-allocated, empty |X509_STORE_CTX|, or NULL
39633951
// on error.

0 commit comments

Comments
 (0)