Re: WatchdogInterval

[Christophe JAILLET]

Le 21/06/2017 à 14:19, Jim Jagielski a écrit :

This is weird... At least on my system, if I set WatchdogInterval
to *anything*, httpd will refuse to start up. Even if I set it
to the default. It's like the mere existence of the directive
in the config file is enough to stop it.

WTF?


Works for me with 2.4.x and trunk.

CJ

Re: Problems using Apache::Test on Debian (and Ubuntu)

[Joachim Zobel]

Hi.

Just wanted to point out that as a workaround running as root worked for 
me (on debian stretch using cpan2deb).


Sincerely,

Joachim

--
Papier ist gebundenes CO2. Bitte drucken Sie diese EMail aus und archivieren 
Sie sie.

Re: WatchdogInterval

[Noel Butler]

On 21/06/2017 22:19, Jim Jagielski wrote:

> This is weird... At least on my system, if I set WatchdogInterval
> to *anything*, httpd will refuse to start up. Even if I set it
> to the default. It's like the mere existence of the directive
> in the config file is enough to stop it.
> 
> WTF?

I don't use it Jim, but ... enabling it on dev box confirms your finding


wont start, trace is 

0596 write(2, "AH00526: Syntax error on line 42 "..., 71) = 71
10596 write(2, "Duplicate WatchdogInterval direct"..., 54) = 54

-- 
Kind Regards, 

Noel Butler 

This Email, including any attachments, may contain legally 
privileged
information, therefore remains confidential and subject to copyright
protected under international law. You may not disseminate, discuss, or
reveal, any part, to anyone, without the authors express written
authority to do so. If you are not the intended recipient, please notify
the sender then delete all copies of this message including attachments,
immediately. Confidentiality, copyright, and legal privilege are not
waived or lost by reason of the mistaken delivery of this message. Only
PDF [1] and ODF [2] documents accepted, please do not send proprietary
formatted documents 

 

Links:
--
[1] http://www.adobe.com/
[2] http://en.wikipedia.org/wiki/OpenDocument

signature.asc
Description: OpenPGP digital signature

Re: [users@httpd] if directive not being respected in Apache 2.4.6

[Jacob Champion]

On 06/21/2017 09:02 AM, William A Rowe Jr wrote:

++1

EXEC_ON_READ is meaningful to we devs, but not for a general audience.

'Immediate Global' or something similar?  What other 2 or 3 word caption 
works to describe this well for end users researching Define, or 
LoadModule, etc?


"Preprocessor Directive"? "Load-Time Directive"? I'll give it some thought.

--Jacob

Re: buildbot failure in on httpd-trunk

[Jacob Champion]

On 06/21/2017 11:58 AM, Jacob Champion wrote:

On 06/21/2017 11:31 AM, Yann Ylavic wrote:

The "make depend" (after initial ./configure) is missing?

Is dependency generation not part of the default make invocation?


...from testing, it looks like it is not. Seems like we should change 
that, since (I assume) it just bit Jim. And now that I think about it, 
I'm almost certain it's bitten me before.


I generally expect a 'make' invocation to Do the Right Thing (tm), from 
scratch.


--Jacob

Re: buildbot failure in on httpd-trunk

[Jacob Champion]

On 06/21/2017 11:31 AM, Yann Ylavic wrote:

The "make depend" (after initial ./configure) is missing?

Is dependency generation not part of the default make invocation?

--Jacob

Re: buildbot failure in on httpd-trunk

[Yann Ylavic]

On Wed, Jun 21, 2017 at 6:06 PM, Jacob Champion  wrote:
> On 06/21/2017 09:00 AM, build...@apache.org wrote:
>>
>> The Buildbot has detected a new failure on builder httpd-trunk while
>> building . Full details are available at:
>>  https://ci.apache.org/builders/httpd-trunk/builds/682
>
>
> It works! \o/
>
> Now to figure out why it took until the clean rebuild to catch, instead of
> the immediate build after the commit.

The "make depend" (after initial ./configure) is missing?


Regards,
Yann.

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

[Jacob Champion]

On 06/20/2017 02:19 PM, Jacob Champion wrote:
Here's why I'm asking: if I were to propose the attached patch for 
backport, what is the test case that *should* fail but doesn't? 
(proxy_fcgi.t passes, no problem.) And once we get that test case, can 
we show that it actually addresses a valid PHP-FPM use case, or is it a 
feature without a user?


Any thoughts on this?

--Jacob

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

[Jacob Champion]

On 06/20/2017 06:39 PM, Jan Ehrhardt wrote:

BTW: the developers at Directadmin are aware of this bug.
https://forum.directadmin.com/showthread.php?t=54952=2=281618#post281618
I ran into it while updating my Centos 6 servers.


Thanks for the heads up. CC's are coming in on the related bugs in 
Bugzilla as well.


Anyone opposed to a users@ announcement on the issue?

--Jacob

Re: svn commit: r1799375 - /httpd/httpd/trunk/server/util.c

[Jacob Champion]

Reverted while the veto is in effect.

On 06/20/2017 11:08 PM, William A Rowe Jr wrote:

Sorry but I reraise my objection and veto worthless cpu cycles.


Yep, but it's public now, which was my goal.

Background for the uninitiated: CVE-2017-7668 is a buffer overrun caused 
by Bill's patch in the Strict-HTTP backport, which inverted 
test_char_table's classification of the NULL character from "is an HTTP 
token stop" to "is NOT an HTTP token stop". A loop that relied on the 
previous behavior to stop at the end of strings was broken, causing a 
vulnerability.


My patch introduces redundant checks to ensure that even if NULL's 
classification changes in the future, all loops using test_char_table 
are provably correct without code changes, since we've already screwed 
this up once.


(Please understand that my goal here is not to point fingers at you, 
Bill, for the bug. I'm very thankful that you spent as much time as you 
did on the HTTP-strict backport. My goal is to scrutinize how we follow 
up on our mistakes after they're made, to try to make sure they don't 
happen again, and to figure out why a logically correct patch is being 
*vetoed* by you.)


The correct fix to your concern is to document all expected behavior of 
the null but in gen_test_char.c - and in such tests a /* !c && */ 
notation is fine.


That's a personal judgment call, and it deserves the opinion of others 
on the list. You've used a veto to ensure that my preferred solution 
can't even reach trunk and get votes for backport. That is my major 
concern here.


If my patch goes to trunk and gets votes despite your concerns, it seems 
like that should be good enough. If it can't, and your preferred patch 
does get votes, great! At least it was all done fairly.


Due to the way we assemble the code, I'm not convinced it that any 
compiler can optimize away these infrequent cases.


I was under the impression that it was on you, as the one exercising the 
veto, to prove your case technically.


But that's really neither here nor there, because in the end, I'm 
comfortable trading a handful of nanoseconds for provable correctness 
and improved auditability. And I think others should be able to express 
their opinion on the matter *without* the shadow of a veto over the 
alternative that you dislike. Why would anyone else here spend time 
arguing for a patch that's already procedurally DOA?


But there were only two questionable values for \0, and in this case the 
answer is obvious. Invert the rule to a TOKEN char from the rather 
dubious TOKEN_STOP definition. Solved.


Patches welcome.

--Jacob

buildbot success in on httpd-trunk

[buildbot]

The Buildbot has detected a restored build on builder httpd-trunk while 
building . Full details are available at:
https://ci.apache.org/builders/httpd-trunk/builds/683

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: bb_slave6_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'httpd-trunk-on-commit' 
triggered this build
Build Source Stamp: [branch httpd/httpd/trunk] 1799482
Blamelist: dferradal,jim

Build succeeded!

Sincerely,
 -The Buildbot

Re: buildbot failure in on httpd-trunk

[Jacob Champion]

On 06/21/2017 09:12 AM, William A Rowe Jr wrote:
If two commits occur in a short enough period of time, the datestamp of 
the newly refreshed source may be earlier than the .o generated by the 
first update.


The build system won't SVN-update in the middle of a build, and I assume 
the build bots are not set up to use-commit-times (?!) since that would 
break a bunch of stuff. I think this is just an old-fashioned Makefile 
dependency bug.


--Jacob

Re: buildbot failure in on httpd-trunk

[William A Rowe Jr]

If two commits occur in a short enough period of time, the datestamp of the
newly refreshed source may be earlier than the .o generated by the first
update.

On Jun 21, 2017 11:06, "Jacob Champion"  wrote:

> On 06/21/2017 09:00 AM, build...@apache.org wrote:
>
>> The Buildbot has detected a new failure on builder httpd-trunk while
>> building . Full details are available at:
>>  https://ci.apache.org/builders/httpd-trunk/builds/682
>>
>
> It works! \o/
>
> Now to figure out why it took until the clean rebuild to catch, instead of
> the immediate build after the commit.
>
> --Jacob
>

Re: buildbot failure in on httpd-trunk

[Jacob Champion]

On 06/21/2017 09:06 AM, Jacob Champion wrote:
Now to figure out why it took until the clean rebuild to catch, instead 
of the immediate build after the commit.


Oh, there's probably no Makefile dependency on the header, right? Hmm.

--Jacob

Re: buildbot failure in on httpd-trunk

[Jacob Champion]

On 06/21/2017 09:00 AM, build...@apache.org wrote:

The Buildbot has detected a new failure on builder httpd-trunk while building . 
Full details are available at:
 https://ci.apache.org/builders/httpd-trunk/builds/682


It works! \o/

Now to figure out why it took until the clean rebuild to catch, instead 
of the immediate build after the commit.


--Jacob

Re: buildbot failure in on httpd-trunk

[William A Rowe Jr]

mod_proxy_balancer.c: In function 'balancer_handler':
mod_proxy_balancer.c:1144:25: error: 'HCHECK_WATHCHDOG_INTERVAL'
undeclared (first use in this function)
 if (ival >= HCHECK_WATHCHDOG_INTERVAL) {
 ^
mod_proxy_balancer.c:1144:25: note: each undeclared identifier is
reported only once for each function it appears in


On Jun 21, 2017 11:00,  wrote:

> The Buildbot has detected a new failure on builder httpd-trunk while
> building . Full details are available at:
> https://ci.apache.org/builders/httpd-trunk/builds/682
>
> Buildbot URL: https://ci.apache.org/
>
> Buildslave for this Build: bb_slave6_ubuntu
>
> Build Reason: The Nightly scheduler named 'httpd-trunk-nightly-clean'
> triggered this build
> Build Source Stamp: [branch httpd/httpd/trunk] HEAD
> Blamelist:
>
> BUILD FAILED: failed compile
>
> Sincerely,
>  -The Buildbot
>
>
>
>

buildbot failure in on httpd-trunk

[buildbot]

The Buildbot has detected a new failure on builder httpd-trunk while building . 
Full details are available at:
https://ci.apache.org/builders/httpd-trunk/builds/682

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: bb_slave6_ubuntu

Build Reason: The Nightly scheduler named 'httpd-trunk-nightly-clean' triggered 
this build
Build Source Stamp: [branch httpd/httpd/trunk] HEAD
Blamelist: 

BUILD FAILED: failed compile

Sincerely,
 -The Buildbot

Re: svn commit: r1799356 - in /httpd/httpd/branches/2.2.x: ./ server/scoreboard.c

[Jacob Champion]

On 06/21/2017 01:38 AM, Yann Ylavic wrote:

On Wed, Jun 21, 2017 at 10:19 AM, Joe Orton  wrote:

I believe many people find the "if constant CMP variable" style
irritating.  My internal parser has to reset after reading that because
I assume the LHS is variable on first parse, so I have to mentally flip
the logic to understand it.  I don't know why really, it's stupid.

The double layer of redundant parentheses are also annoying and force me
to re-parse the statement to try to work out why they might be needed.


+1, same brain issues here :)


Fair enough. :D

--Jacob

Re: Timeouts and other time-related granularity

[Eric Covener]

On Wed, Jun 21, 2017 at 10:19 AM, Jim Jagielski  wrote:
> It would be useful to mark those directives supporting the "extended
> time format" as such, but we don't have that format documented
> anyplace...
>
> As someone who has limited docs-sense, what is the best way
> to do that. Should we create some entry somewhere in our manual
> that specifies that format and then, in the directive's syntax
> field, link to that format?

I would suggest writing it up once in
http://httpd.apache.org/docs/2.4/mod/directive-dict.html then
appending a brief paragraph to each directive that uses it w/ a
prominent link.

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

Re: Timeouts and other time-related granularity

[Jim Jagielski]

Recommendations on how to do it better welcomed!

Re: Timeouts and other time-related granularity

[Jim Jagielski]

It would be useful to mark those directives supporting the "extended
time format" as such, but we don't have that format documented
anyplace...

As someone who has limited docs-sense, what is the best way
to do that. Should we create some entry somewhere in our manual
that specifies that format and then, in the directive's syntax
field, link to that format?

Re: Timeouts and other time-related granularity

[Jim Jagielski]

Last I checked, these already are.

> On Jun 21, 2017, at 9:36 AM, Stefan Eissing  
> wrote:
> 
> s->timeout
> s->keep_alive_timeout
> 
> ?
> 
>> Am 21.06.2017 um 15:34 schrieb Jim Jagielski :
>> 
>> I've started making some of the more obvious changes to
>> support sub-single-second values for various modules... 1st
>> was the watchdog and hcheck modules.
>> 
>> It does seem like "everyplace" we have timeouts, we should take
>> full advantage of the finer granularity we store anyway. Looking
>> at reqtimeout and that seems ripe as well. But that seems to
>> require some struct field changes, so I might hold off and
>> look for lower hanging fruit.
> 

Re: svn commit: r1799375 - /httpd/httpd/trunk/server/util.c

[William A Rowe Jr]

On Wed, Jun 21, 2017 at 1:08 AM, William A Rowe Jr  wrote:
>
> But there were only two questionable values for \0, and in this case the
> answer is obvious. Invert the rule to a TOKEN char from the rather dubious
> TOKEN_STOP definition. Solved.

... for trunk, IMO. I don't suggest we touch this code again on
2.4 branch. For trunk, when I encounter the concept of a STOP
I envision there is a set of defined delimiters (e.g. punctuation/ctrls)
which serve as terminators. What HTTP defined was the set of
token characters in the bnf. T_HTTP_NOT_TOKEN would have
been a better name for the existing rule. T_HTTP_TOKEN is
simply a better understood rule.

On to whether element [NUL] tests aught to be trustable or
unused; My research (unsure if this was shared earlier) starts
with tests of isfn('\0');

isalnum  false
isalpha  false
iscntrl  true
isdigit  false
isgraph  false
islower  false
isprint  false
ispunct  false
isspace  false
isupper  false
isxdigit false
isascii  true
isblank  false

In each case above, the isfn('\0') results are plainly obvious, right
down to iscntrl(NUL)'s and isascii(NUL)'s result. Our resulting
test_char table values include element [NUL] as well, and it needs
to be sensible for the many purposes...

Our declarations;

T_ESCAPE_SHELL_CMD false (correct, \0 is EOS, not a char)
T_ESCAPE_PATH_SEGMENT  false (correct, \0 is EOS, not a char)
T_OS_ESCAPE_PATH   false (correct, \0 is EOS, not a char)
T_HTTP_TOKEN_STOP  true (correct, \0 is a non-token byte)
T_ESCAPE_LOGITEM   false (correct, \0 is EOS not a char)
T_ESCAPE_FORENSIC  true (interesting/debatable)
T_ESCAPE_URLENCODEDfalse (correct, \0 is EOS not a char)
T_HTTP_CTRLS   true (correct, \0 is NUL, non VCHAR)
T_VCHAR_OBSTEXTfalse (correct, \0 is NUL < 0x80)

In the _ESCAPE's we are escaping a string, and NUL's terminate
a string and are not escaped. Any NUL byte traveling the wire
needs to be evaluated by the appropriate TOKEN, CTRLS or
OBSTEXT rule by HTTP RFC.

The T_ESCAPE_FORENSIC case inverts the general rule for
command lines and paths we are constructing, but as a symbol
over the wire, I can agree with the idea of logging an escaped \0
in the case of a NUL input byte. This differs from logging in that
our logging API has no counted-string semantic (if it did, I guess
we would encrypt \0's.)

Re: Timeouts and other time-related granularity

[Stefan Eissing]

s->timeout
s->keep_alive_timeout

?

> Am 21.06.2017 um 15:34 schrieb Jim Jagielski :
> 
> I've started making some of the more obvious changes to
> support sub-single-second values for various modules... 1st
> was the watchdog and hcheck modules.
> 
> It does seem like "everyplace" we have timeouts, we should take
> full advantage of the finer granularity we store anyway. Looking
> at reqtimeout and that seems ripe as well. But that seems to
> require some struct field changes, so I might hold off and
> look for lower hanging fruit.

Re: Timeouts and other time-related granularity

[Jim Jagielski]

I've started making some of the more obvious changes to
support sub-single-second values for various modules... 1st
was the watchdog and hcheck modules.

It does seem like "everyplace" we have timeouts, we should take
full advantage of the finer granularity we store anyway. Looking
at reqtimeout and that seems ripe as well. But that seems to
require some struct field changes, so I might hold off and
look for lower hanging fruit.

Re: WatchdogInterval

[Jim Jagielski]

Hmmm... has to do with wd_interval_set handling.

No idea why the "error" isn't reported but that's what it
is.

> On Jun 21, 2017, at 8:19 AM, Jim Jagielski  wrote:
> 
> This is weird... At least on my system, if I set WatchdogInterval
> to *anything*, httpd will refuse to start up. Even if I set it
> to the default. It's like the mere existence of the directive
> in the config file is enough to stop it.
> 
> WTF?

WatchdogInterval

[Jim Jagielski]

This is weird... At least on my system, if I set WatchdogInterval
to *anything*, httpd will refuse to start up. Even if I set it
to the default. It's like the mere existence of the directive
in the config file is enough to stop it.

WTF?

Re: svn commit: r1799356 - in /httpd/httpd/branches/2.2.x: ./ server/scoreboard.c

[Yann Ylavic]

On Wed, Jun 21, 2017 at 10:19 AM, Joe Orton  wrote:
> On Tue, Jun 20, 2017 at 10:00:35AM -0700, Jacob Champion wrote:
>> On 06/20/2017 09:47 AM, wr...@apache.org wrote:
>> > Log:
>> > Make the range test legible
>> Hmm, out of curiosity, is the legibility you mention from the
>> parenthesization change or the switch to greater-than-or-equal for one side?
>>
>> 
>>
>> I kind of like reading code that has all less-than comparisons, instead of
>> mixed less-than and greater-than, because it means the logic is closer to
>> the mathematics and the number line. For example,
>
> I believe many people find the "if constant CMP variable" style
> irritating.  My internal parser has to reset after reading that because
> I assume the LHS is variable on first parse, so I have to mentally flip
> the logic to understand it.  I don't know why really, it's stupid.
>
> The double layer of redundant parentheses are also annoying and force me
> to re-parse the statement to try to work out why they might be needed.

+1, same brain issues here :)


Regards,
Yann.

Re: svn commit: r1799356 - in /httpd/httpd/branches/2.2.x: ./ server/scoreboard.c

[Joe Orton]

On Tue, Jun 20, 2017 at 10:00:35AM -0700, Jacob Champion wrote:
> On 06/20/2017 09:47 AM, wr...@apache.org wrote:
> > Log:
> > Make the range test legible
> Hmm, out of curiosity, is the legibility you mention from the
> parenthesization change or the switch to greater-than-or-equal for one side?
> 
> 
> 
> I kind of like reading code that has all less-than comparisons, instead of
> mixed less-than and greater-than, because it means the logic is closer to
> the mathematics and the number line. For example,

I believe many people find the "if constant CMP variable" style 
irritating.  My internal parser has to reset after reading that because 
I assume the LHS is variable on first parse, so I have to mentally flip 
the logic to understand it.  I don't know why really, it's stupid.

The double layer of redundant parentheses are also annoying and force me 
to re-parse the statement to try to work out why they might be needed.

Regards, Joe

Re: [PATCH 2.2] fix ap_get_scoreboard_*

[Joe Orton]

On Tue, Jun 20, 2017 at 11:48:53AM -0500, William A Rowe Jr wrote:
> Joe, I compromised on your fix and retained parens for legibility,
> following the pattern of the other fix.
> 
> Committed as r1799356, thanks

Thanks a lot Bill!  I will survive the extra parentheses... :)

Re: svn commit: r1799375 - /httpd/httpd/trunk/server/util.c

[William A Rowe Jr]

Sorry but I reraise my objection and veto worthless cpu cycles.

The correct fix to your concern is to document all expected behavior of the
null but in gen_test_char.c - and in such tests a /* !c && */ notation is
fine.

Due to the way we assemble the code, I'm not convinced it that any compiler
can optimize away these infrequent cases.

But there were only two questionable values for \0, and in this case the
answer is obvious. Invert the rule to a TOKEN char from the rather dubious
TOKEN_STOP definition. Solved.

On Jun 20, 2017 18:08,  wrote:

> Author: jchampion
> Date: Tue Jun 20 23:08:19 2017
> New Revision: 1799375
>
> URL: http://svn.apache.org/viewvc?rev=1799375=rev
> Log:
> util.c: ensure all TEST_CHAR loops stop at the null terminator
>
> In the aftermath of CVE-2017-7668, decouple the business logic ("is NULL
> a T_HTTP_CTRL") from the postcondition ("must not go past the end of the
> string"). The NULL-byte classification in the TEST_CHAR table may change
> in the future.
>
> Modified:
> httpd/httpd/trunk/server/util.c
>
> Modified: httpd/httpd/trunk/server/util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.
> c?rev=1799375=1799374=1799375=diff
> 
> ==
> --- httpd/httpd/trunk/server/util.c (original)
> +++ httpd/httpd/trunk/server/util.c Tue Jun 20 23:08:19 2017
> @@ -1526,7 +1526,7 @@ AP_DECLARE(const char *) ap_parse_token_
>  while (!string_end) {
>  const unsigned char c = (unsigned char)*cur;
>
> -if (!TEST_CHAR(c, T_HTTP_TOKEN_STOP)) {
> +if (c && !TEST_CHAR(c, T_HTTP_TOKEN_STOP)) {
>  /* Non-separator character; we are finished with leading
>   * whitespace. We must never have encountered any trailing
>   * whitespace before the delimiter (comma) */
> @@ -1600,7 +1600,7 @@ AP_DECLARE(const char *) ap_parse_token_
>   */
>  AP_DECLARE(const char *) ap_scan_http_field_content(const char *ptr)
>  {
> -for ( ; !TEST_CHAR(*ptr, T_HTTP_CTRLS); ++ptr) ;
> +for ( ; *ptr && !TEST_CHAR(*ptr, T_HTTP_CTRLS); ++ptr) ;
>
>  return ptr;
>  }
> @@ -1610,7 +1610,7 @@ AP_DECLARE(const char *) ap_scan_http_fi
>   */
>  AP_DECLARE(const char *) ap_scan_http_token(const char *ptr)
>  {
> -for ( ; !TEST_CHAR(*ptr, T_HTTP_TOKEN_STOP); ++ptr) ;
> +for ( ; *ptr && !TEST_CHAR(*ptr, T_HTTP_TOKEN_STOP); ++ptr) ;
>
>  return ptr;
>  }
> @@ -1620,7 +1620,7 @@ AP_DECLARE(const char *) ap_scan_http_to
>   */
>  AP_DECLARE(const char *) ap_scan_vchar_obstext(const char *ptr)
>  {
> -for ( ; TEST_CHAR(*ptr, T_VCHAR_OBSTEXT); ++ptr) ;
> +for ( ; *ptr && TEST_CHAR(*ptr, T_VCHAR_OBSTEXT); ++ptr) ;
>
>  return ptr;
>  }
>
>
>