Mailing List Archive

Re: svn commit: r1874577 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_util_stapling.c
On 02/27/2020 01:43 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Thu Feb 27 12:43:51 2020
> New Revision: 1874577
>
> URL: http://svn.apache.org/viewvc?rev=1874577&view=rev
> Log:
> mod_ssl: Fix memory leak of OCSP stapling response.
>
> The OCSP_RESPONSE is either ignored or serialized (i2d_OCSP_RESPONSE) in the
> TLS response/handshake extension, so it must be freed.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c
>

> Modified: httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c?rev=1874577&r1=1874576&r2=1874577&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c Thu Feb 27 12:43:51 2020
> @@ -873,15 +873,21 @@ static int stapling_cb(SSL *ssl, void *a
> if (rsp && ((ok == TRUE) || (mctx->stapling_return_errors == TRUE))) {
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01956)
> "stapling_cb: setting response");
> - if (!stapling_set_response(ssl, rsp))
> - return SSL_TLSEXT_ERR_ALERT_FATAL;
> - return SSL_TLSEXT_ERR_OK;
> + if (!stapling_set_response(ssl, rsp)) {
> + rv = SSL_TLSEXT_ERR_ALERT_FATAL;
> + }
> + else {
> + rv = SSL_TLSEXT_ERR_OK;
> + }
> }
> - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01957)
> - "stapling_cb: no suitable response available");
> -
> - return SSL_TLSEXT_ERR_NOACK;
> + else {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01957)
> + "stapling_cb: no suitable response available");
> + rv = SSL_TLSEXT_ERR_NOACK;
> + }
> + OCSP_RESPONSE_free(rsp); /* NULL safe */

Out of curiosity: Why do you think that OCSP_RESPONSE_free is NULL safe?

Regards

RĂ¼diger
Re: svn commit: r1874577 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_util_stapling.c [ In reply to ]
On Fri, Feb 28, 2020 at 11:02 AM Ruediger Pluem <rpluem@apache.org> wrote:
>
> Out of curiosity: Why do you think that OCSP_RESPONSE_free is NULL safe?

All _free() functions have always been NULL-safe in openssl, quite
unlikely that they changed this..
I just verified for OCSP_RESPONSE_free() just in case :p and it seems
to be a "derived" ASN1 struct, thus NULL-safe per
"crypto/asn1/asn1_lib.c" in latest version (if I read the code
correctly).

Regards,
Yann.