Subject: [Curdle] Review of draft-ietf-curdle-pkix-04

From: David Benjamin <[email-masked]>
Date: Wed, 29 Mar 2017 17:31:31 +0000

Overall, the document looks good to me. I've actually just implemented it
along with the TLS bits in BoringSSL and our Go testing implementation.
Seems to work. (Though I haven't done anything with the signature yet, just
parsing out keys. Our X.509 story is a little funny since Chrome currently
uses OS verifiers.)

I have a handful of comments. They are all entirely editorial and mostly
nitpicks.

In section 3,

   The same algorithm identifiers are used for identifying a public key,
   identifying a private key and identifying a signature (for the four
   EdDSA related OIDs).  Additional encoding information is provided
   below for each of these locations.

"four EdDSA related OIDs" should now be "two EdDSA related OIDs"

In section 4,

   o  subjectPublicKey contains the byte stream of the public key.
      While the encoded public keys for the current algorithms are all
      an even number of octets, future curves could change that.

I would suggest replacing the last sentence with "For the algorithms
defined in this document, the encoded public keys are an even number of
octets." If future curves do something funny, they'll be defined with
independent OIDs and won't be bound by any explanatory notes in this
document.

In section 5,

   If the keyUsage extension is present in a certificate that indicates
   id-X25119 or id-X448 in SubjectPublicKeyInfo, then the following MUST
   be present:

"id-X25119" should be "id-X25519".

Also, a exceedingly nitpicky nitpick: the various lists of keyUsage values
are all indented differently. I'm not sure if this is an artifact of some
centering thing or a formatting error.

In section 7,

   For the keys defined in this document, the private key is always an
   opaque byte sequence.  The ASN.1 type CurvePrivateKey is defined in
   this document to hold the byte sequence.  Thus when encoding a
   OneAsymmetricKey object, the private key is wrapped in an
   CurvePrivateKey object and wrapped by the OCTET STRING of the
   'privateKey' field.

The 'privateKey' field is described with single quotes, but in the
following paragraph, the "privateKey", "privateKeyAlgorithm", and
"publicKey" fields are described with double quotes.

In section 8,

   When the curve is known, use a more specific string.  For the id-
   EdDSA25519 value use the string "Ed25519".  For id-EdDSA448 use
   "Ed448".

In keeping with its own advice and to match the names in section 3,
"id-EdDSA25519" should be "id-Ed25519" and "id-EdDSA448" should be
"id-Ed448".  The names also show up elsewhere in the document and should
match.

In section 10.2,

The OIDs in the sample certificate are referred to as "EdDSA 25519
signature algorithm" and "ECDH 25519 key agreement", contradicting the
document's own advice on human-readable names.

David

---

From: David Benjamin <[email-masked]>
Date: Wed, 29 Mar 2017 17:33:34 +0000

> "id-Ed448".  The names also show up elsewhere in the document and should

Er, by "the names", I meant "id-EdDSA25519". That is, there are other
instances which should be switched too. (Actually "id-Ed25519" only shows
up once, but I think that is a better name.)

---

From: Jim Schaad <[email-masked]>
Date: Wed, 29 Mar 2017 13:19:05 -0500

From: Curdle [mailto:[email-masked]] On Behalf Of David Benjamin
Sent: Wednesday, March 29, 2017 12:32 PM
To: curdle <[email-masked]>
Subject: [Curdle] Review of draft-ietf-curdle-pkix-04

Overall, the document looks good to me. I've actually just implemented it along with the TLS bits in BoringSSL and our Go testing implementation. Seems to work. (Though I haven't done anything with the signature yet, just parsing out keys. Our X.509 story is a little funny since Chrome currently uses OS verifiers.)

I have a handful of comments. They are all entirely editorial and mostly nitpicks.

In section 3,

fixed

In section 4,

I would suggest replacing the last sentence with "For the algorithms defined in this document, the encoded public keys are an even number of octets." If future curves do something funny, they'll be defined with independent OIDs and won't be bound by any explanatory notes in this document.

How about

        The algorithms defined in this document always encode the public key in an even number of octets.

        If future algorithms change this, they should consider using a new OID value.

In section 5,

   be present:

Also, a exceedingly nitpicky nitpick: the various lists of keyUsage values are all indented differently. I'm not sure if this is an artifact of some centering thing or a formatting error.

Fixed 

In section 7,

   'privateKey' field.

The 'privateKey' field is described with single quotes, but in the following paragraph, the "privateKey", "privateKeyAlgorithm", and "publicKey" fields are described with double quotes.

In section 8,

   "Ed448".

In keeping with its own advice and to match the names in section 3, "id-EdDSA25519" should be "id-Ed25519" and "id-EdDSA448" should be "id-Ed448".  The names also show up elsewhere in the document and should match.

fixed

In section 10.2,

The OIDs in the sample certificate are referred to as "EdDSA 25519 signature algorithm" and "ECDH 25519 key agreement", contradicting the document's own advice on human-readable names.

Fixed – I apparently did this when I re-mapped the OIDs and forgot to make the entire document consistent

Jim

David

---

From: Russ Housley <[email-masked]>
Date: Wed, 29 Mar 2017 14:21:59 -0400

>         If future algorithms change this, they should consider using a new OID value.

I do not think you need the second sentence.  A changes would be a different algorithm, which is of course not in the document.

Russ