Re: SSL_CTX_set_ecdh_auto noop OpenSSL 1.1.0?

2017-02-16 Thread William A Rowe Jr 
On Thu, Feb 16, 2017 at 4:39 PM, Yann Ylavic  wrote:
> On Thu, Feb 16, 2017 at 11:33 PM, Yann Ylavic  wrote:
>> On Thu, Feb 16, 2017 at 10:52 PM, William A Rowe Jr  
>> wrote:
>>> I'm not clear that this was a good usage of the current API...
>>>
>>> In file included from httpd-2.x/modules/ssl/ssl_private.h:90:0,
>>>  from httpd-2.x/modules/ssl/ssl_engine_init.c:29:
>>> httpd-2.x/modules/ssl/ssl_engine_init.c: In function 
>>> ‘ssl_init_server_certs’:
>>> include/openssl/ssl.h:1287:51: warning: statement with no effect
>>> [-Wunused-value]
>>>  # define SSL_CTX_set_ecdh_auto(dummy, onoff)  ((onoff) != 0)
>>>^
>>> httpd-2.x/modules/ssl/ssl_engine_init.c:1328:9: note: in expansion of
>>> macro ‘SSL_CTX_set_ecdh_auto’
>>>  SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
>>>  ^
>>
>> Looks like OpenSSL missed a cast to void in its macro implementation.
>> I think goal was to still evaluate "onoff", but in this case
>> "((void)((onoff) != 0))" or the usual "do (void)((onoff) != 0); while
>> (0)" would have been more clean/compatible...
>
> Wait, SSL_CTX_set_ecdh_auto() used to return an int so the macro is
> right actually.
> Hmm, picky compiler :)
>
>>
>> I guess we'll have to work around this with our own (void) casting.
>
> That still holds...

It was complaining about the unused 'dummy'. IMO, and perhaps the fact
that (1 != 0) evaluates to a constant expression.

Easier to drop it from the compilation path for 1.1.0+

Re: SSL_CTX_set_ecdh_auto noop OpenSSL 1.1.0?

2017-02-16 Thread Yann Ylavic 
On Thu, Feb 16, 2017 at 11:33 PM, Yann Ylavic  wrote:
> On Thu, Feb 16, 2017 at 10:52 PM, William A Rowe Jr  
> wrote:
>> I'm not clear that this was a good usage of the current API...
>>
>> In file included from httpd-2.x/modules/ssl/ssl_private.h:90:0,
>>  from httpd-2.x/modules/ssl/ssl_engine_init.c:29:
>> httpd-2.x/modules/ssl/ssl_engine_init.c: In function ‘ssl_init_server_certs’:
>> include/openssl/ssl.h:1287:51: warning: statement with no effect
>> [-Wunused-value]
>>  # define SSL_CTX_set_ecdh_auto(dummy, onoff)  ((onoff) != 0)
>>^
>> httpd-2.x/modules/ssl/ssl_engine_init.c:1328:9: note: in expansion of
>> macro ‘SSL_CTX_set_ecdh_auto’
>>  SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
>>  ^
>
> Looks like OpenSSL missed a cast to void in its macro implementation.
> I think goal was to still evaluate "onoff", but in this case
> "((void)((onoff) != 0))" or the usual "do (void)((onoff) != 0); while
> (0)" would have been more clean/compatible...

Wait, SSL_CTX_set_ecdh_auto() used to return an int so the macro is
right actually.
Hmm, picky compiler :)

>
> I guess we'll have to work around this with our own (void) casting.

That still holds...

Re: SSL_CTX_set_ecdh_auto noop OpenSSL 1.1.0?

2017-02-16 Thread Yann Ylavic 
On Thu, Feb 16, 2017 at 10:52 PM, William A Rowe Jr  wrote:
> I'm not clear that this was a good usage of the current API...
>
> In file included from httpd-2.x/modules/ssl/ssl_private.h:90:0,
>  from httpd-2.x/modules/ssl/ssl_engine_init.c:29:
> httpd-2.x/modules/ssl/ssl_engine_init.c: In function ‘ssl_init_server_certs’:
> include/openssl/ssl.h:1287:51: warning: statement with no effect
> [-Wunused-value]
>  # define SSL_CTX_set_ecdh_auto(dummy, onoff)  ((onoff) != 0)
>^
> httpd-2.x/modules/ssl/ssl_engine_init.c:1328:9: note: in expansion of
> macro ‘SSL_CTX_set_ecdh_auto’
>  SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
>  ^

Looks like OpenSSL missed a cast to void in its macro implementation.
I think goal was to still evaluate "onoff", but in this case
"((void)((onoff) != 0))" or the usual "do (void)((onoff) != 0); while
(0)" would have been more clean/compatible...

> This looks like a no-op now in OpenSSL 1.1.0.
>
> /*
>  * ...otherwise, enable auto curve selection (OpenSSL 1.0.2 and later)
>  * or configure NIST P-256 (required to enable ECDHE for earlier versions)
>  */
> else {
> #if defined(SSL_CTX_set_ecdh_auto)
> SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
> #else
> eckey = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
> SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
> #endif

I guess we'll have to work around this with our own (void) casting.

Re: SSL_CTX_set_ecdh_auto noop OpenSSL 1.1.0?

2017-02-16 Thread Jacob Champion 

On 02/16/2017 01:52 PM, William A Rowe Jr wrote:

This looks like a no-op now in OpenSSL 1.1.0.


https://github.com/openssl/openssl/issues/1437 seems to explain it.

--Jacob

SSL_CTX_set_ecdh_auto noop OpenSSL 1.1.0?

2017-02-16 Thread William A Rowe Jr 
I'm not clear that this was a good usage of the current API...

In file included from httpd-2.x/modules/ssl/ssl_private.h:90:0,
 from httpd-2.x/modules/ssl/ssl_engine_init.c:29:
httpd-2.x/modules/ssl/ssl_engine_init.c: In function ‘ssl_init_server_certs’:
include/openssl/ssl.h:1287:51: warning: statement with no effect
[-Wunused-value]
 # define SSL_CTX_set_ecdh_auto(dummy, onoff)  ((onoff) != 0)
   ^
httpd-2.x/modules/ssl/ssl_engine_init.c:1328:9: note: in expansion of
macro ‘SSL_CTX_set_ecdh_auto’
 SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
 ^

This looks like a no-op now in OpenSSL 1.1.0.

/*
 * ...otherwise, enable auto curve selection (OpenSSL 1.0.2 and later)
 * or configure NIST P-256 (required to enable ECDHE for earlier versions)
 */
else {
#if defined(SSL_CTX_set_ecdh_auto)
SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
#else
eckey = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
#endif