On Wed, Apr 15, 2020 at 06:42:17PM +0100, Stuart Henderson wrote:
> On 2020/04/15 19:20, Giovanni Bechis wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA512
> >
> > Hi,
> > memory leak in mod_ssl fixed and take maintainership, ok ?
> >
> > Cheers
> > Giovanni
> >
> > Index: Makefile
> > ===================================================================
> > RCS file: /cvs/ports/www/apache-httpd/Makefile,v
> > retrieving revision 1.102
> > diff -u -p -r1.102 Makefile
> > - --- Makefile 31 Mar 2020 14:30:33 -0000 1.102
> > +++ Makefile 15 Apr 2020 17:06:22 -0000
> > @@ -5,8 +5,11 @@ COMMENT= apache HTTP server
> > V= 2.4.43
> > DISTNAME= httpd-${V}
> > PKGNAME= apache-httpd-${V}
> > +REVISION= 0
> >
> > CATEGORIES= www net
> > +
> > +MAINTAINER= Giovanni Bechis <giova...@openbsd.org>
> >
> > HOMEPAGE= https://httpd.apache.org/
> >
> > Index: patches/patch-modules_ssl_ssl_util_stapling_c
> > ===================================================================
> > RCS file: patches/patch-modules_ssl_ssl_util_stapling_c
> > diff -N patches/patch-modules_ssl_ssl_util_stapling_c
> > - --- /dev/null 1 Jan 1970 00:00:00 -0000
> > +++ patches/patch-modules_ssl_ssl_util_stapling_c 15 Apr 2020 17:06:22
> > -0000
> > @@ -0,0 +1,84 @@
> > +$OpenBSD$
> > +
> > +# Memory leak in mod_ssl
> > (https://bz.apache.org/bugzilla/show_bug.cgi?id=63687)
>
> icing's version was already committed upstream, it's probably better to use
> that I think?
>
> https://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c?view=log
>
this is icing's version, shouldn't I mention the bz as a referral ?
Giovanni
> > +--- modules/ssl/ssl_util_stapling.c.orig
> > ++++ modules/ssl/ssl_util_stapling.c
> > +@@ -134,6 +134,7 @@ int ssl_stapling_init_cert(server_rec *s, apr_pool_t *
> > + X509 *issuer = NULL;
> > + OCSP_CERTID *cid = NULL;
> > + STACK_OF(OPENSSL_STRING) *aia = NULL;
> > ++ int rv = 1; /* until further notice */
> > +
> > + if (x == NULL)
> > + return 0;
> > +@@ -158,16 +159,18 @@ int ssl_stapling_init_cert(server_rec *s, apr_pool_t
> > *
> > + SSL_CTX_set_tlsext_status_cb(mctx->ssl_ctx, stapling_cb);
> > + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(10177)
> > "OCSP stapling added via hook");
> > + }
> > +- return 1;
> > ++ goto cleanup;
> > + }
> > +
> > + if (mctx->stapling_enabled != TRUE) {
> > + /* mod_ssl's own implementation is not enabled */
> > +- return 1;
> > ++ goto cleanup;
> > + }
> > +
> > +- if (X509_digest(x, EVP_sha1(), idx, NULL) != 1)
> > +- return 0;
> > ++ if (X509_digest(x, EVP_sha1(), idx, NULL) != 1) {
> > ++ rv = 0;
> > ++ goto cleanup;
> > ++ }
> > +
> > + cinf = apr_hash_get(stapling_certinfo, idx, sizeof(idx));
> > + if (cinf) {
> > +@@ -181,18 +184,18 @@ int ssl_stapling_init_cert(server_rec *s, apr_pool_t
> > *
> > + APLOGNO(02814) "ssl_stapling_init_cert: no
> > OCSP URI "
> > + "in certificate and no SSLStaplingForceURL "
> > + "configured for server %s",
> > mctx->sc->vhost_id);
> > +- return 0;
> > ++ rv = 0;
> > + }
> > +- return 1;
> > ++ goto cleanup;
> > + }
> > +
> > + cid = OCSP_cert_to_id(NULL, x, issuer);
> > +- X509_free(issuer);
> > + if (!cid) {
> > + ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x,
> > APLOGNO(02815)
> > + "ssl_stapling_init_cert: can't create CertID "
> > + "for OCSP request");
> > +- return 0;
> > ++ rv = 0;
> > ++ goto cleanup;
> > + }
> > +
> > + aia = X509_get1_ocsp(x);
> > +@@ -201,7 +204,8 @@ int ssl_stapling_init_cert(server_rec *s, apr_pool_t *
> > + ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x,
> > + APLOGNO(02218) "ssl_stapling_init_cert: no OCSP
> > URI "
> > + "in certificate and no SSLStaplingForceURL set");
> > +- return 0;
> > ++ rv = 0;
> > ++ goto cleanup;
> > + }
> > +
> > + /* At this point, we have determined that there's something to store
> > */
> > +@@ -222,8 +226,10 @@ int ssl_stapling_init_cert(server_rec *s, apr_pool_t *
> > + mctx->sc->vhost_id);
> > +
> > + apr_hash_set(stapling_certinfo, cinf->idx, sizeof(cinf->idx), cinf);
> > +-
> > +- return 1;
> > ++
> > ++cleanup:
> > ++ X509_free(issuer);
> > ++ return rv;
> > + }
> > +
> > + static certinfo *stapling_get_certinfo(server_rec *s, X509 *x,
> > modssl_ctx_t *mctx,
> >
> > -----BEGIN PGP SIGNATURE-----
> >
> > iQIzBAEBCgAdFiEEqg3TnG6R3qYMxl94+r7qCYlyWOUFAl6XQkUACgkQ+r7qCYly
> > WOXJ1Q/+PGkrfKUjhHW1jhJEltrtXHvLGna+QfOsX9+JyCnTIq1qBeIeWmQRgwYB
> > seiDHupE2Mi2+ytwwzilV7f88GiHL6i+hd00kQiMHsOGkowr7x86hRWZc1kfNrcT
> > iQTUOKj38Ri78Xjyx/9kj1+vKDGtplD+eyuo9pVFH6HWDXKIV0Q7k7Jl7IffDmdY
> > 9NeKGhwxvrnscjUliMOLBCyucsB04XcbtRyQZlxjsGnBLyyOWCeJR7o0CKUy3jE0
> > JZ3dQ5mdig+ZYbUsDnd5uMmBcWJV5uqu4lGMezCfhSf+fBQ6nO9L3J9NhPWRUcFu
> > EH01l/rBggE50bfL59tNCbutaEGIpesDSbN5nI54ugSb7FVm9vKO26WPsDgufr3s
> > fSZOY01qNi4Tyevik9Q8NTO4MSdcevlkSvh9InX/bKP3udX0Rj96X/qLMiwPlkYy
> > ffHzibnu3Fg4Z9EPWRe19PZ0QjBViQ7Z8iWGIgZd44aIj5AP42ZFbK8ptVAKqsd9
> > M0VwpDooUv8UDzFiBtu3M1NCatA/2I4CVXRdwQgAdXoe5OnaJGB11Z1Yfvgjxgr2
> > p39mSNZAAKIlLpzUVFFAyrv4QjvkphWLbeTLJ2yghpeQbRwfcLnW/wEXxA0d4Ehg
> > CUpq4IPNf/QUCnUQHHBscnd8NyWZ0T+tocHxtCRdoGalac1pCJg=
> > =nsHV
> > -----END PGP SIGNATURE-----
>
signature.asc
Description: PGP signature
