You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by William A Rowe Jr <wr...@rowe-clan.net> on 2015/06/15 18:33:31 UTC
TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Reviewing the spec, I cannot find where Sambar server is permitted to
insert whitespace. I further reviewed the ABNF appendix, and it does not
appear there, either.
The spec seems unambiguous;
chunk = chunk-size [ chunk-ext ] CRLF
chunk-data CRLF
chunk-size = 1*HEXDIG
last-chunk = 1*("0") [ chunk-ext ] CRLF
There is no opportunity to use whitespace outside of chunk-ext.
chunk-ext = *( ";" chunk-ext-name [ "=" chunk-ext-val ] )
chunk-ext-name = token
chunk-ext-val = token / quoted-string
The rules in section 3.2.3 have become extremely strict;
3.2.3 <https://tools.ietf.org/html/rfc7230#section-3.2.3>. Whitespace
This specification uses three rules to denote the use of linear
whitespace: OWS (optional whitespace), RWS (required whitespace), and
BWS ("bad" whitespace).
The OWS rule is used where zero or more linear whitespace octets
might appear. For protocol elements where optional whitespace is
preferred to improve readability, a sender SHOULD generate the
optional whitespace as a single SP; otherwise, a sender SHOULD NOT
generate optional whitespace except as needed to white out invalid or
unwanted protocol elements during in-place message filtering.
The RWS rule is used when at least one linear whitespace octet is
required to separate field tokens. A sender SHOULD generate RWS as a
single SP.
The BWS rule is used where the grammar allows optional whitespace
only for historical reasons. A sender MUST NOT generate BWS in
messages. A recipient MUST parse for such bad whitespace and remove
it before interpreting the protocol element.
And section 3.6.1 of RFC2616 made no accommodation for whitespace, in
the first place.
I think Sambar is wrong and we should not be supporting this.
If we make provision to support this, we should be disallowing
by default and add a directive to change the behavior.
Thoughts?
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Jun 15, 2015 6:11 PM, "Roy T. Fielding" <fi...@gbiv.com> wrote:
>
> > On Jun 15, 2015, at 9:33 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:
> >
> > Reviewing the spec, I cannot find where Sambar server is permitted to
insert whitespace. I further reviewed the ABNF appendix, and it does not
appear there, either.
>
> Right, this was a deliberate decision to reduce the number of infinite
stream possibilities.
> We can still read a few SP and discard for robustness, but it should be
limited to the same
> few characters as leading zeros.
So as things stand, this is BWS (or just drop the W :) As such we don't
pass it through verbatim, so we are honoring the BWS contract of not
repeating the mistake. With HttpStrict on head, we can return to Yann's
original proposal of treating such headers as errors.
> Sambar Server has been EOL for 7 years with no available source code for
review, so it's
> behavior is no longer relevant to the standard.
+1
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
> On Jun 15, 2015, at 9:33 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> Reviewing the spec, I cannot find where Sambar server is permitted to insert whitespace. I further reviewed the ABNF appendix, and it does not appear there, either.
Right, this was a deliberate decision to reduce the number of infinite stream possibilities.
We can still read a few SP and discard for robustness, but it should be limited to the same
few characters as leading zeros.
Sambar Server has been EOL for 7 years with no available source code for review, so it's
behavior is no longer relevant to the standard.
....Roy
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Jun 15, 2015 1:26 PM, "Graham Leggett" <mi...@sharp.fm> wrote:
>
> On 15 Jun 2015, at 7:00 PM, Jeff Trawick <tr...@gmail.com> wrote:
>
> > 1.3 (or 1.3-based servers) put whitespace there.
> > 1.3.x, 2.0.x, 2.2.x, and 2.4.x (for all released x so far) accepts
whitespace there.
> > We can't change that by default in a stable branch.
>
> +1.
>
> We need to be liberal in what we accept. I don’t even think a “strict”
mode serves much purpose.
That 'thinking' by many server authors created the stack of CVE-2005 issues
identified by Watchfire as low, medium and worse severities.
This is not metadata. It is transport-layer data. And we have precedent
for closing vulnerabilities in 2005, and the several SSL vendors have done
similar.
That said, httpd 2 has always done the right thing with respect to not
passing hop-by-hop data verbatim. So we are in a bit better position than
some, and this mitigates the surface area of the attack.
The recent enhancements to further protect from split requests is also
reassuring.
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by Graham Leggett <mi...@sharp.fm>.
On 15 Jun 2015, at 7:00 PM, Jeff Trawick <tr...@gmail.com> wrote:
> 1.3 (or 1.3-based servers) put whitespace there.
> 1.3.x, 2.0.x, 2.2.x, and 2.4.x (for all released x so far) accepts whitespace there.
> We can't change that by default in a stable branch.
+1.
We need to be liberal in what we accept. I don’t even think a “strict” mode serves much purpose.
Regards,
Graham
—
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/15/2015 07:00 PM, Jeff Trawick wrote:
> On Mon, Jun 15, 2015 at 12:33 PM, William A Rowe Jr <wrowe@rowe-clan.net <ma...@rowe-clan.net>> wrote:
>
>
> 1.3 (or 1.3-based servers) put whitespace there.
> 1.3.x, 2.0.x, 2.2.x, and 2.4.x (for all released x so far) accepts whitespace there.
> We can't change that by default in a stable branch.
>
> This could be perhaps implemented in conjunction with sf's HttpStrict (?) stuff in trunk (I have no clue what that does
> in practice, but it sounds right).
+1
Regards
Rüdiger
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Jun 16, 2015 at 10:50 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Jun 16, 2015 at 8:09 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>> Note in STATUS I've requested that you split the approved patch from
>> security@ that seemed to be lost in long and winding patch versioning from
>> the spaces accepted. A patch should correct one thing, not several, it
>> makes these too difficult to review when folks have a small window of free
>> time. Your proposed rolled-up patch didn't correspond to trunk, and the
>> 'parsing' flag seems unnecessary. Two error messages would have been easier
>> on reviewers anyways.
>>
>> Hopefully all constructive criticism easily agreed to?
>
> No pb, that's how things go ahead!
>
> I can certainly split the patch (i.e. the two first hunks only address
> the 2.4.14's "defect") and commit what's already accepted from patch
> v5 (the three remaining hunks).
> The latter however includes the 'parsing' flag which is meant
> precisely to address the different semantic between trunk and 2.4.x
> (so far): in 2.4.x the parsing errors are handled by
> bail_out_on_error() whereas in trunk the caller decides (eg. a handler
> can return a HTTP status and hence, possibly, an ErrorDocument...).
> Without the 'parsing' flag, we'd also have to use two separate log
> messages (with a new AH for 2.4.x only), so I think it also helps
> commonality between the two codes (that could be addressed by
> splitting the same way in trunk though...).
>
> BTW, I do that now.
Done in r1685904 (missing backport) and r1685910 (proposal update).
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Jun 16, 2015 at 8:09 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> Note in STATUS I've requested that you split the approved patch from
> security@ that seemed to be lost in long and winding patch versioning from
> the spaces accepted. A patch should correct one thing, not several, it
> makes these too difficult to review when folks have a small window of free
> time. Your proposed rolled-up patch didn't correspond to trunk, and the
> 'parsing' flag seems unnecessary. Two error messages would have been easier
> on reviewers anyways.
>
> Hopefully all constructive criticism easily agreed to?
No pb, that's how things go ahead!
I can certainly split the patch (i.e. the two first hunks only address
the 2.4.14's "defect") and commit what's already accepted from patch
v5 (the three remaining hunks).
The latter however includes the 'parsing' flag which is meant
precisely to address the different semantic between trunk and 2.4.x
(so far): in 2.4.x the parsing errors are handled by
bail_out_on_error() whereas in trunk the caller decides (eg. a handler
can return a HTTP status and hence, possibly, an ErrorDocument...).
Without the 'parsing' flag, we'd also have to use two separate log
messages (with a new AH for 2.4.x only), so I think it also helps
commonality between the two codes (that could be addressed by
splitting the same way in trunk though...).
BTW, I do that now.
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Note in STATUS I've requested that you split the approved patch from
security@ that seemed to be lost in long and winding patch versioning from
the spaces accepted. A patch should correct one thing, not several, it
makes these too difficult to review when folks have a small window of free
time. Your proposed rolled-up patch didn't correspond to trunk, and the
'parsing' flag seems unnecessary. Two error messages would have been
easier on reviewers anyways.
Hopefully all constructive criticism easily agreed to?
On Tue, Jun 16, 2015 at 12:31 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Jun 16, 2015 at 7:22 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> > Sooo.... in order to get 2.4.15 out, it would be nice to have
> > a patch :)
>
> Isn't the one proposed in STATUS suitable (section SHOWSTOPPERS)?
> It has been positively tested by Steffen in [1] and also passes the
> new framework tests from [2].
>
> [1]
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201506.mbox/%3CFB1A6FB8F6F046AAA243E9583A4CA9BD%40father%3E
> [2] http://svn.apache.org/r1685463
>
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Jun 16, 2015 at 7:22 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> Sooo.... in order to get 2.4.15 out, it would be nice to have
> a patch :)
Isn't the one proposed in STATUS suitable (section SHOWSTOPPERS)?
It has been positively tested by Steffen in [1] and also passes the
new framework tests from [2].
[1] http://mail-archives.apache.org/mod_mbox/httpd-dev/201506.mbox/%3CFB1A6FB8F6F046AAA243E9583A4CA9BD%40father%3E
[2] http://svn.apache.org/r1685463
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by Jim Jagielski <ji...@jaguNET.com>.
Sooo.... in order to get 2.4.15 out, it would be nice to have
a patch :)
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by Jim Jagielski <ji...@jaguNET.com>.
>
> 1.3 (or 1.3-based servers) put whitespace there.
> 1.3.x, 2.0.x, 2.2.x, and 2.4.x (for all released x so far) accepts whitespace there.
> We can't change that by default in a stable branch.
>
++1
Re: TWS ";" LWS permitted by RFC 7230 4.1.1? Apparently, no.
Posted by Jeff Trawick <tr...@gmail.com>.
On Mon, Jun 15, 2015 at 12:33 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:
> Reviewing the spec, I cannot find where Sambar server is permitted to
> insert whitespace. I further reviewed the ABNF appendix, and it does not
> appear there, either.
>
> The spec seems unambiguous;
>
> chunk = chunk-size [ chunk-ext ] CRLF
> chunk-data CRLF
> chunk-size = 1*HEXDIG
> last-chunk = 1*("0") [ chunk-ext ] CRLF
>
>
> There is no opportunity to use whitespace outside of chunk-ext.
>
>
> chunk-ext = *( ";" chunk-ext-name [ "=" chunk-ext-val ] )
> chunk-ext-name = token
> chunk-ext-val = token / quoted-string
>
>
> The rules in section 3.2.3 have become extremely strict;
>
>
> 3.2.3 <https://tools.ietf.org/html/rfc7230#section-3.2.3>. Whitespace
>
> This specification uses three rules to denote the use of linear
> whitespace: OWS (optional whitespace), RWS (required whitespace), and
> BWS ("bad" whitespace).
>
> The OWS rule is used where zero or more linear whitespace octets
> might appear. For protocol elements where optional whitespace is
> preferred to improve readability, a sender SHOULD generate the
> optional whitespace as a single SP; otherwise, a sender SHOULD NOT
> generate optional whitespace except as needed to white out invalid or
> unwanted protocol elements during in-place message filtering.
>
> The RWS rule is used when at least one linear whitespace octet is
> required to separate field tokens. A sender SHOULD generate RWS as a
> single SP.
>
> The BWS rule is used where the grammar allows optional whitespace
> only for historical reasons. A sender MUST NOT generate BWS in
> messages. A recipient MUST parse for such bad whitespace and remove
> it before interpreting the protocol element.
>
>
> And section 3.6.1 of RFC2616 made no accommodation for whitespace, in the first place.
>
>
> I think Sambar is wrong and we should not be supporting this.
>
>
> If we make provision to support this, we should be disallowing
>
> by default and add a directive to change the behavior.
>
>
> Thoughts?
>
>
1.3 (or 1.3-based servers) put whitespace there.
1.3.x, 2.0.x, 2.2.x, and 2.4.x (for all released x so far) accepts
whitespace there.
We can't change that by default in a stable branch.
This could be perhaps implemented in conjunction with sf's HttpStrict (?)
stuff in trunk (I have no clue what that does in practice, but it sounds
right).
--
Born in Roswell... married an alien...
http://emptyhammock.com/