Proposed, 2 week termination of mod_authnz_ldap component

2016-08-04 Thread William A Rowe Jr
As previously identified and familiar to all who are working
with httpd trunk, httpd has not compiled against apr trunk
in about 6 years.

It seems time to evict the component from httpd core, given
that it is neither supportable or maintainable. The ABI contract
offered by APR, of no external header dependencies, no binary
dependencies, and all the rest were never observed by the
porters or authors.

Made well aware of this, they chose a flamewar over actually
solving the deficiencies, and in the subsequent years, repaired
none of the defects. Offered an entirely workable httpd-specific
solution to resolve the issues, maintainer vetoed it.

It seems overdue to drop this component from core on trunk,
until it actually functions again.

I'll let this dialog run for 2 weeks.

Thoughts?

Bill


Re: HTTP/1.1 strict ruleset

2016-08-04 Thread William A Rowe Jr
On Thu, Aug 4, 2016 at 8:05 PM, William A Rowe Jr 
wrote:

> On Thu, Aug 4, 2016 at 5:21 PM, Roy T. Fielding  wrote:
>
>> On Aug 4, 2016, at 3:02 PM, William A Rowe Jr 
>> wrote:
>>
>> If consensus here agrees that no out-of-spec behavior should be tolerated
>> anymore, I'll jump on board. I'm already in the consensus block that says
>> we should not ship a new major.minor without disallowing all of this
>> garbage.
>>
>> It would be helpful if other PMC members would weigh in yea or nay on
>> dropping out-of-spec behaviors from 2.4 and 2.2 maintenance branches.
>>
>>
>> That would be weird.  One of us is going to create a patch.  That
>> specific patch is
>> going to be voted upon for backport.  If anyone wants to veto it, they
>> are free
>> to do so with justification.
>>
>
> You don't seem to comprehend the idea behind consensus, which is what I'm
> appealing for. You first among them all were perfectly happy to champion
> stupid
> design considerations as 'beyond Bill's authority' as a committer and
> reviewer,
> and sadly while I sat in the chair of httpd and got to eat dirt.
>
> I'm not trying that again, httpd 2.0 does not entirely compile against apr
> 2.0-dev,
> and likely never will due to this stalemate.
>
> I'm entirely willing to invite vetos with my code, if I wasn't I wouldn't
> commit.
> But if you go back through the archives, you will realize several of you
> were
> entirely on the wrong side of problem-solving. If you would personally like
> to invite vetoes, please be our guest.
>

And FWIW you were on record that a veto does not demand a justification,
so let that settle in.


Re: HTTP/1.1 strict ruleset

2016-08-04 Thread William A Rowe Jr
On Thu, Aug 4, 2016 at 5:21 PM, Roy T. Fielding  wrote:

> On Aug 4, 2016, at 3:02 PM, William A Rowe Jr  wrote:
>
> If consensus here agrees that no out-of-spec behavior should be tolerated
> anymore, I'll jump on board. I'm already in the consensus block that says
> we should not ship a new major.minor without disallowing all of this
> garbage.
>
> It would be helpful if other PMC members would weigh in yea or nay on
> dropping out-of-spec behaviors from 2.4 and 2.2 maintenance branches.
>
>
> That would be weird.  One of us is going to create a patch.  That specific
> patch is
> going to be voted upon for backport.  If anyone wants to veto it, they are
> free
> to do so with justification.
>

You don't seem to comprehend the idea behind consensus, which is what I'm
appealing for. You first among them all were perfectly happy to champion
stupid
design considerations as 'beyond Bill's authority' as a committer and
reviewer,
and sadly while I sat in the chair of httpd and got to eat dirt.

I'm not trying that again, httpd 2.0 does not entirely compile against apr
2.0-dev,
and likely never will due to this stalemate.

I'm entirely willing to invite vetos with my code, if I wasn't I wouldn't
commit.
But if you go back through the archives, you will realize several of you
were
entirely on the wrong side of problem-solving. If you would personally like
to invite vetoes, please be our guest.


Re: HTTP/1.1 strict ruleset

2016-08-04 Thread Roy T. Fielding
> On Aug 4, 2016, at 3:02 PM, William A Rowe Jr  wrote:
> 
> On Thu, Aug 4, 2016 at 3:46 PM, Roy T. Fielding  > wrote:
> > On Aug 3, 2016, at 4:33 PM, William A Rowe Jr  > > wrote:
> >
> > So it seems pretty absurd we are coming back to this over
> > three years later, but is there any reason to preserve pre-RFC 2068
> > behaviors? I appreciate that Stefan was trying to avoid harming
> > existing deployment scenarios, but even as I'm about to propose
> > that we backport all of this to 2.4 and 2.2, I have several questions;
> 
> In general, I don't see a need for any "strict" options. The only changes we
> made to parsing in RFC7230 were for the sake of security and known failures
> to interoperate. This is exactly the feature we are supposed to be handling
> automatically on behalf of our users: secure, correct, and interoperable
> handling and generation of HTTP messaging.  We should not need to configure 
> it.
> 
> Note that the MUST requirements in RFC7230 are not optional. We either 
> implement
> them as specified or we are not compliant with HTTP. 
>  
> Understood. And that describes my attitude toward 2.6/3.0 next release.
> 
> We live in an ecosystem with literally hundreds of thousands of legitimate
> custom http clients asking httpd server for datum. Most projects would
> effectively declare their last major.minor release static, and fix the defects
> while doing all enhancement in their next release. This isn't that project.

By that logic, we can't make any changes in minor versions.  I disagree that
we have ever treated versioning in that way.  If a user doesn't like a bug fix,
they don't have to install the new version, or they have the code to unfix it.

> Because httpd fixes and introduces dozens of bugs each major.minor
> subversion release, and we truly agree that we want every user to move
> to the most recently released major.minor, breaking hundreds of these
> applications with *no recourse* in their build or configuration is 
> frustrating.

Leaving existing users in a broken state of non-compliance with the primary
Internet standard we are claiming to implement just because of unsubstantiated
FUD is far more frustrating.  Bugs get fixed. Users choose whether or not to 
install.
If we find a real problem in a deployed client that causes the bug fix to be 
intolerable,
then of course we will need to configure workarounds.  But we are not in that 
place.

> If consensus here agrees that no out-of-spec behavior should be tolerated
> anymore, I'll jump on board. I'm already in the consensus block that says
> we should not ship a new major.minor without disallowing all of this garbage.
> 
> It would be helpful if other PMC members would weigh in yea or nay on
> dropping out-of-spec behaviors from 2.4 and 2.2 maintenance branches. 

That would be weird.  One of us is going to create a patch.  That specific 
patch is
going to be voted upon for backport.  If anyone wants to veto it, they are free
to do so with justification.

Roy



Re: HTTP/1.1 strict ruleset

2016-08-04 Thread Yann Ylavic
On Fri, Aug 5, 2016 at 12:02 AM, William A Rowe Jr  wrote:
>
> It would be helpful if other PMC members would weigh in yea or nay on
> dropping out-of-spec behaviors from 2.4 and 2.2 maintenance branches.

IMHO we should keep an opt *out* strict mode for new errors 400 we
generate such early (no workaround), at least for 2.2, but probably
also for 2.4.
It shouldn't overcomplicate the code anyway.


Re: HTTP/1.1 strict ruleset

2016-08-04 Thread William A Rowe Jr
On Thu, Aug 4, 2016 at 3:46 PM, Roy T. Fielding  wrote:

> > On Aug 3, 2016, at 4:33 PM, William A Rowe Jr 
> wrote:
> >
> > So it seems pretty absurd we are coming back to this over
> > three years later, but is there any reason to preserve pre-RFC 2068
> > behaviors? I appreciate that Stefan was trying to avoid harming
> > existing deployment scenarios, but even as I'm about to propose
> > that we backport all of this to 2.4 and 2.2, I have several questions;
>
> In general, I don't see a need for any "strict" options. The only changes
> we
> made to parsing in RFC7230 were for the sake of security and known failures
> to interoperate. This is exactly the feature we are supposed to be handling
> automatically on behalf of our users: secure, correct, and interoperable
> handling and generation of HTTP messaging.  We should not need to
> configure it.
>
> Note that the MUST requirements in RFC7230 are not optional. We either
> implement
> them as specified or we are not compliant with HTTP.
>

Understood. And that describes my attitude toward 2.6/3.0 next release.

We live in an ecosystem with literally hundreds of thousands of legitimate
custom http clients asking httpd server for datum. Most projects would
effectively declare their last major.minor release static, and fix the
defects
while doing all enhancement in their next release. This isn't that project.

Because httpd fixes and introduces dozens of bugs each major.minor
subversion release, and we truly agree that we want every user to move
to the most recently released major.minor, breaking hundreds of these
applications with *no recourse* in their build or configuration is
frustrating.

If consensus here agrees that no out-of-spec behavior should be tolerated
anymore, I'll jump on board. I'm already in the consensus block that says
we should not ship a new major.minor without disallowing all of this
garbage.

It would be helpful if other PMC members would weigh in yea or nay on
dropping out-of-spec behaviors from 2.4 and 2.2 maintenance branches.


Re: svn commit: r1754579 - /httpd/httpd/trunk/server/gen_test_char.c

2016-08-04 Thread William A Rowe Jr
On Thu, Aug 4, 2016 at 4:29 PM, Yann Ylavic  wrote:

> On Thu, Aug 4, 2016 at 11:10 PM, William A Rowe Jr 
> wrote:
> > On Thu, Aug 4, 2016 at 3:52 PM, Yann Ylavic 
> wrote:
> >>
> >> On Thu, Aug 4, 2016 at 9:33 PM, William A Rowe Jr 
> >> wrote:
> >> >
> >> > It seems correcting the table is the correct way to go, by direct
> >> > observation, and placing great faith that other than 0x15/0x37,
> >> > the discrepancies between ASCII <> EBCDIC C0 mappings do
> >> > not vary widely between EBCDIC mapping choices.
> >>
> >> Maybe to be sure we could compare the current 'ucharmap' with some
> >> result of apr_xlate_conv_byte() for each byte?
> >
> >
> > Perhaps a VALIDATE_TABLE define for the builder, especially when
> > --with-maintainer-mode is given?
>
> Maybe a small temporary main() run once would be enough, it's not as
> if it could change anytime soon, no?
>

Then we may as well build the table at runtime as a register_hooks
callback... which is a possibility.

The downside is that the table would be more difficult to validate, not
less.


Re: svn commit: r1754579 - /httpd/httpd/trunk/server/gen_test_char.c

2016-08-04 Thread Yann Ylavic
On Thu, Aug 4, 2016 at 11:10 PM, William A Rowe Jr  wrote:
> On Thu, Aug 4, 2016 at 3:52 PM, Yann Ylavic  wrote:
>>
>> On Thu, Aug 4, 2016 at 9:33 PM, William A Rowe Jr 
>> wrote:
>> >
>> > It seems correcting the table is the correct way to go, by direct
>> > observation, and placing great faith that other than 0x15/0x37,
>> > the discrepancies between ASCII <> EBCDIC C0 mappings do
>> > not vary widely between EBCDIC mapping choices.
>>
>> Maybe to be sure we could compare the current 'ucharmap' with some
>> result of apr_xlate_conv_byte() for each byte?
>
>
> Perhaps a VALIDATE_TABLE define for the builder, especially when
> --with-maintainer-mode is given?

Maybe a small temporary main() run once would be enough, it's not as
if it could change anytime soon, no?


Re: svn commit: r1754579 - /httpd/httpd/trunk/server/gen_test_char.c

2016-08-04 Thread William A Rowe Jr
On Thu, Aug 4, 2016 at 3:52 PM, Yann Ylavic  wrote:

> On Thu, Aug 4, 2016 at 9:33 PM, William A Rowe Jr 
> wrote:
> >
> > It seems correcting the table is the correct way to go, by direct
> > observation, and placing great faith that other than 0x15/0x37,
> > the discrepancies between ASCII <> EBCDIC C0 mappings do
> > not vary widely between EBCDIC mapping choices.
>
> Maybe to be sure we could compare the current 'ucharmap' with some
> result of apr_xlate_conv_byte() for each byte?
>

Perhaps a VALIDATE_TABLE define for the builder, especially when
--with-maintainer-mode is given?

Without some specific define, invoking actual apr functions, again,
violates the ability to cross-compile.


Re: svn commit: r1754579 - /httpd/httpd/trunk/server/gen_test_char.c

2016-08-04 Thread Yann Ylavic
On Thu, Aug 4, 2016 at 9:33 PM, William A Rowe Jr  wrote:
>
> It seems correcting the table is the correct way to go, by direct
> observation, and placing great faith that other than 0x15/0x37,
> the discrepancies between ASCII <> EBCDIC C0 mappings do
> not vary widely between EBCDIC mapping choices.

Maybe to be sure we could compare the current 'ucharmap' with some
result of apr_xlate_conv_byte() for each byte?


Re: HTTP/1.1 strict ruleset

2016-08-04 Thread Roy T. Fielding
> On Aug 3, 2016, at 4:33 PM, William A Rowe Jr  wrote:
> 
> So it seems pretty absurd we are coming back to this over
> three years later, but is there any reason to preserve pre-RFC 2068
> behaviors? I appreciate that Stefan was trying to avoid harming
> existing deployment scenarios, but even as I'm about to propose
> that we backport all of this to 2.4 and 2.2, I have several questions;

In general, I don't see a need for any "strict" options. The only changes we
made to parsing in RFC7230 were for the sake of security and known failures
to interoperate. This is exactly the feature we are supposed to be handling
automatically on behalf of our users: secure, correct, and interoperable
handling and generation of HTTP messaging.  We should not need to configure it.

Note that the MUST requirements in RFC7230 are not optional. We either implement
them as specified or we are not compliant with HTTP.  So, the specific issues of

https://tools.ietf.org/html/rfc7230#section-3

   A sender MUST NOT send whitespace between the start-line and the
   first header field.  A recipient that receives whitespace between the
   start-line and the first header field MUST either reject the message
   as invalid or consume each whitespace-preceded line without further
   processing of it (i.e., ignore the entire line, along with any
   subsequent lines preceded by whitespace, until a properly formed
   header field is received or the header section is terminated).

   The presence of such whitespace in a request might be an attempt to
   trick a server into ignoring that field or processing the line after
   it as a new request, either of which might result in a security
   vulnerability if other implementations within the request chain
   interpret the same message differently.  Likewise, the presence of
   such whitespace in a response might be ignored by some clients or
   cause others to cease parsing.

and

https://tools.ietf.org/html/rfc7230#section-3.2.4

   No whitespace is allowed between the header field-name and colon.  In
   the past, differences in the handling of such whitespace have led to
   security vulnerabilities in request routing and response handling.  A
   server MUST reject any received request message that contains
   whitespace between a header field-name and colon with a response code
   of 400 (Bad Request).  A proxy MUST remove any such whitespace from a
   response message before forwarding the message downstream.


must be complied with regardless of any "strict" config setting.

Some of those other things under "strict" seem a bit wonky. For example,
changing the Host header field when the incoming request URI is absolute
is fine by default but needs to be a configurable option for gateways.
Trying to validate IPv4/6 vs DNS doesn't work in intranet environments
that use local name servers.  The Location field-value is no longer required
to be absolute ("https://tools.ietf.org/html/rfc7231#section-7.1.2;).

> 1. offer a logging-only option? Why? It seems like a simple
>choice, follow the spec, or don't. If you want to see what's
>going on, Wireshark, Fiddler and dozens of other tools let
>you inspect the conversation.
> 
> 2. leave the default as 'not-strict'? Seems we should most
>strongly recommend that the server observe RFC's 2068,
>2616 and 723x, and not tolerate ancient behavior by default
>unless the admin insists on being foolish.

As far as the Internet is concerned, RFC723x is the new law of the land.
There is no reason to support obsolete RFCs.  No reason at all.  This has
nothing to do with semantic versioning or binary compatibility -- it is
simply doing what the product says it does: serve HTTP.

> 3. retain these legacy faulty behaviors in httpd 2.next/3.0?
>Seems that once we agree on a backport, the ancient
>side of this logic should all just disappear from trunk.
> 
> 4. detail the error to the error log? Again, there are inspection
>tools, but more importantly, no visual user-agent is going
>to send this garbage, and automated requests are going
>to discard the 400 response. Seems we can save a lot of
>code throwing away the details that just don't help, and
>are generally the product of abusive traffic.
> 
> Thoughts?

I think we just need to state in the log the reason for a 400 error. I don't 
like
sending invalid client-provided data back in a response, even when encoded.

Whitespace before the first header field can log a static message.
Whitespace after a field-name could log the field-name (no need to log the
field value). Invalid characters can be noted as "in a field-name" without
further data, or as "in a field-value" with only the field-name logged.

These are all post-error details off the critical path, so I don't buy the CPU
argument.  However, I do think our error handling in protocol.c has become
so verbose that it obscures the rest of the code.  Maybe it would be better if
we just 

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

2016-08-04 Thread Jacob Champion

On 08/04/2016 01:11 PM, William A Rowe Jr wrote:

At our kindest, we would like to let people keep upgrading on the 2.2
or 2.4 branches of httpd for other fixes, without breaking their
deployments.

I'm 100% in favor of recognizing-and-rejecting (and terminating the
connection) for any obs-fold occurrences on 2.6 / 3.0, if that's the
group consensus.


+1 to both.

--Jacob



Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

2016-08-04 Thread William A Rowe Jr
Thanks for the feedback...

On Thu, Aug 4, 2016 at 3:02 PM, Roy T. Fielding  wrote:

> On Aug 3, 2016, at 2:28 PM, William A Rowe Jr  wrote:
>
> So AIUI, the leading SP / TAB whitespace in a field is a no-op (usually
> represented by a single space by convention), and trailing whitespace
> in the field value is a no-op, all leading tabs/spaces (beyond one SP)
> in the obs-fold line is a no-op. Is there any reason to preserve trailing
> spaces before the obs-fold?
>
>
> Not given our implementation.  The buffer efficiency argument is for other
> kinds of parsers that are not reading just one line at a time.
>

And because we are merging line-at-a-time, we have both factors, the cost
of stripping back trailing whitespace before an obs-fold (we should be doing
this at the end of the header field value in any case), v.s. buffer
allocation
for no-op whitespace.

If not, then stripping trailing whitespace from the line prior to obs-fold
> and
> eating all leading whitespace on the obs-fold line will result in a single
> SP
> character, which should be just fine unless spaces were significant within
> a quoted value. The only way for the client to preserve such significant
> spaces would be to place them after the opening quote before the obs-fold.
>
>
> obs-fold is not allowed inside quoted text, so we need not worry about
> messing with such a construct.
>

That solves that, thanks for the clarity.


> Note that obs-fold has been formally deprecated outside of message/http.
> We can remove its handling at any time we are willing to accept the risk
> of strange error reports.  I do not believe it is part of our versioning
> contract.
>

At our kindest, we would like to let people keep upgrading on the 2.2 or
2.4
branches of httpd for other fixes, without breaking their deployments.

I'm 100% in favor of recognizing-and-rejecting (and terminating the
connection)
for any obs-fold occurrences on 2.6 / 3.0, if that's the group consensus.

It still must be supported in media type message/http, but that shouldn't
apply
to anything in the core of httpd. Third party module authors would have to
make
appropriate accommodations if they directly handle this content.


Re: svn commit: r1754579 - /httpd/httpd/trunk/server/gen_test_char.c

2016-08-04 Thread William A Rowe Jr
On Thu, Aug 4, 2016 at 2:54 PM, Eric Covener  wrote:

> On Thu, Aug 4, 2016 at 3:33 PM, William A Rowe Jr 
> wrote:
> > It seems correcting the table is the correct way to go, by direct
> > observation
>
> #error if it's not the EBCDIC platform we made the observation on?   I
> don't know how much of a technicality another EBCDIC platform that
> comes anywhere near building moderm releases is.
>

I agree, and the problem is that the platform we are building on isn't
necessarily the target, which would apply across different EBCDIC
architectures.

You can get into trouble with this with your LANG setting even under
nearly-ASCII architectures, for example.


Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

2016-08-04 Thread Roy T. Fielding
> On Aug 3, 2016, at 2:28 PM, William A Rowe Jr  wrote:
> 
> So AIUI, the leading SP / TAB whitespace in a field is a no-op (usually
> represented by a single space by convention), and trailing whitespace 
> in the field value is a no-op, all leading tabs/spaces (beyond one SP) 
> in the obs-fold line is a no-op. Is there any reason to preserve trailing 
> spaces before the obs-fold?

Not given our implementation.  The buffer efficiency argument is for other
kinds of parsers that are not reading just one line at a time.

> If not, then stripping trailing whitespace from the line prior to obs-fold and
> eating all leading whitespace on the obs-fold line will result in a single SP
> character, which should be just fine unless spaces were significant within
> a quoted value. The only way for the client to preserve such significant 
> spaces would be to place them after the opening quote before the obs-fold.

obs-fold is not allowed inside quoted text, so we need not worry about
messing with such a construct.

Note that obs-fold has been formally deprecated outside of message/http.
We can remove its handling at any time we are willing to accept the risk
of strange error reports.  I do not believe it is part of our versioning 
contract.

Roy



Re: svn commit: r1754579 - /httpd/httpd/trunk/server/gen_test_char.c

2016-08-04 Thread Eric Covener
On Thu, Aug 4, 2016 at 3:33 PM, William A Rowe Jr  wrote:
> It seems correcting the table is the correct way to go, by direct
> observation

#error if it's not the EBCDIC platform we made the observation on?   I
don't know how much of a technicality another EBCDIC platform that
comes anywhere near building moderm releases is.

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


Re: svn commit: r1754579 - /httpd/httpd/trunk/server/gen_test_char.c

2016-08-04 Thread William A Rowe Jr
On Thu, Aug 4, 2016 at 2:01 PM, Eric Covener  wrote:

> On Mon, Aug 1, 2016 at 3:22 PM, William A Rowe Jr 
> wrote:
> > We have a few choices, but the bottom line is that we treat /r/n
> > as 0x0a 0x15 on ebcdic, and perhaps fix our iconv mapping.
> >
> > Choice 1; map both 0x15 and 0x37 to ASCII 0x0d, which grows the
> > number of ascii equivalents by one. Both would be treated at CTRL.
> >
> > Choice 2; invert our current mapping, ASCII NL to EBCDIC LF and
> > visa-versa. That would leave 0x37 'unguarded' and allowed as opaque
> > text chars.
> >
> > Choice 3; treat the entire C1 codeplane on EBCDIC as CTRLs, and
> > ignore some 32 'opaque bytes' as unsupportable.
>
> How about #2 with the below -- but using apr_xlate so it is the same as
> runtime:
>
> http://people.apache.org/~covener/patches/ebcdic-gen_test_char.diff
>

Runtime is the issue, this module is set up for cross compilation, including
between native ASCII architectures. The patch breaks all cross-compilation,
AIUI, not exclusively EBCDIC target builds.

I suspect that the generator is probably wrong for cross compilation on
an ASCII origin/build box targeting an EBCDIC OS in the first place.

It seems correcting the table is the correct way to go, by direct
observation, and placing great faith that other than 0x15/0x37,
the discrepancies between ASCII <> EBCDIC C0 mappings do
not vary widely between EBCDIC mapping choices. Whether we
fix cross compilation of an ASCII build to an EBCDIC target is
a different question.

An alternative is to directly speak iconv, /shrug.

(I am not happy about changing the non-ebcdic build here, but it
> should act just like a static support program)
>

Static support programs don't have to be invoked on the build environment,
today.

But I'm still uneasy about leaving 0x37 unguarded.


Re: svn commit: r1754579 - /httpd/httpd/trunk/server/gen_test_char.c

2016-08-04 Thread Eric Covener
On Mon, Aug 1, 2016 at 3:22 PM, William A Rowe Jr  wrote:
> We have a few choices, but the bottom line is that we treat /r/n
> as 0x0a 0x15 on ebcdic, and perhaps fix our iconv mapping.
>
> Choice 1; map both 0x15 and 0x37 to ASCII 0x0d, which grows the
> number of ascii equivalents by one. Both would be treated at CTRL.
>
> Choice 2; invert our current mapping, ASCII NL to EBCDIC LF and
> visa-versa. That would leave 0x37 'unguarded' and allowed as opaque
> text chars.
>
> Choice 3; treat the entire C1 codeplane on EBCDIC as CTRLs, and
> ignore some 32 'opaque bytes' as unsupportable.

How about #2 with the below -- but using apr_xlate so it is the same as runtime:

http://people.apache.org/~covener/patches/ebcdic-gen_test_char.diff

(I am not happy about changing the non-ebcdic build here, but it
should act just like a static support program)


Re: mod_remoteip DNS address resolution

2016-08-04 Thread Rainer Jung

Am 04.08.2016 um 17:46 schrieb Yann Ylavic:

On Thu, Aug 4, 2016 at 3:30 PM, Rainer Jung  wrote:


- apr_ipsubnet_create() has some logic, that for instance accepts "192.168"
as input with NULL mask_or_numbits and returns sub 192.168.0.0 and mask
255.255.0.0.


Hmm, indeed, but this looks buggy to me.
Shouldn't apr_ipsubnet_create() be fixed with:

-rv = parse_ip(*ipsub, ipstr, mask_or_numbits == NULL);
+rv = parse_ip(*ipsub, ipstr, mask_or_numbits != NULL);

?


At least code archaeology shows this condition goes back very long way. 
It seems to me NULL was used as a marker for abbreviated IPv4 network 
address, like 192.168 meaning 192.168/16. Simply switching the test 
could break stuff like simple IP ACLs ("Allow from 192.168"). Not tested 
with httpd though, I was just doing some tests by calling 
apr_ipsubnet_create() directly from a small standalone program to 
confirm its behavior.


Regards,

Rainer







Frequent wake-ups for mpm_event

2016-08-04 Thread Luca Toscano
Hi Apache Devs,

there is an interesting bugzilla ticket about mpm_event and frequent
wake-ups: https://bz.apache.org/bugzilla/show_bug.cgi?id=57399

Would it be possible to avoid them adding APR_POLLSET_WAKEABLE to the
event_pollset flags and calling apr_pollset_wakeup right after
apr_skiplist_insert?

I might have completely misunderstood the code, but I wanted to bring up
the issue to have more expert opinions from the list. The skiplist use case
is still a bit unclear to me even if we discussed it a while ago in another
email thread.

Thanks!

Regards,

Luca


Re: mod_remoteip DNS address resolution

2016-08-04 Thread Yann Ylavic
On Thu, Aug 4, 2016 at 3:30 PM, Rainer Jung  wrote:
>
> - apr_ipsubnet_create() has some logic, that for instance accepts "192.168"
> as input with NULL mask_or_numbits and returns sub 192.168.0.0 and mask
> 255.255.0.0.

Hmm, indeed, but this looks buggy to me.
Shouldn't apr_ipsubnet_create() be fixed with:

-rv = parse_ip(*ipsub, ipstr, mask_or_numbits == NULL);
+rv = parse_ip(*ipsub, ipstr, mask_or_numbits != NULL);

?


Re: mod_remoteip DNS address resolution

2016-08-04 Thread Rainer Jung

Am 04.08.2016 um 13:36 schrieb Yann Ylavic:

On Thu, Aug 4, 2016 at 10:14 AM, Rainer Jung  wrote:


Something like "RemoteIPLookups (On|Off|NNN)". "On" would be current
behavior, "Off" would be "No DNS and use connection IP if address is
invalid", "NNN" would be "No DNS and return status NNN if address is
invalid". Default "On" or "Off" for 2.4 and "Off" for trunk.


+1



Note that we don't have an "IP address string to numeric IP" conversion
function at hand. APR has apr_inet_pton(), but unfortunately it currently is
not made public via the header files. We could probably copy it in and make
public for future versions.


We have apr_ipsubnet_create() which can validate whether a string is a
valid IP address (among other things).


I did some experiments with it. There's two problems with 
apr_ipsubnet_create():


- it produces a apr_ipsubnet_t which IMHO is an opaque type. Of course 
we could cast by using our knowledge of what that type actually is, but 
by that we break our ability to later change that type in APR. Or we add 
accessors for the members of the struct to APR or we make the struct 
definition public in APR.


- apr_ipsubnet_create() has some logic, that for instance accepts 
"192.168" as input with NULL mask_or_numbits and returns sub 192.168.0.0 
and mask 255.255.0.0. So one has to set mask_or_numbits to some 
(correct) value, IMHO "32" for IPv4 and "128" for IPv6. That means one 
has to first check the string IP address whether it is likely IPv4 or 
IPv6, then set mask_or_numbits to the string representation of the 
address size and then let apr_ipsubnet_create() convert everything back. 
Not especially efficient.


But of course probably the shortest in term of code complexity.

Regards,

Rainer



Re: [ANNOUNCE] Apache HTTP Server 2.4.23 Released [I]

2016-08-04 Thread Luca Toscano
2016-08-04 14:56 GMT+02:00 Eric Covener :

> On Thu, Aug 4, 2016 at 8:29 AM, Mark Blackman 
> wrote:
> > Classification: For internal use only
> >
> > Hi,
> >
> > Could I recommend that text about Apache 2.2 EOL notification be added
> to the 2.2 section on http://httpd.apache.org please?
> >
> > Regards,
> > Mark
> >
> >> -Original Message-
> >> From: Jim Jagielski [mailto:j...@jagunet.com]
> >> Sent: 05 July 2016 14:04
> >> To: httpd 
> >> Cc: us...@httpd.apache.org
> >> Subject: [ANNOUNCE] Apache HTTP Server 2.4.23 Released.
> >>
> >> ..
> >>
> >> Please note that Apache Web Server Project will only provide maintenance
> >> releases of the 2.2.x flavor through June of 2017, and will provide some
> >> security patches beyond this date through at least December of 2017.
> >> Minimal maintenance patches of 2.2.x are expected throughout this
> period,
> >> and users are strongly encouraged to promptly complete their transitions
> >> to the the 2.4.x flavor of httpd to benefit from a much larger
> assortment
> >> of minor security and bug fixes as well as new features.
> >
>
> Good catch, thanks Mark.
>

https://httpd.apache.org front page updated!

Luca


Re: [ANNOUNCE] Apache HTTP Server 2.4.23 Released [I]

2016-08-04 Thread Eric Covener
On Thu, Aug 4, 2016 at 8:29 AM, Mark Blackman  wrote:
> Classification: For internal use only
>
> Hi,
>
> Could I recommend that text about Apache 2.2 EOL notification be added to the 
> 2.2 section on http://httpd.apache.org please?
>
> Regards,
> Mark
>
>> -Original Message-
>> From: Jim Jagielski [mailto:j...@jagunet.com]
>> Sent: 05 July 2016 14:04
>> To: httpd 
>> Cc: us...@httpd.apache.org
>> Subject: [ANNOUNCE] Apache HTTP Server 2.4.23 Released.
>>
>> ..
>>
>> Please note that Apache Web Server Project will only provide maintenance
>> releases of the 2.2.x flavor through June of 2017, and will provide some
>> security patches beyond this date through at least December of 2017.
>> Minimal maintenance patches of 2.2.x are expected throughout this period,
>> and users are strongly encouraged to promptly complete their transitions
>> to the the 2.4.x flavor of httpd to benefit from a much larger assortment
>> of minor security and bug fixes as well as new features.
>

Good catch, thanks Mark.


RE: [ANNOUNCE] Apache HTTP Server 2.4.23 Released [I]

2016-08-04 Thread Mark Blackman
Classification: For internal use only

Hi,

Could I recommend that text about Apache 2.2 EOL notification be added to the 
2.2 section on http://httpd.apache.org please?

Regards,
Mark

> -Original Message-
> From: Jim Jagielski [mailto:j...@jagunet.com]
> Sent: 05 July 2016 14:04
> To: httpd 
> Cc: us...@httpd.apache.org
> Subject: [ANNOUNCE] Apache HTTP Server 2.4.23 Released.
>
> ..
>
> Please note that Apache Web Server Project will only provide maintenance
> releases of the 2.2.x flavor through June of 2017, and will provide some
> security patches beyond this date through at least December of 2017.
> Minimal maintenance patches of 2.2.x are expected throughout this period,
> and users are strongly encouraged to promptly complete their transitions
> to the the 2.4.x flavor of httpd to benefit from a much larger assortment
> of minor security and bug fixes as well as new features.


---
This e-mail may contain confidential and/or privileged information. If you are 
not the intended recipient (or have received this e-mail in error) please 
notify the sender immediately and delete this e-mail. Any unauthorized copying, 
disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and 
regulatory disclosures and to 
http://www.db.com/unitedkingdom/content/privacy.htm for information about 
privacy.


Re: mod_remoteip DNS address resolution

2016-08-04 Thread Yann Ylavic
On Thu, Aug 4, 2016 at 10:14 AM, Rainer Jung  wrote:
>
> Something like "RemoteIPLookups (On|Off|NNN)". "On" would be current
> behavior, "Off" would be "No DNS and use connection IP if address is
> invalid", "NNN" would be "No DNS and return status NNN if address is
> invalid". Default "On" or "Off" for 2.4 and "Off" for trunk.

+1

>
> Note that we don't have an "IP address string to numeric IP" conversion
> function at hand. APR has apr_inet_pton(), but unfortunately it currently is
> not made public via the header files. We could probably copy it in and make
> public for future versions.

We have apr_ipsubnet_create() which can validate whether a string is a
valid IP address (among other things).


Regards,
Yann.


Re: svn commit: r1750953 - /httpd/httpd/trunk/server/util_script.c

2016-08-04 Thread Luca Toscano
2016-08-02 10:17 GMT+02:00 Luca Toscano :

>
> 2016-08-01 21:13 GMT+02:00 Jacob Champion 
>>
>>
>> As stated above, this is not my first choice -- but I wouldn't oppose it
>> if that's what the consensus comes to.
>>
>>  else if (!ap_cstr_casecmp(w, "Last-Modified")) {
>>> -apr_time_t parsed_date = apr_date_parse_rfc(l);
>>> +apr_time_t parsed_date = apr_date_parse_http(l);
>>>
>>
>> apr_date_parse_http() is not good enough; IIUC, it completely ignores
>> timezones, which further corrupts non-GMT Last-Modified stamps. We either
>> want strict parsing or actual correction, not something in the middle.
>
>
> You are right, but this is the current behavior, this is why I used
> apr_date_parse_http. My idea was to keep what we have at the moment, adding
> a clear log that states the inconsistency found by httpd and why it has
> been corrected. In this case it would simply mean that a value considered
> "in the future" from GMT would be logged and corrected accordingly. This
> would still leave out other wrong use cases like a datetime string in the
> US/West timezone "in the future" if converted in GMT, but to solve it we'd
> need to interpret the Last-Modified header value with the
> apr_date_parse_rfc function and afaiu we still need to reach an agreement :)
>
> So I am really in favor of using apr_date_parse_rfc but I wanted to come
> up with a possible to solution to satisfy everybody and help our users with
> some clue.
>
> Example about the wrong use case mentioned by me for everybody (it helped
> me a lot so it might also be useful to other people reading):
>
> ===
> HTTP/1.1 200 OK
> Date: Tue, 02 Aug 2016 07:54:39 GMT
> Server: Apache/2.5.0-dev (Unix)
> Last-Modified: Tue, 02 Aug 2016 00:54:39 GMT
> Content-Type: text/html; charset=UTF-8
>
> Last-Modified sent: Tue, 02 Aug 2016 00:54:39 -0700
> Now in GMT: Tue, 02 Aug 2016 07:54:39 +
> ===
>
> As you can see, the FCGI script sets a Last-Modified header value now() in
> the "America/Los_Angeles" timezone, but it gets interpreted by httpd as a
> GMT datestring "in the past". So for example "now() + 2 hours" in the
> "America/Los_Angeles" timezone would still get interpreted as "in the past"
> by httpd, avoiding any datetime correction to GMT now().
>
> Same example with the code change that we have originally proposed:
>
> ===
> HTTP/1.1 200 OK
> Date: Tue, 02 Aug 2016 08:08:14 GMT
> Server: Apache/2.5.0-dev (Unix)
> Last-Modified: Tue, 02 Aug 2016 08:08:14 GMT
> Content-Type: text/html; charset=UTF-8
>
> Last-Modified sent: Tue, 02 Aug 2016 01:08:14 -0700
> Now in GMT: Tue, 02 Aug 2016 08:08:14 +
> ===
>
> The debate is all around, as far as I have understood, to if httpd should
> be able to do the above "interpretation" or not.
>
>
> 2016-08-01 23:13 GMT+02:00 Jacob Champion :
>
>> On 08/01/2016 01:35 PM, William A Rowe Jr wrote:
>>
>>> So this alternative is not my first choice. Invalid headers should
>>> really either be corrected (if the correction is obvious, safe, and
>>> helpful), or dropped entirely. Or the entire response should be
>>> 500'd, but we run into major compatibility breaks if we choose that
>>> route.
>>>
>>> No, I agree that a 500 is not an option. Drop it, or fix it loudly in
>>> the logs,
>>> but we can't break existing deployments (which don't accept non-GMT
>>> dates today, FWIW).
>>>
>>
>> Based on conversations with Luca, existing deployments do "accept"
>> non-GMT dates, silently corrupting the value, due to the use of the
>> *_parse_http() function.
>>
>> I'm happy to react to bad input following a parsable date string, e.g. any
>>> non-"GMT" signifier, as entirely bad input worth emitting to the error
>>> log.
>>> Let's help CGI authors find their bugs instead of generating unexpected
>>> results, such as 5-8 hour wrong "Last Modified" dates in the US and
>>> now() throughout Europe, owing to the time zone deltas.
>>>
>>
>> Cool. I'm not opposed to loudly dropping non-GMT timestamps (though I
>> still prefer fixing them up).
>
>
> Wouldn't this introduce a breaking change? I know that everybody is
> supposed to look into the changelog before upgrading httpd but as we often
> see this is not the case and we could trigger nasty issues in various
> installations imho. Compared to this solution I'd much prefer to go for the
> proposed one, invasive but less disruptive :)
>
> As Jacob said, more opinions are very welcome!
>
> One general comment: I really like discussions but in this case I feel
> that we would have solved the issue in 20 minutes talking in person :)
> So maybe a conversation on IRC whenever everybody has a bit of time could
> help reaching a final consensus?
>


Trying with a new patch: http://apaste.info/xHz

I added two separate logs, one for non-GMT header values and the other one
for datetime strings considered in 

mod_remoteip DNS address resolution

2016-08-04 Thread Rainer Jung


Hi there,

I learned that mod_remoteip does IP address resolution including DNS 
when it processes a token from the configured RemoteIPHeader. In the 
observed case, two different customers using F5 load balancers had a 
numeric IP address in the header which was followed without white space 
or separator character by "%N", "N" a number. That thing was said to be 
a partition tag. Of course one doesn't want such stuff in 
X-Forwarded-For, but mod_remoteip falling back to DNS was a surprise for me.


Although we clearly express in the docs "It is critical to only enable 
this behavior from intermediate hosts (proxies, etc) which are trusted 
by this server, since it is trivial for the remote useragent to 
impersonate another useragent." I would prefer if IP address resolution 
with DNS would be configurable - mostly due to performance reasons.


So I think we need a directive for mod_remoteip to enable/disable DNS 
lookups.


We also need to decide what to do, if the header value is not a valid 
numeric IP address and DNS is turned off. We could fall back to using 
the connection IP address and proceed with the request or we could throw 
an error like a Bad Request, Forbidden, Internal Server Error or similar.


Something like "RemoteIPLookups (On|Off|NNN)". "On" would be current 
behavior, "Off" would be "No DNS and use connection IP if address is 
invalid", "NNN" would be "No DNS and return status NNN if address is 
invalid". Default "On" or "Off" for 2.4 and "Off" for trunk.


Note that we don't have an "IP address string to numeric IP" conversion 
function at hand. APR has apr_inet_pton(), but unfortunately it 
currently is not made public via the header files. We could probably 
copy it in and make public for future versions.


WDYT?

Rainer