This is a combined Gen-ART and OPS-Dir review. Boilerplate for both follows ... I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at: <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. Please resolve these comments along with any other Last Call comments you may receive. I have reviewed this document as part of the Operational directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the operational area directors. Document editors and WG chairs should treat these comments just like any other last call comments. Document: draft-ietf-httpbis-header-compression-10 Reviewer: David Black Review Date: Jan 12, 2015 IETF LC End Date: Jan 14, 2015 IESG Telechat date: Jan 22, 2014 Summary: This draft is on the right track, but has open issues described in the review. The draft describes a new compression format for HTTP, HPACK. The draft is generally well-written although not the easiest read due to the amount of detail necessary to specify this compression format. I did not check the examples in Appendix C in detail. I found a couple of major issues, and a bunch of minor one, plus the usual crop of nits. The first major issue involves the dense packing of static and dynamic table indices, and what appears to be an inability to ever change this and HPACK in general (if that's a "feature," an explanation of why is in order). The second major issue is that I can't find the list of fields that are required to use the never-indexed syntax for security reasons. All of the minor issues should be straightforward to resolve. For the OPS-Dir review, most of the operational considerations are for the HTTP version 2 protocol that uses this compression format, and hence belong in the HTTP version 2 draft, and not here. Major issues: -1- Section 2.3.3 Indices between 1 and the length of the static table (inclusive) refer to elements in the static table (see Section 2.3.1). Indices strictly greater than the length of the static table refer to elements in the dynamic table (see Section 2.3.2). Having the dynamic table indices start immediately after the static table indices is asking for problems if the static table is ever extended. This dense use of table indices looks like it was done to pack as much as possible into the index range 1-127, which has an efficient 1-byte representation in HPACK, but the static table can never be changed without going to a completely new HPACK format. I think the real concern here is what the intentions are for revising HPACK in the future, including expansion of the static table, and how the resulting multiple versions of HPACK would be signaled. My sense from this draft is that HPACK cannot be changed, ever, period - I'm not sure whether that's a wise design choice - it definitely needs to be explained. -2- Where is the list of fields to which the never indexed representation MUST be applied for security reasons? Minor issues: -A- Section 1.3: Dynamic Table: The dynamic table (see Section 2.3.2) is a header table used to associate stored header fields to index values. This table is dynamic and specific to an encoding or decoding context. Need to define "header table" before using it in this definition, or point to the discussion of the term in Section 1. -B- Section 4.2 This paragraph is unclear on what has to be communicated: Multiple updates to the maximum table size can occur between the sending of two header blocks. In the case that the value of this parameter is changed more than once, if any changed value is smaller than the new maximum size, the smallest value for the parameter MUST be sent in an encoding context update. The altered maximum size is always sent, resulting in at most two encoding context updates. This ensures that the decoder is able to perform eviction based on the decoder table size (see Section 4.3). I suggest: Multiple updates to the maximum table size can occur between the sending of two header blocks. In the case that this size is changed more than once in this interval, the smallest maximum table size that occurs in that interval MUST be sent in an encoding context update. The final maximum size is always sent, resulting in at most two encoding context updates. This ensures that the decoder is able to perform eviction based on reductions in decoder table size (see Section 4.3). -C- Section 4.4: This paragraph is unclear on whether eviction occurs before or after adding an entry: Whenever a new entry is to be added to the dynamic table, entries are evicted from the end of the dynamic table until the size of the dynamic table is less than or equal to (maximum size - new entry size), or until the table is empty. I suggest inserting "(before the new entry is added)" after "until the size of the dynamic table" -D- Section 4.4: If the representation of the added entry references the name of an entry in the dynamic table, the referenced name is cached prior to performing eviction to avoid having the name inadvertently evicted. Cached where and how? Please explain. -E- Section 5.1 N is supposed to be the number of bits in the prefix, which makes the use of N in "Value (N)" incorrect in Figure 2. I think just deleting "(N)" in the figure will fix this. -F- Section 7.1.3 This section applies only to intermediaries that are aware of HPACK (and presumably use it). That should be stated, along with a reminder that an HPACK-unaware intermediary that does HPACK-unaware compression may create vulnerabilities to attacks like CRIME. Nits/editorial comments: -- Section 1: As Web pages have grown to include dozens to hundreds of requests, "include dozens to hundreds of requests" -> "require dozens to hundreds of requests to retrieve" -- Section 1.3: Header Field: A name-value pair. Both the name and value are treated as opaque sequences of octets. Indicate what header or headers these fields come from. Static Table: The static table (see Section 2.3.1) is a header table used to associate static header fields to index values. "associate static header fields" -> "statically associate commonly used header fields" -- Section 2.2: To decompress header blocks, a decoder only needs to maintain a dynamic table (see Section 2.3.2) as a decoding context. No other state is needed. "state is needed" -> "dynamic state is needed" The static table is part of decoding state, it just doesn't change. -- Section 2.4: The rationale for the additional format that forbids ever encoding a field should be repeated here (it's stated in Section 2.3). -- Section 3.1: Once a header field is decoded and added to the reconstructed header list, it cannot be removed from it. "it cannot be removed from it" -> "the field cannot be removed" -- Section 5.1: I found this text hard to read because the figures come before the text that explains them. At a minimum add "as shown in Figure X" to the paragraphs after both figures 2 and 3 (for X=2 and X=3). idnits complained that it couldn't find an IANA Considerations section. Please add an empty one (stating that there are no IANA Considerations) if/when the draft is revised. --- Selected RFC 5706 Appendix A Q&A for OPS-Dir review --- A.1. Operational Considerations These are almost completely missing. They should be in the main HTTP version 2 draft, which I have not reviewed as part of this review. The HPACK format appears to have been designed to never be changed. I hope that's a well-thought out choice - see major issue -1-. A.2. Management Considerations These are N/A - they would apply to the main HTTP version 2 protocol which discusses use of HPACK. A.3. Documentation Obviously, the HTTP version 2 protocol will have a major operational impact on the Internet, but that's a matter for the HTTP version 2 draft, not this one. The shepherd writeup refers obliquely to interop events, so I infer that this compression format has been implemented and at least "smoke-tested." Thanks, --David ---------------------------------------------------- David L. Black, Distinguished Engineer EMC Corporation, 176 South St., Hopkinton, MA 01748 +1 (508) 293-7953 FAX: +1 (508) 293-7786 david.black@xxxxxxx Mobile: +1 (978) 394-7754 ----------------------------------------------------