From: Marc Jorge <openbsd@cypher-fox.com>
Subject: Re: relayd: free original cert after X509_dup in ssl_update_certificate
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Thu, 21 May 2026 16:37:39 +0200

On 5/21/26 12:08 AM, Theo Buehler wrote:

> While this seems correct, I wonder: should this X509_dup() not simply be
> deleted? This looks like a leftover from older code prior to the libtls
> rewrite. The cert is freshly deserialized from the PEM held in oldcert,
> so round tripping it through DER (which is what the X509_dup() does)
> doesn't really change anything.
>
> Error checking X509_set_pubkey and X509_set_issuer name wouldn't hurt
> either.
You are absolutely right. Here is the fixed patch.
Compiled and tested using tls inspection


Index: src/usr.sbin/relayd/ssl.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/ssl.c,v
diff -u -p -r1.39 ssl.c
--- src/usr.sbin/relayd/ssl.c    16 May 2026 13:16:50 -0000 1.39
+++ src/usr.sbin/relayd/ssl.c    21 May 2026 14:35:55 -0000
@@ -127,12 +127,15 @@ ssl_update_certificate(const uint8_t *ol
          name[1], sizeof(name[1])))
          goto done;

-    if ((cert = X509_dup(cert)) == NULL)
-        goto done;
-
      /* Update certificate key and use our CA as the issuer */
-    X509_set_pubkey(cert, pkey);
-    X509_set_issuer_name(cert, X509_get_subject_name(cacert));
+    if (!X509_set_pubkey(cert, pkey)) {
+        log_warnx("%s: X509_set_pubkey failed", __func__);
+        goto done;
+    }
+    if (!X509_set_issuer_name(cert, X509_get_subject_name(cacert))) {
+        log_warnx("%s: X509_set_issuer_name failed", __func__);
+        goto done;
+    }

      /* Sign with our CA */
      if (!X509_sign(cert, capkey, EVP_sha256())) {