Re: [secdir] secdir review of draft-ietf-httpbis-p1-messaging-25

[Ben Laurie <[e-mail]>]

On 19 December 2013 03:01, Mark Nottingham <[e-mail]> wrote:
> Hi Ben,
>
> Thanks for the detailed review. A few preliminary comments below.
>
>
> On 19 Dec 2013, at 5:02 am, Ben Laurie <[e-mail]> wrote:
>
>> I have reviewed this document as part of the security directorate's
>> ongoing effort to review all IETF documents being processed by the
>> IESG.  These comments were written primarily for the benefit of the
>> security area directors.  Document editors and WG chairs should treat
>> these comments just like any other last call comments.
>>
>> Summary: HTTP/1.1 is complicated, with many pitfalls. The draft does
>> not give a great deal of assistance in dealing with them, and, indeed,
>> encourages s/w to trip over these pitfalls.
>>
>> 2.4 Caches
>>
>> As well as poisoning attacks, websites can leave themselves exposed by
>> failing to set appropriate caching values - for example, allowing
>> caches to cache pages containing pesonal information, which is later
>> served to the wrong person. The I-D should discuss this issue and give
>> concrete advice on how to avoid inappropriate caching, whilst still
>> permitting appropriate caching.
>>
>> Likewise, caches can be fooled into serving cached data inappropriaely
>> - for example, in response to a subtly different request.
>>
>> Part 6, which is devoted to caches, appears to give no advice
>> whatsoever, other than applications should "emit appropriate
>> Cache-Control response header fields".
>>
>> It also, strangely, blames cache poisoning on implementation flaws,
>> whilst exactly those types of flaws are encouraged in the next section of this
>> I-D: "Unless noted otherwise, a recipient MAY attempt to recover a
>> usable  protocol element from an invalid construct."
>
> Do you have a suggestion for text we should include (either here or in p6)?

I would say that recipients MUT NOT attempt recover a usable...

Also, for "appropriate Cache-Control response header fields", I'd
suggest including examples for likely scenarios (e.g. "can be freely
cached, must not be cached at all, ever, can be cached for anyone with
this particular cookie" [if that's even possible]).

>> 2.5.  Conformance and Error Handling
>>
>> Obviously very important for security. Is littered with vague
>> generalities like
>>
>> "A sender MUST NOT generate protocol elements that convey a meaning
>> that is known by that sender to be false."
>>
>> "Within a given message, a sender MUST NOT generate protocol elements
>> or syntax alternatives that are only allowed to be generated by
>> participants in other roles (i.e., a role that the sender does not
>> have for that message)."
>>
>> "When a received protocol element is parsed, the recipient MUST be
>>   able to parse any value of reasonable length that is applicable to
>>   the recipient's role and matches the grammar defined by the
>>   corresponding ABNF rules."
>>
>> without giving any concrete advice (other than, presumably, "read and
>> memorise the whole xxx pages of RFCs A, B, C ... Z"). Are there
>> matrices of what is allowed, for example? Where? How long is
>> "reasonable"? What do you do if the length is, in fact,
>> "unreasonable"?
>
> If we were designing HTTP from scratch, I would agree with you. However, we're not; it is arguably the most widely deployed and diversely implemented application protocols. There are few (if any) fields where a single recommendation about "reasonable" size (for example) would be appropriate, because HTTP is used for everything from Web browsers to embedded controllers. We can't make these things non-conformant retrospectively.

You could, perhaps, distinguish between what might be found in the
wild (that you want to make conformant, for some reason) and what you
advise for new implementations.

> We have, however, introduced recommendations (not requirements) around minimum field lengths, etc. in most places. If you find a particular one lacking, please do bring it to our attention.
>
>
>> As mentioned above, dangerous behaviour is encouraged on the part of
>> receivers - i.e. to attempt to make some sense of invalid constructs.
>>
>> Also, "A recipient MUST interpret a received protocol element
>> according to the semantics defined for it by this specification,
>> including extensions to this specification, unless the recipient has
>> determined (through experience or configuration) that the sender
>> incorrectly implements what is implied by those semantics." is a
>> licence for arbitraty behaviour.
>>
>> No mention at all of any of this in Security Considerations.
>>
>> 2.6. Protocol Versioning
>>
>> "A client MAY send a lower request version if it is known that the
>> server incorrectly implements the HTTP specification, but only after
>> the client has attempted at least one normal request and determined
>> from the response status code or header fields (e.g., Server) that the
>> server improperly handles higher request versions." - more
>> encouragement to ignore the spec, and doubtless introduce yet more
>> interesting security flaws.
>
> How is this "ignoring the spec"?

You allow clients to fall back to older specs (i.e. ignore this spec)
on essentially arbitrary grounds.

>> 3.  Message Format
>>
>> "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)." - ditto.
>
> Can you please give us more substantive input? What is the *specific* flaw here?

The issue is encouraging interpreting content that may or may not be
HTTP as if it were (combined with other admonishments to guess at its
meaning as you go). This is how request splitting attacks work, for
example.

In other words, recipients should be strict in their interpretation
lest they are fooled into misinterpretation.

>> 3.1.1.  Request Line
>>
>> "Recipients of an invalid request-line SHOULD respond with either a
>> 400 (Bad Request) error or a 301 (Moved Permanently) redirect with the
>> request-target properly encoded." - ditto.
>
> And again...

See above. The response should be a 400, never a 301.

>> 3.2.2.  Field Order
>>
>> "A server MUST wait until the entire header section is received before
>> interpreting a request message, since later header fields might
>> include conditionals, authentication credentials, or deliberately
>> misleading duplicate header fields that would impact request
>> processing."
>>
>> Good advice, but sadly, no substance - it goes on to say:
>>
>> "A recipient MAY combine multiple header fields with the same field
>> name into one "field-name: field-value" pair, without changing the
>> semantics of the message, by appending each subsequent field value to
>> the combined field value in order, separated by a comma.  The order in
>> which header fields with the same field name are received is therefore
>> significant to the interpretation of the combined field value"
>>
>> Which gives no useful advice about "duplicate header fields that would
>> impact request processing", and worse, means that intermediates and
>> endpoints have essentially no chance of predicting the behaviour of
>> machines up/downstream from them.
>
> We've taken the approach of identifying security-sensitive headers and giving such advice in their specification, since this section is going to be read by implementers of the generic message parsers, not the folks who need to understand the security import of this design.

OK. It would probably be useful to have a list somewhere, rather than
obliging everyone, e.g. code auditors, to read all n documents.

>> 3.2.4.  Field Parsing
>>
>> "A server that receives an obs-fold in a request message that is not
>> within a message/http container MUST either reject the message by
>> sending a 400 (Bad Request), preferably with a representation
>> explaining that obsolete line folding is unacceptable, or replace each
>> received obs-fold with one or more SP octets prior to interpreting the
>> field value or forwarding the message downstream." - again,
>> encouraging dangerous behaviour.
>
> Please explain how it is dangerous.

Again, non-strict interpretation can lead (and historically has led)
to misinterpretation, which leads to exploitable bugs.

>> 3.3.2.  Content-Length
>>
>> Oddly, advice is given on what to when multiple identical content
>> lengths are received, but not when they're different...
>>
>> "If a message is received that has multiple Content-Length header
>> fields with field-values consisting of the same decimal value, or a
>> single Content-Length header field with a field value containing a
>> list of identical decimal values (e.g., "Content-Length: 42, 42"),
>> indicating that duplicate Content-Length header fields have been
>> generated or combined by an upstream message processor, then the
>> recipient MUST either reject the message as invalid or replace the
>> duplicated field-values with a single valid Content-Length field
>> containing that decimal value prior to determining the message body
>> length or forwarding the message."
>>
>> (though the next section does say what to do: for a change, reject
>> it).
>
> So, it is given. What's the issue - would you like this section rearranged? If so, please come out and say so; if not, please explain.

Yeah, it just seemed odd to deal with one case in one place and the
other in a completely different section. It also is a little odd to
collapse multiple content-lengths (which is incorrect according to the
spec) into a single one, just because they happen to match.

>> "Since there is no way to distinguish a successfully completed,
>> close-delimited message from a partially-received message interrupted
>> by network failure, a server SHOULD generate encoding or length-
>> delimited messages whenever possible.  The close-delimiting feature
>> exists primarily for backwards compatibility with HTTP/1.0."
>>
>> Surely closed connections _can_ be distinguished from interrupted
>> ones?
>
> Not if the client is behind a proxy, and not if the HTTP API doesn't expose this information. That said, "no way" is too strong here; should be at least qualified with a "sometimes."

Perhaps the proxy should indicate if this has happened?