From: op <op@openbsd.org>
Date: Thu, 25 May 2023 07:46:21 +0000 (+0000)
Subject: Forcibly update the EVP_PKEY's internal key
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=508824d40368da803a3da7e0c87c1127f6c01f6b;p=openbsd

Forcibly update the EVP_PKEY's internal key

To aid privilege separation, libtls maintains application-specific data
on the key inside the EVP_PKEY abstraction because the EVP API doesn't
provide a way to do that on the EVP_PKEY itself.

OpenSSL 3 changed behavior of EVP_PKEY_get1_RSA() and related functions.
These now return a struct from some cache.  Thus, modifying the RSA will
no longer modify the EVP_PKEY like it did previously, which was clearly
implied to be the case in the older documentation.
This is a subtle breaking change that affects several applications.

While this is documented, no real solution is provided.  The transition
plan from one OpenSSL major version to the next one tends to involve
many #ifdef in the ecosystem, and the only suggestion provided by the
new documentation is to switch to a completely unrelated, new API.

Instead, forcibly reset the internal key on EVP_PKEY after modification,
this way the change is picked up also by OpenSSL 3.

Fixes issue 1171 in OpenSMTPD-portable

ok tb@, jsing@
---

diff --git a/lib/libtls/tls.c b/lib/libtls/tls.c
index f3e7148f0d1..989339dc033 100644
--- a/lib/libtls/tls.c
+++ b/lib/libtls/tls.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls.c,v 1.95 2023/05/14 07:26:25 op Exp $ */
+/* $OpenBSD: tls.c,v 1.96 2023/05/25 07:46:21 op Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -410,12 +410,18 @@ tls_keypair_setup_pkey(struct tls *ctx, struct tls_keypair *keypair, EVP_PKEY *p
 			tls_set_errorx(ctx, "RSA key setup failure");
 			goto err;
 		}
-		if (ctx->config->sign_cb == NULL)
-			break;
-		if ((rsa_method = tls_signer_rsa_method()) == NULL ||
-		    RSA_set_ex_data(rsa, 1, ctx->config) == 0 ||
-		    RSA_set_method(rsa, rsa_method) == 0) {
-			tls_set_errorx(ctx, "failed to setup RSA key");
+		if (ctx->config->sign_cb != NULL) {
+			rsa_method = tls_signer_rsa_method();
+			if (rsa_method == NULL ||
+			    RSA_set_ex_data(rsa, 1, ctx->config) == 0 ||
+			    RSA_set_method(rsa, rsa_method) == 0) {
+				tls_set_errorx(ctx, "failed to setup RSA key");
+				goto err;
+			}
+		}
+		/* Reset the key to work around caching in OpenSSL 3. */
+		if (EVP_PKEY_set1_RSA(pkey, rsa) == 0) {
+			tls_set_errorx(ctx, "failed to set RSA key");
 			goto err;
 		}
 		break;
@@ -425,12 +431,18 @@ tls_keypair_setup_pkey(struct tls *ctx, struct tls_keypair *keypair, EVP_PKEY *p
 			tls_set_errorx(ctx, "EC key setup failure");
 			goto err;
 		}
-		if (ctx->config->sign_cb == NULL)
-			break;
-		if ((ecdsa_method = tls_signer_ecdsa_method()) == NULL ||
-		    ECDSA_set_ex_data(eckey, 1, ctx->config) == 0 ||
-		    ECDSA_set_method(eckey, ecdsa_method) == 0) {
-			tls_set_errorx(ctx, "failed to setup EC key");
+		if (ctx->config->sign_cb != NULL) {
+			ecdsa_method = tls_signer_ecdsa_method();
+			if (ecdsa_method == NULL ||
+			    ECDSA_set_ex_data(eckey, 1, ctx->config) == 0 ||
+			    ECDSA_set_method(eckey, ecdsa_method) == 0) {
+				tls_set_errorx(ctx, "failed to setup EC key");
+				goto err;
+			}
+		}
+		/* Reset the key to work around caching in OpenSSL 3. */
+		if (EVP_PKEY_set1_EC_KEY(pkey, eckey) == 0) {
+			tls_set_errorx(ctx, "failed to set EC key");
 			goto err;
 		}
 		break;