From: Mike Jones <Michael.Jones@microsoft.com>
To: Carsten Bormann <cabo@tzi.org>
Thread-Topic: [apps-discuss] Appsdir review for
 draft-ietf-jose-json-web-algorithms-33
Thread-Index: Ac/oVSxcdPW/TNiPTRiX1p7gtScCIgACOMIAAD/dNXAAAX+S8A==
Date: Thu, 16 Oct 2014 17:09:05 +0000
Message-ID: <4E1F6AAD24975D4BA5B16804296739439BB139D6@TK5EX14MBXC286.redmond.corp.microsoft.com>
References: <4E1F6AAD24975D4BA5B16804296739439BB0FB8B@TK5EX14MBXC286.redmond.corp.microsoft.com>
 <AD74AEDE-AC3E-46D9-A6C7-99B009548D26@tzi.org> 
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Received-SPF: Pass (protection.outlook.com: domain of microsoft.com designates
 131.107.125.37 as permitted sender)
 receiver=protection.outlook.com; 
 client-ip=131.107.125.37; helo=mail.microsoft.com;
Archived-At: http://mailarchive.ietf.org/arch/msg/jose/lsZZawJ9mTAlwd6UcecDgGQTCrU
Cc: "draft-ietf-jose-json-web-algorithms.all@tools.ietf.org"
 <draft-ietf-jose-json-web-algorithms.all@tools.ietf.org>,
 "jose@ietf.org" <jose@ietf.org>, "iesg@ietf.org" <iesg@ietf.org>,
 "apps-discuss@ietf.org" <apps-discuss@ietf.org>,
 Matt Miller <mamille2@cisco.com>
Subject: Re: [jose] [apps-discuss] Appsdir review for
 draft-ietf-jose-json-web-algorithms-33
Precedence: list

Adding one more reply below - this time new proposed text from Matt Miller.=
..

-----Original Message-----
From: Mike Jones=20
Sent: Thursday, October 16, 2014 9:53 AM
To: 'Carsten Bormann'
Cc: apps-discuss@ietf.org; draft-ietf-jose-json-web-algorithms.all@tools.ie=
tf.org; Matt Miller; iesg@ietf.org; jose@ietf.org
Subject: RE: [apps-discuss] Appsdir review for draft-ietf-jose-json-web-alg=
orithms-33

Replies to your replies are inline below.  Thanks again for your thoughtful=
 and detailed review.

				-- Mike

> -----Original Message-----
> From: Carsten Bormann [mailto:cabo@tzi.org]
> Sent: Wednesday, October 15, 2014 2:55 AM
> To: Mike Jones
> Cc: apps-discuss@ietf.org; draft-ietf-jose-json-web-=20
> algorithms.all@tools.ietf.org; Matt Miller; iesg@ietf.org;=20
> jose@ietf.org
> Subject: Re: [apps-discuss] Appsdir review for=20
> draft-ietf-jose-json-web-
> algorithms-33
>=20
> Hi Mike,
>=20
> thanks for getting to this review - I don't envy you for this herculean j=
ob.
> A few more comments/clarifications inline below.
>=20
> Gr=FC=DFe, Carsten
>=20
> On 15 Oct 2014, at 10:51, Mike Jones <Michael.Jones@microsoft.com> wrote:
>=20
> > Thanks for your review, Carsten.  I apologize for not responding to=20
> > it until
> now.  It had gotten sorted into a folder and I wasn't aware of it=20
> until Kathleen brought it to my attention.  Replies are inline below...
> >
> >> -----Original Message-----
> >> From: apps-discuss [mailto:apps-discuss-bounces@ietf.org] On Behalf=20
> >> Of Carsten Bormann
> >> Sent: Thursday, October 02, 2014 12:22 AM
> >> To: apps-discuss@ietf.org; draft-ietf-jose-json-web-=20
> >> algorithms.all@tools.ietf.org
> >> Cc: iesg@ietf.org; jose@ietf.org
> >> Subject: [apps-discuss] Appsdir review for
> >> draft-ietf-jose-json-web-algorithms-
> >> 33
> >>
> >> I have been selected as the Applications Area Directorate reviewer=20
> >> for this draft (for background on appsdir, please see=20
> >> http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirec
> >> to
> >> rate
> >> ).
> >>
> >> Please resolve these comments along with any other Last Call=20
> >> comments you may receive. Please wait for direction from your=20
> >> document shepherd or AD before posting a new version of the draft.
> >>
> >> Document:  draft-ietf-jose-json-web-algorithms-33
> >> Title: JSON Web Algorithms (JWA)
> >> Reviewer: Carsten Bormann
> >> Review Date: 2014-10-02
> >> IESG Telechat date: 2014-10-02
> >>
> >> Summary: This draft is ready for publication as a standards track=20
> >> RFC, with a few nits corrected.
> >>
> >> However, some additional editorial improvements might improve the=20
> >> security outcome when it is referenced by application developers.
> >>
> >> Major issues: None.
> >>
> >> Minor issues:
> >>
> >> 5.2:
> >> Add a reference that defines PKCS #7 padding.
> >
> > I'll plan on adding a reference to RFC 2315 for this.
>=20
> That would work (section reference would be 10.3) Alternatively, you=20
> could reference RFC 5652 (section 6.3), which is an Internet Standard (ST=
D70).
>=20
> >> 5.2.2.2
> >> Does "the PKCS #7 padding is removed" entail checking all of its bytes=
?
> >
> > Should this be changed to "the PKCS #7 padding bytes are checked and=20
> > then
> removed"?
>=20
> I think it is good to alert the reader to this need.
>=20
> >> 6.2.1
> >> Is the intention that the sentence containing "point compression is=20
> >> not supported" also applies to any future registered value of "crv"?
> >> A similar comment applies to other specifications in 6.2.1.x, e.g.,=20
> >> the reference to SEC1 representation to x and y.
> >
> > It would apply to any future curves registered that use the "x", "y"=20
> > point
> representation.  Others could define new key types that use a=20
> different representation or we could refine the definition of=20
> "kty":"EC" to make the representation specific to the "crv" value.
> >
> > A discussion happened on the JOSE thread "JWK Elliptic Curve key
> representations and new curves".  The conclusion suggested by Richard=20
> Barnes and seconded by Stephen Farrell was to table defining any new=20
> key representations for new elliptic curves until the CRFG=20
> recommendations to TLS have been made.  That ended the discussion for now=
.
>=20
> Right, I didn't want to anticipate that discussion; I would be happy=20
> if it didn't appear that the avenue for any evolution on point=20
> compression/compact points were closed.

I can add language saying that future curve registrations may use different=
 parameters to represent the curve value.  That may get a little convoluted=
 because currently it's the "kty" that determines which parameters are used=
.  But I agree with trying to accommodate new key representations for new c=
urves.

> >> 6.2.1.1
> >> =BBAdditional "crv" values MAY be
> >> used, provided they are understood by implementations using that=20
> >> Elliptic Curve key.=AB How are conflicts between such implementation=20
> >> defined values and future registered values handled?
> >
> > Conflicts are avoided using the IANA JSON Web Key Elliptic Curve=20
> > registry
> defined in Section 7.6.
>=20
> Oh, but that's not what the text says.  I read this as a general=20
> license to use random locally defined crv values.  Maybe the need for=20
> registration needs to be clarified.

I'll delete the sentence "Additional 'crv' values MAY be used, provided the=
y are understood by implementations using that Elliptic Curve key."  The re=
gistry is already talked about in the previous sentences.

> >> 6.3.2:
> >> The MAY accept partially overrides the MUST include?
> >> Is the latter thus really a SHOULD?
> >
> > "d" is REQUIRED.  The others SHOULD be included, and if any are=20
> > included, the
> others MUST be included.  Those are requirements on producers.
> >
> > Consumers may choose to accept keys that don't include all the CRT
> parameters.  Given that they can be computed from "n", "e", and "d"=20
> and sometimes they're not available, a previous reviewer had asked=20
> this language about consumers to be included.  It's a case of "Be=20
> conservative in what you send, be liberal in what you accept".
>=20
> [I'm not inserting my usual rant here that this bon mot may apply to=20
> implementations, but never to specifications.]
>=20
> > It's not clear to me that there's any value in relaxing the=20
> > requirements on
> producers.
>=20
> Well, if the producer MUST send "all of the others" if "any of the other"
> parameters are present, it is not clear to me what the point of "MAY choo=
se
>    to accept an RSA private key that does not contain a complete set of
>    the private key parameters other than "d"" is other than enabling=20
> producers to send just that.
> (More importantly, it is not clear when a producer should be taking=20
> that license.)

I can delete the sentence " The consumer of a JWK MAY choose to accept an R=
SA private key that does not contain a complete set of the private key para=
meters..."

> >> 7.1:
> >> It is interesting that a mere registration (vetted only by a DE)=20
> >> can change the IETF consensus base specifications by making an=20
> >> algorithm
> "Required".
> >
> > It is expected that the designated experts will be appointed, in=20
> > part, for their
> cryptographic expertise.
>=20
> There has been more discussion about the requirements levels here that=20
> I don't want to repeat.
> My comment here was that a DE (even with the best crypto knowledge) is=20
> in a hard place updating the consensus about *desirability of=20
> implementation* - there are more aspects to this than the technical ones.

Thinking about this some more, I agree that "Required" is a special case.  =
I'm thinking that a reasonable safeguard for this case is to require that a=
pproval of both a designated expert and a sitting security area director be=
 required to make an algorithm "Required" or to change it from "Required" t=
o something else.  Kathleen and Carsten, would that work for you?

> >> 8.
> >> I am unable to find a "security considerations" section in NIST SP 800=
-38A.
> >> 800-38D at least has a "practical considerations" section, is that mea=
nt?
> >> (Etc., I haven't checked all the references.) In general, I believe=20
> >> a security considerations section is most useful where it provides=20
> >> more directed guidance instead of saying the equivalent of "here is a =
textbook".
> >
> > There are 14 subsections with directed guidance, plus more in the=20
> > JWS, JWE,
> and JWK specifications.  If there is other directed guidance you'd=20
> like to see added, please supply proposed text.
> >
> > Having skimmed through it, I agree that 800-30A doesn't contain=20
> > clear enough
> security guidance to merit inclusion in the list.  I'll remove it.  If=20
> there are other specifications that you'd like to see added or removed, p=
lease let me know.
>=20
> This was more of a general comment that "Here is a reference to a textboo=
k"
> type security considerations are less useful than specific, directed=20
> references.  I haven't checked all those referenced documents, so I'm=20
> not in a position to recommend changes here.  Maybe something for a=20
> checklist to apply to the next security spec.
>=20
> >
> >> 8.7 is not clear: is it NOT RECOMMENDED to reuse an entire set of=20
> >> key material (including IV), or to reuse any part of it?
> >
> > I agree that this is ambiguous, as worded.  Its advice seems to be=20
> > so broad as
> to be impractical, as never reusing a key means that a new key would=20
> have to be redistributed for each encryption, which then requires a=20
> key to use for the distribution...  This text came from=20
> http://tools.ietf.org/html/draft-miller-jose-
> jwe-protected-jwk-02#section-8.1, which was incorporated into the=20
> document after a working group decision to do so.  Matt Miller, you=20
> were the author of this text.  Could you clarify what the intent was?  Th=
anks.

Matt proposed the following revised text.  I agree that this is significant=
ly clearer and more actionable than the previous version:

It is NOT RECOMMENDED to reuse the same entire set of key material (Key Enc=
ryption Key, Content Encryption Key, Initialization Vector,
etc.) to encrypt multiple JWK or JWK Set objects, or to encrypt the same JW=
K or JWK Set object multiple times.  One suggestion for preventing re-use i=
s to always generate at least one piece of key material for each encryption=
 operation (e.g., a new Content Encryption Key, a new Initialization Vector=
, and/or a new PBES2 Salt), based on the considerations noted in this docum=
ent as well as from RFC 4086 [RFC4086].

> >> Nits/editorial comments:
> >>
> >> 6.3.2.x:
> >> The constant repetition of =BBIt is represented as the base64url=20
> >> encoding of the value's unsigned big endian representation as an=20
> >> octet
> sequence.
> >> The octet sequence MUST utilize the minimum number of octets to=20
> >> represent the value.=AB almost ensures that an implementer will stop=20
> >> reading the details (well, I did, and I did not write a program to=20
> >> verify the same phrase is used everywhere; if any parameter were=20
> >> using a different encoding, that sure would be missed).  Why not=20
> >> define
> another abstraction like base64url and use this?
> >
> > I did just verify that they are all consistent.  Yes, if you're=20
> > reading the spec
> linearly I'm sure it feels repetitive, but if you're an implementer=20
> going straight to a definition to implement it, having a complete=20
> description all in one place is valuable.  I understand your request=20
> but it's not clear to me that this rises to the level that a separate=20
> definition that the developer then has to go find is warranted.
>=20
> My point was corroborated by the tiny error Richard found.  Repeating=20
> the same information again and again drowns out the important=20
> information.  A single paragraph at the end of 6.3's intro of the form:
>=20
>    Each of the positive integer parameters defined in this section
>    is represented as the base64url encoding
>    of the value's unsigned big endian representation as an octet
>    sequence.  The octet sequence MUST utilize the minimum number of
>    octets to represent the value.

OK - I'll see what I can do in this regard.  Editorially, it's a little bit=
 complicated because there are different subsections for public and private=
 key parameters.

> Note that the repetitive text already has a reference that needs to be=20
> looked up in that it uses the term "base64url encoding", which has a=20
> slightly refined meaning here from that in RFC 4648.  So an even=20
> better way to handle this would be to add a new term "posint encoding"=20
> or some such to section 1.1, turning e.g. the entirety of 6.3.2.2 into
>=20
>            The "p" (first prime factor) member contains the first=20
> prime factor, in POSINT encoding.
>=20
>=20
> >
> >> 6.2.3.1: This is not a positive integer?  6.2.3.x mentions this otherw=
ise.
> >
> > I may not understand this remark.  I'm guessing you're referring to=20
> > 6.3.2.1 and
> the fact that the description includes the word "unsigned".  This is=20
> there to make it clear that no sign bit will be present in the=20
> representation - which is especially important since the high-order=20
> bit is always set for "n" and "d" for correctly generated keys.
>=20
> (Sorry, this is indeed about 6.3.2.x, not 6.2.3.x.) That was not what=20
> I meant:  All 6.3.2.x have the form =BBThe "p" (first prime factor)=20
> member contains the first prime factor, a positive integer.=AB
> 6.3.2.1 does not have the ", a positive integer", but I can't see a=20
> reason for that (d clearly cannot be zero, and it is still an unsigned va=
lue).
> This is likely to confuse an implementer that reads this text closely.
> (And I'm offering the fact that you are overlooking that inconsistency=20
> as another corroboration of my previous point.)

I can delete the unnecessary phrase "a positive integer", subject to how it=
 does or doesn't fit into the edit above that you asked for.

> >> 7.1.1
> >> =BBExample description=AB is not a useful example for an "Algorithm De=
scription".
> >> (Same comment for 7.x.1.)
> >
> > See the actual registrations in Section 7.1.2 for more useful examples.
>=20
> Maybe just take out the not so useful examples in 7.x.1?

All other registration templates that I have seen include examples of all p=
arameters.  This is only one of the parameters in the template, and all nee=
d to be retained.  Would you be happier with the e.g. text "Example algorit=
hm"?

> >> 8.3:
> >> s/because it/because it is/
> >
> > Good catch
> >
> >> [sec1]
> >> (Given the date, this is probably referencing V2.0 of this spec.)
> >
> > Hmmm - the version I have cached locally came from
> http://www.secg.org/collateral/sec1_final.pdf and is dated September=20
> 20, 2000.  But the site appears to be down at present.  The version=20
> referenced by Wikipedia at=20
> http://www.secg.org/download/aid-385/sec1_final.pdf is missing too.=20
> http://secg.org/download/aid-780/sec1-v2.pdf is missing too.  Can you poi=
nt me to a good link?
>=20
> I just had a look and embarrassingly found that the copy I'm using=20
> comes from the personal space of a researcher:
> http://perso.univ-rennes1.fr/sylvain.duquesne/master/standards/sec1-v2
> .pdf
> Not very satisfying as a reference.
>=20
> >
> > RFC 5915 includes this reference:
> >
> >   [SECG1]    Standards for Efficient Cryptography Group (SECG), "SEC 1:
> >              Elliptic Curve Cryptography", Version 2.0, May 2009.
>=20
> That looks good: We don't need a URL, as long as we are certain which=20
> version we are referencing.
>=20
> > Worst comes to worst, I'll either reference that or explicitly=20
> > reference the 1.0
> version.  But if I'm going to reference the 2.0 version, I'd like to=20
> be able to read a copy so that I can verify that the way we're using=20
> it is consistent with the actual spec, including the section numbers.
> >
> >> [usascii]
> >> The reference to ANSI X3.4:1986 should probably be replaced by a=20
> >> reference to RFC 20.  There is little reason to reference a=20
> >> somewhat hard to obtain external document ($60!) when we have an=20
> >> RFC about the
> same subject.
> >
> > OK
> >
> >> (Tables in Appendix A need some formatting.)
> >
> > Agreed.  It's on my to-do list.  Jim Schaad recently told me to "Add=20
> > a width
> attribute to the ttcol with either "digits" (for fixed width) or=20
> "digits%" (for percent width) to the column in question".  I plan to give=
 it a try.
> >
> >> _______________________________________________
> >> apps-discuss mailing list
> >> apps-discuss@ietf.org
> >> https://www.ietf.org/mailman/listinfo/apps-discuss
> >
> > 				Thanks again,
> > 				-- Mike
> >
> >
> >