From 4f19ead7b58ba35687d31d3bd67085999cc603da Mon Sep 17 00:00:00 2001 From: tb Date: Sun, 24 Jul 2022 19:54:46 +0000 Subject: [PATCH] Prepare to provide TS_VERIFY_CTX accessors The setters make no sense since they do not free the old members and return what was passed in instead of returning the old struct member so that the caller has a chance of freeing them. This has the side effect that calling a setter a second time will likely result in a leak. TS_VERIFY_CTX_set_imprint() was "fixed" upstream by adding a free() but the other three setters were missed since discussing the contributor's CLA was more important. Also missed was that adding frees will result in double frees: careful consumers like openssl/ruby have workarounds for the strange existing semantics. Add a compat #define for TS_VERIF_CTS_set_certs() that made it into the public API with a typo. A good illustration of the amount of thought and care that went into the OpenSSL 1.1 API by both the implementers and the reviewers. Amazing job overall. We will be stuck with this nonsense for a long time. ok jsing kn --- lib/libcrypto/ts/ts.h | 15 +++++++- lib/libcrypto/ts/ts_verify_ctx.c | 66 +++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/lib/libcrypto/ts/ts.h b/lib/libcrypto/ts/ts.h index 3c6baf82e07..83bd6829ae4 100644 --- a/lib/libcrypto/ts/ts.h +++ b/lib/libcrypto/ts/ts.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ts.h,v 1.16 2022/07/24 19:25:36 tb Exp $ */ +/* $OpenBSD: ts.h,v 1.17 2022/07/24 19:54:46 tb Exp $ */ /* Written by Zoltan Glozik (zglozik@opentsa.org) for the OpenSSL * project 2002, 2003, 2004. */ @@ -682,6 +682,19 @@ void TS_VERIFY_CTX_init(TS_VERIFY_CTX *ctx); void TS_VERIFY_CTX_free(TS_VERIFY_CTX *ctx); void TS_VERIFY_CTX_cleanup(TS_VERIFY_CTX *ctx); +#if defined(LIBRESSL_INTERNAL) +int TS_VERIFY_CTX_add_flags(TS_VERIFY_CTX *ctx, int flags); +int TS_VERIFY_CTX_set_flags(TS_VERIFY_CTX *ctx, int flags); +BIO *TS_VERIFY_CTX_set_data(TS_VERIFY_CTX *ctx, BIO *bio); +X509_STORE *TS_VERIFY_CTX_set_store(TS_VERIFY_CTX *ctx, X509_STORE *store); +/* R$ special */ +#define TS_VERIFY_CTS_set_certs TS_VERIFY_CTX_set_certs +STACK_OF(X509) *TS_VERIFY_CTX_set_certs(TS_VERIFY_CTX *ctx, + STACK_OF(X509) *certs); +unsigned char *TS_VERIFY_CTX_set_imprint(TS_VERIFY_CTX *ctx, + unsigned char *imprint, long imprint_len); +#endif + /* * If ctx is NULL, it allocates and returns a new object, otherwise * it returns ctx. It initialises all the members as follows: diff --git a/lib/libcrypto/ts/ts_verify_ctx.c b/lib/libcrypto/ts/ts_verify_ctx.c index 83ef54a8947..ef0ec6ca7f8 100644 --- a/lib/libcrypto/ts/ts_verify_ctx.c +++ b/lib/libcrypto/ts/ts_verify_ctx.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ts_verify_ctx.c,v 1.10 2022/07/24 08:16:47 tb Exp $ */ +/* $OpenBSD: ts_verify_ctx.c,v 1.11 2022/07/24 19:54:46 tb Exp $ */ /* Written by Zoltan Glozik (zglozik@stones.com) for the OpenSSL * project 2003. */ @@ -114,6 +114,70 @@ TS_VERIFY_CTX_cleanup(TS_VERIFY_CTX *ctx) TS_VERIFY_CTX_init(ctx); } +/* + * XXX: The following accessors demonstrate the amount of care and thought that + * went into OpenSSL 1.1 API design and the review thereof: for whatever reason + * these functions return what was passed in. Correct memory management is left + * as an exercise for the reader... Unfortunately, careful consumers like + * openssl-ruby assume this behavior, so we're stuck with this insanity. The + * cherry on top is the TS_VERIFY_CTS_set_certs() [sic!] function that made it + * into the public API. + * + * Outstanding job, R$ and tjh, A+. + */ + +int +TS_VERIFY_CTX_add_flags(TS_VERIFY_CTX *ctx, int flags) +{ + ctx->flags |= flags; + + return ctx->flags; +} + +int +TS_VERIFY_CTX_set_flags(TS_VERIFY_CTX *ctx, int flags) +{ + ctx->flags = flags; + + return ctx->flags; +} + +BIO * +TS_VERIFY_CTX_set_data(TS_VERIFY_CTX *ctx, BIO *bio) +{ + ctx->data = bio; + + return ctx->data; +} + +X509_STORE * +TS_VERIFY_CTX_set_store(TS_VERIFY_CTX *ctx, X509_STORE *store) +{ + ctx->store = store; + + return ctx->store; +} + +STACK_OF(X509) * +TS_VERIFY_CTX_set_certs(TS_VERIFY_CTX *ctx, STACK_OF(X509) *certs) +{ + ctx->certs = certs; + + return ctx->certs; +} + +unsigned char * +TS_VERIFY_CTX_set_imprint(TS_VERIFY_CTX *ctx, unsigned char *imprint, + long imprint_len) +{ + free(ctx->imprint); + + ctx->imprint = imprint; + ctx->imprint_len = imprint_len; + + return ctx->imprint; +} + TS_VERIFY_CTX * TS_REQ_to_TS_VERIFY_CTX(TS_REQ *req, TS_VERIFY_CTX *ctx) { -- 2.20.1