Gen-ART and OPS-Dir review of draft-ietf-httpbis-header-compression-10

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@emc.com        Mobile: +1 (978) 394-7754
----------------------------------------------------

Received on Wednesday, 14 January 2015 08:10:02 UTC