Prepare to provide TS_VERIFY_CTX accessors
authortb <tb@openbsd.org>
Sun, 24 Jul 2022 19:54:46 +0000 (19:54 +0000)
committertb <tb@openbsd.org>
Sun, 24 Jul 2022 19:54:46 +0000 (19:54 +0000)
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
lib/libcrypto/ts/ts_verify_ctx.c

index 3c6baf8..83bd682 100644 (file)
@@ -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:
index 83ef54a..ef0ec6c 100644 (file)
@@ -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)
 {