Internet X.509 Public Key Infrastructure: Additional Algorithm Identifiers for RSASSA-PSS and ECDSA using SHAKEs
draft-ietf-lamps-pkix-shake-15

Note: This ballot was opened for revision 11 and is now closed.

Roman Danyliw Yes

Benjamin Kaduk Yes

Comment (2019-06-24 for -11)
Thanks for this document; I only have editorial-nit-level comments.

Section 2

   This document describes cryptographic algorithm identifiers for
   several cryptographic algorithms which use variable length output
   SHAKE functions introduced in [SHA3] which can be used with the
   Internet X.509 Certificate and CRL profile [RFC5280].

nit(?): Is "describes" or "defines" more appropriate?  (Given that
NIST has already allocated some of the OIDs in question, I could go
either way.)
I'd also suggest further rewording, perhaps as:

   This document defines cryptographic algorithm identifiers for several
   cryptographic algorithms that use the variable length output SHAKE
   functions introduced in [SHA3]; these algorithms can be used with the
   Internet X.509 Certificate and CRL profile [RFC5280].

--

   This specification describes the identifiers for SHAKEs to be used in
   X.509 and their meaning.

nit: this seems pretty redundant with the first paragraph of the
section.

Section 5.1

   Signatures are used in a number of different ASN.1 structures.  As
   shown in the ASN.1 representation from [RFC5280] below, an X.509
   certificate a signature is encoded with an algorithm identifier in
   the signatureAlgorithm attribute and a signatureValue attribute that
   contains the actual signature.

nit: "an X.509 certificate a signature is encoded" is not grammatical; I
think there's a missing "in" at the start?

   The identifiers defined in Section 4 can be used as the
   AlgorithmIdentifier in the signatureAlgorithm field in the sequence
   Certificate and the signature field in the sequence tbsCertificate in
   X.509 [RFC5280].  [...]

nit: I'm a bit confused by the usage "sequence tbsCertificate" -- the
name of the ASN.1 SEQUENCE is TBSCertificate, with tbsCertificate
reflecting the field name for this sequence as it appears in the
Certificate.  (Contrariwise, "the sequence Certificate" makes sense to
me, as that is the type name of an ASN.1 SEQUENCE.)  I do note that the
sentence "This field MUST contain the same algorithm identifier as the
signature field in the sequence tbsCertificate (Section 4.1.2.3" appears
in RFC 5280, which includes the same phrasing.

Section 5.1.1

   The RSASSA-PSS algorithm is defined in [RFC8017].  When id-RSASSA-
   PSS-SHAKE128 or id-RSASSA-PSS-SHAKE256 specified in Section 4 is
   used, the encoding MUST omit the parameters field.  [...]

Is this requirement redundant with the one in Section 4?
(Similarly in Section 5.1.2.)

   The hash algorithm to hash a message being signed and the hash
   algorithm as the mask generation function used in RSASSA-PSS MUST be
   the same, SHAKE128 or SHAKE256 respectively.  [...]

nit: I think just "as" is not the right grammar, here, and we want "used
as" instead.

   SHAKE128 and id-RSASSA-PSS-SHAKE256 respectively.  The mgfSeed is the
   seed from which mask is generated, an octet string [RFC8017].  As
   explained in Step 9 of section 9.1.1 of [RFC8017], the output length
   of the MGF is emLen - hLen - 1 bytes. emLen is the maximum message
   length ceil((n-1)/8), where n is the RSA modulus in bits. hLen is 32
   and 64-bytes for id-RSASSA-PSS-SHAKE128 and id-RSASSA-PSS-SHAKE256
   respectively.  Thus when SHAKE is used as the MGF, the SHAKE output
   length maskLen is (n - 264) or (n - 520) bits respectively.  For
   example, when RSA modulus n is 2048, the output length of SHAKE128 or
   SHAKE256 as the MGF will be 1784 or 1528-bits when id-RSASSA-PSS-
   SHAKE128 or id-RSASSA-PSS-SHAKE256 is used respectively.

nit: Absent some external requirement for the RSA modulus to be a
multiple of 8 bits (that I have forgotten about), it seems we need to be
more careful about transtioning from the byte length of the MGF output
to the bit length of SHAKE output needed, as the ceil() function will
vary with the modulus of n base 8.

Section 5.2

   is an OID and optionally associated parameters.  The conventions and
   encoding for RSASSA-PSS and ECDSA public keys algorithm identifiers
   are as specified in Section 2.3 of [RFC3279], Section 3.1 of
   [RFC4055] and Section 2.1 of [RFC5480].

I think this might be better if it calls out sections 2.3.1 and 2.3.5 of
RFC 3279 explicitly rather than globbing in a bunch of unrelated
subsections.

   The identifier parameters, as explained in Section 4, MUST be absent.

This feels like the fourth time we've said that parameters are absent...

Appendix A

nit: Does "Deterministic" need to be in the comments for the ECDSA smime
capabilities?  It's not really something the peer can verify.

Ignas Bagdonas No Objection

Deborah Brungard No Objection

Alissa Cooper No Objection

Suresh Krishnan No Objection

Mirja Kühlewind No Objection

Barry Leiba No Objection

Comment (2019-06-24 for -11)
I just have some editorial comments, all minor:

General:
Whenever you use "respectively", throughout the document, it needs a comma before; it also needs a comma after unless it's at the end of a sentence.  There are also some cases where you use "respectively" incorrectly, and I've noted those below.

-- Section 2 --
A set of nits:

In "several cryptographic algorithms which use", make it "that" instead of "which"... better anyway, but especially with the subsequent "which" (that should have a comma before it).

You need a comma after "SHA3-512".

"d-bits-long" needs both hyphens.

"second-preimage-resistance" is a compound modifier and needs two hyphens (two instances here).

The comma after "And" doesn't belong.

-- Section 5.1 --

   Conforming
   client implementations that process RSASSA-PSS or ECDSA with SHAKE
   signatures when processing certificates and CRLs MUST recognize the
   corresponding OIDs.

I find the double "process" a little hard to parse.  Do you mean this?:

NEW
   Conforming
   client implementations that process certificates and CRLs using RSASSA-PSS
   or ECDSA with SHAKE MUST recognize the corresponding OIDs.
END

-- Section 5.1.1 --

   The hash algorithm to hash a message being signed and the hash
   algorithm as the mask generation function used in RSASSA-PSS MUST be
   the same, SHAKE128 or SHAKE256 respectively.

There's something wrong here, and I think it's the "respectively."  I think you're saying that the two algorithms must be the same as each other, but "respectively" says that the first must be 128 and the second must be 256.  I think you want this instead:

NEW
   The hash algorithm to hash a message being signed and the hash
   algorithm as the mask generation function used in RSASSA-PSS MUST be
   the same: both SHAKE128 or  both SHAKE256.
END

The "respectively" in the sentence following that is also wrong; please rephrase that one as well (and "output length" should NOT be hyphenated).

In the final paragraph, "The RSASSA-PSS saltLength MUST be 32 or 64 bytes respectively," is wrong (you can't really inherit the context from another paragraph); you need to say something like, "The RSASSA-PSS saltLength MUST be 32 or 64 bytes for id-RSASSA-PSS-SHAKE128 or id-RSASSA-PSS-SHAKE256, respectively."  Probably better to say, "The RSASSA-PSS saltLength MUST be 32 bytes for id-RSASSA-PSS-SHAKE128 or 64 bytes for id-RSASSA-PSS-SHAKE256."

-- Section 5.1.2 --

   It is RECOMMENDED that conforming CA implementations that generate
   ECDSA with SHAKE signatures in certificates or CRLs generate such
   signatures with a deterministically generated, non-random k in
   accordance with all the requirements specified in [RFC6979].

Take or leave this one as you please, but I find the passive voice both more confusing and unnecessary in this sentence (because you do have a clear subject already), and I think this is easier to read:

NEW
   Conforming CA implementations that generate ECDSA with
   SHAKE signatures in certificates or CRLs SHOULD generate such
   signatures with a deterministically generated, non-random k in
   accordance with all the requirements specified in [RFC6979].
END

Later in that paragraph, two instances of "these standards" and one of "the standards" seem to refer to [X9.62] and [SEC1], so I think it should say "those standards" (to make it clear that you're not talking about any standards defined in *this* document).

Alvaro Retana No Objection

Adam Roach No Objection

Comment (2019-06-25 for -11)
Thanks to the authors for a well-written and easy to read document. I have
only one minor comment.

This document updates RFC 3279. It would be helpful if the abstract indicated
this fact. (cf. https://www.ietf.org/standards/ids/checklist/ §3.1.D.1)

Martin Vigoureux No Objection

Éric Vyncke No Objection

Comment (2019-06-25 for -11)
Thank you all for the work put into this document.

== COMMENTS ==

-- Section 1 / Change log --

May I assume that the issues by the two reviews of -08 are solved in -11 ? 


-- Section 4 --

== NITS ==

-- Abstract --

Just wondering why CRL acronym is expanded while SHAKE & ECDSA are not.

-- section 6 --

Also wondering why in some IANA entries "SHAKE" is in lower case while in others in upper case.

Magnus Westerlund No Objection