You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rainer Jung <ra...@kippdata.de> on 2015/05/22 18:29:32 UTC
Two questions on mod_ssl source code details
1) In other code I see
EC_KEY_free(ecdh);
after
EC_KEY *ecdh = EC_KEY_new_by_curve_name(...)
and using ecdh, e.g. in
SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
Should we add the free? Or is it not needed? Anyone knows why?
2) In modules/ssl/ssl_private.h I see
/**
* The following features all depend on TLS extension support.
* Within this block, check again for features (not version numbers).
*/
#if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_set_tlsext_host_name)
#define HAVE_TLSEXT
and then further checks and defines for OCSP, Session Tickets, SRP,
ALPN, all inside this "if" block.
Is it really true, that they are only supported if
SSL_set_tlsext_host_name is defined? That function seems to belong only
to SNI.
Should we switch the code to:
/**
* The following features all depend on TLS extension support.
* Within this block, check again for features (not version numbers).
*/
#if !defined(OPENSSL_NO_TLSEXT)
#define HAVE_TLSEXT
#if defined(SSL_set_tlsext_host_name)
#define HAVE_SNI
#endif
and then use HAVE_SNI where appropriate.
Regards,
Rainer
Re: Two questions on mod_ssl source code details
Posted by Rainer Jung <ra...@kippdata.de>.
>> 1) In other code I see
>>
>> EC_KEY_free(ecdh);
>>
>> after
>>
>> EC_KEY *ecdh = EC_KEY_new_by_curve_name(...)
>> and using ecdh, e.g. in
>> SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
>>
>> Should we add the free? Or is it not needed? Anyone knows why?
>
> This was added in r1666363:
>
> * mod_ssl: fix small memory leak in ssl_init_server_certs when ECDH is used.
> SSL_CTX_set_tmp_ecdh increases reference count, so we have to call EC_KEY_free,
> otherwise eckey will not be freed.
Damn, I was working my way though old patches to port them to Tomcat
tcnative and forgot to check the current code.
>> 2) In modules/ssl/ssl_private.h I see
>>
>> /**
>> * The following features all depend on TLS extension support.
>> * Within this block, check again for features (not version numbers).
>> */
>> #if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_set_tlsext_host_name)
>>
>> #define HAVE_TLSEXT
>
> I guess this was (one of) the first TLS extention added to OpenSSL,
> hence OPENSSL_NO_TLSEXT was probably defined at the same time as
> SSL_set_tlsext_host_name.
> This code checks if extensions are not disabled (OPENSSL_NO_TLSEXT),
> but that's relevent only if they exist in OpenSSL
> (SSL_set_tlsext_host_name).
>
>>
>> Should we switch the code to:
>>
>> /**
>> * The following features all depend on TLS extension support.
>> * Within this block, check again for features (not version numbers).
>> */
>> #if !defined(OPENSSL_NO_TLSEXT)
>
> That would be true before OPENSSL_NO_TLSEXT existed...
Yup, got it.
Thanks a bunch,
Rainer
Re: Two questions on mod_ssl source code details
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, May 22, 2015 at 6:29 PM, Rainer Jung <ra...@kippdata.de> wrote:
>
> 2) In modules/ssl/ssl_private.h I see
>
> /**
> * The following features all depend on TLS extension support.
> * Within this block, check again for features (not version numbers).
> */
> #if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_set_tlsext_host_name)
>
> #define HAVE_TLSEXT
I guess this was (one of) the first TLS extention added to OpenSSL,
hence OPENSSL_NO_TLSEXT was probably defined at the same time as
SSL_set_tlsext_host_name.
This code checks if extensions are not disabled (OPENSSL_NO_TLSEXT),
but that's relevent only if they exist in OpenSSL
(SSL_set_tlsext_host_name).
>
> Should we switch the code to:
>
> /**
> * The following features all depend on TLS extension support.
> * Within this block, check again for features (not version numbers).
> */
> #if !defined(OPENSSL_NO_TLSEXT)
That would be true before OPENSSL_NO_TLSEXT existed...
Regards,
Yann.
Re: Two questions on mod_ssl source code details
Posted by Rainer Jung <ra...@kippdata.de>.
Am 22.05.2015 um 18:35 schrieb Yann Ylavic:
> On Fri, May 22, 2015 at 6:29 PM, Rainer Jung <ra...@kippdata.de> wrote:
>> 1) In other code I see
>>
>> EC_KEY_free(ecdh);
>>
>> after
>>
>> EC_KEY *ecdh = EC_KEY_new_by_curve_name(...)
>> and using ecdh, e.g. in
>> SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
>>
>> Should we add the free? Or is it not needed? Anyone knows why?
>
> This was added in r1666363:
>
> * mod_ssl: fix small memory leak in ssl_init_server_certs when ECDH is used.
> SSL_CTX_set_tmp_ecdh increases reference count, so we have to call EC_KEY_free,
> otherwise eckey will not be freed.
Ha! It is in trunk and 2.2, but the backport/changes in 2.4 were
incomplete. Exactly the free is missing. Proposed now for 2.4 in STATUS.
Regards,
Rainer
Re: Two questions on mod_ssl source code details
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, May 22, 2015 at 6:29 PM, Rainer Jung <ra...@kippdata.de> wrote:
> 1) In other code I see
>
> EC_KEY_free(ecdh);
>
> after
>
> EC_KEY *ecdh = EC_KEY_new_by_curve_name(...)
> and using ecdh, e.g. in
> SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
>
> Should we add the free? Or is it not needed? Anyone knows why?
This was added in r1666363:
* mod_ssl: fix small memory leak in ssl_init_server_certs when ECDH is used.
SSL_CTX_set_tmp_ecdh increases reference count, so we have to call EC_KEY_free,
otherwise eckey will not be freed.
Regards,
Yann.