Subject: [lamps] Fwd: AD Review draft-ietf-lamps-pkix-shake-06

From: Eric Rescorla <[email-masked]>
Date: Sat, 22 Dec 2018 06:43:07 -0800

CCing the WG because I was wrong about the aliases.

---------- Forwarded message ---------
From: Eric Rescorla <[email-masked]>
Date: Fri, Dec 21, 2018 at 4:11 PM
Subject: AD Review draft-ietf-lamps-pkix-shake-06
To: <[email-masked]>

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D4946

IMPORTANT
S 5.1.2.
>      [SEC1] if they have a stated policy that requires conformance to
>      these standards.  These standards may have not specified SHAKE128 and
>      SHAKE256 as hash algorithm options.  However, SHAKE128 and SHAKE256
>      with output length being 32 and 64 octets respectively are
>      subtitutions for 256 and 512-bit output hash algorithms such as
>      SHA256 and SHA512 used in the standards.

I'm not sure why you are requiring deterministic ECDSA with SHAKE. Is
there any specific reason why it's more important there than with
other hashes.

S 5.2.
>      and CRLs.  Conforming client implementations that process RSASSA-PSS
>      or ECDSA with SHAKE public key when processing certificates and CRLs
>      MUST recognize the corresponding OIDs.  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].

This is going to be a major disincentive to use SHAKE. If you don't
know what the verifier will support, you're going to want to use
RSAEncryption, but then this says you can't use SHAKE.
COMMENTS
S 2.
>      In the SHA-3 family, two extendable-output functions (SHAKEs),
>      SHAKE128 and SHAKE256, are defined.  Four other hash function
>      instances, SHA3-224, SHA3-256, SHA3-384, and SHA3-512 are also
>      defined but are out of scope for this document.  A SHAKE is a
>      variable length hash function.  The output length, in bits, of a
>      SHAKE is defined by the d parameter.  The corresponding collision and

This is the first mention of d. I think you mean something like "A
SHAKE is defined as a function SHAKE(M, d) where the output is a
digest of message M that is d bits long)"

S 4.
>      capitals, as shown here.
>
>   4.  Identifiers
>
>      This section defines four new OIDs for RSASSA-PSS and ECDSA when
>      SHAKE128 and SHAKE256 are used.  The same algorithm identifiers are

This is a little unclear. I think you mean "four new OIDs, for RSASSA-
PSS and ECDSA with each of SHAKE-128 and SHAKE-256"

S 4.
>      for each use of SHAKE128 or SHAKE256 in RSASSA-PSS and ECDSA.  In
>      summary, when hashing messages to be signed, output lengths of
>      SHAKE128 and SHAKE256 are 256 and 512 bits respectively.  When the
>      SHAKEs are used as mask generation functions RSASSA-PSS, their output
>      length is (n - 264) or (n - 520) bits respectively, where n is a RSA
>      modulus size in bits.

"n is the RSA modulus size in bits"

S 5.1.
>   5.1.  Signatures
>
>      Signatures can be placed in a number of different ASN.1 structures.
>      The top level structure for an X.509 certificate, to illustrate how
>      signatures are frequently encoded with an algorithm identifier and a
>      location for the signature, is

This sentence is ungrammatical

S 5.1.
>            signatureValue       BIT STRING  }
>
>      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].

What goes in "parameters"?

S 5.1.
>      ECDSA with SHAKE signatures in certificates and CRLs.  Conforming
>      client implementations that process RSASSA-PSS or ECDSA with SHAKE
>      signatures when processing certificates and CRLs MUST recognize the
>      corresponding OIDs.  Encoding rules for RSASSA-PSS and ECDSA
>      signature values are specified in [RFC4055] and [RFC5480]
>      respectively.

Is it forbidden to verify RSAPSSA/SHAKE signatures with RSAEncryption
in the cert? I expect not.

S 5.1.1.
>      seed from which mask is generated, an octet string [RFC8017].  The
>      output length is (n - 264)/8 or (n - 520)/8 bytes respectively, where
>      n is the RSA modulus in bits.  For example, when RSA modulus n is
>      2048, the output length of SHAKE128 or SHAKE256 as the MGF will be
>      223 or 191-bits when id-RSASSA-PSS-SHAKE128 or id-RSASSA-PSS-SHAKE256
>      is used respectively.

I think you are mixing bits and bytes here, because 2048 - 264 != 223.

It would also be helpful to explain what's going on here, by
explaining how these values are computed.

S 5.1.2.
>      [X9.62].  When the id-ecdsa-with-SHAKE128 or id-ecdsa-with-SHAKE256
>      (specified in Section 4) algorithm identifier appears, the respective
>      SHAKE function (SHAKE128 or SHAKE256) is used as the hash.  The
>      encoding MUST omit the parameters field.  That is, the
>      AlgorithmIdentifier SHALL be a SEQUENCE of one component, the OID id-
>      ecdsa-with-SHAKE128 or id-ecdsa-with-SHAKE256.

Shouldn't we be requiring that the hash match the size of the curve.

S 5.2.
>      key exclusively to RSASSA-PSS, the AlgorithmIdentifiers for RSASSA-
>      PSS defined in Section 4 can be used as the algorithm field in the
>      SubjectPublicKeyInfo sequence [RFC5280].  The identifier parameters,
>      as explained in section Section 4, MUST be absent.  The RSASSA-PSS
>      algorithm functions and output lengths are the same as defined in
>      Section 5.1.1.

I must be missing something. what does this text refer to, given the
text above about what's in the certificate.

S 7.
>      of the longer one.  It is a similar situation as truncating a 512-bit
>      output of SHA-512 by taking its 256 left-most bits.  These 256 left-
>      most bits are a prefix of the 512-bit output.
>
>      Implementations must protect the signer's private key.  Compromise of
>      the signer's private key permits masquerade attacks.

This seems unnecessary

S 7.
>      computing power increases, the work factor or time required to break
>      a particular cryptographic algorithm may decrease.  Therefore,
>      cryptographic algorithm implementations should be modular allowing
>      new algorithms to be readily inserted.  That is, implementers should
>      be prepared to regularly update the set of algorithms in their
>      implementations.

This also seems unnecessary.