Re: [jose] [apps-discuss] Appsdir review for draft-ietf-jose-json-web-algorithms-33

Kathleen Moriarty <kathleen.moriarty.ietf@gmail.com> Thu, 16 October 2014 21:15 UTC

Return-Path: <kathleen.moriarty.ietf@gmail.com>
X-Original-To: jose@ietfa.amsl.com
Delivered-To: jose@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B2A5B1A87AC; Thu, 16 Oct 2014 14:15:14 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.998
X-Spam-Level:
X-Spam-Status: No, score=-1.998 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, NORMAL_HTTP_TO_IP=0.001, SPF_PASS=-0.001] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xxM_PWbNA4lA; Thu, 16 Oct 2014 14:15:08 -0700 (PDT)
Received: from mail-la0-x22b.google.com (mail-la0-x22b.google.com [IPv6:2a00:1450:4010:c03::22b]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id D0F521A87A0; Thu, 16 Oct 2014 14:15:06 -0700 (PDT)
Received: by mail-la0-f43.google.com with SMTP id mc6so3704156lab.16 for <multiple recipients>; Thu, 16 Oct 2014 14:15:05 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=Pqz3aCOh4/pehJk/ysCyg977XVhyPcw9vbbMkum2hnw=; b=IuzGj6laAESwMHA1CMHXgImUr35W9snbkRnvZSezWw6TReEmQhwK/tXyiwRZfsdtbN E/X2B7BBWSqIhGFGwdXRQgHuVxpeydjKOx6Znm1GIKlcBri5AIj4BmFk803IltNppt24 8Og76fWHc41LELwAP0vX1m89wSdw03wRFxx91rHx3aImGmWWpeMC+PggNhq7M4ra50Qk 6WnDB3sGe6TwEi3R5f88eTSG6gPwwUhLkHLukg0+zlhMUlfNQRn4rFUR94pfeyD1BiGI ITHb980pomCrTmiqkzv7x2qMXOTN/lobdC0hJ3Q5wT8vThdAeNTMZ1P4D0jW9Fip5jh2 eqAA==
MIME-Version: 1.0
X-Received: by 10.152.87.171 with SMTP id az11mr4142474lab.97.1413494105076; Thu, 16 Oct 2014 14:15:05 -0700 (PDT)
Received: by 10.112.95.36 with HTTP; Thu, 16 Oct 2014 14:15:04 -0700 (PDT)
In-Reply-To: <CAHbuEH4p6QK2f9FF-fU7YJzggjXJG0=MM86G7dK5edRtyuVAFA@mail.gmail.com>
References: <4E1F6AAD24975D4BA5B16804296739439BB0FB8B@TK5EX14MBXC286.redmond.corp.microsoft.com> <AD74AEDE-AC3E-46D9-A6C7-99B009548D26@tzi.org> <4E1F6AAD24975D4BA5B16804296739439BB139D6@TK5EX14MBXC286.redmond.corp.microsoft.com> <CAHbuEH4p6QK2f9FF-fU7YJzggjXJG0=MM86G7dK5edRtyuVAFA@mail.gmail.com>
Date: Thu, 16 Oct 2014 17:15:04 -0400
Message-ID: <CAHbuEH41w0LXsUqn4iUKJxtYcXid+n4FPsWM0aJ31cYVqKA6Tw@mail.gmail.com>
From: Kathleen Moriarty <kathleen.moriarty.ietf@gmail.com>
To: Mike Jones <Michael.Jones@microsoft.com>
Content-Type: multipart/alternative; boundary="001a11c238ae067a8e050590be14"
Archived-At: http://mailarchive.ietf.org/arch/msg/jose/emFx8KpspsfSf4r6PYAQ0dGJ2uA
Cc: "apps-discuss@ietf.org" <apps-discuss@ietf.org>, "draft-ietf-jose-json-web-algorithms.all@tools.ietf.org" <draft-ietf-jose-json-web-algorithms.all@tools.ietf.org>, "iesg@ietf.org" <iesg@ietf.org>, "jose@ietf.org" <jose@ietf.org>, Matt Miller <mamille2@cisco.com>, Carsten Bormann <cabo@tzi.org>
Subject: Re: [jose] [apps-discuss] Appsdir review for draft-ietf-jose-json-web-algorithms-33
X-BeenThere: jose@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: Javascript Object Signing and Encryption <jose.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/jose>, <mailto:jose-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/jose/>
List-Post: <mailto:jose@ietf.org>
List-Help: <mailto:jose-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/jose>, <mailto:jose-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 16 Oct 2014 21:15:15 -0000

On Thu, Oct 16, 2014 at 4:27 PM, Kathleen Moriarty <
kathleen.moriarty.ietf@gmail.com> wrote:

> I'm just chiming in on one section, the designated review requirements
> right now.
>
> On Thu, Oct 16, 2014 at 1:09 PM, Mike Jones <Michael.Jones@microsoft.com>
> wrote:
>
>> Adding one more reply below - this time new proposed text from Matt
>> Miller...
>>
>> -----Original Message-----
>> From: Mike Jones
>> Sent: Thursday, October 16, 2014 9:53 AM
>> To: 'Carsten Bormann'
>> Cc: apps-discuss@ietf.org;
>> draft-ietf-jose-json-web-algorithms.all@tools.ietf.org; Matt Miller;
>> iesg@ietf.org; jose@ietf.org
>> Subject: RE: [apps-discuss] Appsdir review for
>> draft-ietf-jose-json-web-algorithms-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-
>> > algorithms.all@tools.ietf.org; Matt Miller; iesg@ietf.org;
>> > jose@ietf.org
>> > Subject: Re: [apps-discuss] Appsdir review for
>> > draft-ietf-jose-json-web-
>> > algorithms-33
>> >
>> > Hi Mike,
>> >
>> > thanks for getting to this review - I don't envy you for this herculean
>> job.
>> > A few more comments/clarifications inline below.
>> >
>> > Grüße, Carsten
>> >
>> > On 15 Oct 2014, at 10:51, Mike Jones <Michael.Jones@microsoft.com>
>> wrote:
>> >
>> > > Thanks for your review, Carsten.  I apologize for not responding to
>> > > it until
>> > now.  It had gotten sorted into a folder and I wasn't aware of it
>> > until Kathleen brought it to my attention.  Replies are inline below...
>> > >
>> > >> -----Original Message-----
>> > >> From: apps-discuss [mailto:apps-discuss-bounces@ietf.org] On Behalf
>> > >> Of Carsten Bormann
>> > >> Sent: Thursday, October 02, 2014 12:22 AM
>> > >> To: apps-discuss@ietf.org; draft-ietf-jose-json-web-
>> > >> 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
>> > >> for this draft (for background on appsdir, please see
>> > >> http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirec
>> > >> to
>> > >> rate
>> > >> ).
>> > >>
>> > >> Please resolve these comments along with any other Last Call
>> > >> comments you may receive. Please wait for direction from your
>> > >> 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
>> > >> RFC, with a few nits corrected.
>> > >>
>> > >> However, some additional editorial improvements might improve the
>> > >> 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.
>> >
>> > That would work (section reference would be 10.3) Alternatively, you
>> > could reference RFC 5652 (section 6.3), which is an Internet Standard
>> (STD70).
>> >
>> > >> 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
>> > > then
>> > removed"?
>> >
>> > I think it is good to alert the reader to this need.
>> >
>> > >> 6.2.1
>> > >> Is the intention that the sentence containing "point compression is
>> > >> 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.,
>> > >> the reference to SEC1 representation to x and y.
>> > >
>> > > It would apply to any future curves registered that use the "x", "y"
>> > > point
>> > representation.  Others could define new key types that use a
>> > different representation or we could refine the definition of
>> > "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
>> > Barnes and seconded by Stephen Farrell was to table defining any new
>> > key representations for new elliptic curves until the CRFG
>> > recommendations to TLS have been made.  That ended the discussion for
>> now.
>> >
>> > Right, I didn't want to anticipate that discussion; I would be happy
>> > if it didn't appear that the avenue for any evolution on point
>> > 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 curves.
>>
>> > >> 6.2.1.1
>> > >> »Additional "crv" values MAY be
>> > >> used, provided they are understood by implementations using that
>> > >> Elliptic Curve key.« How are conflicts between such implementation
>> > >> defined values and future registered values handled?
>> > >
>> > > Conflicts are avoided using the IANA JSON Web Key Elliptic Curve
>> > > registry
>> > defined in Section 7.6.
>> >
>> > Oh, but that's not what the text says.  I read this as a general
>> > license to use random locally defined crv values.  Maybe the need for
>> > registration needs to be clarified.
>>
>> I'll delete the sentence "Additional 'crv' values MAY be used, provided
>> they are understood by implementations using that Elliptic Curve key."  The
>> registry 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
>> > > 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"
>> > and sometimes they're not available, a previous reviewer had asked
>> > this language about consumers to be included.  It's a case of "Be
>> > conservative in what you send, be liberal in what you accept".
>> >
>> > [I'm not inserting my usual rant here that this bon mot may apply to
>> > implementations, but never to specifications.]
>> >
>> > > It's not clear to me that there's any value in relaxing the
>> > > requirements on
>> > producers.
>> >
>> > 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
>> choose
>> >    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
>> > producers to send just that.
>> > (More importantly, it is not clear when a producer should be taking
>> > that license.)
>>
>> I can delete the sentence " The consumer of a JWK MAY choose to accept an
>> RSA private key that does not contain a complete set of the private key
>> parameters..."
>>
>> > >> 7.1:
>> > >> It is interesting that a mere registration (vetted only by a DE)
>> > >> can change the IETF consensus base specifications by making an
>> > >> algorithm
>> > "Required".
>> > >
>> > > It is expected that the designated experts will be appointed, in
>> > > part, for their
>> > cryptographic expertise.
>> >
>> > There has been more discussion about the requirements levels here that
>> > I don't want to repeat.
>> > My comment here was that a DE (even with the best crypto knowledge) is
>> > in a hard place updating the consensus about *desirability of
>> > 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 approval of both a designated expert and a sitting security area
>> director be required to make an algorithm "Required" or to change it from
>> "Required" to something else.  Kathleen and Carsten, would that work for
>> you?
>>
>
> The AD is in the appeals line, so that doesn't quite work:
> http://tools.ietf.org/html/draft-leiba-cotton-iana-5226bis-08#section-5.2.1
>
> I think you need to leave it as-is with a set of DEs.
>

Please do change the text to require more than one DE.


>
> Thanks,
> Kathleen
>
>
>
>> > >> 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
>> meant?
>> > >> (Etc., I haven't checked all the references.) In general, I believe
>> > >> a security considerations section is most useful where it provides
>> > >> more directed guidance instead of saying the equivalent of "here is
>> a textbook".
>> > >
>> > > There are 14 subsections with directed guidance, plus more in the
>> > > JWS, JWE,
>> > and JWK specifications.  If there is other directed guidance you'd
>> > like to see added, please supply proposed text.
>> > >
>> > > Having skimmed through it, I agree that 800-30A doesn't contain
>> > > clear enough
>> > security guidance to merit inclusion in the list.  I'll remove it.  If
>> > there are other specifications that you'd like to see added or removed,
>> please let me know.
>> >
>> > This was more of a general comment that "Here is a reference to a
>> textbook"
>> > type security considerations are less useful than specific, directed
>> > references.  I haven't checked all those referenced documents, so I'm
>> > not in a position to recommend changes here.  Maybe something for a
>> > checklist to apply to the next security spec.
>> >
>> > >
>> > >> 8.7 is not clear: is it NOT RECOMMENDED to reuse an entire set of
>> > >> key material (including IV), or to reuse any part of it?
>> > >
>> > > I agree that this is ambiguous, as worded.  Its advice seems to be
>> > > so broad as
>> > to be impractical, as never reusing a key means that a new key would
>> > have to be redistributed for each encryption, which then requires a
>> > key to use for the distribution...  This text came from
>> > http://tools.ietf.org/html/draft-miller-jose-
>> > jwe-protected-jwk-02#section-8.1, which was incorporated into the
>> > document after a working group decision to do so.  Matt Miller, you
>> > were the author of this text.  Could you clarify what the intent was?
>> Thanks.
>>
>> Matt proposed the following revised text.  I agree that this is
>> significantly clearer and more actionable than the previous version:
>>
>> It is NOT RECOMMENDED to reuse the same entire set of key material (Key
>> Encryption Key, Content Encryption Key, Initialization Vector,
>> etc.) to encrypt multiple JWK or JWK Set objects, or to encrypt the same
>> JWK or JWK Set object multiple times.  One suggestion for preventing re-use
>> is 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 document as well as from RFC 4086 [RFC4086].
>>
>> > >> Nits/editorial comments:
>> > >>
>> > >> 6.3.2.x:
>> > >> The constant repetition of »It 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.« almost ensures that an implementer will stop
>> > >> reading the details (well, I did, and I did not write a program to
>> > >> verify the same phrase is used everywhere; if any parameter were
>> > >> using a different encoding, that sure would be missed).  Why not
>> > >> define
>> > another abstraction like base64url and use this?
>> > >
>> > > I did just verify that they are all consistent.  Yes, if you're
>> > > reading the spec
>> > linearly I'm sure it feels repetitive, but if you're an implementer
>> > going straight to a definition to implement it, having a complete
>> > description all in one place is valuable.  I understand your request
>> > but it's not clear to me that this rises to the level that a separate
>> > definition that the developer then has to go find is warranted.
>> >
>> > My point was corroborated by the tiny error Richard found.  Repeating
>> > the same information again and again drowns out the important
>> > information.  A single paragraph at the end of 6.3's intro of the form:
>> >
>> >    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
>> > looked up in that it uses the term "base64url encoding", which has a
>> > slightly refined meaning here from that in RFC 4648.  So an even
>> > better way to handle this would be to add a new term "posint encoding"
>> > or some such to section 1.1, turning e.g. the entirety of 6.3.2.2 into
>> >
>> >            The "p" (first prime factor) member contains the first
>> > prime factor, in POSINT encoding.
>> >
>> >
>> > >
>> > >> 6.2.3.1: This is not a positive integer?  6.2.3.x mentions this
>> otherwise.
>> > >
>> > > I may not understand this remark.  I'm guessing you're referring to
>> > > 6.3.2.1 and
>> > the fact that the description includes the word "unsigned".  This is
>> > there to make it clear that no sign bit will be present in the
>> > representation - which is especially important since the high-order
>> > bit is always set for "n" and "d" for correctly generated keys.
>> >
>> > (Sorry, this is indeed about 6.3.2.x, not 6.2.3.x.) That was not what
>> > I meant:  All 6.3.2.x have the form »The "p" (first prime factor)
>> > member contains the first prime factor, a positive integer.«
>> > 6.3.2.1 does not have the ", a positive integer", but I can't see a
>> > reason for that (d clearly cannot be zero, and it is still an unsigned
>> value).
>> > This is likely to confuse an implementer that reads this text closely.
>> > (And I'm offering the fact that you are overlooking that inconsistency
>> > 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
>> > >> »Example description« is not a useful example for an "Algorithm
>> Description".
>> > >> (Same comment for 7.x.1.)
>> > >
>> > > See the actual registrations in Section 7.1.2 for more useful
>> examples.
>> >
>> > 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
>> parameters.  This is only one of the parameters in the template, and all
>> need to be retained.  Would you be happier with the e.g. text "Example
>> algorithm"?
>>
>> > >> 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, 2000.  But the site appears to be down at present.  The version
>> > referenced by Wikipedia at
>> > http://www.secg.org/download/aid-385/sec1_final.pdf is missing too.
>> > http://secg.org/download/aid-780/sec1-v2.pdf is missing too.  Can you
>> point me to a good link?
>> >
>> > I just had a look and embarrassingly found that the copy I'm using
>> > 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.
>> >
>> > >
>> > > RFC 5915 includes this reference:
>> > >
>> > >   [SECG1]    Standards for Efficient Cryptography Group (SECG), "SEC
>> 1:
>> > >              Elliptic Curve Cryptography", Version 2.0, May 2009.
>> >
>> > That looks good: We don't need a URL, as long as we are certain which
>> > version we are referencing.
>> >
>> > > Worst comes to worst, I'll either reference that or explicitly
>> > > reference the 1.0
>> > version.  But if I'm going to reference the 2.0 version, I'd like to
>> > be able to read a copy so that I can verify that the way we're using
>> > 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
>> > >> reference to RFC 20.  There is little reason to reference a
>> > >> somewhat hard to obtain external document ($60!) when we have an
>> > >> 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
>> > > a width
>> > attribute to the ttcol with either "digits" (for fixed width) or
>> > "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
>> > >
>> > >
>> > >
>>
>>
>
>
> --
>
> Best regards,
> Kathleen
>



-- 

Best regards,
Kathleen