Re: [lamps] Benjamin Kaduk's Yes on draft-ietf-lamps-pkix-shake-11: (with COMMENT)

["Panos Kampanakis (pkampana)" <[e-mail]>]

Hi Ben,

Then you for the review. 

I addressed all your comments. The details and the language used are described in the git issue https://github.com/csosto-pk/adding-shake-to-pkix/issues/47#issuecomment-505640399 And the actual updates are in https://github.com/csosto-pk/adding-shake-to-pkix/commit/1f056710eea828a4f2cfd8f349563a4b1a5af85a Please check them out to see if they look OK. 

I am planning to reupload the draft tomorrow or Monday. 

Thanks,
Panos 


-----Original Message-----
From: Spasm <[e-mail]> On Behalf Of Benjamin Kaduk via Datatracker
Sent: Monday, June 24, 2019 2:41 PM
To: The IESG <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]
Subject: [lamps] Benjamin Kaduk's Yes on draft-ietf-lamps-pkix-shake-11: (with COMMENT)

Benjamin Kaduk has entered the following ballot position for
draft-ietf-lamps-pkix-shake-11: Yes

When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-lamps-pkix-shake/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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.


_______________________________________________
Spasm mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/spasm