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

[Ruediger Pluem]

On 7/1/24 3:01 PM, Ruediger Pluem wrote:
> 
> 
> On 6/27/24 3:48 PM, Ruediger Pluem wrote:
>>
>>
>> On 6/27/24 12:47 PM, Yann Ylavic wrote:
>>> On Thu, Jun 27, 2024 at 12:38 PM Yann Ylavic  wrote:

 On Thu, Jun 27, 2024 at 12:07 PM Ruediger Pluem  wrote:
>
> How about the below? I am yet undecided if I should follow the below 
> approach to have
> two completely separate callbacks depending on the OpenSSL version or one 
> with a lot of
> #If statements in it, but as much shared code as possible. Thoughts?

 Hm, I wouldn't duplicate the whole thing depending on openssl version.

 Something like the attached maybe? modssl_io_cb() will just consider
 up to APR_INT32_MAX bytes, more shouldn't happen anyway and it's more
 than enough for debug logs..
>>>
>>> We could even log the real length still if it matters, see attached v2.
>>
>> Thanks. Unfortunately the meaning of rc varies depending on if we have the 
>> ex or the non ex callback.
>> This is not in the documentation but only in the OpenSSL code :-(.
>> Furthermore the processed amount of data is in *processed in the ex case 
>> whereas in the non ex case it is in
>> rc. The intended amount of data to be processed is in len in the ex case and 
>> in argi in the non ex case.
>> Hence I propose the patch below. Of course we can have a longer debate if 
>> the len parameter to ssl_io_data_dump
>> should be int or size_t :-). And yes dumping more than APR_INT32_MAX bytes 
>> to the log might be bad.
> 
> Any further comments on whether we should limit the dump to APR_INT32_MAX or 
> not? I would guess that it will not matter
> in practice, but I am still open to it.
> BTW: I guess
> 
>  int len = data_len & APR_INT32_MAX;
> 
> would be wrong. It would need to be
> 
>  int len = data_len > APR_INT32_MAX ? APR_INT32_MAX : (int)data_len;
> 
> instead.

Updated patch.

Index: modules/ssl/ssl_engine_io.c
===
--- modules/ssl/ssl_engine_io.c (revision 1918813)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -2308,7 +2308,7 @@
 #define DUMP_WIDTH 16

 static void ssl_io_data_dump(conn_rec *c, server_rec *s,
- const char *b, long len)
+ const char *b, size_t len)
 {
 char buf[256];
 int i, j, rows, trunc, pos;
@@ -2361,7 +2361,7 @@
 }
 if (trunc > 0)
 ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
-"| %04ld - ", len + trunc);
+"| %04" APR_SIZE_T_FMT " - ", len + 
(size_t)trunc);
 ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
 
"+-+");
 }
@@ -2379,9 +2379,9 @@
 conn_rec *c;
 server_rec *s;
 #if OPENSSL_VERSION_NUMBER >= 0x3000L
-(void)len;
-(void)processed;
+(void)argi;
 #endif
+(void)argl;

 if ((ssl = (SSL *)BIO_get_callback_arg(bio)) == NULL)
 return rc;
@@ -2391,7 +2391,20 @@

 if (   cmd == (BIO_CB_WRITE|BIO_CB_RETURN)
 || cmd == (BIO_CB_READ |BIO_CB_RETURN) ) {
+#if OPENSSL_VERSION_NUMBER >= 0x3000L
+#define requested_len (len)
+#define actual_len (*processed)
+/*
+ * On OpenSSL >= 3 rc uses the meaning of the BIO_read_ex and
+ * BIO_write_ex functions return value and not the one of
+ * BIO_read and BIO_write. Hence 0 indicates an error.
+ */
+if (rc > 0) {
+#else
+#define requested_len ((size_t)argi)
+#define actual_len ((size_t)rc)
 if (rc >= 0) {
+#endif
 const char *dump = "";
 if (APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)) {
 if (argp != NULL)
@@ -2400,23 +2413,28 @@
 dump = "(Oops, no memory buffer?)";
 }
 ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
-"%s: %s %ld/%d bytes %s BIO#%pp [mem: %pp] %s",
+"%s: %s %" APR_SIZE_T_FMT "/%" APR_SIZE_T_FMT
+" bytes %s BIO#%pp [mem: %pp] %s",
 MODSSL_LIBRARY_NAME,
 (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" : "read"),
-(long)rc, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? 
"to" : "from"),
+actual_len, requested_len,
+(cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "to" : "from"),
 bio, argp, dump);
 if (*dump != '\0' && argp != NULL)
-ssl_io_data_dump(c, s, argp, rc);
+ssl_io_data_dump(c, s, argp, actual_len);
 }
 else {
 ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
-"%s: I/O error, %d bytes expected to %s on BIO#%pp [mem: 
%pp]",
-MODSSL_LIBRARY_NAME, argi,
+"%s: I/O error, %" APR_SIZE_T_FMT
+" bytes expected to %s on BIO#%pp [mem: %pp]",
+MODSSL_LIBR

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

[Eric Covener]

Looking for a third to do a quick followup release.

On Mon, Jul 1, 2024 at 5:19 PM  wrote:
>
> Author: ylavic
> Date: Mon Jul  1 21:19:05 2024
> New Revision: 1918798
>
> URL: http://svn.apache.org/viewvc?rev=1918798&view=rev
> Log:
> Vote
>
> 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=1918798&r1=1918797&r2=1918798&view=diff
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Mon Jul  1 21:19:05 2024
> @@ -154,7 +154,7 @@ RELEASE SHOWSTOPPERS:
>*) maintain ct trusted flag across ap_internal_fast_redirect
>   Trunk version of patch: https://svn.apache.org/r1918795.
>   2.4.x patch:  svn merge -c 1918795 ^/httpd/httpd/trunk .
> - +1 covener
> + +1 covener, ylavic
>
>  PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
>[ start all new proposals below, under PATCHES PROPOSED. ]
>
>


-- 
Eric Covener
cove...@gmail.com

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

[Ruediger Pluem]

I would be fine if we add:

1918814
1918815
1918823

Unfortunately they cause some conflicts:

modules/dav/main/ms_wdv.c
modules/filters/mod_crypto.c

are not present in 2.4.x an can just be ignored.
The patch to modules/http/byterange_filter.c needs to be slightly modified to:

Index: modules/http/byterange_filter.c
===
--- modules/http/byterange_filter.c (revision 1918813)
+++ modules/http/byterange_filter.c (working copy)
@@ -503,10 +503,10 @@
 /* Is ap_make_content_type required here? */
 const char *orig_ct = ap_make_content_type(r, r->content_type);

-ap_set_content_type(r, apr_pstrcat(r->pool, "multipart",
+ap_set_content_type_ex(r, apr_pstrcat(r->pool, "multipart",
use_range_x(r) ? "/x-" : "/",
"byteranges; boundary=",
-   ap_multipart_boundary, NULL));
+   ap_multipart_boundary, NULL), 1);

 if (orig_ct) {
 bound_head = apr_pstrcat(r->pool,



Regards

Rüdiger

On 7/2/24 1:02 PM, Eric Covener wrote:
> Looking for a third to do a quick followup release.
> 
> On Mon, Jul 1, 2024 at 5:19 PM  wrote:
>>
>> Author: ylavic
>> Date: Mon Jul  1 21:19:05 2024
>> New Revision: 1918798
>>
>> URL: http://svn.apache.org/viewvc?rev=1918798&view=rev
>> Log:
>> Vote
>>
>> 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=1918798&r1=1918797&r2=1918798&view=diff
>> ==
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Mon Jul  1 21:19:05 2024
>> @@ -154,7 +154,7 @@ RELEASE SHOWSTOPPERS:
>>*) maintain ct trusted flag across ap_internal_fast_redirect
>>   Trunk version of patch: https://svn.apache.org/r1918795.
>>   2.4.x patch:  svn merge -c 1918795 ^/httpd/httpd/trunk .
>> - +1 covener
>> + +1 covener, ylavic
>>
>>  PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
>>[ start all new proposals below, under PATCHES PROPOSED. ]
>>
>>
> 
> 

[RFC/PATCH] Avoid redefining apr_filepath_merge() for UNC path checks

[Evgeny Kotkov via dev]

Hi all,

Currently, `httpd.h` redefines the `apr_filepath_merge()` function on Win32
as follows:
```
#ifdef WIN32
#define apr_filepath_merge  ap_filepath_merge
#endif
```

The intent behind this redefinition seems to be to retroactively apply
the recently introduced UNC path checks to any existing calls of
apr_filepath_merge(), including those in third-party module code.

However, this approach has several downsides, because the security-related
behavior of the code is now tied to the inclusion of :

- Utility layers that do not include httpd.h but use apr_filepath_merge()
  will need to include this header to ensure appropriate security behavior.

- Refactorings that involve adding or removing #include  can
  inadvertently change the security properties of the code.

- It becomes challenging to understand the code behavior by inspection alone.

Personally, I find these issues quite significant.

An alternative would be to explicitly use ap_filepath_merge() everywhere and
remove the platform-specific define.  This alternative is demonstrated in the
attached patch:
[[[
* include/httpd.h
  (apr_filepath_merge): Remove this Win32-specific #define, and …

* modules/*, server/*:
  …replace all its usages with ap_filepath_merge().  This ensures explicit and
  consistent security-related behavior that doesn't depend on the inclusion
  of the  header.
]]]

What do you think?


Thanks,
Evgeny Kotkov
Index: include/httpd.h
===
--- include/httpd.h (revision 1918820)
+++ include/httpd.h (working copy)
@@ -2875,10 +2875,6 @@
  apr_int32_t flags,
  apr_pool_t *p);
 
-#ifdef WIN32
-#define apr_filepath_merge  ap_filepath_merge
-#endif
-
 /* Win32/NetWare/OS2 need to check for both forward and back slashes
  * in ap_normalize_path() and ap_escape_url().
  */
Index: modules/arch/win32/mod_isapi.c
===
--- modules/arch/win32/mod_isapi.c  (revision 1918820)
+++ modules/arch/win32/mod_isapi.c  (working copy)
@@ -990,7 +990,7 @@
 
 #ifdef WIN32
 /* We need to make this a real Windows path name */
-apr_filepath_merge(&file, "", file, APR_FILEPATH_NATIVE, r->pool);
+ap_filepath_merge(&file, "", file, APR_FILEPATH_NATIVE, r->pool);
 #endif
 
 *buf_size = apr_cpystrn(buf_data, file, *buf_size) - buf_data;
Index: modules/dav/fs/quota.c
===
--- modules/dav/fs/quota.c  (revision 1918820)
+++ modules/dav/fs/quota.c  (working copy)
@@ -98,10 +98,10 @@
 break;
 
 case APR_DIR:
-rv = apr_filepath_merge(&newpath, path, finfo.name, 0, r->pool);
+rv = ap_filepath_merge(&newpath, path, finfo.name, 0, r->pool);
 if (rv != APR_SUCCESS) {
 ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
-  "apr_filepath_merge \"%s\" \"%s\" failed",
+  "ap_filepath_merge \"%s\" \"%s\" failed",
   path, finfo.name);
 goto out;
 }
Index: modules/filters/mod_include.c
===
--- modules/filters/mod_include.c   (revision 1918820)
+++ modules/filters/mod_include.c   (working copy)
@@ -1697,9 +1697,9 @@
 char *newpath;
 
 /* be safe; only files in this directory or below allowed */
-rv = apr_filepath_merge(&newpath, NULL, tag_val,
-APR_FILEPATH_SECUREROOTTEST |
-APR_FILEPATH_NOTABSOLUTE, r->pool);
+rv = ap_filepath_merge(&newpath, NULL, tag_val,
+   APR_FILEPATH_SECUREROOTTEST |
+   APR_FILEPATH_NOTABSOLUTE, r->pool);
 
 if (rv != APR_SUCCESS) {
 error_fmt = APLOGNO(02668) "unable to access file \"%s\" "
@@ -1834,9 +1834,9 @@
 char *newpath;
 
 /* be safe; only files in this directory or below allowed */
-rv = apr_filepath_merge(&newpath, NULL, parsed_string,
-APR_FILEPATH_SECUREROOTTEST |
-APR_FILEPATH_NOTABSOLUTE, ctx->dpool);
+rv = ap_filepath_merge(&newpath, NULL, parsed_string,
+   APR_FILEPATH_SECUREROOTTEST |
+   APR_FILEPATH_NOTABSOLUTE, ctx->dpool);
 
 if (rv != APR_SUCCESS) {
 error_fmt = "unable to include file \"%s\" in parsed file %s";
Index: modules/lua/mod_lua.c
===
--- modules/lua/mod_lua.c   (revision 1918820)
+++ modules/lua/mod_lua.c   (working copy)
@@ -196,8 +196,8 @@
 
 if (filename) {
 char *file;
-apr

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

[Eric Covener]

updated proposal, reset Yann's vote just in case.

On Tue, Jul 2, 2024 at 7:29 AM Ruediger Pluem  wrote:
>
> I would be fine if we add:
>
> 1918814
> 1918815
> 1918823
>
> Unfortunately they cause some conflicts:
>
> modules/dav/main/ms_wdv.c
> modules/filters/mod_crypto.c
>
> are not present in 2.4.x an can just be ignored.
> The patch to modules/http/byterange_filter.c needs to be slightly modified to:
>
> Index: modules/http/byterange_filter.c
> ===
> --- modules/http/byterange_filter.c (revision 1918813)
> +++ modules/http/byterange_filter.c (working copy)
> @@ -503,10 +503,10 @@
>  /* Is ap_make_content_type required here? */
>  const char *orig_ct = ap_make_content_type(r, r->content_type);
>
> -ap_set_content_type(r, apr_pstrcat(r->pool, "multipart",
> +ap_set_content_type_ex(r, apr_pstrcat(r->pool, "multipart",
> use_range_x(r) ? "/x-" : "/",
> "byteranges; boundary=",
> -   ap_multipart_boundary, NULL));
> +   ap_multipart_boundary, NULL), 1);
>
>  if (orig_ct) {
>  bound_head = apr_pstrcat(r->pool,
>
>
>
> Regards
>
> Rüdiger
>
> On 7/2/24 1:02 PM, Eric Covener wrote:
> > Looking for a third to do a quick followup release.
> >
> > On Mon, Jul 1, 2024 at 5:19 PM  wrote:
> >>
> >> Author: ylavic
> >> Date: Mon Jul  1 21:19:05 2024
> >> New Revision: 1918798
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1918798&view=rev
> >> Log:
> >> Vote
> >>
> >> 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=1918798&r1=1918797&r2=1918798&view=diff
> >> ==
> >> --- httpd/httpd/branches/2.4.x/STATUS (original)
> >> +++ httpd/httpd/branches/2.4.x/STATUS Mon Jul  1 21:19:05 2024
> >> @@ -154,7 +154,7 @@ RELEASE SHOWSTOPPERS:
> >>*) maintain ct trusted flag across ap_internal_fast_redirect
> >>   Trunk version of patch: https://svn.apache.org/r1918795.
> >>   2.4.x patch:  svn merge -c 1918795 ^/httpd/httpd/trunk .
> >> - +1 covener
> >> + +1 covener, ylavic
> >>
> >>  PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
> >>[ start all new proposals below, under PATCHES PROPOSED. ]
> >>
> >>
> >
> >



-- 
Eric Covener
cove...@gmail.com

Re: svn commit: r1918845 - /httpd/httpd/tags/2.4.61-rc1-candidate/

[Rainer Jung]

Are we missing a CHANGES entry for the ct fixes?

[VOTE] Release httpd-2.4.61-rc1 as httpd-2.4.61

[Eric Covener]

Hi all,

Please find below the proposed release tarball and signatures:

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

=== Different from template ===
I would like to call an expedited VOTE (due to regression in 2.4.60)
over the next 1-2 days to release this candidate tarball
httpd-2.4.61-rc1 as 2.4.61:

(sorry)
=== end not from a template

[ ] +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: ccdc02f78ebf615002dbcab19c8dd9e124b99207b6fed4eecce7562e64c647c9
*httpd-2.4.61-rc1.tar.gz
sha512: 
ad5894ff83f06a8a39bedb26e32a4381ac6806ae0c8624c0dbf631781eb59431cef37af0e459dff0f5618628ed06adbee013a7a714447ed1de4542294e62b885
*httpd-2.4.61-rc1.tar.gz

The candidate source is found at

and at .

-- 
Eric Covener
cove...@gmail.com

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

[Rainer Jung]

Small correction below (broken URL) ...

Am 02.07.24 um 15:29 schrieb Eric Covener:

Hi all,

Please find below the proposed release tarball and signatures:

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

=== Different from template ===
I would like to call an expedited VOTE (due to regression in 2.4.60)
over the next 1-2 days to release this candidate tarball
httpd-2.4.61-rc1 as 2.4.61:

(sorry)
=== end not from a template

[ ] +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: ccdc02f78ebf615002dbcab19c8dd9e124b99207b6fed4eecce7562e64c647c9
*httpd-2.4.61-rc1.tar.gz
sha512: 
ad5894ff83f06a8a39bedb26e32a4381ac6806ae0c8624c0dbf631781eb59431cef37af0e459dff0f5618628ed06adbee013a7a714447ed1de4542294e62b885
*httpd-2.4.61-rc1.tar.gz

The candidate source is found at





(strip one "tags")


and at .

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

[Stefan Eissing via dev]

> Am 02.07.2024 um 15:29 schrieb Eric Covener :
> 
> Hi all,
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> === Different from template ===
> I would like to call an expedited VOTE (due to regression in 2.4.60)
> over the next 1-2 days to release this candidate tarball
> httpd-2.4.61-rc1 as 2.4.61:
> 
> (sorry)
> === end not from a template
> 
> [ ] +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, macOS 14.5

Thanks for all the hard work put into this, Eric.

- Stefan

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

[Rainer Jung]

Am 02.07.24 um 15:29 schrieb Eric Covener:

Hi all,

Please find below the proposed release tarball and signatures:

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


No signatures yet ...

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

[Eric Covener]

On Tue, Jul 2, 2024 at 9:47 AM Rainer Jung  wrote:
>
> Am 02.07.24 um 15:29 schrieb Eric Covener:
> > Hi all,
> >
> > Please find below the proposed release tarball and signatures:
> >
> > https://dist.apache.org/repos/dist/dev/httpd/

Thanks, checking on it, it seems like I did not get the usual gpg prompt.

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

[Eric Covener]

On Tue, Jul 2, 2024 at 9:49 AM Eric Covener  wrote:
>
> On Tue, Jul 2, 2024 at 9:47 AM Rainer Jung  wrote:
> >
> > Am 02.07.24 um 15:29 schrieb Eric Covener:
> > > Hi all,
> > >
> > > Please find below the proposed release tarball and signatures:
> > >
> > > https://dist.apache.org/repos/dist/dev/httpd/
>
> Thanks, checking on it, it seems like I did not get the usual gpg prompt.

On the way.



-- 
Eric Covener
cove...@gmail.com

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

[Mario Brandt]

+1 on Debian 12 x64

Eric Covener  schrieb am Di., 2. Juli 2024, 15:30:

> Hi all,
>
> Please find below the proposed release tarball and signatures:
>
> https://dist.apache.org/repos/dist/dev/httpd/
>
> === Different from template ===
> I would like to call an expedited VOTE (due to regression in 2.4.60)
> over the next 1-2 days to release this candidate tarball
> httpd-2.4.61-rc1 as 2.4.61:
>
> (sorry)
> === end not from a template
>
> [ ] +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: ccdc02f78ebf615002dbcab19c8dd9e124b99207b6fed4eecce7562e64c647c9
> *httpd-2.4.61-rc1.tar.gz
> sha512:
> ad5894ff83f06a8a39bedb26e32a4381ac6806ae0c8624c0dbf631781eb59431cef37af0e459dff0f5618628ed06adbee013a7a714447ed1de4542294e62b885
> *httpd-2.4.61-rc1.tar.gz
>
> The candidate source is found at
> <
> https://svn.apache.org/repos/asf/httpd/httpd/tags/tags/2.4.61-rc1-candidate
> >
> and at .
>
> --
> Eric Covener
> cove...@gmail.com
>

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

[Frank Gingras]

On Tue, Jul 2, 2024 at 10:18 AM Mario Brandt  wrote:

> +1 on Debian 12 x64
>
> Eric Covener  schrieb am Di., 2. Juli 2024, 15:30:
>
>> Hi all,
>>
>> Please find below the proposed release tarball and signatures:
>>
>> https://dist.apache.org/repos/dist/dev/httpd/
>>
>> === Different from template ===
>> I would like to call an expedited VOTE (due to regression in 2.4.60)
>> over the next 1-2 days to release this candidate tarball
>> httpd-2.4.61-rc1 as 2.4.61:
>>
>> (sorry)
>> === end not from a template
>>
>> [ ] +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: ccdc02f78ebf615002dbcab19c8dd9e124b99207b6fed4eecce7562e64c647c9
>> *httpd-2.4.61-rc1.tar.gz
>> sha512:
>> ad5894ff83f06a8a39bedb26e32a4381ac6806ae0c8624c0dbf631781eb59431cef37af0e459dff0f5618628ed06adbee013a7a714447ed1de4542294e62b885
>> *httpd-2.4.61-rc1.tar.gz
>>
>> The candidate source is found at
>> <
>> https://svn.apache.org/repos/asf/httpd/httpd/tags/tags/2.4.61-rc1-candidate
>> >
>> and at .
>>
>> --
>> Eric Covener
>> cove...@gmail.com
>
>
+1

A quick test ran fine on Slackware15 here as well; it feels odd to use
AddType for that DSO after all these years.

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

[Eric Covener]

On Tue, Jul 2, 2024 at 9:29 AM Eric Covener  wrote:
>
> Hi all,
>
> Please find below the proposed release tarball and signatures:
>
> https://dist.apache.org/repos/dist/dev/httpd/
>
> === Different from template ===
> I would like to call an expedited VOTE (due to regression in 2.4.60)
> over the next 1-2 days to release this candidate tarball
> httpd-2.4.61-rc1 as 2.4.61:
>
> (sorry)
> === end not from a template
>
> [ ] +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 aix/xlc/ppc64 usual proxy/ssl/perl culprits only.

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

[Steffen]

+1 on Windows. 

Changes for 2.4.61 is empty. 

Eric, Rüdiger, Yann and others, thanks for all the work. 

> Op 2 jul 2024 om 15:30 heeft Eric Covener  het volgende 
> geschreven:
> Hi all,
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> === Different from template ===
> I would like to call an expedited VOTE (due to regression in 2.4.60)
> over the next 1-2 days to release this candidate tarball
> httpd-2.4.61-rc1 as 2.4.61:
> 
> (sorry)
> === end not from a template
> 
> [ ] +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: ccdc02f78ebf615002dbcab19c8dd9e124b99207b6fed4eecce7562e64c647c9
> *httpd-2.4.61-rc1.tar.gz
> sha512: 
> ad5894ff83f06a8a39bedb26e32a4381ac6806ae0c8624c0dbf631781eb59431cef37af0e459dff0f5618628ed06adbee013a7a714447ed1de4542294e62b885
> *httpd-2.4.61-rc1.tar.gz
> 
> The candidate source is found at
> 
> and at .
> 
> --
> Eric Covener
> cove...@gmail.com

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

[giovanni]

On 7/2/24 3:29 PM, Eric Covener wrote:

Hi all,

Please find below the proposed release tarball and signatures:

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

=== Different from template ===
I would like to call an expedited VOTE (due to regression in 2.4.60)
over the next 1-2 days to release this candidate tarball
httpd-2.4.61-rc1 as 2.4.61:

(sorry)
=== end not from a template

[ ] +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: ccdc02f78ebf615002dbcab19c8dd9e124b99207b6fed4eecce7562e64c647c9
*httpd-2.4.61-rc1.tar.gz
sha512: 
ad5894ff83f06a8a39bedb26e32a4381ac6806ae0c8624c0dbf631781eb59431cef37af0e459dff0f5618628ed06adbee013a7a714447ed1de4542294e62b885
*httpd-2.4.61-rc1.tar.gz

The candidate source is found at

and at .


+1 Fedora 40

Thanks for all the hard work !!
 Giovanni


OpenPGP_signature.asc
Description: OpenPGP digital signature

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

[Joe Orton]

On Tue, Jul 02, 2024 at 09:29:53AM -0400, Eric Covener wrote:
> Hi all,
> 
> Please find below the proposed release tarball and signatures:
> 
> https://dist.apache.org/repos/dist/dev/httpd/
> 
> === Different from template ===
> I would like to call an expedited VOTE (due to regression in 2.4.60)
> over the next 1-2 days to release this candidate tarball
> httpd-2.4.61-rc1 as 2.4.61:
> 
> (sorry)
> === end not from a template
> 
> [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.

No apologies necessary! Thanks again Eric.

+1 for release, tests pass on Fedora 40, RHEL8+9, CentOS Stream 10.

Regards, Joe

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

[Eric Covener]

I plan to release in another ~24h barring any bad feedback relative to
2.4.60, if anyone wants closer to the usually 72h for any reason,
please comment.

On Tue, Jul 2, 2024 at 9:29 AM Eric Covener  wrote:
>
> Hi all,
>
> Please find below the proposed release tarball and signatures:
>
> https://dist.apache.org/repos/dist/dev/httpd/
>
> === Different from template ===
> I would like to call an expedited VOTE (due to regression in 2.4.60)
> over the next 1-2 days to release this candidate tarball
> httpd-2.4.61-rc1 as 2.4.61:
>
> (sorry)
> === end not from a template
>
> [ ] +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: ccdc02f78ebf615002dbcab19c8dd9e124b99207b6fed4eecce7562e64c647c9
> *httpd-2.4.61-rc1.tar.gz
> sha512: 
> ad5894ff83f06a8a39bedb26e32a4381ac6806ae0c8624c0dbf631781eb59431cef37af0e459dff0f5618628ed06adbee013a7a714447ed1de4542294e62b885
> *httpd-2.4.61-rc1.tar.gz
>
> The candidate source is found at
> 
> and at .
>
> --
> Eric Covener
> cove...@gmail.com



-- 
Eric Covener
cove...@gmail.com

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

[Yann Ylavic]

On Tue, Jul 2, 2024 at 3:45 PM Eric Covener  wrote:
>
> Hi all,
>
> Please find below the proposed release tarball and signatures:
>
> https://dist.apache.org/repos/dist/dev/httpd/
>
> === Different from template ===
> I would like to call an expedited VOTE (due to regression in 2.4.60)
> over the next 1-2 days to release this candidate tarball
> httpd-2.4.61-rc1 as 2.4.61:
>
> (sorry)
> === end not from a template

[X] +1: It's not just good, it's good enough!

Tested on Debian stable and testing/sid, all passes.

Thanks Eric!

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

[Rainer Jung]

Am 02.07.24 um 15:29 schrieb Eric Covener:

Hi all,

Please find below the proposed release tarball and signatures:

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

=== Different from template ===
I would like to call an expedited VOTE (due to regression in 2.4.60)
over the next 1-2 days to release this candidate tarball
httpd-2.4.61-rc1 as 2.4.61:

(sorry)
=== end not from a template

[ ] +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 and many thanks for the hard work of all who contributed to this 
complex release and especially Eric.


Build and tested on RHEL 6, 7, 8 and 9 plus SLES 11, 12 and 15 plus 
Solaris 10 Sparc using OpenSSL 3.1.6.

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

[Yann Ylavic]

On Tue, Jul 2, 2024 at 10:57 AM Ruediger Pluem  wrote:
>
> On 7/1/24 3:01 PM, Ruediger Pluem wrote:
> >
> >
> > On 6/27/24 3:48 PM, Ruediger Pluem wrote:
> >>
> >>
> >> On 6/27/24 12:47 PM, Yann Ylavic wrote:
> >>> On Thu, Jun 27, 2024 at 12:38 PM Yann Ylavic  wrote:
> 
>  On Thu, Jun 27, 2024 at 12:07 PM Ruediger Pluem  
>  wrote:
> >
> > How about the below? I am yet undecided if I should follow the below 
> > approach to have
> > two completely separate callbacks depending on the OpenSSL version or 
> > one with a lot of
> > #If statements in it, but as much shared code as possible. Thoughts?
> 
>  Hm, I wouldn't duplicate the whole thing depending on openssl version.
> 
>  Something like the attached maybe? modssl_io_cb() will just consider
>  up to APR_INT32_MAX bytes, more shouldn't happen anyway and it's more
>  than enough for debug logs..
> >>>
> >>> We could even log the real length still if it matters, see attached v2.
> >>
> >> Thanks. Unfortunately the meaning of rc varies depending on if we have the 
> >> ex or the non ex callback.
> >> This is not in the documentation but only in the OpenSSL code :-(.
> >> Furthermore the processed amount of data is in *processed in the ex case 
> >> whereas in the non ex case it is in
> >> rc. The intended amount of data to be processed is in len in the ex case 
> >> and in argi in the non ex case.
> >> Hence I propose the patch below. Of course we can have a longer debate if 
> >> the len parameter to ssl_io_data_dump
> >> should be int or size_t :-). And yes dumping more than APR_INT32_MAX bytes 
> >> to the log might be bad.
> >
> > Any further comments on whether we should limit the dump to APR_INT32_MAX 
> > or not? I would guess that it will not matter
> > in practice, but I am still open to it.
> > BTW: I guess
> >
> >  int len = data_len & APR_INT32_MAX;
> >
> > would be wrong. It would need to be
> >
> >  int len = data_len > APR_INT32_MAX ? APR_INT32_MAX : (int)data_len;
> >
> > instead.

Right! Maybe APR_UINT16_MAX is enough, I don't think we should log
more undecipherable text than that :)
IIRC an SSL/TLS payload is 16K (still?) anyway so we shouldn't be
called for more than that (though multiple times)?

>
> Updated patch.
>
> Index: modules/ssl/ssl_engine_io.c
> ===
> --- modules/ssl/ssl_engine_io.c (revision 1918813)
> +++ modules/ssl/ssl_engine_io.c (working copy)
> @@ -2308,7 +2308,7 @@
>  #define DUMP_WIDTH 16
>
>  static void ssl_io_data_dump(conn_rec *c, server_rec *s,
> - const char *b, long len)
> + const char *b, size_t len)
>  {
>  char buf[256];
>  int i, j, rows, trunc, pos;
> @@ -2361,7 +2361,7 @@
>  }
>  if (trunc > 0)
>  ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
> -"| %04ld - ", len + trunc);
> +"| %04" APR_SIZE_T_FMT " - ", len + 
> (size_t)trunc);
>  ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
>  
> "+-+");
>  }
> @@ -2379,9 +2379,9 @@
>  conn_rec *c;
>  server_rec *s;
>  #if OPENSSL_VERSION_NUMBER >= 0x3000L
> -(void)len;
> -(void)processed;
> +(void)argi;
>  #endif
> +(void)argl;
>
>  if ((ssl = (SSL *)BIO_get_callback_arg(bio)) == NULL)
>  return rc;
> @@ -2391,7 +2391,20 @@
>
>  if (   cmd == (BIO_CB_WRITE|BIO_CB_RETURN)
>  || cmd == (BIO_CB_READ |BIO_CB_RETURN) ) {
> +#if OPENSSL_VERSION_NUMBER >= 0x3000L
> +#define requested_len (len)
> +#define actual_len (*processed)
   requested_len = len
> +/*
> + * On OpenSSL >= 3 rc uses the meaning of the BIO_read_ex and
> + * BIO_write_ex functions return value and not the one of
> + * BIO_read and BIO_write. Hence 0 indicates an error.
> + */
> +if (rc > 0) {
> +#else
> +#define requested_len ((size_t)argi)
> +#define actual_len ((size_t)rc)
>  if (rc >= 0) {
> +#endif
Maybe use variables like:
#if OPENSSL_VERSION_NUMBER >= 0x3000L
   apr_size_t requested_len = len;
   int ok = (rc > 0);
#else
   apr_size_t requested_len = argi;
   int ok = (rc >= 0);
#endif
   if (ok) {
#if OPENSSL_VERSION_NUMBER >= 0x3000L
   apr_size_t actual_len = *processed;
#else
   apr_size_t actual_len = rc;
#endif
>  const char *dump = "";
>  if (APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)) {
>  if (argp != NULL)
> @@ -2400,23 +2413,28 @@
>  dump = "(Oops, no memory buffer?)";
>  }
>  ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
> -"%s: %s %ld/%d bytes %s BIO#%pp [mem: %pp] %s",
> +"%s: %s %" APR_SIZE_T_FMT "/%" APR_SIZE_T_FMT
> +" bytes %s BI

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

[Ruediger Pluem]

On 7/3/24 2:59 AM, Yann Ylavic wrote:
> On Tue, Jul 2, 2024 at 10:57 AM Ruediger Pluem  wrote:
>>
>> On 7/1/24 3:01 PM, Ruediger Pluem wrote:
>>>
>>>
>>> On 6/27/24 3:48 PM, Ruediger Pluem wrote:


 On 6/27/24 12:47 PM, Yann Ylavic wrote:
> On Thu, Jun 27, 2024 at 12:38 PM Yann Ylavic  wrote:
>>
>> On Thu, Jun 27, 2024 at 12:07 PM Ruediger Pluem  
>> wrote:
>>>
>>> How about the below? I am yet undecided if I should follow the below 
>>> approach to have
>>> two completely separate callbacks depending on the OpenSSL version or 
>>> one with a lot of
>>> #If statements in it, but as much shared code as possible. Thoughts?
>>
>> Hm, I wouldn't duplicate the whole thing depending on openssl version.
>>
>> Something like the attached maybe? modssl_io_cb() will just consider
>> up to APR_INT32_MAX bytes, more shouldn't happen anyway and it's more
>> than enough for debug logs..
>
> We could even log the real length still if it matters, see attached v2.

 Thanks. Unfortunately the meaning of rc varies depending on if we have the 
 ex or the non ex callback.
 This is not in the documentation but only in the OpenSSL code :-(.
 Furthermore the processed amount of data is in *processed in the ex case 
 whereas in the non ex case it is in
 rc. The intended amount of data to be processed is in len in the ex case 
 and in argi in the non ex case.
 Hence I propose the patch below. Of course we can have a longer debate if 
 the len parameter to ssl_io_data_dump
 should be int or size_t :-). And yes dumping more than APR_INT32_MAX bytes 
 to the log might be bad.
>>>
>>> Any further comments on whether we should limit the dump to APR_INT32_MAX 
>>> or not? I would guess that it will not matter
>>> in practice, but I am still open to it.
>>> BTW: I guess
>>>
>>>  int len = data_len & APR_INT32_MAX;
>>>
>>> would be wrong. It would need to be
>>>
>>>  int len = data_len > APR_INT32_MAX ? APR_INT32_MAX : (int)data_len;
>>>
>>> instead.
> 
> Right! Maybe APR_UINT16_MAX is enough, I don't think we should log
> more undecipherable text than that :)
> IIRC an SSL/TLS payload is 16K (still?) anyway so we shouldn't be
> called for more than that (though multiple times)?
> 
>>
>> Updated patch.
>>
>> Index: modules/ssl/ssl_engine_io.c
>> ===
>> --- modules/ssl/ssl_engine_io.c (revision 1918813)
>> +++ modules/ssl/ssl_engine_io.c (working copy)
>> @@ -2308,7 +2308,7 @@
>>  #define DUMP_WIDTH 16
>>
>>  static void ssl_io_data_dump(conn_rec *c, server_rec *s,
>> - const char *b, long len)
>> + const char *b, size_t len)
>>  {
>>  char buf[256];
>>  int i, j, rows, trunc, pos;
>> @@ -2361,7 +2361,7 @@
>>  }
>>  if (trunc > 0)
>>  ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
>> -"| %04ld - ", len + trunc);
>> +"| %04" APR_SIZE_T_FMT " - ", len + 
>> (size_t)trunc);
>>  ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
>>  
>> "+-+");
>>  }
>> @@ -2379,9 +2379,9 @@
>>  conn_rec *c;
>>  server_rec *s;
>>  #if OPENSSL_VERSION_NUMBER >= 0x3000L
>> -(void)len;
>> -(void)processed;
>> +(void)argi;
>>  #endif
>> +(void)argl;
>>
>>  if ((ssl = (SSL *)BIO_get_callback_arg(bio)) == NULL)
>>  return rc;
>> @@ -2391,7 +2391,20 @@
>>
>>  if (   cmd == (BIO_CB_WRITE|BIO_CB_RETURN)
>>  || cmd == (BIO_CB_READ |BIO_CB_RETURN) ) {
>> +#if OPENSSL_VERSION_NUMBER >= 0x3000L
>> +#define requested_len (len)
>> +#define actual_len (*processed)
>requested_len = len
>> +/*
>> + * On OpenSSL >= 3 rc uses the meaning of the BIO_read_ex and
>> + * BIO_write_ex functions return value and not the one of
>> + * BIO_read and BIO_write. Hence 0 indicates an error.
>> + */
>> +if (rc > 0) {
>> +#else
>> +#define requested_len ((size_t)argi)
>> +#define actual_len ((size_t)rc)
>>  if (rc >= 0) {
>> +#endif
> Maybe use variables like:
> #if OPENSSL_VERSION_NUMBER >= 0x3000L
>apr_size_t requested_len = len;
>int ok = (rc > 0);
> #else
>apr_size_t requested_len = argi;
>int ok = (rc >= 0);
> #endif
>if (ok) {
> #if OPENSSL_VERSION_NUMBER >= 0x3000L
>apr_size_t actual_len = *processed;
> #else
>apr_size_t actual_len = rc;
> #endif

I am fine with this. Maybe my desire to save stack memory by not using further
variables was a bit over the top here.

>>  const char *dump = "";
>>  if (APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)) {
>>  if (argp != NULL)
>> @@ -2400,23 +2413,28 @@
>>  dump = "(Oops, no memory