On Tue, Dec 22, 2015 at 04:44:09PM -0800, Jim Schaad wrote:
> Here are some comments on the current draft.
>
> * Abstract: The abstract needs to be expanded to include the names of the
> algorithms that are being included here. (Note that references in an
> abstract are not considered kosher so don't do that.) You need to be able
> to read the abstract without the rest of the document and be able to decide
> if it is the correct document.
Did add the algorithm names.
> * Section 1: Putting a reference to the new algorithms in the first
> sentence would be a reasonable edit. I would also probably at least
> consider putting the set of algorithms here by name. Think of reading the
> introduction in 10 years when a new set of algorithms has been created.
Did add the algorithm names and references.
> * Section 1: The IETF does not have the concept of extending documents.
> Documents generally fall into the categories of a) stand alone, b) updates
> an existing document, c) obsoletes and existing document. The second
> paragraph either should be removed or re-phrased. Possibly something along
> the lines of "This document defines the conventions to be used in the
> context of [JWA] and [JWK]." Although I am not sure that I even care for
> that.
Changed the prhrasing to be in line with that.
> * Section 1.2: I am not sure that this qualifies as being notation. I
> would probably change the title
Moved it to section 1 (it is just one paragraph).
> * Section 2: In response to a question that you had asked earlier on the
> list, but which I did not respond to at the time. I believe that you could
> define any of the existing parameter to be a required parameter for a new
> key type. You would need to have a good justification for doing so, but it
> would be possible to define 'alg' as a required parameter for the key type
> 'OKP'. Having to do simple checks for things like
> {"kty":"OKP","crv":"Ed25519", "alg":"Ed25519p", "x":"...."} as being
> illegal seems problematic.
OHOH, merging the things may cause problems with X25519 and X448. Since
those are used with ECDH-ES, which currently seems to be used with 4
algorithms (direct keying and 3x AES-KW with different keysizes).
OTOH, the other types have just one algorithm they work with.
> * Section 2: You have introduced the term 'subtype of the key' without
> making it clear where this term is coming from. This is not a field in the
> registry as seems to be implied by the text.
Since OKP is a new keytype, I thought I need to define fields for it.
Noticed that IANA considerations has a typo: It still refers to "algorithm
group" instead of "key subtype" (I changed the terminology at some point,
but that got left).
> * Section 2: If you don't like the terms crv and x, perhaps you should
> seriously think about using different names for these fields. That is the
> feeling I am getting from the Note in this section.
The draft originally did use different names, but various people in the
WG wanted those to be "crv" and "x", damn what that impiles about the
meaning.
> * Section 3.1.1: For formatting I would remove the colons in the table.
Removed in internal copy.
> * Section 3.1.1: The table is a bit confusing given that all of the columns
> have the same value. Without a better description of the headers I don't
> find it to be very useful.
This is artifact of having different alg values for what is logically the
same operation with different keys.
> * Section 3.1.1: I think that I have a real problem with the concept of all
> of the parameters of a signature algorithm being defined by the "crv" field
> rather than the "alg" field. This seems to be a real misuse of what crv
> currently represents. If you do combine things into one algorithm I would
> really do some renaming of the key fields. (See note above on use of 'alg'
> instead of 'crv'.)
See above.
> * Section 3.1.2 and 3.1.3: Yes - I like the fact that there is not a full
> description of the signature processing here!!!
I only put in what goes where. There aren't a lot of values invovled.
> * Section 3.2 - what is the 'alg' parameter supposed to be here? It is not
> in the table. (See above notes on the table.)
AFAICT: "enc", "ECDH-ES+A128KW", "ECDH-ES+A192KW" or "ECDH-ES+A256KW"
(which are the ECDH-ES -type things).
> * Section 3.2.1 - You need to specify what the KDF to be used is.
Standard ECDH-ES KDF processing from JWA. I added a reference to JWA
section 4.6.2 in internal copy.
> * Section 4 - s/information what key/information about what key/
Fixed on internal copy.
-Ilari
_______________________________________________
jose mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/jose