Re: Stephen Farrell's Discuss on draft-ietf-httpbis-http2-16: (with DISCUSS and COMMENT)

On 26/01/15 19:12, Martin Thomson wrote:
> Hi Stephen,
> 
> I've collected the edits I've made into a pull request that you can
> review at your leisure:
> 
> https://github.com/http2/http2-spec/pull/707
> 
> On 22 January 2015 at 04:45, Stephen Farrell <stephen.farrell@cs.tcd.ie> wrote:
>> 8.2.2 says that clients MUST validate that a proxy is
>> configured for the correspondsing request. Since you say
>> MUST, don't you need to say what exacftly is meant there?
> 
> The text in question:
> Clients receiving a pushed response MUST validate that either the
> server is authoritative (see Section 10.1), or the proxy that provided
> the pushed response is configured for the corresponding request.
> 
> I'm not against adding additional clarification here:
> 
> "In other words, the client would be willing to send a request for the
> identified resource to the server or proxy."
> 
>> If that is somewhere here, I'm not seeing it.
> 
> From RFC 7230, from which we reference for this definition explicitly:
>    A "<x:dfn>proxy</x:dfn>" is a message-forwarding agent that is
> selected by the
>    client, usually via local configuration rules, to receive requests
>    for some type(s) of absolute URI and attempt to satisfy those
>    requests via translation through the HTTP interface.

I'll clear and make this a comment as you are correct that it's
no different a problem. The MUST-but-I-can't-tell-you-how still
jars a bit though and maybe should be added to a 6919bis:-) Or
if you could re-word without the MUST maybe, but I'll leave that
to you as editor.

>> (Putting this one first as I'd really like an answer
>> but will not be blocking on it no matter how much I
>> dislike the answer:-)
>>
>> - 6.1, and elsewhere, I just want to check that the 256
>> octet limit on padding isn't too restrictive a design. Is it
>> still possible to say arrange that every response from a
>> server has the same size? (Say from a server that is only
>> used for serving images and where there could be a 2KB
>> variation in file size or something.) I think that can work
>> via HEADERS and CONTINUATIONs but wanted to check the limits
>> of flexibility here. I'm similarly interested in the limits
>> within which a client can add padding to a request, but I
>> think the same trick works if it works at all.
> 
> We talked to a couple of people who really are doing padding.  It
> turns out that no one pads that much.  That 2KB variation, even if it
> weren't addressed by the below, is more often broken into sets.
> 
> And the padding isn't as limited as it seems: For DATA, padding is
> effectively unlimited, because you can just make more padded DATA
> frames as you need.  For HEADERS and PUSH_PROMISE, additional padding
> can be obtained other ways too.  (A new, dummy header field, for
> example.)

Cool. I'm keen that padding be such that people can experiment
with how to use it to counter traffic analysis and that is likely
to require flexibility for experiments.

> 
>> - 3.1: a question on the text to be removed about
>> draft-specific identifiers - did the WG consider how to
>> handle drafts produced from now until the RFC issues?  I've
>> not yet looked to see if there are normative dependencies
>> that might lengthen that process, but if there might be it
>> could be useful to discuss in the WG if it turns out there
>> are a bunch of discusses that might take a while and a few
>> versions to sort out. If the IESG eval is clean enough and
>> there are no delaying normative refs then that's probably
>> not needed.
> 
> Well... we discussed it. I believe that the plan was to fight the IESG
> tooth and nail if they want to change things.
> 
> In practice, that text will remain until the RFC editors gets the
> draft.  At that point, if we're making breaking changes, we're not
> going to publish.
> 
> We have no unresolved normative dependencies.
> 
>> - 3.2, the last para before 3.2.1 is hard to read, maybe it
>> was added late but it seems to use concepts not yet
>> explained at that point (e.g. half-closed) Maybe a forward
>> ref to Figure 2 would be enough to fix.
> 
> I'll add some forward references.
> 
>> - 5.3.4, "can be moved" in 1st sentence - I don't get why
>> it's ok to not say MUST or SHOULD there, iff the change is
>> as a result of a peer's action. Shouldn't a 404 for a
>> dependecy have a predictable impact on the overall page
>> load?
> 
> Ahh, yes, priorities are advisory.  And removal of old or dead nodes
> is largely at the discretion of the implementation.  In an ideal
> world, implementations would have infinite state for the
> prioritization graph and we wouldn't have to worry about this, but
> they don't.
> 
> If a node has to be removed, then prioritization information might be
> lost, so there isn't much point in being all normative about how to do
> it properly.  That is what the remainder of the section is all about:
> what the implications of various choices are regarding removal.
> 
>> - 6.10, odd that there's no padding here
> 
> See above answer.  This is entirely deliberate.
> 
>> - 8.1, a bit of abnf here might have helped, ah well
> 
> It was suggested.  Didn't happen.  Probably because the whole mess was
> less complex at that time.  I blame Julian for insisting we continue
> to support 1xx.
> 
>> - 8.1.2.3, I assume the underscors in "_:authority_" are a
>> typo, if not they are possibly confusing
> 
> Yep, a mistake in use of <spanx>.
> 
>> - end of p57, does the last para mean that a header field
>> value can be split betwee a HEADERS frame and the subsequent
>> CONTINUATION frame (with possible padding in between when
>> looked at on-the-wire)? If so, then I think you might want
>> to be clearer about that since I could see it leading to
>> interop issues.
> 
> http://http2.github.io/http2-spec/#HeaderBlock explains this already.
> I've expanded on this a little.

Well I read it again now and glazed over again;-) But I guess
it'd be clear if I were reading it to write code.

> 
>> - p64, DNS-ID is what (dNSName?)? And "Common Name" might be
>> better as commonName, but both probably need a reference.
>> (I don't care about nitty capitalisations, but wouldn't some
>> coders need to reference?)
> 
> Ahh, I used to have a 6125 reference.  Restored.  "Common Name" is now
> correctly "CN-ID".
> 
>> - 9.1, which TLS library allows a client to know that it'll
>> shortly be time to re-key? Doing so is doable, as e.g. NTP
>> has a mechanism like that for pre-announcing leap seconds
>> I'm told. Buti I'm not sure anyone actually does it at all
>> today.
> 
> Since we prohibit renegotiation, rekeying won't happen until we get TLS 1.3.
> 
> I'm sure that what will happen in practice is that the connection will
> suddenly drop.  Until someone happens to get annoyed about that.
> 
> In any case, I haven't run the numbers, but I believe that we will run
> out of streams first, unless there are large resources (or a
> disproportionately large number of TLS records).

Right. I think the current text assumes that TLS libraries may
now or in future warn an application above that "it'll be time
to rekey soon." I don't think that's likely to happen, they'll
either rekey automagically or fail to de|encrypt I'd say. Not
that it'll happen of course. Until it does;-)

> 
>> - 9.2.1 - the renegotiation text is confusing. First para on
>> that starts with "MUST be disable" next one says "MAY use"
>> for client cert confid. I think adding something like "Other
>> than as noted below" before the start of  the 1st renego
>> para (the 3rd para of 9.2.1) might fix this, but could be
>> some more editing would be better.
> 
> Fair point.  I've had a bash at making it more appropriately
> conditional than that.

I see more discussion on that and am fine with whatever you
all conclude.

> 
>> - 9.2.2 - Isn't it sad that there are so many undesirable
>> TLS ciphersuites. Sorry about that;-)
> 
> Don't apologize.  Help in fixing it would be appreciated though :)

I did suggest a two-bit ciphersuite ID already I think:-)

> 
>> - 10.1 - I think this could be a lot clearer about which
>> pushed things are to be thrown away and I think being
>> clearer about that might avoid some future problems.
> 
> I think that I'm going to take Richard's expansion here.  Check on his
> DISCUSS responses for the outcome of that.
> 
>> - 10.5.1 has a few typos, no harm being nice to the RFC
>> editor and fixing those if you're pushing out a -17
> 
> Yep, a little cleanup always helps.
> 
>> - 10.7 - what general purpose padding does TLS1.2 provide?
>> I'm sad that this section is so negative - does that
>> indicate that people haven't really tried this out so much?
>> While it is clear there can be failed attempts at using
>> padding to thwart traffic analysis I think having this
>> mechanism defined so we can in future learn how to better
>> mitigate traffic analysis is a good thing and we ought not
>> be so down on that.
> 
> TLS 1.2 only pads to block boundaries, for some ciphers.  However,
> this is an optimistic forward reference based on a thin hope that TLS
> might provide a generic record padding mechanism, like
> draft-pironti-....  I still hope for that to come about.

Me too, but I'm not sure this ought look forward to that
outcome quite so explicitly.

Cheers,
S.

> 
>> - For the record, I'm also sad that this isn't all and only
>> over TLS with the option of opportunistic security for http:
>> schemed URIs but I accept that the wg debated that and
>> decided for this.
> 
> You are not alone in being sad.  You can take consolation in the fact
> that it's not being implemented by some.
> 

Received on Monday, 26 January 2015 22:08:13 UTC