Re: [jose] Pete Resnick's Discuss on draft-ietf-jose-json-web-encryption-33: (with DISCUSS and COMMENT)

[Mike Jones <[e-mail]>]

Thanks for your review, Pete.  I've added the working group so they're aware of your comments.

> -----Original Message-----
> From: Pete Resnick [mailto:[e-mail]]
> Sent: Thursday, October 02, 2014 7:24 AM
> To: The IESG
> Cc: [e-mail]; draft-ietf-jose-json-web-
> [e-mail]
> Subject: Pete Resnick's Discuss on draft-ietf-jose-json-web-encryption-33: (with
> DISCUSS and COMMENT)
> 
> Pete Resnick has entered the following ballot position for
> draft-ietf-jose-json-web-encryption-33: Discuss
> 
> When responding, please keep the subject line intact and reply to all email
> addresses included in the To and CC lines. (Feel free to cut this introductory
> paragraph, however.)
> 
> 
> Please refer to http://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> http://datatracker.ietf.org/doc/draft-ietf-jose-json-web-encryption/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> 3.1: Like JWS, why can't I have unprotected header in compact serialization.
> Seems like a serious problem.

Same answer as for JWS.  I presume that you're willing with withdraw this DISCUSS as well, like the one on JWS discussed during the telechat?

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> There are a bunch of comments here that also apply to JWS. It's a real shame
> you did not factor out what's common between these documents and not
> repeat everything in a slightly different way. I'm sure that there will be a change
> in one document that won't get changed in the other.
> Such is life.

Where text was previously actually the same, we editorially left in in JWS and referenced it from JWE some time ago.  If there are examples remaining of duplicated text that you believe should be removed from JWE, please let us know what they are.

> 1: Lose the last paragraph. It's not a core goal. Certainly not in the charter.

Producing a compact URL-safe serialization has always been a core goal.  I'm writing this in an airplane without network access (yes, such places still exist :-) ) and don't have the actual charter text in front me but I know that Karen O'Donoghue sent proposed charter text to the IESG a while back that included the language "using a compact URL-safe representation" because I have a local copy of the e-mail.

> 3: "base64url encoded for transmission"? Why is "transmission" in here?
> These things are in base64url independent of transmission aren't they?

Thanks.  I agree that the "for transmission" language is unnecessary verbiage.

> 3.1: A pointer to section 7.1 is probably appropriate here.

OK

> 3.2:
> 
>    In the JWE JSON Serialization, a JWE object is represented as the
>    combination of these eight values...
> 
> That's not true, and potentially confusing. In the JSON Serialization, the JWE
> object is represented as a JSON object with 8 members:
> 
>   protected, whose value is BASE64URL(UTF8(JWE Protected Header)),
>   unprotected, whose value is JWE Shared Unprotected Header...
> 
> etc.

I'm not certain what specific confusion you're citing.  Is it that some of those 8 values may be absent?  If so, I suppose we could say "potential values" rather than "values".  Or if you have other proposed wording, I'd be glad to hear it.

> 3.3: As in JWS:
> 
> - "If this were part of an Protected Header, it would be encoded as..."
> 
> Please show the regular JSON Serialization, not just the compact. The compact is
> lousy as an overview example. If you want to add the compact after the JSON,
> that's OK.

The same answer applies as for the parallel comment on JWS.

> 4.1.2: So I want to understand the "MUST reject" as used here. What happens if
> the implementation doesn't reject? Is the decrypt simply going to fail (in which
> case the MUST reject is not helpful), or is there some attack vector here?

This was discussed on the telechat and several people were in favor leaving this language as-is, citing precedents from other RFCs.  I believe you said that you would identify any particular uses that you thought would be problematic.

> 5.1:
> 
> It looks like steps 1, 2, 3, and 6 are actually one big step. They should probably
> be combined and clarified in some way.

Actually, all of 2-6 are steps dependent upon the key management mode used to determine and encode the CEK value.  It's not clear to me, editorially, that combining a bunch of simple steps with a single action into a bigger combined step with multiple conditional actions would result in a clearer specification.

> 4 and 5 could be reasonably combined.

Their actions are different.

> 18 is not a step.

I agree with you.  As with the parallel comment in JWS, I'll think about how to revise this to either make the step actionable or eliminate it.

> 20 - Delete everything after the first sentence (and optionally point to section 7).

Same answer as for the parallel comment on the JWS spec.

> 5.2: If these things are steps, start with the verb. See 2 below as an example, but
> look at the other steps as well.

Fair enough.  I agree that these should either start with a verb or a condition, followed by a verb.  I'll plan to revise accordingly.

>    1 - Say what to parse out and refer to section 7. Again, don't privilege
> compact.

Same answer as for the parallel comment on the JWS spec.

>    2 - Make the verb first:
> 
>         Decode the base64url encoding of JWE Protected Header, the JWE
>         Encrypted Key, the JWE Initialization Vector, the JWE
>         Ciphertext, the JWE Authentication Tag, and the JWE AAD. The
>         decoding MUST follow the restriction that no padding characters
>         have been used.

Good - thanks for the specific proposed wording.

>    4 could be significantly shortened if you didn't separate the serializations. Just
> make it the union. If some of the members are absent (for whatever reason),
> that's fine.

I disagree with combining them, as the explanation would be far less clear to developers.

>    4 and 5 should be combined.

I agree with you that checking for duplicate names actually has to happen during 4, so I'll look at combining them as you suggest.

>    9-12 seem like they could be combined/refactored in some useful way.

Same answer as to your comment on 5.1.

>    (I'll also note here that the "recipient" construct is still weird to me. It's just a
> "key owner" or something like that, right?)

This was the terminology that was chosen by the working group.  I'm offline at the moment so can't check immediately, but I believe that it was also terminology used by CMS.  I *know* that it was terminology used by ekr and Joe Hildebrand in their submission to the working group, parts of which (including language in the steps) are part of this specification.

>    15 and 16 - Ditch the parenthetical.

I disagree.  These instructions are written to be as clear to developers as possible, so they're more likely to build correct implementations.  Including a few bits of pertinent information in context when relevant, even if redundant with other parts of the spec, seems like a reasonable editorial practice.

>    19 is kind of silly. If decryption failed, it failed. No need to additionally "reject"
> (whatever that is supposed to mean). The second and third sentences are just
> implementation detail. Delete 19.

Actually, no, it can succeed for some recipients and fail for others.  That's the point of this step.  The "implementation detail" is there to ensure that implementations let applications know which succeeded and which failed in the case when there can be multiple recipients.

> 9: It took me a bit to figure out why this section is here. This is only a problem
> for the Compact Serialization. The second bullet makes this
> clear: For the JSON Serialization, the answer is, "They're different if they're
> different." I suggest rewriting this section to make it clear that you're trying to
> help folks who need to distinguish between compact serializations.

It's also needed for the JSON Serializations, hence bullet 2.  Bullets 3 and 4 also apply to both serializations.  Only bullet 1 is specific to the compact serializations.

> Appendix A: Leaving the JSON Serialization until the end and putting a compact
> serialization in every example stinks. Let's not make it harder for implementers
> to figure out how to use the JSON Serialization.

Other than the last example, the examples are there to describe properties of algorithms, not of serializations, so both serializations aren't needed to illustrate those points.  The simpler compact serialization does just fine for that purpose.  That being said, draft-ietf-jose-cookbook is chock full of examples of both serializations.  It should be coming to the IESG pretty soon as well.

				Thanks again,
				-- Mike