[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: draft-ietf-pkix-ecc-subpubkeyinfo-02
Alfred,
Thanks for the review. Comments inline...
Curious how those on the list feel about the #11 (i.e., reference to FIPS
180-3 - which is still draft).
spt
>-----Original Message-----
>(1) Section 1
>
>The second text paragraph (below the algorithm list) says:
>
> [...]. To promote
> interoperability, this document indicates which is required.
>
>This is considered ambiguous:
>Do you mean "required to implement", "required to use", or both ?
>
>From details specified later in the draft, it becomes clear
>that perhaps the former choice had been intended.
>I suggest to clarify this at first place, stating unambiguously:
>
> [...]. To promote
> interoperability, this document indicates which is required
> to implement.
>
>A similar addition should be applied at the end of the third
>text paragraph of this section.
I take your point I'll make the suggested changes. Rambling on a bit ...
You could say we're trying to do both. You'd be required to implement the
MUST, SHOULDs, etc. if you want to claim conformance - if you don't that's
okay you just claim conformance. We'd like to say required to use on the
internet if you're going to use ECC algorithms - but really thats not up to
us.
>(2) Section 2
>
>(2a)
>1st paragraph: s/subjectPublilcKeyInfo/subjectPublicKeyInfo/
> ^
Fixed.
>(2b)
>Spurious replicated "are" in the middle of the section:
>
> The type AlgorithmIdentifier is parameterized to allow legal sets of
> values to be specified by constraining the type with an information
> object set. There are two parameterized types for
>AlgorithmIdentifier
> vvvv ^^^^^^^^^
>| are defined in this document: ECPKAlgorithms (see paragraph 2.1) and
> HashFunctions (see paragraph 2.1.1.2.5).
>---
> The type AlgorithmIdentifier is parameterized to allow legal sets of
> values to be specified by constraining the type with an information
> object set. There are two parameterized types for
>AlgorithmIdentifier
>| defined in this document: ECPKAlgorithms (see paragraph 2.1) and
> HashFunctions (see paragraph 2.1.1.2.5).
Fixed.
>(3) Section 2.1
>
>(3a)
>In the section title, I suggest using plural:
>
> 2.1. Elliptic Curve Public Key Algorithm Identifiers
Fixed.
>(3b)
>In the first paragraph: s/ECPKCAlgorithms/ECPKAlgorithms/
Fixed.
>(4) Section 2.1.1
>
>(4a)
>"ansi-x9-62" twice appears in Section 2.1.1.1 without being
>formally introduced in the memo.
>I suggest to deal with that issue in Section 2.1.1, where this
>OID is used implicitely, by changing:
>
> The algorithm identifier is:
>
> id-ecPublicKey OBJECT IDENTIFIER ::= {
> iso(1) member-body(2) us(840) ansi-X9-62(10045) keyType(2) 1 }
>---
> The algorithm identifier is:
>
>| id-ecPublicKey OBJECT IDENTIFIER ::= { ansi-X9-62 keyType(2) 1 }
>|
>| where ansi-X9-62 is defined as:
>|
>| ansi-X9-62 OBJECT IDENTIFIER ::= {
>| iso(1) member-body(2) us(840) 10045 }
>
>(Please correct an ASN.1 layman's trial if that's incorrect!)
I have replaced ansi-X9-62 with "iso(1) member-body(2) us(840) 10045" in
2.1.1.1. The two are equivalent - so it's a matter of style. I like to
explicitly list the OIDs but I guess I missed these two.
>(4b)
>In the ECParameters syntax, please correct:
>
> specifiedCuve SpecifiedCurve,
>---
>| specifiedCurve SpecifiedCurve,
> ^
Fixed.
>(4c)
>In the ECParameters fields explanations, I suggest adding two
>commas (in an attempt to follow RFC-Editor favored style):
>
> specifiedCurve allows all of the required values to be explicitly
>| specified. This choice MAY be supported, and if it is,
> ^
> implicitCurve MUST also be supported. See paragraph 2.1.1.2.
>
>and:
>
> implicitCurve allows the elliptic curve parameters to be
> inherited from the issuer's certificate. This choice MAY be
> supported, but if subordinate certificates use the same
> namedCurve as their superior, then the subordinate certificate
>| MUST use the namedCurve option. That is, implicitCurve is only
> ^
> supported if the superior doesn't use the namedCurve option.
Fixed.
>(5) Section 2.1.1.1
>
>(5a)
>First paragraph: s/ECParamaters/ECParameters/
> ^ ^
Fixed.
>(5b)
>For the 'ansi-x9-62' OID issue, see (4a) above!
>Alternatively, for secp192r1 and secpr256r1 the fully expanded
>version of the object identifier might be specified.
Fixed. I fully expanded it.
>(6) Sections 2.1.1.2 and 2.1.1.2.1
>
>(6a) 2.1.1.2, 1st paragraph
>Please correct and improve the readability:
>
> The specified field in ECParameters is the SpecifiedCurve type.
> SpecifiedCurve uses the following ASN.1 structure:
>--- vvvvv
>| The specifiedCurve field in ECParameters is the
>SpecifiedCurve type
>| chosen; it uses the following ASN.1 structure:
> ^^^^^^^^^^^^
I think it's clearer if we say: "The specifiedCurve field in ECParameters is
the SpecifiedCurve type. SpecifiedCurve uses the following ASN.1 structure:"
>(6b) base point naming
>
>The text in section 2.1.1.2 uses "P" to denote the base point
>on the elliptic curve -- once in the syntax, and twice in the
>field explanations for SpecifiedCurve.
>Contrary to that, Section 2.1.1.2.1 uses "G" for the same
>purpose (three times in the long text paragraph below the syntax).
>
>That's confusing, at best.
>I suggest to consistently use a single formula symbol / name
>for the base point, either "P" or "G" -- it's your choice!.
Fixed. I picked P and changed 2.1.1.2.1.
>Also, in the field explanations for SpecifiedCurve in 2.1.1.2,
>I suggest changing:
>
> base specifies the base point P of the elliptic curve E, ...
>---
>| base specifies the base point P on the elliptic curve E, ...
> ^
Fixed.
>(6c)
>
>Throughout the text: s/psuedorandomly/pseudorandomly/g
> ^^ ^^
>
>(one occurrence in 2.1.1.2, five occurrences in 2.1.1.2.1)
Fixed.
>(6d) 2.1.1.2.1, 1st paragraph
>
>The text says:
> vvvvvv
> The version field in SpecifiedCurve is the SpecifiedCurveVersion
> type. [...]
> ^^^^
>
>I suggest to change this and either say:
> vv
>| The version field in SpecifiedCurve is of
>SpecifiedCurveVersion type.
> [...]
>
>or:
> vvvvvvv
>| The version field in SpecifiedCurve is of type
>SpecifiedCurveVersion.
> [...]
>
>or:
> vvv
>| The version field in SpecifiedCurve has SpecifiedCurveVersion type.
> [...]
>
>(Again, it's your choice ...)
>
>Similar changes should be applied in the first line in the
>subsequent sections 2.1.1.2.2, 2.1.1.2.3, 2.1.1.2.4, and 2.1.1.2.5.
>(For brevity, I omit mentioning that below.)
Fixed. I choose your 1st suggestion.
>(7) Section 2.1.1.2.2
>
>The last paragraph should be corrected:
>
> Two FieldTypes defined herein: [...]
>--- vvvv
>| Two FieldTypes are defined herein: [...]
Fixed.
>(8) Section 2.1.1.2.2.2
>
>Near the end of the section: s/pentaomial/pentanomial/
> ^
Fixed.
>(9) Sections 2.1.1.2.3 and 2.1.1.2.4
>
>In the second-to-last paragraph of 2.1.1.2.3, the Note ...
>
> [...]. Note that these
> octet strings may represent an elliptic curve point in compressed
> or uncompressed form. Implementations that support elliptic
> curve according to this document MUST support the uncompressed
> form and MAY support the compressed form.
>
>... comes to surprise and is confusing in this context.
>It turns out that this Note belongs into the explanation of 'Base'
>and hence should be moved to the end of Section 2.1.1.2.4.
I'd say the note also belongs in 2.1.1.2.4. It still belongs in 2.1.1.2.3
because they're also octet strings. Others thoughts?
>(10) Section 2.1.1.2.5
>
>In the second line, please use less literary style:
>
> HashAlgorithm use the following ASN.1 syntax:
>---
>| HashAlgorithm uses the following ASN.1 syntax:
> ^
Fixed.
>(11) Sections 2.1.2 and 2.2
>
>I suggest to more clearly use "elliptic curve cryptograpy" and
>its abbreviation, "ECC", in place of the too terse "elliptic
>curve" and its abbreviation "EC" in these paragraphs; perhaps
>it would even be better to not use the abbreviation in 2.1.2 -- i.e.:
>
>(11a) Section 2.1.2, first paragraph
>
> Algorithms used with EC fall in to different categories: signature
> and key agreement algorithms. ECDSA uses the ecPublicKey described
> in 2.1.1. Two sets of key agreement algorithms are
>identified herein:
> Elliptic Curve Diffie-Hellman (ECDH) key agreement scheme and
> Elliptic Curve Menezes-Qu-Vanstone (ECMQV) key agreement scheme. All
> algorithms are identified by an OID and have PARMS. The OID varies
> based on the algorithm but the PARMS are always ECParameters (see
> paragraph 2.1.1).
>--- vvvvvvvvvvvvvvvvvvvvvvvvvvv
>| Algorithms used with elliptic curve cryptography fall in to
>different
> categories: signature and key agreement algorithms. ECDSA uses the
> ecPublicKey described in 2.1.1. Two sets of key agreement
>algorithms
>| are identified herein: the Elliptic Curve Diffie-Hellman (ECDH) key
> vvv|^^^
>| agreement scheme and the Elliptic Curve Menezes-Qu-Vanstone (ECMQV)
> key agreement scheme. All algorithms are identified by an OID and
> have PARMS. The OID varies based on the algorithm but the PARMS are
> always ECParameters (see paragraph 2.1.1).
>
>(I also have added two missing articles.)
Fixed.
>(11b) Section 2.2, first paragraph
>
> The subjectPublicKey from SubjectPublicKeyInfo is the ECC
>public key.
> Implementations that support elliptic curve according to this
> document MUST support the uncompressed form and MAY support the
> compressed form of the ECC public key. [...]
>---
> The subjectPublicKey from SubjectPublicKeyInfo is the ECC
>public key.
>| Implementations of elliptic curve cryptography according to this
> ^^ ^^^^^^^^^^^^
> document MUST support the uncompressed form and MAY support the
> compressed form of the ECC public key. [...]
>
>(I also have simplified the language, avoiding one instance of
>the threefold "support".)
Fixed.
>(12) References
>
>You might also plan for quoting the upcoming FIPS PUB 180-3
>for the SHS, and use the (version independent) ref. tag
>'[SHS]' in place of '[SHA2]'.
I'm good with making this change but I'm curious what others think?