From: Theo Buehler <tb@theobuehler.org>
Subject: Re: iked: free untrusted cert stacks properly
To: Jonathan Matthew <jonathan@d14n.org>
Cc: tech@openbsd.org, tobhe@openbsd.org
Date: Tue, 2 Sep 2025 07:17:15 +0200

On Tue, Sep 02, 2025 at 03:12:48PM +1000, Jonathan Matthew wrote:
> We've observed the iked CA process growing in size over time, with no
> bound as far as we can tell.  Here's one that's been running for a couple
> of months:
> 
> _iked    83201  0.0 44.8 763208 455168 ??  Ipc     4Jun25   10:43.73 iked: ca (iked)
> 
> after staring at this for a while I realised the CA process doesn't
> correctly free stacks of untrusted certs when it receives a cert bundle
> to validate.  sk_X509_free() doesn't call X509_free(), so it only frees the
> memory allocated for the x509 struct itself.
> sk_X509_pop_free(stack, X509_free) works better.
> 
> With the diff below, memory usage is stable and everything still seems to
> work properly.
> 
> ok?

ok tb

> 
> 
> Index: ca.c
> ===================================================================
> RCS file: /cvs/src/sbin/iked/ca.c,v
> diff -u -p -u -p -r1.102 ca.c
> --- ca.c	18 Jun 2024 05:08:41 -0000	1.102
> +++ ca.c	2 Sep 2025 05:07:55 -0000
> @@ -321,7 +321,7 @@ ca_decode_cert_bundle(struct iked *env, 
>  	if (ret != 0)
>  		log_info("%s: failed to decode cert bundle",
>  		    SPI_SH(sh, __func__));
> -	sk_X509_free(untrusted);
> +	sk_X509_pop_free(untrusted, X509_free);
>  	return ret;
>  }
>  
> @@ -659,7 +659,7 @@ ca_getcert(struct iked *env, struct imsg
>  				    type, issuer);
>  				X509_free(issuer);
>  				if (ret == 0) {
> -					sk_X509_free(untrusted);
> +					sk_X509_pop_free(untrusted, X509_free);
>  					return (0);
>  				}
>  			} else
> @@ -699,7 +699,7 @@ ca_getcert(struct iked *env, struct imsg
>  
>  	ret = proc_composev(&env->sc_ps, PROC_IKEV2, cmd, iov, iovcnt);
>  	ibuf_free(key.id_buf);
> -	sk_X509_free(untrusted);
> +	sk_X509_pop_free(untrusted, X509_free);
>  
>  	return (ret);
>  }
>