Re: 2nd Working Group Last Call: draft-ietf-httpbis-encryption-encoding-03.txt

[Martin Thomson <[e-mail]>]

Thanks for the review Julian.

Thanks for making your comments actionable.  And sorry it took so
long, illness and other work conspired against me.

PR: https://github.com/httpwg/http-extensions/pull/249

I've just responded with commit IDs below if I agreed with your
comment and thought that it needed no discussion.

On 13 October 2016 at 20:54, Julian Reschke <[e-mail]> wrote:
>    The "aesgcm" content coding uses a fixed record size.  The resulting
>    encoding is either a single record, or a series of fixed-size
>    records.  The final record, or a lone record, MUST be shorter than
>    the fixed record size.
>
> That seems incorrect. If it's "either a single record or a series of
> fix-sized records", how then can then last record be shorter?

How about:
 The "aesgcm" content coding uses a fixed record size.  The resulting
encoding is
 any number of fixed-size records - which could be zero records - followed by a
 single partial record.  The partial record MUST be shorter than the
fixed record
 size.
[ddf5f8e]

> 3.  The Encryption HTTP Header Field
>
>    If the payload is encrypted more than once (as reflected by having
>    multiple content codings that imply encryption), each application of
>    the content coding is reflected in a separate Encryption header field
>    value in the order in which they were applied.
>
> I believe we need to clarify the precise interaction between new content
> codings and this header field. I'll assume any new content coding needs to
> opt-in to use this field value as well, so it's clear how to remove entries
> when unwrapping codings.

I think that I have some text that will help.  I think that the answer is:

Content codings that use the Encryption header field MUST always include a
value for the header field when the content coding has been applied.  If no
parameters are needed, then a dummy value is necessary to avoid confusion about
which set of parameters applies to which content coding.  This requirement
applies to uses of the `aesgcm` content coding, which does not need a dummy
value because the `salt` parameter is mandatory.
[97b3c12] and [67b65df]

>    Encryption header field values with multiple instances of the same
>    parameter name are invalid.
>
> ...of the same parameter name in a single encryption_params production...
[a0c51a2]

> (which reminds me that you can't have '_' in ABNF production names)
[1b376a8]

>    Crypto-Key header field values with multiple instances of the same
>    parameter name are invalid.
[a0c51a2]

> 5.1.  Encryption of a Response
>
>    Here, a successful HTTP GET response has been encrypted using input
>    keying material that is identified by a URI.
>
> by a URI? (leftover from earlier versions?)
[bd0eef2]

>    Note that the media type has been changed to "application/octet-
>    stream" to avoid exposing information about the content.
>
> Maybe mention the alternative of not sending it at all.
[4e510be]

> 6.1.  Key and Nonce Reuse
>
>    (...)
>
>    If a content encryption key is reused - that is, if input keying
>    material and salt are reused - this could expose the plaintext and
>    the authentication key, nullifying the protection offered by
>    encryption.  Thus, if the same input keying material is reused, then
>    the salt parameter MUST be unique each time.  This ensures that the
>    content encryption key is not reused.  An implementation SHOULD
>    generate a random salt parameter for every message; a counter could
>    achieve the same result.
>
> So maybe the requirement is uniqueness, not randomness?

Yes, and it says that just above: "the salt parameter MUST be unique
each time".  The "SHOULD" here is advice (good advice I believe).

> Maybe [the abstract] should mention that it also adds a mechanism to parametrize
> content codings in general?

I don't believe that it does, or should.  See earlier email about
crypto being special (though we might argue that point if you like).

> 3.1.  Encryption Header Field Parameters
>
>   salt:  The "salt" parameter contains a base64url-encoded octets
>       [RFC7515] that is used as salt in deriving a unique content
>       encryption key (see Section 3.2).  ...
>
> As "base64url" is defined upfront, citing here RFC7515 is a bit confusing
> (as when you follow the reference, you end up with a spec that happens to
> include that definition, but it largely about something else)
[63850db]

>       cek_info = "Content-Encoding: aesgcm" || 0x00 || context
>
> I was a bit confused by this until I realized that it's pseudo-code, not
> ABNF. Maybe "concat(....)" would be clearer than "||" (which, in the
> languages I use, definitively does not mean concatenation).

Maybe I can just explain it.  || is more compact.
[80225a4]

> maybe ...value *that* is produced...
[7f3a35c]

> ...*a* context...
[31c4bc4]

> Might be better to cite the slightly newer
> <https://www.w3.org/TR/2013/REC-xmlenc-core1-20130411/>. In any case, please
> add the W3C short name to the series element, such as in:
[8ea4739]

Interesting thing I learned from this: there are rules for "fixing"
non-ASCII names in xml2rfc.