Re: [Update] Support for OpenSSL 1.1.0

2016-03-23 Thread Rainer Jung

not before 2.4.19 -> not before 2.4.20 ...

Am 23.03.2016 um 15:18 schrieb Rainer Jung:

OpenSSL 1.1.0 pre 4 = Beta 1 is out.

I did another round of compatibility updates for mod_ssl. Apart form
fixing Bugs, the OpenSSL 1.1.0 API is supposed to stay stable now. So I
hope mod_ssl can stabilize now.

The current code runs the test suite with 1.0.2 and with 1.1.0 without
any ssl related failures.

I'll let it settle a bit before suggesting backport to 2.4, definitely
not before 2.4.19.

More critical eyes on the 1.1.0 specific code paths very welcome!


Re: [Update] Support for OpenSSL 1.1.0

2016-03-23 Thread Rainer Jung

OpenSSL 1.1.0 pre 4 = Beta 1 is out.

I did another round of compatibility updates for mod_ssl. Apart form 
fixing Bugs, the OpenSSL 1.1.0 API is supposed to stay stable now. So I 
hope mod_ssl can stabilize now.


The current code runs the test suite with 1.0.2 and with 1.1.0 without 
any ssl related failures.


I'll let it settle a bit before suggesting backport to 2.4, definitely 
not before 2.4.19.


More critical eyes on the 1.1.0 specific code paths very welcome!

Regards,

Rainer


Re: [Update] Support for OpenSSL 1.1.0

2016-02-14 Thread Rainer Jung
The nice people at OpenSSL have already committed the two patches 
(renegotiation with ECDHE ciphers, detecting HTTP-on-HTTPS) and I think 
I found an easy way to trigger renegotiation without polling (using 
SSL_peek).


The current code runs the test suite with 1.0.2 and with 1.1.0 without 
any ssl related failures.


I'll let it settle a bit and test again once OpenSSL 1.1.0pre3 is out 
before suggesting backport to 2.4. I also need to set up the test suite 
environment for 2.4 with support for OpenSSL 0.9.8 to check against 
regressions.


Regards,

Rainer


[Update] Support for OpenSSL 1.1.0

2016-02-13 Thread Rainer Jung
I have send a candidate patch for the "talking http on https" patch to 
the OpenSSL project. Using this patch and another fix I applied to trunk 
for reneg handling in the proxy client case (mod_proxy talking https to 
a backend), I'm now down to one remaining test suite failure.


More precisely the following points are open

- reneg for ECDHE and maybe other ciphers
  IMHO broken in OpenSSL itself. Opened a case there, because I can 
reproduce with openssl command s_server and s_client, ie. without any 
Apache involvement. Steve has already taken the ticket there.


  https://rt.openssl.org/Ticket/Display.html?id=4303

- "talking http on https": The patch for OpenSSL is not big and mostly 
consists of the older OpenSSL 1.0.2 code but they have to check, whether 
I have put it at the right place. I verified it works by running the 
Apache test suite, which contains tests using the "talking http on 
https" feature.


  https://rt.openssl.org/Ticket/Display.html?id=4304

- Test suite failure test 3 in t/security/CVE-2009-3555.t. The test 
sends two requests pipelined, where the first one needs a reneg. 
Pre-1.1.0 the first requests succeeds and then the connection is closed. 
Using 1.1.0 the reneg fails, the first request get a 403 and the 
connection is closed. For this there's still some analysis needed on our 
side.


All other tests succeed, some non-SSL tests fails for prefork and 
worker, but they did before the changes and they fail with OpenSSL 1.0.2 
also.


Once the last test breakage is fixed, I plan to go through the changes 
in order to remove pre-1.1.0 OpenSSL specific code where these versions 
can use the newer as well. Currently pre-1.1.0 OpenSSL versions use the 
exact same code path as before the changes.



If you want to do tests on your own, what you need is:

- OpenSSL 1.1.0pre2 plus two patches:


https://github.com/openssl/openssl/commit/311f27852a18fb9c10f0c1283b639f12eea06de2

  https://rt.openssl.org/Ticket/Attachment/62645/38635/http-on-https.patch

- Fix to use a non-ECDHE cipher in the test suite

--- t/conf/ssl/ssl.conf.in  2016-02-12 17:21:44.857749000 +0100
+++ t/conf/ssl/ssl.conf.in2016-02-12 23:15:18.493357000 +0100
@@ -33,7 +33,8 @@
 CustomLog logs/ssl_request_log ssl
 

-SSLCipherSuite ALL:!ADH:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP:+eNULL
+SSLCipherSuite 
AES128-SHA256:ALL:!ADH:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP:+eNULL

+SSLHonorCipherOrder On

 
 SSLPassPhraseDialog 
exec:@ServerRoot@/conf/ssl/httpd-passphrase.pl



- Depending on how you link apr-util crypto build also against OpenSSL 
1.1.0. apr trunk but also apr-util 1.5.x head supports this.


Regards,

Rainer



Re: Support for OpenSSL 1.1.0

2016-02-11 Thread Rainer Jung
The list is getting shorter. The test suite currently only shows a few 
failures due to the missing "talking http on https" support.


Am 09.02.2016 um 11:20 schrieb Rainer Jung:


Open problems:

1) HTTP on HTTPS

OpenSSL 1.1.0 currently doesn't support the "HTTP spoken on HTTPS port"
error. The code to detect HTTP was removed due to a major rewrite of the
state engine. The OpenSSL project is willing to review patches for
reintroducing the feature there but currently doesn't plan to work on it
themselves.


I'll try tackling this next. Nut sure how well it will go.


2) Renegotiation

It needs to be implemented differently. The OpenSSL project suggest to
try reading application data until the renegotiation has finished. I
committed some rather ugly code that does loop waiting for reneg, but it
has a couple of problems:


Still using poll, but better state tracking now.


a) it will not work for EC or DH ciphers. Some opaque structure element
in the ssl struct is already set and confuses the state machine. I hope
to get some helpful feedback from the OpenSSL project for this.


Still open.


5) ssl_engine_kernel.c

In ssl_callback_Info() the explicit state constants
SSL3_ST_SR_CLNT_HELLO_A and SSL23_ST_SR_CLNT_HELLO_A are used which no
longer exist. I can't find obvious replacements in the list of new state
constants:

2100 int state = SSL_get_state((SSL *)ssl);
2101
2102 if (state == SSL3_ST_SR_CLNT_HELLO_A
2103 || state == SSL23_ST_SR_CLNT_HELLO_A) {
2104 scr->reneg_state = RENEG_ABORT;
2105 ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(02042)
2106   "rejecting client initiated renegotiation");
2107 }


That shouldn't be too hard. Will look into it.

Regards,

Rainer


Support for OpenSSL 1.1.0

2016-02-09 Thread Rainer Jung

I started adding support for OpenSSL 1.1.0 in trunk.

As some might know, the OpenSSL API changes and especially many 
structures have been made opaque.


I resolved all the stuff that could be done on a local/formal level, but 
some items remain, where I'm not sure how to proceed. I have marked all 
of them with "XXX: OpenSSL 1.1.0:" in the sources.


I tested a straight backport to 2.4 against OpenSSL 1.1.0pre2 plus patch 
https://github.com/openssl/openssl/commit/311f27852a18fb9c10f0c1283b639f12eea06de2 
and there were only 7 ssl test failures.


Open problems:

1) HTTP on HTTPS

OpenSSL 1.1.0 currently doesn't support the "HTTP spoken on HTTPS port" 
error. The code to detect HTTP was removed due to a major rewrite of the 
state engine. The OpenSSL project is willing to review patches for 
reintroducing the feature there but currently doesn't plan to work on it 
themselves.


2) Renegotiation

It needs to be implemented differently. The OpenSSL project suggest to 
try reading application data until the renegotiation has finished. I 
committed some rather ugly code that does loop waiting for reneg, but it 
has a couple of problems:


a) it will not work for EC or DH ciphers. Some opaque structure element 
in the ssl struct is already set and confuses the state machine. I hope 
to get some helpful feedback from the OpenSSL project for this.


b) the loop impl currently assumes we wait for client certs during the 
reneg. It will hang for the full loop duration when only the cipher 
changed but no certs will be send. We need a better loop end check.


3) ssl_engine_ocsp.c

In modssl_verify_ocsp() the following code accesses the struct member 
"valid", for which currently no accessor function exists in 1.1.0:


268 else if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
269 /* don't do OCSP checking for valid self-issued certs */
270 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
271   "Skipping OCSP check for valid self-issued cert");
272 X509_STORE_CTX_set_error(ctx, X509_V_OK);
273 return 1;
274 }


4) ssl_util_stapling.c

In ssl_stapling_init_cert() there's a compiler warning:
  "passing argument 1 of 'sk_value' from incompatible pointer type
   expected 'const struct _STACK *' but argument is of type
   'struct stack_st_OPENSSL_STRING *'":

179cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0));


5) ssl_engine_kernel.c

In ssl_callback_Info() the explicit state constants 
SSL3_ST_SR_CLNT_HELLO_A and SSL23_ST_SR_CLNT_HELLO_A are used which no 
longer exist. I can't find obvious replacements in the list of new state 
constants:


2100 int state = SSL_get_state((SSL *)ssl);
2101
2102 if (state == SSL3_ST_SR_CLNT_HELLO_A
2103 || state == SSL23_ST_SR_CLNT_HELLO_A) {
2104 scr->reneg_state = RENEG_ABORT;
2105 ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(02042)
2106   "rejecting client initiated renegotiation");
2107 }


Regards,

Rainer



Re: Support for OpenSSL 1.1.0

2016-02-09 Thread Dr Stephen Henson
On 09/02/2016 14:36, Rainer Jung wrote:
> Hi Steve,
> 
> thanks a lot for your review and comments. More inline.
> 
> Am 09.02.2016 um 13:34 schrieb Dr Stephen Henson:
>> On 09/02/2016 10:20, Rainer Jung wrote:
>>>
>>> 3) ssl_engine_ocsp.c
>>>
>>> In modssl_verify_ocsp() the following code accesses the struct member 
>>> "valid",
>>> for which currently no accessor function exists in 1.1.0:
>>>
>>> 268 else if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
>>> 269 /* don't do OCSP checking for valid self-issued certs */
>>> 270 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
>>> 271   "Skipping OCSP check for valid self-issued cert");
>>> 272 X509_STORE_CTX_set_error(ctx, X509_V_OK);
>>> 273 return 1;
>>> 274 }
>>>
>>>
>>
>> It's not clear what that code is supposed to do. The check isn't for
>> "self-issued" because that would just require comparing the subject and 
>> issuer
>> names it is actually checking for a self signed certificate.
> 
>> Is it supposed to be skipping OCSP checking for a trusted root?
> 
> The svn log message says "Don't do OCSP checks for valid self-issued certs". 
> The
> change was discussed here
> 
> http://marc.info/?t=12921462952=1=2
> 
> with some older discussion here
> 
> http://marc.info/?t=11963686385=1=2
> 
> As far as I get it, it is meant as an optimization to skip OCSP in cases where
> it isn't needed or useful. But I'm far from being an expert here. Kaspar, who
> introduced it originally formulated "prevents mod_ssl from doing unnecessary
> OCSP checks (valid self-issued certs, i.e. trust anchors configured through
> SSLCACertificate{File,Path})".
> 
> I'll CC Kaspar directly.
> 

OK it looks like at that point the certificate has been verified anyway so you
don't gain anything by checking cert->valid.

Steve.
-- 
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shen...@opensslfoundation.com


Re: Support for OpenSSL 1.1.0

2016-02-09 Thread Rainer Jung

Hi Steve,

thanks a lot for your review and comments. More inline.

Am 09.02.2016 um 13:34 schrieb Dr Stephen Henson:

On 09/02/2016 10:20, Rainer Jung wrote:


3) ssl_engine_ocsp.c

In modssl_verify_ocsp() the following code accesses the struct member "valid",
for which currently no accessor function exists in 1.1.0:

268 else if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
269 /* don't do OCSP checking for valid self-issued certs */
270 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
271   "Skipping OCSP check for valid self-issued cert");
272 X509_STORE_CTX_set_error(ctx, X509_V_OK);
273 return 1;
274 }




It's not clear what that code is supposed to do. The check isn't for
"self-issued" because that would just require comparing the subject and issuer
names it is actually checking for a self signed certificate.



Is it supposed to be skipping OCSP checking for a trusted root?


The svn log message says "Don't do OCSP checks for valid self-issued 
certs". The change was discussed here


http://marc.info/?t=12921462952=1=2

with some older discussion here

http://marc.info/?t=11963686385=1=2

As far as I get it, it is meant as an optimization to skip OCSP in cases 
where it isn't needed or useful. But I'm far from being an expert here. 
Kaspar, who introduced it originally formulated "prevents mod_ssl from 
doing unnecessary OCSP checks (valid self-issued certs, i.e. trust 
anchors configured through SSLCACertificate{File,Path})".


I'll CC Kaspar directly.


4) ssl_util_stapling.c

In ssl_stapling_init_cert() there's a compiler warning:
   "passing argument 1 of 'sk_value' from incompatible pointer type
expected 'const struct _STACK *' but argument is of type
'struct stack_st_OPENSSL_STRING *'":

179cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0));



In ssl_private.h the checks like this:

#ifndef sk_OPENSSL_STRING_value
#define sk_OPENSSL_STRING_value sk_value
#endif

no longer work because stacks are now inline functions. If you put that block
round an appropriate #ifdef it should be fine.

I had a quick look at the changes and did notice that some of the direct
structure access (extensions, X509_NAME) is unnecessary in existing versions of
OpenSSL. So in some cases you don't need to only use them for 1.1: they'll work
in all versions of OpenSSL but it's only in 1.1 they are enforced.


I'll get this tested/included. I kept the direct structure accesses for 
pre-1.1.0 just for the moment to stay on the safe side. Once this works 
reasonably well, I'll clean up the code to reduce the version dependent 
ifdefs.


Getting closer :)

Regards,

Rainer


Re: Support for OpenSSL 1.1.0

2016-02-09 Thread Dr Stephen Henson
On 09/02/2016 10:20, Rainer Jung wrote:
> 
> 3) ssl_engine_ocsp.c
> 
> In modssl_verify_ocsp() the following code accesses the struct member "valid",
> for which currently no accessor function exists in 1.1.0:
> 
> 268 else if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
> 269 /* don't do OCSP checking for valid self-issued certs */
> 270 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
> 271   "Skipping OCSP check for valid self-issued cert");
> 272 X509_STORE_CTX_set_error(ctx, X509_V_OK);
> 273 return 1;
> 274 }
> 
> 

It's not clear what that code is supposed to do. The check isn't for
"self-issued" because that would just require comparing the subject and issuer
names it is actually checking for a self signed certificate.

Is it supposed to be skipping OCSP checking for a trusted root?

> 4) ssl_util_stapling.c
> 
> In ssl_stapling_init_cert() there's a compiler warning:
>   "passing argument 1 of 'sk_value' from incompatible pointer type
>expected 'const struct _STACK *' but argument is of type
>'struct stack_st_OPENSSL_STRING *'":
> 
> 179cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0));
>

In ssl_private.h the checks like this:

#ifndef sk_OPENSSL_STRING_value
#define sk_OPENSSL_STRING_value sk_value
#endif

no longer work because stacks are now inline functions. If you put that block
round an appropriate #ifdef it should be fine.

I had a quick look at the changes and did notice that some of the direct
structure access (extensions, X509_NAME) is unnecessary in existing versions of
OpenSSL. So in some cases you don't need to only use them for 1.1: they'll work
in all versions of OpenSSL but it's only in 1.1 they are enforced.

Steve.
-- 
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shen...@opensslfoundation.com