Re: [WIP] mod_authnz_ldap / mod_ldap support for APR v2 LDAP API

2024-04-18 Thread Joe Orton
On Thu, Apr 18, 2024 at 09:40:21AM +0100, Graham Leggett via dev wrote:
> Hi all,
> 
> The attached patch is a current work in progress patch for httpd-trunk to use 
> the new apr_ldap API that just landing in APR.
> 
> The highlights:
> 
> - Complete replacement of the previous API.
> - Will work against apr-2 (just landed) and apr-util-1.7 (after backport).
> - Linking to the underlying LDAP API has been removed and is no longer needed.

This design decision seems surprising to me, what does this add? Adding 
another abstraction layer to allow runtime selection of the LDAP library 
seems like a step backwards (a lot of complexity with no benefit). 
Unlike with e.g. a database backend selection users don't care about 
picking/configuring among many LDAP libraries.

Regards, Joe



mod_cgi/cgid unification for 2.4.x

2024-04-16 Thread Joe Orton
I proposed the monster mod_cgi/cgid unification backport for 2.4.x, but 
wanted to give a bit more context here rather than in STATUS. PR is 
here: https://github.com/apache/httpd/pull/209

1) stderr handling is a significant regression in mod_cgid compared to 
mod_cgi so it is worth fixing this IMO (PR 54221)

2) we have shipped this patch in Fedora for a few years, so I am 
quite confident that the file descriptor passing feature works well *on 
Linux*. I have no idea how well it works on non-Linux platforms, hence 
keeping it opt-in (./configure --enable-cgid-fdpassing).

3) there should be no behaviour changes to mod_cgi or mod_cgid if the 
fdpassing feature is not enabled, except where bug fixes to either were 
missing and are provided by the unification of common code.

4) because the bulk of the patch is de-duplication, this is a net 
reduction in lines of code shipped despite the new fd-passing code. 
diffstat:

 .github/workflows/linux.yml |8 -
 changes-entries/pr54221.txt |3 
 modules/generators/cgi_common.h |  639 
+
 modules/generators/config5.m4   |   11 +
 modules/generators/mod_cgi.c|  586 

 modules/generators/mod_cgid.c   |  642 
+-
 6 files changed, 913 insertions(+), 976 deletions(-)

Since the github PR link is a series of patches, attached the complete 
patch here as well in case that's preferred for review.

Regards, Joe
diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml
index ddacd4af19..6d4379d165 100644
--- a/.github/workflows/linux.yml
+++ b/.github/workflows/linux.yml
@@ -48,11 +48,11 @@ jobs:
   - name: Shared MPMs, all-modules
 config: --enable-mods-shared=reallyall --enable-mpms-shared=all
   # 
-
-  - name: Event MPM, all-modules, mod_cgid only
-config: --enable-mods-shared=reallyall --with-mpm=event 
--disable-cgi
+  - name: Event MPM, all-modules, mod_cgid fdpassing
+config: --enable-mods-shared=reallyall --with-mpm=event 
--disable-cgi --enable-cgid-fdpassing
   # 
-
-  - name: Event MPM, all-modules, no CMSG_DATA
-config: --enable-mods-shared=reallyall --with-mpm=event 
ac_cv_have_decl_CMSG_DATA=no
+  - name: Event MPM, all-modules, mod_cgid w/o fdpassing
+config: --enable-mods-shared=reallyall --with-mpm=event 
--disable-cgi
   # 
-
   - name: Default, all-modules + install
 config: --enable-mods-shared=reallyall
diff --git a/changes-entries/pr54221.txt b/changes-entries/pr54221.txt
new file mode 100644
index 00..62b75ea4dd
--- /dev/null
+++ b/changes-entries/pr54221.txt
@@ -0,0 +1,3 @@
+  *) mod_cgid: Optional support for file descriptor passing, fixing
+ error log handling (configure --enable-cgid-fdpassing) on Unix
+ platforms. PR 54221.  [Joe Orton]
diff --git a/modules/generators/cgi_common.h b/modules/generators/cgi_common.h
new file mode 100644
index 00..66f9418f21
--- /dev/null
+++ b/modules/generators/cgi_common.h
@@ -0,0 +1,639 @@
+/* Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "apr.h"
+#include "apr_strings.h"
+#include "apr_buckets.h"
+#include "apr_lib.h"
+#include "apr_poll.h"
+
+#define APR_WANT_STRFUNC
+#define APR_WANT_MEMFUNC
+#include "apr_want.h"
+
+#include "httpd.h"
+#include "util_filter.h"
+#include "util_script.h"
+
+static APR_OPTIONAL_FN_TYPE(ap_ssi_get_tag_and_value) *cgi_pfn_gtv;
+static APR_OPTIONAL_FN_TYPE(ap_ssi_parse_string) *cgi_pfn_ps;
+
+/* These functions provided by mod_cgi.c/mod_cgid.c still. */
+static int l

Re: mod_systemd: refactor to get rid of libsystemd dependency?

2024-04-03 Thread Joe Orton
On Tue, Apr 02, 2024 at 12:25:40PM +0200, Rainer Jung wrote:
> Hi there,
> 
> in the light of the recent xz attack I was wondering, whether we should also
> reduce our library dependencies by no longer using sd_notify() in
> mod_systemd (thus loading libsystemd and all of its dependencies), but
> instead taking the approach to hard code sd_notify functionality.
> 
> I guess the Linux distributors who patched sshd to use libsystemd for
> notification are on their way to do the same for their sshd patches, so we
> might soon get an idea how to do that properly.
> 
> This is not meant to become part of out next release (this week), but
> hopefully we can manage to code it for the next one.
> 
> WDYT: does this make sense?

The trunk mod_systemd has got slightly wider library use than just 
sd_notify - so it is not quite that simple. If there was an alternative 
minimal library implementing the sd_* API parts required, that would 
definitely make sense. I'm not sure that reimplementing them all from 
scratch makes sense (especially multiplied by N projects doing this).

It looks like systemd folks have also changed the library implementation 
to dlopen() the various dependant libraries on demand now rather than 
directly linking to them, which removes the specific attack vector used 
here IIUC.

Regards, Joe



Re: [VOTE] Release httpd-2.4.59-rc1 as httpd-2.4.59

2024-04-03 Thread Joe Orton
On Wed, Apr 03, 2024 at 08:26:09AM -0400, Eric Covener wrote:
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> I would like to call a SHORTENED VOTE to release
> this candidate tarball httpd-2.4.59-rc1 as 2.4.59:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.

+1 from me. Passes test suite on RHEL8+9+CentOS Stream 10. Big thanks for 
RMing.

Also a CI pass: https://github.com/apache/httpd/actions/runs/8538499329

Regards, Joe



Re: Failing test t/apache/pr64339.t

2024-04-03 Thread Joe Orton
On Tue, Apr 02, 2024 at 08:46:46PM -0400, Eric Covener wrote:
> This could be due to none of these happening:
> - mod_mime didn't send a charset from backend
> - no BOM
> - no xml2EncDefault (8859-1 effectively by default) on frontend
> 
> To make the conf match the test code, this works for me to address
> mod_mime on the backend:

Yup. Sorry for wasting your time on this. Thanks for the commit, I had 
the same change uncommitted locally still and missed it.




Re: svn commit: r1915947 - /httpd/httpd/branches/2.4.x/STATUS

2024-03-05 Thread Joe Orton
On Tue, Mar 05, 2024 at 11:25:12AM +0100, jean-frederic clere wrote:
> On 2/29/24 10:54, Joe Orton wrote:
> > On Thu, Feb 22, 2024 at 01:35:06PM -, jfcl...@apache.org wrote:
> > > Author: jfclere
> > > Date: Thu Feb 22 13:35:06 2024
> > > New Revision: 1915947
> > > 
> > > URL: http://svn.apache.org/viewvc?rev=1915947=rev
> > > Log:
> > > Propose. CMake builds fail withi: "fatal error C1083: Cannot open include 
> > > file: 'ap_config_auto.h'"
> > 
> > FYI the r1877693 fix is included in my other backport proposal for
> > htpasswd.
> 
> I have removed my proposal, tested, checked and votes yours. Thanks!

Thanks - now merged.

Regards, Joe



Re: svn commit: r1916068 - in /httpd/httpd/trunk: .github/workflows/linux.yml test/travis_before_linux.sh

2024-03-01 Thread Joe Orton
On Fri, Mar 01, 2024 at 01:52:15PM +0100, Yann Ylavic wrote:
> On Fri, Mar 1, 2024 at 1:42 PM Yann Ylavic  wrote:
> >
> > On Fri, Mar 1, 2024 at 1:24 PM Joe Orton  wrote:
> > >
> > > Do you still want that
> > > TestSSLCA.pm change merged?
> >
> > I think it can be useful for those who test httpd with openssl1 still
> > (not maintained anymore, but we have to keep compatibility in 2.4 at
> > least).
> 
> But the issue with this patch is that it doesn't check which openssl
> version httpd is actually using, so it always generates pkcs#1 keys
> even if not needed.
> If we had a way to check the system's openssl AND httpd's openssl are
> < 3 it would be better, but I don't see how to do this.

I suppose we could export the detected version from configure via apxs 
-q and pick it up in Apache::Test, but I think it would be likely to 
make the whole house of cards even more fragile. So I'm not sure it's 
worth investing effort in that tbh. Better to assume/require that the 
bin/openssl version matches the version mod_ssl uses.

Regards, Joe



Re: svn commit: r1916068 - in /httpd/httpd/trunk: .github/workflows/linux.yml test/travis_before_linux.sh

2024-03-01 Thread Joe Orton
On Fri, Mar 01, 2024 at 12:59:10PM +0100, Yann Ylavic wrote:
> On Fri, Mar 1, 2024 at 11:15 AM  wrote:
> >
> > Author: jorton
> > Date: Fri Mar  1 10:15:13 2024
> > New Revision: 1916068
> >
> > URL: http://svn.apache.org/viewvc?rev=1916068=rev
> > Log:
> > CI: add OpenSSL 3.2, test OpenSSL 3.x using Apache::Test
> > trunk to pick up r1916067.
> 
> I had to modify Apache-Test too when running the perl test framework
> with openssl >= 3.0 and proposed a patch here [1] (not enough karma to
> commit on perl.a.o).
> It was an issue with mod_proxy's client certs IIRC, which r1916067 is
> possibly fixing too, but just in case you are still fighting with this
> ;)

Ah, interesting, thanks. I should read dev@perl more often!

I haven't seen that particularly failure, and trunk seems to now be 
working (touch wood) with 3.1 and 3.2. The Ubuntu runners are all on 
OpenSSL 3.0 anyway, and r1916058 ensures that TestSSLCA.pm is using the 
bin/openssl from the installed version of OpenSSL rather than a 
possibly-mismatched system /usr/bin/openssl. Do you still want that 
TestSSLCA.pm change merged?

Also - I guess the note about *not* accepting PKCS#8 format keys in 
https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslproxymachinecertificatefile
 
is now wrong then?

Regards, Joe



Re: svn commit: r1915947 - /httpd/httpd/branches/2.4.x/STATUS

2024-02-29 Thread Joe Orton
On Thu, Feb 22, 2024 at 01:35:06PM -, jfcl...@apache.org wrote:
> Author: jfclere
> Date: Thu Feb 22 13:35:06 2024
> New Revision: 1915947
> 
> URL: http://svn.apache.org/viewvc?rev=1915947=rev
> Log:
> Propose. CMake builds fail withi: "fatal error C1083: Cannot open include 
> file: 'ap_config_auto.h'"

FYI the r1877693 fix is included in my other backport proposal for 
htpasswd.

> Modified:
> httpd/httpd/branches/2.4.x/STATUS
> 
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1915947=1915946=1915947=diff
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Thu Feb 22 13:35:06 2024
> @@ -243,6 +243,12 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>Github PR: https://github.com/apache/httpd/pull/413
>+1: ylavic, jorton, jfclere
>  
> +   *) htpasswd: Windows : do not include ap_config_auto.h 
> +  trunk patch: https://svn.apache.org/r1877693 
> +  2.4.x patch: svn merge -c 1877693 ^/httpd/httpd/trunk .
> +  +1: jfclere,
> +  
> +
>  PATCHES/ISSUES THAT ARE BEING WORKED
>[ New entries should be added at the START of the list ]
>  
> 
> 



Re: svn commit: r1915740 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/pr61574.txt modules/ssl/ssl_engine_init.c

2024-02-20 Thread Joe Orton
On Fri, Feb 16, 2024 at 09:52:04PM +0100, Christophe JAILLET wrote:
> Le 12/02/2024 à 09:37, jor...@apache.org a écrit :
> > Author: jorton
> > Date: Mon Feb 12 08:37:35 2024
> > New Revision: 1915740
> > 
> > URL: http://svn.apache.org/viewvc?rev=1915740=rev
> > Log:
...
> r1825124 was a follow-up of this patch.
> Should it be backported as well?

Good catch, thank you. Proposed now.

Regards, Joe



Re: libapreq subproject roll call

2024-02-16 Thread Joe Orton
On Fri, Feb 16, 2024 at 08:10:57AM -0500, Eric Covener wrote:
> I count myself as a release vote of last resort only, but i don't
> think we should be committing to future fixes/releases if nearly
> everyone is in this category.

Thanks for raising it here, Eric. Same for me.

Regards, Joe



Re: release apreq 2.18 and mothball the project

2024-02-16 Thread Joe Orton
Joe, you've been warned before to moderate your language. This ends now.

It is completely unacceptable to insult other members of the community 
like this one time, let alone repeatedly. It is unproductive, 
unprofessional, and in violation of the ASF code of conduct. I've taken 
the decision as PMC chair to remove you from dev@ and you are now banned 
from posting in the future.

- Project Chair of HTTP Server, Joe Orton



Re: using changes-entries or write in CHANGES directly

2024-02-14 Thread Joe Orton
On Wed, Feb 14, 2024 at 11:28:15AM +0100, jean-frederic clere wrote:
> So for 2.4.x on my accepted back port I have don't need changes-entries and
> I have to process CHANGES by hands as I missed creating a changes-entries
> file in trunk.

If you file a Github PR for the backport you can still add a 
changes-entries/ file there (same if you create a patch and upload it 
somewhere). It makes the change easy to merge independent of other 
commits, avoiding conflicts adding directly to CHANGES. Works really 
well (thanks Rűdiger!)

Regards, Joe



Re: svn commit: r1915534 - in /httpd/httpd/branches/2.4.x: ./ modules/cache/mod_socache_shmcb.c modules/proxy/balancers/mod_lbmethod_heartbeat.c os/unix/unixd.c server/buildmark.c server/scoreboard.c

2024-02-02 Thread Joe Orton
On Fri, Feb 02, 2024 at 03:29:34PM +0100, Ruediger Pluem wrote:
> > Modified: httpd/httpd/branches/2.4.x/server/util_expr_parse.c
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/util_expr_parse.c?rev=1915534=1915533=1915534=diff
> > ==
> > --- httpd/httpd/branches/2.4.x/server/util_expr_parse.c (original)
> > +++ httpd/httpd/branches/2.4.x/server/util_expr_parse.c Fri Feb  2 11:50:07 
> > 2024
> > @@ -1326,6 +1326,8 @@ YYSTYPE yylval;
> >  
> >goto yysetstate;
> >  
> > +  /* TODO: comppiler warning that this is unused, and it seems to */
> 
> Typo :-) compiler

Thanks -> r1915543



Re: svn commit: r1915516 - in /httpd/httpd/branches/2.4.x: ./ CHANGES configure.in docs/man/htpasswd.1 support/htpasswd.c support/passwd_common.c support/passwd_common.h

2024-02-02 Thread Joe Orton
On Fri, Feb 02, 2024 at 08:29:48AM +0100, Ruediger Pluem wrote:
> 
> 
> On 2/1/24 5:54 PM, jor...@apache.org wrote:
> > Author: jorton
> > Date: Thu Feb  1 16:54:40 2024
> > New Revision: 1915516
> > 
> > URL: http://svn.apache.org/viewvc?rev=1915516=rev
> > Log:
...
> > -" -B  Force bcrypt encryption of the password (very secure)." NL
> > +" -2  Force SHA-256 crypt() hash of the password (secure)." NL
> > +" -5  Force SHA-512 crypt() hash of the password (secure)." NL
> > +" -B  Force bcrypt aencryption of the password (very secure)." NL
> 
> We have a typo above:aencryption

Good catch, thanks. Looks like a couple of the follow-up commits to this 
were missed which fixed that and improved the wording, I'll submit 
another backport.



Re: PR #363

2024-01-25 Thread Joe Orton
On Thu, Jan 25, 2024 at 08:12:24AM +0100, Ruediger Pluem wrote:
> Tried it in r1915391 and it seems to work. Not sure if there are 
> general downsides / objections with regards to symlinks in our 
> repository. But trunk is CTR :-).

Oh, that looks really nice. +1

Thanks to you, Rich, and Mayank Patil.

Regards, Joe



Re: svn commit: r1914804 - in /httpd/httpd/trunk: changes-entries/flushing-chunks.txt modules/http/chunk_filter.c

2023-12-20 Thread Joe Orton
On Wed, 20 Dec 2023, 16:30 Yann Ylavic,  wrote:

> On Wed, Dec 20, 2023 at 5:20 PM Yann Ylavic  wrote:
> >
> > On Wed, Dec 20, 2023 at 4:56 PM  wrote:
> > >
> > > Author: jorton
> > > Date: Wed Dec 20 15:56:15 2023
> > > New Revision: 1914804
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1914804=rev
> > > Log:
> > > * modules/http/chunk_filter.c (ap_http_chunk_filter): For a brigade
> > >   containing [FLUSH EOS], insert the last-chunk terminator before the
> > >   FLUSH rather than between the FLUSH and the EOS.
> > >
> > > Github: closes #400
> >
> > It seems that we should set the last-chunk before the FLUSH only if
> > the latter precedes an EOS?
> >
> > So below:
> >
> > > --- httpd/httpd/trunk/modules/http/chunk_filter.c (original)
> > > +++ httpd/httpd/trunk/modules/http/chunk_filter.c Wed Dec 20 15:56:15
> 2023
> > > @@ -64,7 +64,7 @@ apr_status_t ap_http_chunk_filter(ap_fil
> > >
> > >  for (more = tmp = NULL; b; b = more, more = NULL) {
> > >  apr_off_t bytes = 0;
> > > -apr_bucket *eos = NULL;
> > > +apr_bucket *eos = NULL; /* EOS bucket, or FLUSH preceding EOS
> */
> > >  apr_bucket *flush = NULL;
> > >  /* XXX: chunk_hdr must remain at this scope since it is used
> in a
> > >   *  transient bucket.
> > > @@ -99,8 +99,20 @@ apr_status_t ap_http_chunk_filter(ap_fil
> > >  }
> > >  if (APR_BUCKET_IS_FLUSH(e)) {
> > >  flush = e;
> >
> > Don't set "flush" above.
> >
> > > +
> > > +/* Special case to catch common brigade ending of
> > > + * [FLUSH] [EOS] - insert the last_chunk before
> > > + * the FLUSH rather than between the FLUSH and the
> > > + * EOS. */
> > >  if (e != APR_BRIGADE_LAST(b)) {
> > > -more = apr_brigade_split_ex(b,
> APR_BUCKET_NEXT(e), tmp);
> > > +if (APR_BUCKET_IS_EOS(APR_BUCKET_NEXT(e))) {
> >
> > But here only?
> >
> > > +eos = e;
> > > +/* anything after EOS is dropped, no need
> > > + * to split. */
> > > +}
> > > +else {
> > > +more = apr_brigade_split_ex(b,
> APR_BUCKET_NEXT(e), tmp);
> > > +}
> > >  }
> > >  break;
> > >  }
> > > @@ -173,12 +185,12 @@ apr_status_t ap_http_chunk_filter(ap_fil
> > >   * FLUSH bucket, or appended to the brigade
> > >   */
> > >  e = apr_bucket_immortal_create(CRLF_ASCII, 2,
> c->bucket_alloc);
> > > -if (eos != NULL) {
> > > -APR_BUCKET_INSERT_BEFORE(eos, e);
> > > -}
> > > -else if (flush != NULL) {
> > > +if (flush != NULL) {
> > >  APR_BUCKET_INSERT_BEFORE(flush, e);
> > >  }
> > > +else if (eos != NULL) {
> > > +APR_BUCKET_INSERT_BEFORE(eos, e);
> > > +}
> > >  else {
> > >  APR_BRIGADE_INSERT_TAIL(b, e);
> > >  }
>
> Ah no, this is for every chunk so we are good here.
> For the last-chunk, I think we need
>

I think that's not necessary because in the special case eos=flush, in the
normal case flush is NULL. Actually the reordering of the tests above
doesn't make any difference either, I could have skipped that. In the first
iteration of the patch I renamed eos to "terminator" to make it more
obvious.

Index: modules/http/chunk_filter.c
> ===
> --- modules/http/chunk_filter.c(revision 1914804)
> +++ modules/http/chunk_filter.c(working copy)
> @@ -217,7 +217,7 @@ apr_status_t ap_http_chunk_filter(ap_filter_t *f,
>   * now.
>   */
>  if (eos && !ctx->bad_gateway_seen) {
> -ap_h1_add_end_chunk(b, eos, f->r, ctx->trailers);
> +ap_h1_add_end_chunk(b, flush ? flush : eos, f->r,
> ctx->trailers);
>  }
>
>  /* pass the brigade to the next filter. */
> --
> ?
>
>


Re: [PATCH] mod_deflate: remove filter after seeing EOS

2023-12-20 Thread Joe Orton
On Wed, Dec 20, 2023 at 04:24:32PM +0100, Ruediger Pluem wrote:
> On 12/20/23 4:08 PM, Yann Ylavic wrote:
> > On Wed, Dec 20, 2023 at 2:40 PM Joe Orton  wrote:
> >> https://github.com/apache/httpd/pull/400
> > 
> > Thanks, looks good to me.
> 
> +1

Thanks a lot for the quick reviews. Merged in r1914804 and happy Xmas :)

Regards, Joe



Re: [PATCH] mod_deflate: remove filter after seeing EOS

2023-12-20 Thread Joe Orton
On Wed, Dec 20, 2023 at 10:07:19AM -0500, Eric Norris via dev wrote:
> Thanks Joe, and no need to apologize, that's totally understandable.
> 
> I also appreciate you taking a look at the chunk filter behavior as that
> was actually going to be the next patch I proposed. I had written it here:
> https://github.com/ericnorris/httpd/commit/5f8fa24786b937ab611160b3c765cededa6dcb12,
> though in retrospect I'm not sure why this (or your patch) alone isn't
> enough, and why I thought the mod_deflate patch was also needed.

In the repro case you posted, only one brigade is passed by the handler, 
with that I saw the "delayed last chunk" behaviour but not the Zlib 
double-deinit error log. I think the Zlib error would only be triggered 
by passing a second brigade with a FLUSH.

It is incorrect for a handler to pass anything after EOS but filters are 
also supposed to be robust against it anyway.

> If I understand correctly, either patch would send the flush bucket after
> the last chunk marker and before the EOS bucket (i.e. [crlf last-chunk
> FLUSH EOS]), so there's no need for "userspace" to send an additional flush
> after the EOS bucket, and so mod_deflate wouldn't complain. Does that sound
> right?

I don't think the mod_deflate patch fixed the "delayed last-chunk" 
problem in my testing, but it is definitely correct & necessary to fix 
the Zlib error as above.

> Apologies on my part if that's the case - it had been a few months since I
> had written the patches, so I might have lost that context by the time I
> was able to investigate submitting the patches. I possibly should have
> submitted the chunk filter patch first. Either way, like I said I
> appreciate your time.

Thanks for taking the time to investigate & report it!

Regards, Joe



Re: [PATCH] mod_deflate: remove filter after seeing EOS

2023-12-20 Thread Joe Orton
On Mon, Oct 30, 2023 at 10:47:44AM -0400, Eric Norris via dev wrote:
> Hello again,
> 
> I'd like to politely bump this message to see if anyone would mind
> taking a look at this patch, either here or on GitHub.

Apologies, I got quite distracted by the "rapid reset" security stuff 
earlier in the year. I've merged your patch - thanks!

I was surprised this made a difference to the behaviour on the wire. It 
seems like the chunk filter has suboptimal behaviour here. If you take 
an output brigade like either:

a) [HEAP FLUSH EOS]
b) [HEAP FLUSH EOS FLUSH]

in both cases the chunk filter would output two brigades:

[chunk-hdr HEAP crlf FLUSH] [last-chunk EOS]

Significantly there is no FLUSH in the second brigade even for case (b), 
because the chunk filter also drops everything after EOS. It would be 
clearly better/correct if the chunk filter produces a single brigade for 
this very common output pattern:

[chunk-hdr HEAP crlf last-chunk FLUSH EOS]

correctly preserving the semantics of the FLUSH. I've tried this here:

https://github.com/apache/httpd/pull/400

Regards, Joe

> 
> Eric Norris
> Etsy
> 
> On Mon, Oct 9, 2023 at 2:50 PM Eric Norris  wrote:
> >
> > Hello all,
> >
> > I've submitted a pull request on GitHub here:
> > https://github.com/apache/httpd/pull/387, per the instructions I found
> > at https://httpd.apache.org/contribute. I figured it may also be
> > useful to share the patch here, but please feel free to redirect me as
> > this is my first time contributing.
> >
> > Thanks!
> >
> > Eric Norris
> > Etsy
> >
> >
> > At Etsy, we use mod_php and were investigating what we thought was
> > surprising behavior - that code executed during PHP's shutdown phase
> > prevented the request from terminating, even if we didn't expect to send
> > any additional output to the client.
> >
> > We determined that this was not surprising given mod_php's
> > implementation, but after we developed a proof-of-concept patch that
> > sent an EOS bucket and a flush bucket via a "userland" PHP function, we
> > were surprised that it didn't work when compression was enabled for the
> > request.
> >
> > I was able to reproduce this behavior with a simple Apache module built
> > on top of mod_example_hooks:
> >
> > https://gist.github.com/ericnorris/37c7dac401b50d270867e82a47bdd294
> >
> > It seems that this is because mod_deflate receives the EOS bucket and
> > then calls `deflateEnd` but remains in the filter chain. This means that
> > it receives the next flush bucket and attempts to call
> > `flush_libz_buffer`, which of course fails, causing it to print an
> > AH01385 error to the Apache error log, and perhaps most importantly, to
> > "eat" the flush bucket and not pass it down the bucket brigade.
> >
> > We found that this patch allows us to successfully send an EOS bucket
> > *and* promptly flush the connection so that the client knows the request
> > is finished.
> > ---
> >  modules/filters/mod_deflate.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/modules/filters/mod_deflate.c b/modules/filters/mod_deflate.c
> > index b7a68d2d43..ad753dc618 100644
> > --- a/modules/filters/mod_deflate.c
> > +++ b/modules/filters/mod_deflate.c
> > @@ -941,6 +941,10 @@ static apr_status_t deflate_out_filter(ap_filter_t *f,
> >  }
> >
> >  deflateEnd(>stream);
> > +
> > +/* We've ended the libz stream, so remove ourselves. */
> > +ap_remove_output_filter(f);
> > +
> >  /* No need for cleanup any longer */
> >  apr_pool_cleanup_kill(r->pool, ctx, deflate_ctx_cleanup);
> >
> > --
> > 2.40.1
> 



Re: svn commit: r1914045 - in /httpd/httpd/trunk: changes-entries/ab-source-address.txt docs/man/ab.1 support/ab.c

2023-12-19 Thread Joe Orton
On Wed, Nov 22, 2023 at 05:19:50PM -, minf...@apache.org wrote:
> Author: minfrin
> Date: Wed Nov 22 17:19:49 2023
> New Revision: 1914045
> 
> URL: http://svn.apache.org/viewvc?rev=1914045=rev
> Log:
> Add an option to specify a source address.

Saw this in the backports list - doesn't this duplicate existing the -B 
option for ab - or am I missing something?

Regards, Joe



Re: svn commit: r1914365 - in /httpd/httpd/trunk: changes-entries/ssl-providers.txt docs/log-message-tags/next-number docs/manual/mod/mod_ssl.xml modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_p

2023-12-13 Thread Joe Orton
On Wed, Dec 13, 2023 at 11:33:16AM +0100, Ingo Franzki wrote:
> On 13.12.2023 10:55, Joe Orton wrote:
> > On Wed, Dec 06, 2023 at 01:02:01PM +0100, Yann Ylavic wrote:
> >> Oh, scratch that. Actually the engine API requires a "SSLCryptoDevice
> >> pkcs11" too, so we wouldn't take the !mc->szCryptoDevice path.
> >> Sorry for the noise.
> > 
> > Yes it should remain compatible like that, though you prompted me to 
> > re-read that and it would break for a no-engine build: r1914622.
> Good catch!
> How would one compile without OpenSSL having the engine API ?
> At least currently, any supported OpenSSL version still does have the Engine 
> API. 

If you configure OpenSSL with the "no-engine" flag, you get 
OPENSSL_NO_ENGINE #define'd in the OpenSSL headers, and the engine API 
is not available.

Looks like a few more places which assume ->szCryptoDevice is always 
there, so there may be other build issues. We need a CI config for this.

> > I am not sure but we might want to add a new directive (yay) which loads 
> > a named provider, or we could rely on users doing that in openssl.cnf 
> > since configuring providers may be non-trivial (e.g. [1]).
> I would not try to load a named provider. While loading a named provider can 
> be done using the OpenSSL provider API,
> it is not possible to supply configuration parameters to that provider after 
> loading it. 
> Most provider I know do need specific configuration settings, they won't work 
> without them, especially the PKCS#11 providers. 
> So we must rely on users doing that in openssl.cnf.

That makes sense to me, thank you.

> > Other thing a colleage mentioned was that we may want to expand the list 
> > of URI schemes accepted here from just pkcs11://.
> Sure, the provider code in general should work for any kind of URIs, not only 
> 'pkcs11:'. 
> It would even work for the 'file:' URI, loading the keys/certs from PEM files 
> (like the non-provider/non-engine code is doing already).
> Actually it would even work for file names without a scheme at all, since the 
> default scheme is 'file:' anyway. 
> So it could theoretically replace the non-provider/non-engine load key/cert 
> code (not that I would suggests to change that as of today).

Yup. We would have to be careful to make the logic around loading the 
chain from the file would work exactly the same if a file:// URI was 
used. I'm not sure what the best approach is - try loading everything 
via a store and then fallback to the old way, or again add a new config 
option(s?) SSLBlahURI.

Regards, Joe



Re: svn commit: r1914365 - in /httpd/httpd/trunk: changes-entries/ssl-providers.txt docs/log-message-tags/next-number docs/manual/mod/mod_ssl.xml modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_p

2023-12-13 Thread Joe Orton
On Wed, Dec 06, 2023 at 01:02:01PM +0100, Yann Ylavic wrote:
> Oh, scratch that. Actually the engine API requires a "SSLCryptoDevice
> pkcs11" too, so we wouldn't take the !mc->szCryptoDevice path.
> Sorry for the noise.

Yes it should remain compatible like that, though you prompted me to 
re-read that and it would break for a no-engine build: r1914622.

I am not sure but we might want to add a new directive (yay) which loads 
a named provider, or we could rely on users doing that in openssl.cnf 
since configuring providers may be non-trivial (e.g. [1]).

Other thing a colleage mentioned was that we may want to expand the list 
of URI schemes accepted here from just pkcs11://.

[1] 
https://github.com/tpm2-software/tpm2-openssl/blob/master/docs/initialization.md#tpm-command-transmission-interface-tcti



Re: mod_dav_fs locking / Re: apr_dbm and concurrency

2023-11-24 Thread Joe Orton
On Thu, Nov 23, 2023 at 05:42:10PM +, Emmanuel Dreyfus wrote:
> On Thu, Nov 23, 2023 at 05:36:06PM +0000, Joe Orton wrote:
> > 3) in the mean time I worked up a PR for mod_dav_fs which adds a global 
> > mutex around the dbm lockdb use. This passes my stress tests for the 
> > first time.
> 
> How concurent is the stress test? 

I've been testing with 20 concurrent clients on an 8 core machine.

> In the past, I have been badly hurt by a few WebDAV clients proactively 
> exploring the filesystem using locksdiscovery. That compeltely killed the 
> service. I introduced the DavLockDiscovery directive to work it around.

This is a good point. Was the load in that case PRPOFIND/depth:infinity 
do you know or "just" depth:1? A global lock like in my PR would make 
the PROPFIND load even worse, since the lock is held for the duration of 
the response and there are no concurrent read-only locks.

It might be necessary to disable lock discovery by default then, I don't 
know if any clients rely on or expose that but it's only a "MAY" that 
lock discvovery is possible in RFC4918. I suspect the lock recovery 
mechanism for most users & clients is to delete the lockdb.

Regards, Joe



mod_dav_fs locking / Re: apr_dbm and concurrency

2023-11-23 Thread Joe Orton
Adding dev@httpd to a dev@apr thread about apr_dbm locking being broken.

On Sun, Nov 12, 2023 at 07:43:13AM -0600, Greg Stein wrote:
> Or, apps can stick to an older APR. ... we don't have to carry this forward
> into future versions of APR (IMO).
> 
> To your point, it is probably a single page with code samples to show how
> to do K/V with sqlite, to replace the apr_dbm calls. That's gonna be way
> easier than locking code integrated into apr_dbm.

This seems reasonable to me for the mod_dav use case where the database 
is an implementation detail which users don't care about. For other use 
cases in httpd I'm not so sure, but saying "dbm is dead, sqlite is the 
way" is probably possible for all of them. It does mean someone writing 
a lot of new modules to replace mod_auth*dbm etc tho ;)

There are a few alternative approaches I looked at:

1) we could hack fcntl-based locks to work on Linux by using F_OFD_SETLK 
etc which a sane locking model rather than the weird/dumb F_SETLK which 
has the traditional/POSIX non-thread-safe model. Linux-specific, so...

2) try to shoehorn "proper" locking into apr_dbm is hard because there 
isn't a suitable locking primitive that can be used at this level. Maybe 
the only approach that might work would be filesystem-based locking 
based off open/O_EXCL but this is not exactly reliable.

3) in the mean time I worked up a PR for mod_dav_fs which adds a global 
mutex around the dbm lockdb use. This passes my stress tests for the 
first time. Kind of ugly but this seems like the least ugly option at 
this point other than the "start again with sqlite": 
https://github.com/apache/httpd/pull/395

Any comments/review/flames from either dev@ welcome.

Regards, Joe



Re: [VOTE] Release httpd-2.4.58-rc3 as httpd-2.4.58

2023-10-16 Thread Joe Orton
On Mon, Oct 16, 2023 at 05:08:11PM +0200, Stefan Eissing via dev wrote:
> Hi all,
> 
> after fixing my merge mistake in rc2 (sorry!), we go again:
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> I would like to call a VOTE over the next few days to release
> this candidate tarball httpd-2.4.58-rc3 as 2.4.58:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.

+1 for release from me, sigs good; builds and passes tests on Fedora 39 
and RHEL 8 and 9, my old computers are happy again ;)

Big thanks for fixes and RMing.

Regards, Joe



Re: windows block

2023-10-16 Thread Joe Orton
On Mon, Oct 16, 2023 at 04:45:29PM +0200, Stefan Eissing via dev wrote:
> Do I make an rc3 nevertheless? Do we know someone else using VC?
> 
> Advice appreciated.

+1 for rc3 at r1913027.

Someone using VC needs to work out how to fix that rather than just 
report it's broken, it shouldn't hold progress up for the rest of the 
world.

Regads, Joe



Re: svn commit: r1913006 - in /httpd/httpd/branches/2.4.x: ./modules/http2/h2.h

2023-10-16 Thread Joe Orton
On Mon, Oct 16, 2023 at 03:12:24PM +0200, SteffenAL wrote:
> 
> checkout.
> 
> 
> Looks like h2_ws.h :
> 
> 
> Generating Code...
> h2_ws.h
> C:\VS17\Win32\httpd-2.4\modules\http2\h2.h(173,17): error C2143: syntax
> error: missing ';' before '*'
> C:\VS17\Win32\httpd-2.4\modules\http2\h2.h(173,17): error C4430: missing
> type specifier - int assumed. Note: C++ does not support default-int

Windows compiler errors are often specific to the Windows, you should 
not assume anybody else here will understand (or care) about them. So 
rather than reporting, can you work out what the problem is? Line 173 
uses apr_table_t, does adding #include "apr_tables.h" somewhere help 
here?

Regards, Joe



Re: svn commit: r1913019 - in /httpd/httpd/trunk/modules/http2: h2_session.c h2_ws.c

2023-10-16 Thread Joe Orton
On Mon, Oct 16, 2023 at 02:54:58PM +0200, Ruediger Pluem wrote:
> Fails for me as well. Not sure what fails for Joe such that he removed the 
> include, but if it fails in case H2_USE_WEBSOCKETS is
> not 1 I guess we could move the include (or even all) below the
> 
> #if H2_USE_WEBSOCKETS
> 
> line.

Oh, sorry guys. 

I was building against APR 1.6.x here which doesn't have apr_encode.h, I 
didn't see the apr_pencode use. So how about:

r1913019 + r1913023, +1 for 2.4.x for the pair



Re: [PATCH] fix mod_h2 with older nghttp2

2023-10-16 Thread Joe Orton
BTW is modules/http2 really CTR for 2.4.x? STATUS says only 
mod_proxy_http2 is.

Anyway: I am +1 for r1913005 and r1913019 for 2.4.x, latter was 
sufficient to get 2.4 building on RHEL8 again (if only we had a CI to do 
this, oh...).

Regards, Joe



[PATCH] fix mod_h2 with older nghttp2

2023-10-16 Thread Joe Orton
Looks like this broke with the websockets backport. mod_h2 is failing to 
compile on versions of nghttp2 without 
NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL - looks like this was added in 
nghttp2 v1.34.0 [1] so how about something like this, or is there a 
better way?

(configure check for a declaration is messier)

diff --git a/modules/http2/h2.h b/modules/http2/h2.h
index cfecb3d9a3..06e7087c04 100644
--- a/modules/http2/h2.h
+++ b/modules/http2/h2.h
@@ -20,6 +20,8 @@
 #include 
 #include 
 
+#include 
+
 struct h2_session;
 struct h2_stream;
 
@@ -39,7 +41,7 @@ struct h2_stream;
 #define H2_USE_POLLFD_FROM_CONN 0
 #endif
 
-#if H2_USE_PIPES
+#if H2_USE_PIPES && defined(NGHTTP2_VERSION_NUM) && NGHTTP2_VERSION_NUM >= 
0x012200
 #define H2_USE_WEBSOCKETS   1
 #else
 #define H2_USE_WEBSOCKETS   0



[1] https://github.com/nghttp2/nghttp2/milestone/30?closed=1 



Re: mod_ssl SSL_OP_IGNORE_UNEXPECTED_EOF: "unexpected eof while reading"

2023-09-08 Thread Joe Orton
On Thu, Sep 07, 2023 at 06:46:01PM +0200, Yann Ylavic wrote:
> On Thu, Sep 7, 2023 at 6:09 PM Yann Ylavic  wrote:
> >
> > On Wed, Aug 30, 2023 at 1:22 PM Rainer Jung  wrote:
> > >
> > > OpenSSL 3 flags some abortive shutdowns as an error different to what
> > > 1.1.1 did. This results in info log output in httpd:
> > >
> > > [Tue Aug 29 12:33:06.787210 2023] [ssl:info] [pid 1994673:tid 1994737]
> > > SSL Library Error: error:0A000126:SSL routines::unexpected eof while 
> > > reading
> > > [Tue Aug 29 12:33:06.787374 2023] [ssl:info] [pid 1994673:tid 1994737]
> > > [client 1.2.3.4:54790] AH01998: Connection closed to child 215 with
> > > abortive shutdown (server myserver:443)
> >
> > The info looks legit to me (someone closed the connection with no
> > close_notify), possibly we want to log it at APLOG_DEBUG/TRACEx still
> > if it happens too often?
> > We don't do that though for SSL_ERROR_ZERO_RETURN in openssl < 3, but
> > maybe we should too like in the attached patch (instead of r1912015)?
> 
> Scratch that patch, SSL_ERROR_ZERO_RETURN is actually when
> close_notify was received, we'd rather need to test SSL_ERROR_SYSCALL
> && errno == 0 with openssl < 0, which is more tricky in httpd with the
> EOS bucket vs APR_EOF.
> Hm, not sure we want to complicate this more..

Yeah, I wondered about that too. Maybe we need some kind of "strict 
mode" in mod_ssl which does better/correct close_notify handling?

Regards, Joe



Re: mod_ssl SSL_OP_IGNORE_UNEXPECTED_EOF: "unexpected eof while reading"

2023-09-07 Thread Joe Orton
On Wed, Aug 30, 2023 at 01:21:11PM +0200, Rainer Jung wrote:
> Hi there,
> 
> OpenSSL 3 flags some abortive shutdowns as an error different to what 1.1.1
> did. This results in info log output in httpd:
> 
> [Tue Aug 29 12:33:06.787210 2023] [ssl:info] [pid 1994673:tid 1994737] SSL
> Library Error: error:0A000126:SSL routines::unexpected eof while reading
> [Tue Aug 29 12:33:06.787374 2023] [ssl:info] [pid 1994673:tid 1994737]
> [client 1.2.3.4:54790] AH01998: Connection closed to child 215 with abortive
> shutdown (server myserver:443)
> 
> Some background is given in
> 
>   https://github.com/openssl/openssl/issues/18866
> 
> They introduced a new context option "SSL_OP_IGNORE_UNEXPECTED_EOF" to
> suppress this. Some other software now sets it with SSL_CTX_set_options():

Interesting! Just wondering, is there a reason why we'd only want to 
enable this for server-side operation (mctx->pkp == NULL) not also for 
client-side/proxy operation? Seems like it might be better to enable it 
unconditionally.

Regards, Joe



Re: svn commit: r1911908 - /httpd/httpd/branches/2.4.x/STATUS

2023-08-25 Thread Joe Orton
On Fri, Aug 25, 2023 at 12:11:38PM +0200, Ruediger Pluem wrote:
> > ==
> > --- httpd/httpd/branches/2.4.x/STATUS (original)
> > +++ httpd/httpd/branches/2.4.x/STATUS Fri Aug 25 07:52:31 2023
> > @@ -225,11 +225,12 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
> >*) mod_dav: add DavBasePath (PR 35077)
> >   Trunk version of patch:
> >  https://svn.apache.org/r1911651
> > +https://svn.apache.org/r1911906
> 
> Sorry for being picky, but don't we miss r1911907?

Oh, that rev is docs-only so isn't subject to RTC rules and I didn't 
think about including it here. I will backport the docs bits too though, 
definitely.

Regards, Joe



Re: svn commit: r1910861 - /httpd/httpd/trunk/support/ab.c

2023-07-10 Thread Joe Orton
On Mon, Jul 10, 2023 at 11:12:09AM +0200, Yann Ylavic wrote:
> I think this is not reached with rtnevents == POLLOUT|POLLHUP because
> it takes the first POLLIN|POLLHUP continue-branch.
> I moved the check for POLLOUT first in r1910911, which fixed the issue for me.

Perfect, works for me too - thanks Yann.

Regards, Joe



Re: svn commit: r1910861 - /httpd/httpd/trunk/support/ab.c

2023-07-10 Thread Joe Orton
On Fri, Jul 07, 2023 at 03:52:46PM -, yla...@apache.org wrote:
> Author: ylavic
> Date: Fri Jul  7 15:52:45 2023
> New Revision: 1910861
> 
> URL: http://svn.apache.org/viewvc?rev=1910861=rev
> Log:
> ab: Fix accounting of started connections.
> 
> Revert when a kept alive connection is aborted on read.
> Stop the polling loop when there is nothing to poll anymore, it's simpler.

Not related to this change but I noticed that trunk ab has stopped 
failing if trying to connect to a closed port, strace looks like:

epoll_wait(3, [{events=EPOLLOUT|EPOLLERR|EPOLLHUP, data={u32=33073216, 
u64=33073216}}], 2, 3) = 1 
epoll_wait(3, [{events=EPOLLOUT|EPOLLERR|EPOLLHUP, data={u32=33073216, 
u64=33073216}}], 2, 3) = 1 
...

With HUP set in the returned events it enters the first if() at 
https://github.com/apache/httpd/blob/trunk/support/ab.c#L2523 but 
STATE_CONNECTING is not handled there and so it loops indefinitely. 
That's as far as I debugged it, sorry. Any ideas?



Re: svn commit: r1910820 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/pr60182.txt modules/ssl/ssl_util_stapling.c

2023-07-07 Thread Joe Orton
On Thu, Jul 06, 2023 at 10:58:07PM +0200, Christophe JAILLET wrote:
> Le 06/07/2023 à 18:11, jor...@apache.org a écrit :
> > Author: jorton
> > Date: Thu Jul  6 16:11:56 2023
> > New Revision: 1910820
> > 
> > URL: http://svn.apache.org/viewvc?rev=1910820=rev
> > Log:
> > Merge r1875355 from trunk:
> > 
> > * modules/ssl/ssl_util_stapling.c (stapling_check_response) Don't stop
> >Certificate Revoked messages.
> > 
> >Certificate Revoked Responder messages don't belong to 'error' class.
> >When the server receives one, it MUST be passed on to the client.
> >And stored for the normal period of basic responses.
> > 
> >Also don't log an error each time it is retrieved from cache,
> >only once when it is retrieved from the OCSP responder.
> > 
> > PR: 60182
> > Obtained from: 
> > https://github.com/apache/httpd/commit/7db9795f45fd4688ceb13ee36090e4e2becbc709.diff
> > Submitted by: 
> > Reviewed by: gbechis, icing, ylavic
> 
> Hi,
> 
> I've not seen it in STATUS.
> 
> I'm a bit distant this days, so I may have missed something, but usually
> STATUS is updated just before or after a commit, and I don't see it either.

Hi Christophe - this was voted for in STATUS under "Fix for BZ 66626" 
though I missed that the bug number mentioned is different to the commit 
message. I've fixed the changes entry just now since 66626 is the right 
one. https://github.com/apache/httpd/blob/2.4.x/STATUS#L196

Thanks for checking & thanks for fixing my typo too.

Regards, Joe



Re: [VOTE] Switch read/write repository from Subversion to Git

2023-05-04 Thread Joe Orton
On Thu, May 04, 2023 at 10:34:32AM +0200, Ruediger Pluem wrote:
> This is a formal vote on whether we should move our read/write repository 
> from Subversion to Git.
> This means that our latest read/write repository will be no longer available 
> via svn.apache.org. It
> will be available via Git at 
> https://gitbox.apache.org/repos/asf/httpd-site.git and 
> https://github.com/apache/httpd.git.
> Github also offers the possibility to use a Subversion client:
> https://docs.github.com/en/get-started/working-with-subversion-on-github/support-for-subversion-clients
> 
> 
> [X]: Move the read/write repository from Subversion to Git and leverage the 
> features of Github (for now Actions and PR).
> [ ]: Move the read/write repository from Subversion to Git, but I don't want 
> to work with Github and I will only work with
>  what gitbox.apache.org offers.
> [ ]: Leave everything as is.

Thanks for calling the vote again.

Regards, Joe



Re: ci vs PR approvals? (was: [apache/httpd] Fix a possible NULL pointer dereference in hook_uri2file (PR #355))

2023-05-04 Thread Joe Orton
On Wed, May 03, 2023 at 02:31:35PM -0500, Daniel Gruno wrote:
> I am +1 on moving. I do not have any particular love for git or svn on their
> own, and I realize that the proposed change does make outside contributions
> and certain workflows easier.

+1 for the same reasons here. Might be better to call a formal VOTE 
thread on this with a clearer Subject.



Re: svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c

2023-04-21 Thread Joe Orton
On Wed, Apr 19, 2023 at 08:08:49PM +0200, Yann Ylavic wrote:
> On Fri, Apr 14, 2023 at 4:02 PM  wrote:
> >
> > Author: minfrin
> > Date: Fri Apr 14 14:02:11 2023
> > New Revision: 1909135
> >
> > URL: http://svn.apache.org/viewvc?rev=1909135=rev
> > Log:
> > core: Be explicit if an enclosing directive contains a path or a
> > regex.
> 
> Currently all the tests (framework) are broken due to cmd->regex not
> being properly stacked/scoped.

Trunk has been broken for a week, Graham could you move this work to a 
branch or PR until the regressions are fixed?

Regards, Joe



Re: [VOTE] Release httpd-2.4.57-rc1 as httpd-2.4.57

2023-04-03 Thread Joe Orton
On Sun, Apr 02, 2023 at 12:10:25PM -0400, Eric Covener wrote:
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> I would like to call a VOTE over the next few days to release
> this candidate tarball httpd-2.4.57-rc1 as 2.4.57:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.

+1 for release, CHANGES looks good, sigs match, tests pass on RHEL 8+9. 
Thanks for RMing.

Regards, Joe



Re: svn commit: r1908835 - /httpd/httpd/trunk/.github/workflows/linux.yml

2023-03-31 Thread Joe Orton
On Fri, Mar 31, 2023 at 09:47:05AM +0200, Yann Ylavic wrote:
> On Fri, Mar 31, 2023 at 9:22 AM  wrote:
> >
> > Author: jorton
> > Date: Fri Mar 31 07:21:37 2023
> > New Revision: 1908835
> >
> > URL: http://svn.apache.org/viewvc?rev=1908835=rev
> > Log:
> > Try running CI for 2.* tags. [skip ci]
> 
> It seems that gh actions are not triggered anymore for trunk and
> 2.4.x, but I have no clue as to what the cause can be.
> The Last run was for r1908714 which should not have happened
> (paths-ignore: docs/**), nothing since then but PRs..
> 
> Any ideas?

Yeah I'm not sure why the CI ran for that commit, that's odd.

It looks like the github mirroring has stopped working a few days ago. 
I've created https://issues.apache.org/jira/browse/INFRA-24412

Regards, Joe



Re: svn commit: r1908684 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/cache/mod_cache_disk.c

2023-03-28 Thread Joe Orton
On Fri, Mar 24, 2023 at 08:50:19AM -, gbec...@apache.org wrote:
> Author: gbechis
> Date: Fri Mar 24 08:50:19 2023
> New Revision: 1908684
> 
> URL: http://svn.apache.org/viewvc?rev=1908684=rev
> Log:
> add error message when storing data to temp file fails.
> Github: closes #182

Hi Giovanni, thanks for taking care of the PRs, that is really great to 
see.

FYI when you are committing code written by someone else it is customary 
to include "Submitted by:" in the commit message to give them due 
credit, like:

Submitted by: Name of Person 

There are guidelines here: 
https://httpd.apache.org/dev/guidelines.html#changes-file-and-subversion-logs

If you use Ruediger's apply_trunk_pr.sh script from here: 
https://svn.apache.org/repos/asf/httpd/dev-tools/github/ 
it will do this automatically for you for a PR, including extracting the 
full name properly from the github metadata (which is really neat).

Regards, Joe



Re: svn commit: r1908537 - /httpd/httpd/trunk/modules/ssl/

2023-03-20 Thread Joe Orton
On Sun, Mar 19, 2023 at 09:30:47PM -, yla...@apache.org wrote:
> Author: ylavic
> Date: Sun Mar 19 21:30:47 2023
> New Revision: 1908537
> 
> URL: http://svn.apache.org/viewvc?rev=1908537=rev
> Log:
> mod_ssl: Fix deprecation warnings with openssl-3.

Great stuff, thank you Yann!



Re: svn commit: r1908060 - in /httpd/httpd/trunk/test/modules: http1/htdocs/cgi/ http2/ http2/htdocs/cgi/ md/ tls/ tls/htdocs/a.mod-tls.test/ tls/htdocs/b.mod-tls.test/

2023-03-07 Thread Joe Orton
On Tue, Mar 07, 2023 at 09:15:59AM +0100, Stefan Eissing via dev wrote:
> 
> 
> > Am 06.03.2023 um 17:53 schrieb Joe Orton :
> > 
> > [resent to dev@]
> > 
> > On Sat, Mar 04, 2023 at 01:40:39PM -, ic...@apache.org wrote:
> >> Author: icing
> >> Date: Sat Mar  4 13:40:38 2023
> >> New Revision: 1908060
> >> 
> >> URL: http://svn.apache.org/viewvc?rev=1908060=rev
> >> Log:
> >> Test case updates related to macOS ventura changes:
> >> 
> >> - python 3.11 deprecates the `cg` module, replacing
> >>  url query and multipart form-data handling with new code
> >> - adaptions to changes in openssl/curl behaviours
> >> - all mod_tls test cases now have prefix `test_tls_` for
> >>  easier scoping.
> > 
> > This seems to be failing:
> > 
> > https://github.com/apache/httpd/actions/runs/4341851149/jobs/7581956398
> > 
> > 1) Maybe some new pypi requirement or something?  Looks like the CGI 
> > scripts are now giving 500 errors.
> 
> Yes, for the deprecated `cgi` python module, the `multipart` module
> is recommended by the PyGods to replace parts of it. I have no idea
> how that is named on ubuntu-latest.

It exists but it is prehistoric or something completely different to 
what is in pypi as "multipart" now - apt-get logs say:

Setting up python3-multipart (0.0.5-2) ...

which is not listed here: https://pypi.org/project/multipart/#history

The new error_log is:

[Tue Mar 07 09:34:12.322270 2023] [cgid:error] [pid 51124:tid 139809792149056] 
[client 127.0.0.1:34504] AH01215: stderr from 
/home/runner/work/httpd/httpd/test/gen/apache/htdocs/b.mod-tls.test/vars.py: 
AttributeError: module 'multipart' has no attribute 'parse_form_data'

maybe we should "pip install" the deps here rather than relying on 
Ubuntu packages.

> > 2) What is the path to the relevant error_log when running those tests, 
> > we can tweak the config to grab that file and upload it for easy 
> > diagnosis.
> 
> The server error log on all pytests is found in 
> test/gen/apache/logs/error_log. It is cleared on test start.

Thanks, that works at least.

Regards, Joe



Re: svn commit: r1908060 - in /httpd/httpd/trunk/test/modules: http1/htdocs/cgi/ http2/ http2/htdocs/cgi/ md/ tls/ tls/htdocs/a.mod-tls.test/ tls/htdocs/b.mod-tls.test/

2023-03-06 Thread Joe Orton
[resent to dev@]

On Sat, Mar 04, 2023 at 01:40:39PM -, ic...@apache.org wrote:
> Author: icing
> Date: Sat Mar  4 13:40:38 2023
> New Revision: 1908060
> 
> URL: http://svn.apache.org/viewvc?rev=1908060=rev
> Log:
> Test case updates related to macOS ventura changes:
> 
> - python 3.11 deprecates the `cg` module, replacing
>   url query and multipart form-data handling with new code
> - adaptions to changes in openssl/curl behaviours
> - all mod_tls test cases now have prefix `test_tls_` for
>   easier scoping.

This seems to be failing:

https://github.com/apache/httpd/actions/runs/4341851149/jobs/7581956398

1) Maybe some new pypi requirement or something?  Looks like the CGI 
scripts are now giving 500 errors.

2) What is the path to the relevant error_log when running those tests, 
we can tweak the config to grab that file and upload it for easy 
diagnosis.



Re: [VOTE] [VOTE] Release httpd-2.4.56-rc1 as httpd-2.4.56

2023-03-06 Thread Joe Orton
On Sun, Mar 05, 2023 at 04:31:34PM -0500, Eric Covener wrote:
> Hi all,
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> I would like to call a VOTE over the next few days to release
> this candidate tarball httpd-2.4.56-rc1 as 2.4.56:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.

+1, tests pass on RHEL 8+9 (x86_64), sigs good, thanks for RMing.

Seems there is some tweak required to get Actions to work for a tag 
which I will look into.

Regards, Joe



Re: svn commit: r1907974 - in /httpd/httpd/trunk: CMakeLists.txt modules/dav/fs/config6.m4 modules/dav/fs/mod_dav_fs.c modules/dav/fs/mod_dav_fs.dsp modules/dav/fs/repos.c modules/dav/fs/repos.h modul

2023-03-02 Thread Joe Orton
On Thu, Mar 02, 2023 at 02:36:32PM -, m...@apache.org wrote:
> Author: manu
> Date: Thu Mar  2 14:36:31 2023
> New Revision: 1907974
> 
> URL: http://svn.apache.org/viewvc?rev=1907974=rev
> Log:
> Add RFC4331 quotas for mod_dav_fs

Hi Emmanuel - looks like you forgot to "svn add" the new quota.c in this 
commit.

Regards, Joe



Re: [VOTE] broader RTC exception for 2.4.x CI changes

2023-02-21 Thread Joe Orton
Thanks all, applied in r1907783.

Regards, Joe



Re: svn commit: r1907680 - /httpd/httpd/trunk/modules/dav/main/ms_wdv.c

2023-02-15 Thread Joe Orton
On Wed, Feb 15, 2023 at 02:07:14PM -, m...@apache.org wrote:
> Author: manu
> Date: Wed Feb 15 14:07:14 2023
> New Revision: 1907680
> 
> URL: http://svn.apache.org/viewvc?rev=1907680=rev
> Log:
> Fix warnings

Thanks! There are a still a couple more warnings with GCC 10 from a %s 
argument being passed a constant NULL:

https://github.com/apache/httpd/actions/runs/4184691029/jobs/7250692033#step:10:2774

In function ‘mswdv_combined_lock’,
inlined from ‘dav_mswdv_preprocessing’ at 
/home/runner/work/httpd/httpd/modules/dav/main/ms_wdv.c:693:20:
/home/runner/work/httpd/httpd/include/http_log.h:454:23: error: ‘%s’ directive 
argument is null [-Werror=format-overflow=]
  454 | #define ap_log_rerror ap_log_rerror_
/home/runner/work/httpd/httpd/modules/dav/main/ms_wdv.c:410:5: note: in 
expansion of macro ‘ap_log_rerror’
  410 | ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
  | ^
/home/runner/work/httpd/httpd/include/http_log.h:454:23: error: ‘%s’ directive 
argument is null [-Werror=format-overflow=]
  454 | #define ap_log_rerror ap_log_rerror_
/home/runner/work/httpd/httpd/modules/dav/main/ms_wdv.c:410:5: note: in 
expansion of macro ‘ap_log_rerror’
  410 | ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
  | ^
cc1: all warnings being treated as errors



[VOTE] broader RTC exception for 2.4.x CI changes

2023-02-15 Thread Joe Orton
Per my previous Travis is dead, long live GitHub actions.

I propose to broaden the RTC exception in 2.4.x/STATUS to allow CI 
config changes and scripts to be merged from trunk:

Index: STATUS
===
--- STATUS  (revision 1907679)
+++ STATUS  (working copy)
@@ -141,7 +141,7 @@
 . non-Unix, single-platform code
 . routine APLOGNO() backports
 . .gdbinit
-. Travis integration: .travis.yml and test/travis*.sh
+. CI configuration and scripts
 . mod_tls
 
 RELEASE SHOWSTOPPERS:



CI status update: Travis out, GitHub Actions in

2023-02-15 Thread Joe Orton
Support for Travis has now been dropped, so we are relying only on 
GitHub Actions for CI from today.

There are some gaps in the GHA configuration compared to what we had 
running in Travis: notably, we're currently only testing on the latest 
Ubuntu release, and nothing has been ported back to 2.4.x yet. I will 
propose a vote separately for the 2.4.x RTC exception to extend to CI 
configuration. 

On the positive side, the GHA configuration is a lot more flexible, and 
e.g. not triggering CI on docs changes was a simple change. Any help 
filling remaining gaps would be very welcome, there is a TODO list in 
test/README.ci.

Another significant change is that GHA is configured to require approval 
on PRs from non-committers - currently only for the first PR filed, but 
from March 19th the ASF default will switch to requiring approval for 
every PR filed by a non-contributor. I don't think this is a big deal 
but will require committers to actively review and hit "approve" on each 
such PR to get it tested.

I'm happy to try to answer any questions about GHA, but I'm still 
learning how it works so very much a non-expert here.

Regards, Joe



Re: [VOTE] Release httpd-2.4.55-rc1 as httpd-2.4.55

2023-01-10 Thread Joe Orton
On Tue, Jan 10, 2023 at 10:21:55AM -0500, Eric Covener wrote:
> On Tue, Jan 10, 2023 at 10:17 AM Giovanni Bechis  wrote:
...
> > In file included from /usr/include/openssl/asn1.h:27,
> >  from /usr/include/openssl/rsa.h:21,
> >  from ab.c:169:
> > /usr/include/openssl/bio.h:279:28: note: declared here
> >   279 | OSSL_DEPRECATEDIN_3_0 void BIO_set_callback(BIO *b, BIO_callback_fn 
> > callback);
> >   |^~~~
> > cc1: all warnings being treated as errors
> > -
> >
> > Is this considered a blocker ?
> > This can be workarounded by building with different "-Werror" options.
> >  Giovanni
> 
> I think it's a known issue in ab.c and openssl 3.0
> I think no regression, no veto -- but everyones vote (beyond veto) is
> their own. AFAIK it has been there since 3.0 toleration was added.

Yup - there are many more deprecation warnings in mod_ssl itself too 
when building against OpenSSL 3.x. Some of them are worthwhile fixing 
but IIRC some looked quite involved to fix.

> I was going to send an email on this one, reminded by the recent
> Actions CI activity.  I think we could drop the -wno-error-deprecated
> from CI if ab.c was either fixed or maybe had something in its build
> to set this itself. That way deprecated stuff sneaking in elsewhere
> would not be supresed in maintainer mode.

Adding -Wno-deprecated-declarations or -Wno-error-etc is probably a good 
idea for all OpenSSL 3 builds, yeah.

Regards, Joe



Re: [VOTE] Release httpd-2.4.55-rc1 as httpd-2.4.55

2023-01-10 Thread Joe Orton
On Tue, Jan 10, 2023 at 08:40:52AM -0500, Eric Covener wrote:
> Hi all,
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> I would like to call a VOTE over the next few days to release
> this candidate tarball httpd-2.4.55-rc1 as 2.4.55:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.
> 
> The computed digests of the tarball up for vote are:
> sha256: 5276ea8bc6fff31eed5c82132ae51a0b2ee05f9e6b61a00fa877f6cadab3b638
> *httpd-2.4.55-rc1.tar.gz
> sha512: 
> ca0d03b5e74078977378fe711ca3ed8cf63c109b7dbe73f2c43f7f30f7e522bbe46f93189a183b7675394d57fffb0c2526facd8d40508be984a7a8f64d18f8d6
> *httpd-2.4.55-rc1.tar.gz

+1 for release, thank you for RMing!

Test suite passes on RHEL 8 and 9 (x86_64).

Regards, Joe



Re: svn commit: r1906487 - /httpd/httpd/trunk/modules/dav/main/util.c

2023-01-10 Thread Joe Orton
On Tue, Jan 10, 2023 at 07:30:37AM +0100, Ruediger Pluem wrote:
> On 1/9/23 5:16 PM, Joe Orton wrote:
> > It seems consistent with other error cases to return straight away, but 
> > I'm not following the second part, can you explain more?  An 'N' 
> > followed by whitespace should be caught and treated as an error now (as 
> > desired & expected).
> 
> Sorry for the confusion. It is treated as an error now. I was referring to my
> other approach were it would not be caught. I also found another case that 
> would not be
> caught by my proposal (an 'N' at the end of the string). Hence all good. Your 
> approach
> is the correct and better one.

Ah, great, thanks for looking at it! Any chance of a +1 for the 2.4 
backport? Just need one more vote :)

Regards, Joe



Re: svn commit: r1906487 - /httpd/httpd/trunk/modules/dav/main/util.c

2023-01-09 Thread Joe Orton
On Mon, Jan 09, 2023 at 04:47:33PM +0100, Ruediger Pluem wrote:
> On 1/9/23 1:01 PM, jor...@apache.org wrote:
> > Author: jorton
> > Date: Mon Jan  9 12:01:56 2023
> > New Revision: 1906487
> > 
> > URL: http://svn.apache.org/viewvc?rev=1906487=rev
> > Log:
> > * modules/dav/main/util.c (dav_process_if_header): Fix error
> >   path for "Not" prefix parsing.
> > 
> > Modified:
> > httpd/httpd/trunk/modules/dav/main/util.c
> > 
> > Modified: httpd/httpd/trunk/modules/dav/main/util.c
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/util.c?rev=1906487=1906486=1906487=diff
> > ==
> > --- httpd/httpd/trunk/modules/dav/main/util.c (original)
> > +++ httpd/httpd/trunk/modules/dav/main/util.c Mon Jan  9 12:01:56 2023
> > @@ -801,8 +801,14 @@ static dav_error * dav_process_if_header
> >   "for the same state.");
> >  }
> >  condition = DAV_IF_COND_NOT;
> > +list += 2;
> > +}
> > +else {
> > +return dav_new_error(r->pool, HTTP_BAD_REQUEST,
> > + DAV_ERR_IF_UNK_CHAR, 0,
> > + "Invalid \"If:\" header: "
> > + "Unexpected character in 
> > List");
> 
> Do we want to return here to save cycles or do we do another cycle in 
> the while loop and reuse the error message from 'default:' and thus 
> also ignore single 'N' s that are followed by a ' \t<['?

It seems consistent with other error cases to return straight away, but 
I'm not following the second part, can you explain more?  An 'N' 
followed by whitespace should be caught and treated as an error now (as 
desired & expected).

Regards, Joe



Re: MS-WDV (was Re: Help with buckets)

2022-12-02 Thread Joe Orton
On Fri, Dec 02, 2022 at 08:53:07AM +, Emmanuel Dreyfus wrote:
> Hello
> 
> I made some progress with the combined GET+PROPFIND specified
> by MS-WDV (for a summary, see
> https://lists.apache.org/thread/57s1vvl6k9qpdv5ym7mtcl29bd933w7k )
> 
> Attached is the diff against trunk, form comments.

Hi Emmanuel,

That's a very weird protocol design, wow. Have you tested this with a 
long PROPFIND response? It needs to buffer the entire response in the 
output brigade to work out the length in the "multipart" prefix, but 
mod_dav will flush output brigades down the filter chain regularly 
during the multistatus response processing.

I think this might need to do something more complex, maybe running the 
PROPFIND in a subrequest properly and capturing (buffering) the output 
in a custom filter, rather than using the mod_dav internal API directly. 
Have you tried using ap_sub_req_method_uri()? Not sure this has been 
tried before with mod_dav so might well be something I'm missing.

Regards, Joe



New committer: Emmanuel Dreyfus

2022-11-08 Thread Joe Orton
The Project Management Committee (PMC) for the Apache HTTP Server has 
invited Emmanuel Dreyfus to become a committer and we are pleased to 
announce that they have accepted.

Welcome, Emmanuel!

Regards, Joe



Re: [libapreq2] nits to pick about the patches to util.c over the past few years

2022-10-31 Thread Joe Orton
On Sun, Oct 30, 2022 at 12:09:02AM -0400, Joe Schaefer wrote:
> Forgive me for summarizing, but I didn’t come here expecting help, much
> less collaboration on a solution.  I came here expecting to be scolded for
> having the temerity to critique the quality of the patch sets you’ve been
> shipping of late In libapreq2.  None of my opinions have changed on that
> subject, and won’t for some time.
> 
> The point is I’m part of your extended support channels for libapreq2,
> because it’s easy and fun to help people who use the code.  I’m not here to
> complain, I’m here expecting more empathy for people like me who give their
> time graciously without expecting anything in return other than some
> measure of respect for the effort involved.

Joe, please dial down both the rhetoric and rate of fire.

Constructive criticism is always welcome here, we are quite used to 
doing patch reviews. You've come in with a torrent of not very 
constructive messages, unsurprisingly that won't get a hugely 
sympathetic response from others. We all participate here voluntarily.

Regards, Joe




CVE-2022-22728: libapreq2: libapreq2 multipart form parse memory corruption

2022-08-25 Thread Joe Orton
Severity: important

Description:

A flaw in libapreq2 versions 2.16 and earlier could cause a buffer overflow 
while processing multipart form uploads.  A remote attacker could send a 
request causing a process crash which could lead to a denial of service attack.



[RESULT: PASS] Re: [VOTE] Release libapreq2-2.17

2022-08-25 Thread Joe Orton
Thanks for testing. The release is approved:

PMC votes: +1 from ylavic, jfclere, jorton

I will promote the release and announce it.

Regards, Joe



Re: [VOTE] Release libapreq2-2.17

2022-08-25 Thread Joe Orton
On Thu, Aug 18, 2022 at 12:31:56PM +0100, Joe Orton wrote:
> Hi, I've prepared a candidate release tarball for libapreq2 v2.17 here:
> 
> https://dist.apache.org/repos/dist/dev/httpd/libapreq/
> 
> I would like to call a VOTE over the next week to release this candidate 
> tarball as v2.17:
> 
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.

Adding my own +1, tests fine here on Fedora 36.

Regards, Joe



[VOTE] Release libapreq2-2.17

2022-08-18 Thread Joe Orton
Hi, I've prepared a candidate release tarball for libapreq2 v2.17 here:

https://dist.apache.org/repos/dist/dev/httpd/libapreq/

I would like to call a VOTE over the next week to release this candidate 
tarball as v2.17:

[ ] +1: It's not just good, it's good enough!
[ ] +0: Let's have a talk.
[ ] -1: There's trouble in paradise. Here's what's wrong.

SHA-256 and SHA-512 checksums for the tarball are as follows:

046487f084c12fa1c822affc5f7de56efed9b48905a426e631a6b949c114d86c  
libapreq2-2.17.tar.gz
89b139b8673145d9e2d8fd77d36f878c519c1deb7f9b853cda2a15d34cbb619d1c5e784ba21553f23c2ef07803f07c75a83d96cd770f80e1b36283a4cbb88999
  libapreq2-2.17.tar.gz

The release is prepared from:
https://svn.apache.org/repos/asf/httpd/apreq/branches/v2.17 at r1903514

Regards, Joe



Re: svn commit: r1894982 - /httpd/apreq/trunk/library/util.c

2022-08-18 Thread Joe Orton
On Wed, Aug 17, 2022 at 06:17:23PM +0200, Yann Ylavic wrote:
> I fixed it in r1903496 by requiring that the name in a name=value pair
> only be a token, with no equal sign the attribute is a value only.

Thanks a lot for committing all the fixes, test suite is passing here 
now.

Regards, Joe



Re: svn commit: r1894982 - /httpd/apreq/trunk/library/util.c

2022-08-17 Thread Joe Orton
On Wed, Aug 17, 2022 at 02:05:09PM +0100, Joe Orton wrote:
> On Fri, Nov 12, 2021 at 06:12:58PM -, yla...@apache.org wrote:
> > Author: ylavic
> > Date: Fri Nov 12 18:12:58 2021
> > New Revision: 1894982
> > 
> > URL: http://svn.apache.org/viewvc?rev=1894982=rev
> > Log:
> > apreq_header_attribute: Search for the exact attribute name.
> > 
> > Improve the parsing of the header attributes such that we don't match any
> > special character before that attribute name (e.g. "(boundary=") or let
> > forbidden characters unnoticed.

Yann, it appears this change is also breaking the "params" test case in 
the apreq test suite. A test is trying to parse a content-type like 
header:

https://svn.apache.org/viewvc/httpd/apreq/trunk/library/t/params.c?revision=1903492=markup#l100

it fails when reaching the '/' in "text/plain" which is a non-token 
character:

default:
/* The name is a token */
if (!IS_TOKEN_CHAR(*hde))
return APREQ_ERROR_BADCHAR;

Unless this is an invalid use case (the test case implies otherwise) 
this seems like a regression as well?

Regrads, Joe



Re: svn commit: r1894982 - /httpd/apreq/trunk/library/util.c

2022-08-17 Thread Joe Orton
On Fri, Nov 12, 2021 at 06:12:58PM -, yla...@apache.org wrote:
> Author: ylavic
> Date: Fri Nov 12 18:12:58 2021
> New Revision: 1894982
> 
> URL: http://svn.apache.org/viewvc?rev=1894982=rev
> Log:
> apreq_header_attribute: Search for the exact attribute name.
> 
> Improve the parsing of the header attributes such that we don't match any
> special character before that attribute name (e.g. "(boundary=") or let
> forbidden characters unnoticed.

...
> +look_for_after_quote:
> +switch (*v) {
> +case 0:
> +case '\r':
> +case '\n':
> +done = 1;
> +case ';':
> +case ',':
> +break;
> +case ' ':
> +case '\t':
> +goto look_for_after_quote;

This is an infinite loop. The libapreq test suite is spinning here, 
"make test" from apreq trunk.




Re: [VOTE] Release httpd-2.4.54-rc3 as httpd-2.4.54

2022-06-07 Thread Joe Orton
On Mon, Jun 06, 2022 at 04:25:31PM +0200, Stefan Eissing wrote:
> Here we go again! Sorry for the repeats, but that is why we build candidates, 
> right?
> 
> Hi all,
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> I would like to call a VOTE over the next few days to release
> this candidate tarball httpd-2.4.54-rc3 as 2.4.54:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.
> 
> The computed digests of the tarball up for vote are:
> sha256: c687b99c446c0ef345e7d86c21a8e15fc074b7d5152c4fe22b0463e2be346ffb 
> *httpd-2.4.54-rc3.tar.gz
> sha512: 
> e9599df48a73b07b3a11dd44db2c22a671e8a41cdd5021bb434bbcde39d6fc498d165d9b0c4ed2b66a6321d9760b031c1c1c84c23661dbf44c42c52f637ec4dd
>  *httpd-2.4.54-rc3.tar.gz

+1 for release, passes tests on Fedora 36, RHEL 8 & 9 (x86_64 only).

One note: on F36 I had to manually add a route for the multicast range 
to get t/modules/heartbeat.t to pass, which I guess is a change in the 
default network configuration compared to earlier Fedora releases.

Thanks for RMing! (x3)

Regards, Joe



Re: strcasecmp raises its...

2022-05-19 Thread Joe Orton
On Wed, May 18, 2022 at 05:34:22PM +0200, Ruediger Pluem wrote:
> On 5/18/22 4:55 PM, Joe Orton wrote:
> > I think for httpd it is only safe and sane to run httpd with LANG=C, we 
> > do this in the default service scripts in Fedora/RHEL for a very long 
> > time. Other than the protocol parsing issues you can get in non-C 
> > locales, you can also get "surprises" when sort order can change with 
> > the system locale, impacting e.g. config file load ordering and more.
> 
> Don't you need a locale sensitive case insensitive string comparison in case 
> of case blind file systems which support extended
> latin characters? I know these Germans with their Umlaute :-).

Heh. Well, I got away with it so far :)

> > So IMHO it is probably sufficient & simpler to adjust apachectl to set 
> > LANG=C rather than trying to eliminate strcasecmp, and add another 
> > strcasecmp() reimplementation in APR, in this case.
> 
> We already have this implementation in APR and we use the
> httpd one which is just a forward port from APR to httpd until we require a 
> sufficient recent APR version in several places.
> The question is just if we should use them everywhere and thus do the correct 
> thing no matter what locale is set.

Ah, I missed that, thanks.

+1 from me on doing replacement of strcasecmp() with the 
locale-insensitive versions then. At least with config options, protocol 
data, it is definitely right.

Regards, Joe




Re: strcasecmp raises its...

2022-05-18 Thread Joe Orton
On Wed, May 18, 2022 at 12:53:57PM +0200, Ruediger Pluem wrote:
> 
> 
> On 5/18/22 12:19 PM, Stefan Eissing wrote:
> > 2022 and we discuss strcasecmp() again?
> > 
> > Background: OpenSSL 3.0.3 added OPENSSL_strcasecmp() and friends and there 
> > are several issue around their implementation. Up to this version, they 
> > relied on the POSIX strcasecmp(). Whatever their reasons for their change...
> > 
> > Checking our sources, we have ap_cstr_casecmp() that does the right thing. 
> > But 
> > - we do not use it everywhere
> > - it is not part of APR which relies on the POSIX strcasecmp(), esp. 
> > apr_table does.
> 
> It is, but it may not be used where it possibly should:
> 
> https://apr.apache.org/docs/apr/1.7/group__apr__cstr.html
> 
> > 
> > I want to handshake with you regarding this:
> > 1. should we scan our sources for strcasecmp and replace it with 
> > ap_cstr_casecmp()?
> 
> If I remember correctly ap_cstr_casecmp was only designed to be used for 
> comparisons of HTTP protocol strings as it is locale
> agnostic. Hence I am not sure if it is correct to use it everywhere. From the 
> documentation:
> 
> **
>  * Perform a case-insensitive comparison of two strings @a str1 and @a str2,
>  * treating upper and lower case values of the 26 standard C/POSIX alphabetic
>  * characters as equivalent. Extended latin characters outside of this set
>  * are treated as unique octets, irrespective of the current locale.
> 
> Hence it might be wrong to use it in cases where you need to respect the 
> locale.

Are there really any cases like that in httpd?

I think for httpd it is only safe and sane to run httpd with LANG=C, we 
do this in the default service scripts in Fedora/RHEL for a very long 
time. Other than the protocol parsing issues you can get in non-C 
locales, you can also get "surprises" when sort order can change with 
the system locale, impacting e.g. config file load ordering and more.

So IMHO it is probably sufficient & simpler to adjust apachectl to set 
LANG=C rather than trying to eliminate strcasecmp, and add another 
strcasecmp() reimplementation in APR, in this case.

Regards, Joe



Re: Trouble in rusttls land?

2022-05-11 Thread Joe Orton
On Tue, May 10, 2022 at 10:50:12PM +0200, Stefan Eissing wrote:
> Will look at it tomorrow.

Could we cache the crates if building them is the source of 
unreliability?  It looks like we just have to add a couple of extra 
directories to the cache list:

https://docs.travis-ci.com/user/caching/#rust-cargo-cache

On the other hand, if the aim is to always build against current 
versions of the rust components then maybe you don't want this?

> 
> > Am 10.05.2022 um 15:17 schrieb Ruediger Pluem :
> > 
> > The latest build 0f 2.4.x on Travis failed with mod_tls. Ideas?
> > 
> > https://app.travis-ci.com/github/apache/httpd/jobs/569587665
> > 
> > Regards
> > 
> > Rüdiger
> 



Re: Fwd: FYI: change to travis-ci emailer could cause moderation headaches

2022-05-10 Thread Joe Orton
On Wed, Jan 26, 2022 at 08:58:14AM -0500, Eric Covener wrote:
> I noticed I stopped getting "Travis CI" emails for httpd around 10/21.
> But I see people still discussing CI failures, so I am a little
> confused. Maybe they are only seeing it in the context of PRs.
> 
> Did we lose notifications to dev@ as a result of the below? Or something else?

Travis support think this has now been fixed, they should be using a 
fixed enveloper sender of "bui...@travis-ci.com".  Is there a build 
failure in the dev@ moderation queue for the recent failures, can one of 
the moderators check?

Regards, Joe



Re: [VOTE] Release httpd-2.4.53-rc2 as httpd-2.4.53

2022-03-10 Thread Joe Orton
On Wed, Mar 09, 2022 at 05:19:22PM +0100, Stefan Eissing wrote:
> Hi all,
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> I would like to call a VOTE over the next few days to release
> this candidate tarball httpd-2.4.53-rc2 as 2.4.53:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.
> 
> The computed digests of the tarball up for vote are:
> sha256: 7a045e8e653aaf931f9667f3a7e1943bd81306bf908f316465f737a854d10c16 
> *httpd-2.4.53-rc2.tar.gz
> sha512: 
> 2fa1918aa44eb9984c0a294f416102088ed6874213391acd8c434b062603eb2a9ec64f7b1a218d4fe283d74fc9956650297d0b4dda57dcd8533b075a7321ec78
>  *httpd-2.4.53-rc2.tar.gz
> 
> The SVN candidate source is found at tags/2.4.53-rc2-candidate.

+1 from me, test suite passes on Fedora 35, RHEL8+"9ish".  Thanks again.

Regards, Joe



Re: [VOTE] Release httpd-2.4.53-rc1 as httpd-2.4.53

2022-03-08 Thread Joe Orton
On Tue, Mar 08, 2022 at 02:01:42PM +0100, Ruediger Pluem wrote:
> > I got a new "may be uninitialized" warning with with the GCC 12 shapshot 
> > used in Fedora 36 (which is still under development and can be 
> > unreliable). I think it's unreachable, if we enter here:
> > 
> > https://github.com/apache/httpd/blob/trunk/modules/lua/lua_request.c#L244
> > 
> > r->remaining must be > 0 and hence length > rpos is guaranteed and the 
> > loop will iterate at least once.
> 
> I agree, but I am also fine to init len_read to -1 to avoid this. But I think 
> this is not a blocker.
> Thanks for bringing it to attention. r1898731.

Nice, thank you.  +1 to that for 2.4.x.




Re: [VOTE] Release httpd-2.4.53-rc1 as httpd-2.4.53

2022-03-08 Thread Joe Orton
On Mon, Mar 07, 2022 at 04:55:54PM +0100, Stefan Eissing wrote:
> Hi all,
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> I would like to call a VOTE over the next few days to release
> this candidate tarball httpd-2.4.53-rc1 as 2.4.53:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.
> 
> The computed digests of the tarball up for vote are:
> sha256: 7559255a37adc5cb6f568ba224e435420f0a138cd9564162c9528b8ac08737b3 
> *httpd-2.4.53-rc1.tar.gz
> sha512: 
> a7734e2fa35389678be74bbc18136eef482ff9daecc2c1ed9c2c5eb410182735a94ff6ad248d0d4b6266e90161f2f8052d4871f381723c996ace0412b0219961
>  *httpd-2.4.53-rc1.tar.gz
> 
> The SVN candidate source is found at tags/2.4.53-rc1-candidate.

+1 for release and thanks for RMing.  Passes test suite on Fedora 35, 
RHEL 8, and 9 Beta(ish).

I got a new "may be uninitialized" warning with with the GCC 12 shapshot 
used in Fedora 36 (which is still under development and can be 
unreliable). I think it's unreachable, if we enter here:

https://github.com/apache/httpd/blob/trunk/modules/lua/lua_request.c#L244

r->remaining must be > 0 and hence length > rpos is guaranteed and the 
loop will iterate at least once.

lua_request.c: In function ‘lua_read_body’:
lua_request.c:260:12: error: ‘len_read’ may be used uninitialized 
[-Werror=maybe-uninitialized]
  260 | if (len_read < 0) {
  |^
lua_request.c:247:22: note: ‘len_read’ was declared here
  247 | apr_off_tlen_read, rpos = 0;
  |  ^~~~



Re: candidate branch/tag names

2022-03-08 Thread Joe Orton
On Mon, Mar 07, 2022 at 01:46:51PM +0100, Stefan Eissing wrote:
> Joe,
> 
> I'll change the release candidate tag/branch names and if that works, 
> you can strip some regex from the travis setup.

Looks good to me, thanks Stefan.  I've updated the regexes.

Regards, Joe



Re: backports

2022-03-08 Thread Joe Orton
On Sun, Mar 06, 2022 at 05:56:36PM +0200, Graham Leggett wrote:
> I am however strongly opposed for Github to be our only promotion process.
> 
> CI is great right until the point you get your first unrelated test failure, 
> then it is a nightmare. The collectd project was completely stuck unable to 
> merge a single PR for months and months because their CI broke and nobody had 
> access to fix it. Github presented a “computer says no” button and the 
> project ground to a complete halt. The project is now so backlogged that the 
> chances of getting anything reviewed are slim.
> 
> https://www.mail-archive.com/search?l=colle...@verplant.org=subject:%22Re%5C%3A+%5C%5Bcollectd%5C%5D+SNMPv3%5C%2BDTLS+support+for+collectd%5C-snmp%22=newest=1
> 
> It is inevitable that at some point the generosity of the people 
> supplying the CI will run out, and CI will stop working. We cannot 
> allow ourselves to be jammed up because of this.

I assume the ASF infra budget is still paying for Travis capacity since 
there is no free service there any more (beyond an initial N thousand 
hours per account), and they were in the past:

https://blogs.apache.org/infra/entry/apache_gains_additional_travis_ci

AIUI you can configure github to allow merges even if tests fail, though 
I'm not familiar with that, has anybody played with the settings there?

Regards, Joe



Re: backports

2022-03-08 Thread Joe Orton
On Mon, Mar 07, 2022 at 01:28:19PM +0200, Graham Leggett wrote:
> On 07 Mar 2022, at 11:21, Stefan Eissing  wrote:
> 
> > I'd really like, as a reviewer of backports, you can:
> > - see that it passes all our tests. No need to patch/compile/test locally.
> 
> “No need to patch/compile locally" is not a good idea - currently the 
> travis tests target Ubuntu only, and this is a practical limitation 
> forced upon us by the nature of the Travis service. I want to see 
> reviewers try out the patch on different architectures. I for example 
> work on MacOS, but deploy to Redhat, so those are my two environments. 
> Others will have different environments.
> 
> Our testing needs to be wide and diverse.

Definitely +1 on the sentiment and I'm keen to help with any effort to 
add more diversity to the CI.  Travis natively supports a bunch of 
non-Linux platforms so if someone cares about that they should be able 
to hook it up by tweaking the .travis.yml. 
https://docs.travis-ci.com/user/reference/overview/

(I'll note the empty APLOGNO() check in question here was added because 
some Windows build broke for that case and it blocked a release, IIRC.)

> This way we learn far more than what Github will give to us. Has my 
> patch been broken by another backport ahead of it and is now stale? 
> Very useful to know. It would be nice to be told “your patch is stale” 
> in CI rather than finding that out when the backport is applied.

You get this in Github PRs at least if mergeability test fails.

Regards, Joe



Re: backports

2022-03-08 Thread Joe Orton
On Fri, Mar 04, 2022 at 09:40:49AM -0800, Roy T. Fielding wrote:
> > On Mar 4, 2022, at 6:17 AM, Eric Covener  wrote:
> > 
> > On Fri, Mar 4, 2022 at 9:05 AM Jim Jagielski  wrote:
> >> 
> >> A question: Would it be easier for all this if we moved to being Github 
> >> canon?
> > 
> > I think it is much more straightforward.  The original work, reviews
> > and travis results are all in the same place and nothing is copied
> > around.
> > I have had the same thought a few times this week. But I was hesitant
> > to reopen that thread/discussion because I'm pessimistic we can get
> > anywhere on it.
> 
> I think we are far beyond that point where staying with svn/bugzilla is 
> actively
> hurting the project for little or no benefit.
> 
> I'd +1 a switch just to get real issue management and PRs.

+1 for switching to Github.

Regards, Joe



Re: svn commit: r1898566 - in /httpd/httpd/branches/2.4.x: ./ modules/aaa/ modules/cache/ modules/dav/fs/ modules/dav/lock/ modules/mappers/ modules/proxy/

2022-03-08 Thread Joe Orton
On Fri, Mar 04, 2022 at 09:24:37AM +0100, Stefan Eissing wrote:
> > Am 04.03.2022 um 08:32 schrieb Ruediger Pluem :
> > On 3/3/22 5:40 PM, Joe Orton wrote:
> >> Oh, good question.  I'm not sure how the "branch" variable appears in an 
> >> arbitrary branch but it's possible we'd need to tweak the conditions 
> >> again, yes.  If we used a naming rule of "branches/2.4.x-*" for 2.4.x 
> >> backports would that be reasonable?  This is most common from examples
> > 
> > Sounds reasonable, but given that for candidates we use candidate-2.4.x we 
> > should change this to 2.4.x-candidate if we set a
> > naming convention of branches/2.4.x-*.
> 
> I can change that easily, but the pattern be better: branches/2.4.* 
> since the candidate carries the to be released version, not 2.4.x (the 
> name branches/2.4.x-candidate-2.4.54 is silly and I refuse to go there 
> -.-)

Sounds good to me.  I've changed the conditions in r1898671 to treat 
anything matching "^2.4" like 2.4, matching "^candidate-2.4" is also 
retained for now.

Regards, Joe



Re: svn commit: r1898566 - in /httpd/httpd/branches/2.4.x: ./ modules/aaa/ modules/cache/ modules/dav/fs/ modules/dav/lock/ modules/mappers/ modules/proxy/

2022-03-03 Thread Joe Orton
On Thu, Mar 03, 2022 at 05:11:52PM +0100, Ruediger Pluem wrote:
> On 3/3/22 4:49 PM, Joe Orton wrote:
> > Folks (in no way pointing a finger at Jim who just did merging duty), it 
> > is not hard to test your backport proposals, either in an SVN branch or 
> > a github PR if you want better testing coverage before you submit for 
> > review.
> 
> A quick question on this. If I branch 2.4.x
> 
> 1. Travis will run at all (because their is a .travis.yml in that branch)?

Yup, Travis will definitely run for all branches, e.g. it works for the 
candidate-2.4.x branches:

https://app.travis-ci.com/github/apache/httpd/branches

> 2. But the conditions in .travis.yml will likely not cause travis to run the 
> same tests as for 2.4.x, but likely the trunk ones,
>correct? Hence we need adjusted conditions in .travis.yml and we need to 
> define some kind of naming rules for branches from
>trunk and 2.4.x to ensure that the correct tests and builds are running?

Oh, good question.  I'm not sure how the "branch" variable appears in an 
arbitrary branch but it's possible we'd need to tweak the conditions 
again, yes.  If we used a naming rule of "branches/2.4.x-*" for 2.4.x 
backports would that be reasonable?  This is most common from examples 
at https://svn.apache.org/repos/asf/httpd/httpd/branches/ right now.

Regards, Joe



Re: svn commit: r1898566 - in /httpd/httpd/branches/2.4.x: ./ modules/aaa/ modules/cache/ modules/dav/fs/ modules/dav/lock/ modules/mappers/ modules/proxy/

2022-03-03 Thread Joe Orton
On Thu, Mar 03, 2022 at 01:30:47PM -, j...@apache.org wrote:
> Author: jim
> Date: Thu Mar  3 13:30:46 2022
> New Revision: 1898566
> 
> URL: http://svn.apache.org/viewvc?rev=1898566=rev
> Log:
> dbm backport approved and merged

This has broken the CI with several new warnings and empty APLOGNO()

https://app.travis-ci.com/github/apache/httpd/builds/247346699

Folks (in no way pointing a finger at Jim who just did merging duty), it 
is not hard to test your backport proposals, either in an SVN branch or 
a github PR if you want better testing coverage before you submit for 
review.

Regards, Joe



Re: Fwd: FYI: change to travis-ci emailer could cause moderation headaches

2022-01-26 Thread Joe Orton
On Wed, Jan 26, 2022 at 08:58:14AM -0500, Eric Covener wrote:
> I noticed I stopped getting "Travis CI" emails for httpd around 10/21.
> But I see people still discussing CI failures, so I am a little
> confused. Maybe they are only seeing it in the context of PRs.
> 
> Did we lose notifications to dev@ as a result of the below? Or something else?

Yup, it's apparently exactly that.  I filed: 
https://issues.apache.org/jira/browse/INFRA-22619 a while ago

Does anyone know anybody at Travis so we could escalate this?  It seems 
like having them configure the envelope sender properly would be the 
best fix, or at least, easiest for us.

Regards, Joe



Re: svn commit: r1897156 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number docs/manual/mod/core.xml modules/dav/main/mod_dav.c server/core.c

2022-01-18 Thread Joe Orton
On Mon, Jan 17, 2022 at 04:10:51PM -, minf...@apache.org wrote:
> Author: minfrin
> Date: Mon Jan 17 16:10:51 2022
> New Revision: 1897156
> 
> URL: http://svn.apache.org/viewvc?rev=1897156=rev
> Log:
> core: Allow an optional expression to be specified for an effective
> path in the DirectoryMatch and LocationMatch directives. This allows
> modules like mod_dav to map URLs to URL spaces or to directories on
> the filesystem.

https://app.travis-ci.com/github/apache/httpd/jobs/555883817#L2039

This has introduced new warnings:

In file included from mod_dav.c:51:
mod_dav.c: In function ‘uripath_is_canonical’:
mod_dav.c:774:38: error: passing argument 1 of ‘ap_strchr’ discards ‘const’ 
qualifier from pointer target type [-Werror=discarded-qualifiers]
  774 | dot_pos = strchr(dot_pos + 1, '.')) {
  |  ^~~
/home/travis/build/apache/httpd/include/httpd.h:2469:34: note: in definition of 
macro ‘strchr’
 2469 | # define strchr(s, c)  ap_strchr(s,c)
  |  ^
/home/travis/build/apache/httpd/include/httpd.h:2457:36: note: expected ‘char 
*’ but argument is of type ‘const char *’
 2457 | AP_DECLARE(char *) ap_strchr(char *s, int c);
  |  ~~^



Re: TLS neverbleed design

2022-01-17 Thread Joe Orton
On Wed, Jan 12, 2022 at 03:37:05PM +0100, Stefan Eissing wrote:
> My current, rough idea would be:
> 
> - fork a process, like mod_cgid does, that can be communicated
>   with over a unix domain socket
> - have an ap_hook to load a key and return an opaque key handle
> - have an ap_hooks to sign/encrypt/decrypt data for a key handle
>
> 
> For mod_ssl that would mean:
> - use the hook on loading a key file
>   - if no handle returned, proceed as now, tell SSL_CTX to load the file
>   - on handle, construct a EVP_PKEY that proxies its methods to the
> new hooks

You can do this already with PKCS#11 and it's already supported in 
mod_ssl, reinventing that wheel to add another API 

> Example of how this is done at 
> https://github.com/h2o/neverbleed/blob/master/neverbleed.c
> which AFAICT is used in production at Fastly.
> 
> If we implement this in a new module, that would become usable with
> an additional On|Off directive and no changes to SSL configs.
> 
> > You should be able to deploy something like this with PKCS#11, e.g. 
> > softhsm, p11-kit can proxy PKCS#11 over a Unix domain socket, there are 
> > probably more solutions out there.
> 
> With the proposed hooks interface, one could do an implementation using soft 
> or
> hard hsms. But that would require changing mod_ssl configs as then the
> configuration would need to specify keys by an id other than file names. etc. 
> etc.

Reinventing PKCS#11 as an ap_* level API is hardly without cost, though, 
so I wouldn't wave that away against some "etc etc" costs that users 
would need to tweak configs for.

When I look at this problem from 10,000ft I see two parts:

a) daemon which loads keys and offers key operations over an AF_UNIX 
interface

b) interface to (a) from SSL_* layer

Importantly, both of these seem equally useful in e.g. exim as they are 
in httpd.  You get (b) from supporting PKCS#11 which is already done in 
mod_ssl/OpenSSL.  I don't know how much you can get (a) in a convenient 
way for users, but as a project it mostly orthogonal to httpd except for 
some startup integration stuff.

Regards, Joe



Re: svn commit: r1896976 - /httpd/httpd/branches/2.4.x/STATUS

2022-01-17 Thread Joe Orton
On Sun, Jan 16, 2022 at 03:35:15PM -0600, William A Rowe Jr wrote:
> 4 day ago, you all saw this. 6 years ago, you all started using this on trunk.
> 
> Don't know what I can to do help this along and honor the library 
> author's wishes for all of us to walk away from the dead fork, and use 
> the maintained fork. It's whatever it is, I'm out of here and removing 
> the backport application branch, whoever 3rd upvotes this be prepared 
> to apply this for us all, thanks.

I'm fine with PCRE 10.x as a trunk/2.5 feature.  PCRE upstream have 
maintained 8.x better than e.g. zlib upstream have done in recent years 
(last zlib release in 2017).  So I don't find the fact it's considered 
EOL upstream presents any particular urgency, it's still supported 
downstream on platforms people deploy to.

For 2.4.x I would argue it's better to keep a preference for 8.x over 
10.x so that users aren't switched from one to the other across an 
upgrade - with some new performance trade-off we know about - without 
changing the environment/configure line?

Regards, Joe



Re: Possible apr 1.7.1 showstopper from httpd test framework

2022-01-14 Thread Joe Orton
On Fri, Jan 14, 2022 at 11:37:35AM +0100, Ruediger Pluem wrote:
> 
> 
> On 1/14/22 6:47 AM, William A Rowe Jr wrote:
> > In addition to a broken release of brotli (where enc/dec don't specify
> > -lbrotlicommon,
> > even on trunk, for openssl and other consumers to ferret out that binding), 
> > and
> > lots of fun changes to build flags in curl 7.81 minor release (who does 
> > that?)
> > there appears to be one test failure with date formatting in httpd 2.4.x 
> > branch
> > (including release 2.4.52) and apr 1.7.x branch (including release 1.7.0);
> > 
> > t/modules/include.t . 56/98 # Failed test 64 in
> > t/modules/include.t at line 373
> > 
> > Have not had time to investigate whether this is a change in perl behavior, 
> > or
> > possibly a regression caused by apr datetime handling in 1.7.x itself., but 
> > any
> > release apr-side should hold off just a bit to resolve this question.
> 
> I cannot reproduce this with APR 1.7.x on RedHat 8 and our Travis builds at 
> least for some builds
> on Ubuntu use APR 1.7 as well and do not fail.
> Is this probably a Windows specific issue? Can anyone reproduce on Windows?

IIRC there is some test case which can be sensitive to filesystems, and 
e.g. sometimes fails if NFS mounted?  I may be mixing it up with another 
test. The output of "./t/TEST -v t/modules/include.t" should help 
diagnose.

That phrase "including release 1.7.0" implies it's not a 1.7.x 
regression if it also failed with 1.7.0, Bill? i.e. no reason to hold up 
a release?

Regards, Joe



Re: ocsp.t failing

2022-01-12 Thread Joe Orton
On Wed, Jan 05, 2022 at 12:11:50PM +, Joe Orton wrote:
> On Sat, Dec 25, 2021 at 10:56:59AM +0100, Christophe JAILLET wrote:
> > Hi,
> > 
> > the last travis jobs have failed on trunk because of ocsp.t.
> > 
> > Unless I missed something, the only change with previous successful build is
> > r1896361:
> > 
> > https://github.com/apache/httpd/commit/442b4b167f19e13df918402a7af28fe4a50c2730
> > 
> > I can't see any potential link.
> 
> I wonder if this is due to change in the OpenSSL version in Ubuntu?

Appears not :) Big thanks to Yann for fixing it!

Regards, Joe



Re: TLS neverbleed design

2022-01-12 Thread Joe Orton
On Fri, Jan 07, 2022 at 11:34:47AM +0100, Stefan Eissing wrote:
> A nice new year to everyone!
> 
> I was looking at the design of https://github.com/h2o/neverbleed which
> - loads TLS private keys in a separate process
> - creates EVP_PKEY instances that in the sign callback call into the
>   separate process to create the TLS handshake signature
> 
> This is surprisingly simple. With a little overhead, it keeps the keys
> in a separate address space, inaccessible to any exploits in the traffic
> serving workers.
> 
> I wonder if it is worthwhile to add something similar to our server.

It's definitely an interesting idea, though needs a caveat that if a 
compromised worker can do private key operations via the process then 
the key is not truly isolated.  (That's not to say it's a bad idea, just 
that it's a mitigation/defence-in-depth approach.)

You should be able to deploy something like this with PKCS#11, e.g. 
softhsm, p11-kit can proxy PKCS#11 over a Unix domain socket, there are 
probably more solutions out there.

Regards, Joe



Re: ocsp.t failing

2022-01-05 Thread Joe Orton
On Sat, Dec 25, 2021 at 10:56:59AM +0100, Christophe JAILLET wrote:
> Hi,
> 
> the last travis jobs have failed on trunk because of ocsp.t.
> 
> Unless I missed something, the only change with previous successful build is
> r1896361:
> 
> https://github.com/apache/httpd/commit/442b4b167f19e13df918402a7af28fe4a50c2730
> 
> I can't see any potential link.

I wonder if this is due to change in the OpenSSL version in Ubuntu?

The OpenSSL 3 builds have continued passing consistently.

Can anybody reproduce this on an Ubuntu system?  I don't see a failure 
on Fedora 35 with OpenSSL 1.1.1l.

> BTW, ML don't receive anymore travis failures?

I filed this: https://issues.apache.org/jira/browse/INFRA-22619 but not 
sure how to move it forward, infra have marked it waiting on user.

Regards, Joe



Re: [VOTE] Release httpd-2.4.52-rc1 as httpd-2.4.52

2021-12-17 Thread Joe Orton
On Fri, 17 Dec 2021, 10:56 Ruediger Pluem,  wrote:

>
>
> On 12/17/21 10:54 AM, Joe Orton wrote:
> > On Thu, Dec 16, 2021 at 03:03:13PM +0100, Stefan Eissing wrote:
> >> I would like to call a VOTE over the next few days to release
> >> this candidate tarball httpd-2.4.52-rc1 as 2.4.52:
> >> [X] +1: It's not just good, it's good enough!
> >> [ ] +0: Let's have a talk.
> >> [ ] -1: There's trouble in paradise. Here's what's wrong.
> >>
> >> The computed digests of the tarball up for vote are:
> >> sha256:
> 296c74a8adde1a8acd6617b21fc3d19719ff4fa39319b2bdbd898aca4d5df97f
> *httpd-2.4.52-rc1.tar.gz
> >> sha512:
> b9012096d6658f7d34a3c655eac31b39ffd439c11de6f3e6e9f309d55f4186a4fb26134eb97522e416ae8ca10ed008a14e96fa01a3e3105d9e547f72e2dc3bc2
> *httpd-2.4.52-rc1.tar.gz
> >
> > Thanks for RMing, Stefan.
> >
> > +1 for release, passes test suite on Fedora 35, RHEL 8 and 9 Beta.
>
> Out of curiosity: On RHEL 9 Beta it works and passes tests against the OS
> provided OpenSSL 3?
>

To be pedantic I am testing against a RHEL9 development snapshot rather
than "Beta" though they are quite close, but, yup the tests passed.

Regards, Joe

>


Re: [VOTE] Release httpd-2.4.52-rc1 as httpd-2.4.52

2021-12-17 Thread Joe Orton
On Thu, Dec 16, 2021 at 03:03:13PM +0100, Stefan Eissing wrote:
> I would like to call a VOTE over the next few days to release
> this candidate tarball httpd-2.4.52-rc1 as 2.4.52:
> [X] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.
> 
> The computed digests of the tarball up for vote are:
> sha256: 296c74a8adde1a8acd6617b21fc3d19719ff4fa39319b2bdbd898aca4d5df97f 
> *httpd-2.4.52-rc1.tar.gz
> sha512: 
> b9012096d6658f7d34a3c655eac31b39ffd439c11de6f3e6e9f309d55f4186a4fb26134eb97522e416ae8ca10ed008a14e96fa01a3e3105d9e547f72e2dc3bc2
>  *httpd-2.4.52-rc1.tar.gz

Thanks for RMing, Stefan.

+1 for release, passes test suite on Fedora 35, RHEL 8 and 9 Beta.

Regards, Joe



Re: travis setup for 2.4.x

2021-12-14 Thread Joe Orton
On Tue, Dec 14, 2021 at 11:34:24AM +0100, Stefan Eissing wrote:
> How is the relation between the .travis.yml in trunk and 2.4.x? We have 
> checks for jobs in trunk with
> 
> - if: *condition_not_24x
> 
> and those jobs do not appear in the 2.4.x/.travis.yml. Is this a leftover? 
> Are these 2 clearly separate or merged somehow?

The intent was that .travis.yml (and test/travis*) can be kept the same 
in 2.4.x as in trunk, with the "if" conditions then used to limit 
exactly which tests are run in trunk vs 2.4. Then we should write the 
tests for trunk in a way which should always be fine to backport as-is 
to 2.4.

In practice it gets a bit hard to disentangle some changes, and the 2.4 
.travis.yml is a bit stale, so maybe there is a better practice.  e.g. 
we could simply maintain two completely independent .travis.yml files 
and copy and paste stuff from trunk where required.

Not really sure to be honest, welcome any opinions.

(Also, does anybody know why Travis notifications by e-mail are broken?)

Regards, Joe



Re: release vibes?

2021-12-08 Thread Joe Orton
On Mon, Dec 06, 2021 at 11:36:51AM +0100, Stefan Eissing wrote:
> Friends of httpd, how do you feel about a release in the next two weeks?

Sounds good to me. We may have fewer people around to test it, but at 
least trying to get a release out is better than definitely having no 
release IMO.

Regards, Joe



Re: mod_tls as experimental module?

2021-11-24 Thread Joe Orton
Possibly no strong opinions?  +1 from me anyway.

How hard is it going to be to test in Travis?

On Wed, Nov 24, 2021 at 10:46:03AM +0100, ste...@eissing.org wrote:
> Coming back to this. Since there was no feedback on my post: are people
> just too occupied/opposed/not interested?
> 
> Curious,
> Stefan
> 
> > Am 18.11.2021 um 18:48 schrieb ste...@eissing.org:
> > 
> > How would you feel about adding mod_tls 
> > (https://github.com/abetterinternet/mod_tls) as an experimental module to 
> > Apache httpd?
> > 
> > For people who have not followed that development:
> > - it is a TLS 1.2/1.3 implementation based on rustls, 
> > https://github.com/rustls/rustls
> > - the C API is rustls-ffi, found at https://github.com/rustls/rustls-ffi
> > - it is itself written in C, linking all the Rust things from the 
> > rustls-ffi library
> > - it does not bring any Rust into our code base
> > - functionality wise, it is a clear subset of what mod_ssl offers via 
> > openssl
> >  (e.g. no client certificates now and not as tweakable - at least for now)
> > - it can be co-loaded and co-used with mod_ssl on different ports or 
> > front-/backend roles
> > - performance-wise, according to my plain vanilla tests, it is on par with 
> > mod_ssl
> > 
> > The decision to offer it downstream is of course then made by the distros, 
> > as usual with experimental modules. And if and how it is then used rests 
> > with the users. It is an offered alternative for people.
> > 
> > What would be the benefit to the project?
> > - we offer people an alternative. If they feel the memory safety that Rust 
> > offers is important to them, they can do it with Apache httpd for the TLS 
> > stack.
> > - we could see how people react to this and adapt our TLS offering 
> > accordingly. It being experimental, we remain free to change it. Or remove 
> > it again.
> > 
> > Organizational Things:
> > - the development was done by myself
> > - the work was sponsored by the ISRG (https://www.abetterinternet.org), the 
> > org behind Let's Encrypt, as part of they "memory safety" initiative 
> > (https://www.memorysafety.org)
> > 
> > 
> > Feedback appreciated,
> > 
> > Stefan
> > 
> > PS. On a more personal note:
> > The way this project turned out was a clean separation between C and Rust. 
> > The API offered by rustls-ffi is colored by Rust's immutability/borrowed 
> > memory concepts, but there is nothing Rust special the C code needs to do. 
> > It remains C code. It remains in our core competence.
> > 
> > Working with the rustls people has been nice and productive. The only thing 
> > I can report is that they come from the client TLS side and specific server 
> > needs require some explaining. There are things we can offer to them here.
> > 
> > There are a lot of political things going on right now around OpenSSL and I 
> > definitely want to stay out of that. Again, we can offer this without 
> > having to switch ourself. Even better, customers can use this without 
> > needing to switch completely, as it co-exists. I think that fits into the 
> > concepts our server is designed with.
> > 
> > Thanks for your time.
> > 
> 



Re: openssl 3.0 when

2021-11-02 Thread Joe Orton
On Tue, Nov 02, 2021 at 09:23:32AM +0100, ste...@eissing.org wrote:
> 
> > Am 01.11.2021 um 15:24 schrieb Joe Orton :
> > 
> > On Sun, Oct 31, 2021 at 01:35:09PM +0100, ste...@eissing.org wrote:
> >> I would like us to come to an understanding what our roadmap in
> >> regard to OpenSSL 3.0 is. People keep on asking about it.
> >> 
> >> Yesterday, I spent some hours hacking at mod_ssl and mod_md to
> >> get it running. I managed to compile it, but it was not working
> >> reliably. Maybe I took some wrong turns somewhere. My observations
> >> below.
> > 
> > What are you talking about exactly here?  trunk should compile and run 
> > fine already with 3.0 except if you build OpenSSL without deprecated 
> > functions which AFAIK nobody sane will do, or at least, no sane 
> > distributor will do, because the world is not ready.
> 
> I was trying to make it work without deprecated functions. Sorry,
> to have not been more clear. If we regard 3.0 conformance including
> those, then this is a non-issue, aside from actually testing that
> it still works.

IMO at least it's a non-issue in the "short"-ish term. Other opinions 
are available ;)

Maybe a good transition plan would be to drop use of the deprecated 
functions at the same time we drop support for versions < 3.0, to ease 
the pain of having to support both?  Upstream say they will support 
1.1.1 until the late 2023, since OS vendors will support it beyond that 
I'd expect there's consensus here to support it for longer.  Thoughts?

https://www.openssl.org/policies/releasestrat.html

Regards, Joe



Re: openssl 3.0 when

2021-11-01 Thread Joe Orton
On Sun, Oct 31, 2021 at 01:35:09PM +0100, ste...@eissing.org wrote:
> I would like us to come to an understanding what our roadmap in
> regard to OpenSSL 3.0 is. People keep on asking about it.
> 
> Yesterday, I spent some hours hacking at mod_ssl and mod_md to
> get it running. I managed to compile it, but it was not working
> reliably. Maybe I took some wrong turns somewhere. My observations
> below.

What are you talking about exactly here?  trunk should compile and run 
fine already with 3.0 except if you build OpenSSL without deprecated 
functions which AFAIK nobody sane will do, or at least, no sane 
distributor will do, because the world is not ready.

> With my RM hat on, I see the next release in early December. We 
> have some fixes to ship and maybe the new http2 implementation.
> 
> Personally, I do not see a need for OpenSSL 3.0 in that one. But
> if anyone has plans to do it, it would be good to know.

I would still like to get a Travis job testing against 3.0, on my TODO, 
but I don't know of any compatilibity problems not covered in trunk / 
https://github.com/apache/httpd/pull/258 (outside use of deprecated 
functions anyway).

Regards, Joe


> 
> Kind Regards,
> Stefan
> 
> ---
> Observations hacking on OpenSSL 3.0 compatibility:
> 
> - SRP seems to be gone.
> - the ENGINE API seems to be gone
> - RSA*, DH* and friends are no longer wanted.
>   Instead, the PKEY API offers replacements.
> - This affects reading key parameter from files, afaict.
> - Some minor annoyances with BIO_set_callback and
>   ERR_peek_last..
> - I changed EC key generation in mod_md to the new API,
>   but generation failed at runtime. Maybe a minor glitch
>   on my part.
> - The code overall does not become prettier.
> 
> 
> 



Re: svn commit: r1894220 - in /httpd/httpd/trunk: modules/http2/h2_bucket_beam.c test/modules/http2/test_004_post.py test/modules/http2/test_400_push.py test/modules/http2/test_401_early_hints.py

2021-10-14 Thread Joe Orton
On Thu, Oct 14, 2021 at 11:17:11AM +0200, Ruediger Pluem wrote:
> On 10/14/21 10:59 AM, ic...@apache.org wrote:
> > -
> > +
> > +b = APR_BRIGADE_FIRST(bb);
> >  if (APR_BUCKET_IS_METADATA(b)) {
> >  APR_BUCKET_REMOVE(b);
> >  apr_bucket_setaside(b, beam->pool);
> >  H2_BLIST_INSERT_TAIL(>buckets_to_send, b);
> >  *pwritten += (apr_off_t)b->length;
> 
> Are there meta buckets that have a length that is not zero?
> I mean is it even allowed to define meta buckets with a length != 0?

No, definitely not, all metadata buckets must have length = 0.




Re: Ineract with travis? (was: Errored: apache/httpd#1888 (trunk - 47e6ece))

2021-10-12 Thread Joe Orton
On Tue, Oct 12, 2021 at 11:31:51AM +0200, Yann Ylavic wrote:
> Gavin (infra) gave all the httpd-committers team the "triage" access
> right on github, which follows on travis.
> This also includes managing PRs on github, great!

Ah, brilliant.  Thank you Yann and Gavin.

Regards, Joe



Re: Broken: apache/httpd#2037 (trunk - fa7f375)

2021-10-11 Thread Joe Orton
On Mon, Oct 11, 2021 at 11:03:47AM +0100, Joe Orton wrote:
> On Sat, Oct 09, 2021 at 06:06:32PM +0200, Yann Ylavic wrote:
> > On Sat, Oct 9, 2021 at 5:47 PM Travis CI  wrote:
> > >
> > > Build #2037 was broken
> > 
> > Somehow Travis picked up the wrong Ubuntu version??
> > All the Ubuntu Focal builds with system's APR failed with:
> 
> Odd one, I haven't seen this before.  Looks like it must be a Travis 
> issue, I have e-mailed their support@ address.

They said it was a known issue the fixed over the weekend, so we should 
be good now.



Re: Broken: apache/httpd#2037 (trunk - fa7f375)

2021-10-11 Thread Joe Orton
On Sat, Oct 09, 2021 at 06:06:32PM +0200, Yann Ylavic wrote:
> On Sat, Oct 9, 2021 at 5:47 PM Travis CI  wrote:
> >
> > Build #2037 was broken
> 
> Somehow Travis picked up the wrong Ubuntu version??
> All the Ubuntu Focal builds with system's APR failed with:

Odd one, I haven't seen this before.  Looks like it must be a Travis 
issue, I have e-mailed their support@ address.

Regards, Joe



  1   2   3   4   5   6   7   8   9   10   >