Re: svn commit: r1869392 - in /httpd/httpd/trunk: CHANGES modules/md/md_acme.c modules/md/md_acme_drive.c modules/md/md_curl.c modules/md/md_http.c modules/md/md_version.h modules/md/mod_md_config.c

2019-11-05 Thread Marion & Christophe JAILLET



Le 05/11/2019 à 16:04, ic...@apache.org a écrit :

Author: icing
Date: Tue Nov  5 10:06:15 2019
New Revision: 1869392

URL: http://svn.apache.org/viewvc?rev=1869392=rev
Log:
   *) mod_md v2.2.3:
  - Configuring MDCAChallenges replaces any previous existing challenge 
configuration. It
had been additive before which was not the intended behaviour. [@mkauf]
  - Fixing order of ACME challenges used when nothing else configured. Code 
now behaves as
documented for `MDCAChallenges`. Fixes #156. Thanks again to @mkauf for 
finding this.
  - Fixing a potential, low memory null pointer dereference [thanks to 
@uhliarik].
  - Fixing an incompatibility with a change in libcurl v7.66.0 that added 
unwanted
"transfer-encoding" to POST requests. This failed in directy 
communication with
Let's Encrypt boulder server. Thanks to @mkauf for finding and fixing.


Modified:
 httpd/httpd/trunk/CHANGES
 httpd/httpd/trunk/modules/md/md_acme.c
 httpd/httpd/trunk/modules/md/md_acme_drive.c
 httpd/httpd/trunk/modules/md/md_curl.c
 httpd/httpd/trunk/modules/md/md_http.c
 httpd/httpd/trunk/modules/md/md_version.h
 httpd/httpd/trunk/modules/md/mod_md_config.c

Modified: httpd/httpd/trunk/CHANGES
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1869392=1869391=1869392=diff
==
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue Nov  5 10:06:15 2019
@@ -1,5 +1,15 @@
   -*- coding: utf-8 -*-
  Changes with Apache 2.5.1
+
+  *) mod_md v2.2.3:
+ - Configuring MDCAChallenges replaces any previous existing challenge 
configuration. It
+   had been additive before which was not the intended behaviour. [@mkauf]
+ - Fixing order of ACME challenges used when nothing else configured. Code 
now behaves as
+   documented for `MDCAChallenges`. Fixes #156. Thanks again to @mkauf for 
finding this.
+ - Fixing a potential, low memory null pointer dereference [thanks to 
@uhliarik].
+ - Fixing an incompatibility with a change in libcurl v7.66.0 that added 
unwanted
+   "transfer-encoding" to POST requests. This failed in directy 
communication with
+   Let's Encrypt boulder server. Thanks to @mkauf for finding and fixing. 
[Stefan Eissing]
  
*) mod_proxy: Put mod_proxy_{connect,wstunnel} tunneling code in common in

   proxy_util.  [Yann Ylavic]

Modified: httpd/httpd/trunk/modules/md/md_acme.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/md/md_acme.c?rev=1869392=1869391=1869392=diff
==
--- httpd/httpd/trunk/modules/md/md_acme.c (original)
+++ httpd/httpd/trunk/modules/md/md_acme.c Tue Nov  5 10:06:15 2019
@@ -402,7 +402,7 @@ static apr_status_t md_acme_req_send(md_
  if (req->req_json) {
  body = apr_pcalloc(req->p, sizeof(*body));
  body->data = md_json_writep(req->req_json, req->p, 
MD_JSON_FMT_INDENT);
-if (!body->data) {
+if (!body) {
  rv = APR_EINVAL; goto leave;


This revert r1869018 that I committed on trunk a few days ago.
Not sure if my fix was correct, but in r1869018 this code was changed.
Before we were checking the result of 'md_json_writep()' stored in 
'data', but now the retune valued is stored in 'body->data', so updating 
the check accordingly makes sense to me.


Just my 2c.


CJ



Re: Integration tests running on Docker

2019-11-05 Thread Luca Toscano
Il giorno lun 4 nov 2019 alle ore 19:33 Luca Toscano
 ha scritto:
>
> Hi Joe,
>
> Il giorno lun 4 nov 2019 alle ore 17:16 Joe Orton 
> ha scritto:
> >
> > Here is a proof of concept -
> >
> > https://github.com/notroj/httpd/blob/a73adfc8f1c01fbb6a3d493582f9c49c7c942756/.travis.yml
> >
> > This runs using the full test suite using a few different configurations
> > and also does builds with -Werror and maintainer-mode -
> >
> > https://travis-ci.org/notroj/httpd/builds/607213820
> >
> > ...this should be very easy to extend with more configurations.
> >
> > Can we merge the docker way with this kind of matrix type travis
> > configuration?
> >
>
> The proof of concept looks great, I wanted to test something similar
> especially to get timings. It looks like a single test takes from 2 to
> 5 minutes maximum, that is really impressive, I thought it would have
> been more. With Docker in theory we could use the os matrix, but the
> documentation doesn't really suggest any best practice. My idea was to
> pull different docker images (previously built and uploaded to Docker
> Hub), and depending on the os in the matrix run the correct docker
> command in the "script" section (to get an idea
> https://github.com/apache/arrow/blob/master/.travis.yml, even if they
> don't use docker).
>
> While I research though it might be good to follow your solution and
> just have a simple Travis file for say ubuntu and windows. Even if it
> takes 30 mins to complete, it would be a good feedback for anybody
> committing code to trunk/2.4.x. And it could be possible that this
> solution is good enough for our use cases, only testing will tell us!
> So feel free to commit your Travis config to trunk, maybe following
> the Apache Arrow example? If so others, more experienced with Windows,
> might be able to chime in and add the missing configuration.

This is my idea:

https://github.com/elukey/httpd/blob/trunk/.travis.yml

https://travis-ci.org/elukey/httpd/builds/607536348

Basically what Joe did, but following what the Apache Arrow Project
did. There is a little bit of repetition, but in theory with this
config people are free to add tests for other OSes (osx, windows) and
I will probably be able to add custom ones with Docker to see the
difference in execution timings etc.. (Apache Arrow's test do use
docker, I didn't see it a first).

Luca


Re: Integration tests running on Docker

2019-11-05 Thread Stefan Eissing
Nice work! 

Some weeks ago, I played around with travis to get the mod_h2 tests running, 
but all OS platforms offered included a too old Apache httpd version. Which 
meant I would need to drag in a recent httpd as well. I got distracted before 
doing so and this got put back on the useful-things-to-do-list.

Maybe I should fork your fork and add these tests there...

Do you intend to PR this back to httpd one day?

Cheers, Stefan

> Am 04.11.2019 um 17:16 schrieb Joe Orton :
> 
> Here is a proof of concept -
> 
> https://github.com/notroj/httpd/blob/a73adfc8f1c01fbb6a3d493582f9c49c7c942756/.travis.yml
> 
> This runs using the full test suite using a few different configurations 
> and also does builds with -Werror and maintainer-mode -
> 
> https://travis-ci.org/notroj/httpd/builds/607213820
> 
> ...this should be very easy to extend with more configurations.
> 
> Can we merge the docker way with this kind of matrix type travis 
> configuration?
> 
> Regards, Joe
> 



Re: svn commit: r1869338 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_connect.c modules/proxy/mod_proxy_wstunne

2019-11-05 Thread Rüdiger Plüm



On 11/04/2019 10:55 PM, Yann Ylavic wrote:
> On Mon, Nov 4, 2019 at 10:16 PM Ruediger Pluem  wrote:
>>
>>
>>
>> On 11/04/2019 06:12 PM, Yann Ylavic wrote:
>>> On Mon, Nov 4, 2019 at 4:16 PM Ruediger Pluem  wrote:


>>
>> Just that I get it correct:
>>
>> This would require something like
>>
>> ProxyPass / ws://hostname/
>> ProxyPass / http://hostname/
>>
>> to work and would only work with ProxyPass,
>
> Yes, that's the idea. I think the ProxyPass(es) order matters here
> though, because proxy_trans() walks the aliases in configuration
> order, so if it finds "http" first then "ws" will never be elected.

Exactly order is important here.

>
> We could be more clever, not sure it's worth it...
> My plan is to handle the "ws(s)" schemes in mod_proxy_http directly,
> using the new proxy tunnel interface of this commit to start tunneling
> if/when needed (according to the backend). Then mod_proxy_wstunnel
> would be obsolete.

Could make it easier, but not sure if it's worth it.

>
>> not with RewriteRules and [P] flag, correct?
>
> mod_rewrite wouldn't be concerned indeed, but today one can already:
>
>   RewriteCond %{HTTP:Upgrade} websocket [NC]
>   RewriteRule / ws://hostname/ [P]
>
> to make this work.

In parallel to

RewriteRule ^/(.*) http://hostname/$1 [P]

Good point. In this case order only matters from performance point of view in 
order to limit the amount of RewriteRules
executed for the common case.

Regards

Rüdiger


Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

2019-11-05 Thread Rüdiger Plüm



On 10/28/2019 10:54 AM, Yann Ylavic wrote:
> On Mon, Oct 28, 2019 at 9:24 AM Stefan Eissing
>  wrote:
>>
>> Ok, let me summarize:
>>
>> - SSLProtocol on base server applies, unless vhost has its own setting
>> - no SSLProtocol on base server, SSLProtocol on vhost applies
>> - no SSLProtocol on base server, no SSLProtocol on vhost, possible 
>> SSLProtocol on base vhost applies
>
> That's it, I'd call "base server" the "global server", though, to
> avoid confusion w.r.t. to c->base_server (the "base vhost" in the
> above).
>
> For 2.4.x, this means that there is a behavioural change when:
> - SSLProtocol is specified in a non-base vhost (but this is the point),
> - no SSLProtocol is specified in a non-base vhost AND one is specified
> globally (here the global applies, whereas previously the base vhost's
> applied).
>
> Once/if backported, I plan to completely remove the base vhost from
> the game, in trunk (usual merging applies).

So you want to revert r1868929 after the backport?

As far as I can tell r1868929 keeps the inheritance behavior closer to the
previous 2.4.x and trunk behavior, but is different compared to the
inheritance behavior of already SNI respecting directives like e.g. 
SSLCipherSuite.
Removing r1868929 would bring both (the directives respecting SNI so far
and the ones that NOW respect SNI) to the same inheritance level, correct?

Regards

Rüdiger