[lamps] Mohamed Boucadair's Discuss on draft-ietf-lamps-pq-composite-sigs-15: (with DISCUSS and COMMENT)

[Mohamed Boucadair via Datatracker <[e-mail]>]

Mohamed Boucadair has entered the following ballot position for
draft-ietf-lamps-pq-composite-sigs-15: Discuss

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/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


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



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Hi Mike, John, Massimiliano, Jan, and Scott,

Thank you for the effort put into this specification. The document reads well
... even for an OPS AD :-)

I trust that the test vectors, in particular, were checked by WG participants
other than the authors.

Please find below some comments below. I also flagged some nits and other minor
edits that I will send in a PR for authors convenience.

# ML-DSA is normative for composite ML-DSA

CURRENT:
   Composite ML-DSA is a Post-Quantum / Traditional hybrid signature
   scheme which combines ML-DSA as specified in [FIPS.204] and [RFC9881]

   …

   As this specification uses ML-DSA as a component of all composite
   algorithms, all security considerations from [RFC9881] apply.

Please move RFC9881 from Informative to Normative.

## As I’m there, I’d like to check one related part:

Section 2 says the following:
  This specification assumes a seed-based keygen for ML-DSA.

Maybe I’m misreading this, but is this saying that only the seed mode is
supported? If so, isn’t that a deviating from 9881 where expandedKey is allowed?

# Terminology: I'm adding this as a DISCUSS point as I think this is important
to ensure consistency among specs in this area

Section 2:
   This specification is consistent with the terminology defined in
   [RFC9794].  In addition, the following terminology is used throughout
   this specification:

I interpret this as the terms that follow are not listed in RFC9794 but are an
addition. However, it is not clear to me the rationale followed here. For
example,

## we do have this entry grabbed from RFC9794 with an explicit pointer to that
RFC:

   *COMPOSITE CRYPTOGRAPHIC ELEMENT*: [RFC9794] defines composites as: A
   cryptographic element that incorporates multiple component
   cryptographic elements of the same type in a multi-algorithm scheme.

The citation does not match exactly the entry as defined in 9794:

      A cryptographic element that incorporates multiple component
      cryptographic elements of the same type for use in a multi-
      algorithm scheme, such that the resulting composite cryptographic
      element is exposed as a singular interface of the same type as the
      component cryptographic elements.

## … and this one which is already defined in RFC9794 but repeated here

   *POST-QUANTUM TRADITIONAL (PQ/T) HYBRID SCHEME*: A multi-algorithm
   scheme where at least one component algorithm is a post-quantum
   algorithm and at least one is a traditional algorithm.

## Unless there is a need to deviate from RFC9794, I would suggest to rely on
the terms/definitions in RFC9794 (and make that RFC as normative) and only
define here new terms not listed in RFC9794.

# Given the side-channel implication, any reason why this isn’t a MUST?

CURRENT:
   Note that in step 4 above, both component signature processes are
   invoked, and no indication is given about which one failed.  This
   SHOULD be done in a timing-invariant way to prevent side-channel
   attackers from learning which component algorithm failed.


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

# Abstract: regional requirement, not an absolute one

OLD:
   These combinations are tailored to meet regulatory
   guidelines.

NEW:
   These combinations are tailored to meet regulatory
   Guidelines in certain regions.

# Backward compatibility

## Usual

CURRENT:
   *APPLICATION BACKWARDS COMPATIBILITY*: The usual definition of
   backwards compatibility, meaning whether an upgraded and non-upgraded
   application can successfully establish communication.

### If this is usual definition, then do we need to have a dedicated entry for
it here?

### I suggest

NEW:
   *APPLICATION BACKWARDS COMPATIBILITY*: A property indicating whether an
   upgraded and non-upgraded application can successfully establish
   communication.

## Section 10.2 has the following:

   The term "application backwards compatibility" is used here to mean
   that existing systems as they are deployed today can interoperate
   with the upgraded systems of the future.

Why the definition is repeated here? is there any difference between the intent
here and the relevant entry in Section 1.1?

Unless there is subtlety there, I suggest to delete that text as it is
redundant with the terminology section.

## I don’t parse the following:

CURRENT:
   *PROTOCOL BACKWARDS COMPATIBILITY*: A property whereby a new feature
   can be added to a protocol without requiring any changes to the
   protocol's specification and only minimal changes to its
   implementations (such as adding new identifiers).

Adding a new feature is a change per se!

### Can we please update this definition to better reflect the intended
property?

### Do we need this term at the first place? This is only mentioned once with
self-contained context.

## Internal inconsistency

Section 1:
   Backwards compatibility in the sense of upgraded systems continuing
   to interoperate with legacy systems is not directly covered in this
   specification, but is the subject of Section 10.2.

I’m having troubles to digest the intent of this sentence as it conveys
conflicting messages (at least for me).

## If the intent is to say that backward compatibility is not ensured then
maybe reword to, e.g.:

NEW:
   Backwards compatibility in the sense of upgraded systems continuing
   to interoperate with legacy systems is not directly covered in this
   specification. Refer to Section 10.2 for more details.

# Notations

Can we please add "->" to the list of notations?

# Section 2.1: Broken reference

CURRENT:
   Given this design of Composite ML-DSA, it is possible to split the
   pre-hashing step out from the signature generation process -- see
   {#impl-cons-external-ph} for further discussion and sample
   algorithms.

# Section 2.1: HashML-DSA

CURRENT:
   Note that while the overall construction of Composite ML-DSA is
   similar to that of HashML-DSA, the ML-DSA component inside the
   composite is "pure" ML-DSA; implementing this specification does not
   require an implementation of HashML-DSA.

Shouldn’t this be more explicit that HashML-DSA is disallowed per the same
rationale as in rfc9881#section-8.3?

# Section 3.1

CURRENT:
   Errors produced by the component KeyGen() routines MUST be forwarded
   on to the calling application.

I don’t understand the use of “forwarded” in this context? Isn’t the API call
internal within a system?

Maybe s/forwarded on/communicated?

# Section 3.1.1: Modified process and implications

CURRENT:
   In cases where it is desirable to have a deterministic KeyGen of one
   or both component keys from a seed, this process MAY be modified to
   expose an interface of Composite-ML-DSA<OID>.KeyGen(seed) such that
   one component algorithm is generated from the seed and the other from
   random, or the input seed is cryptographically expanded to produce
   seeds for both components.  Implementation details and security
   analysis of such a modified key generation process is outside the
   scope of this document.

Shouldn’t this need be highlighted in the SEC or OPS cons section so that
appropriate analysis in done before making use of a modified process?

# Section 4: Table 1

Can we please add a pointer to Section 4 of FIPS.204 to indicate the source of
that table?

# Section 6: Obvious

CURRENT:
   Labels are represented here as ASCII strings, but implementers MUST
   convert them to byte strings using the obvious ASCII conversions
   prior to concatenating them with other byte values as described in
   Section 2.2.

do we need to keep “obvious” here? If it was, then why calling it out :-)

# Section 6: Key sizes

CURRENT:
   For all RSA key types and sizes, the exponent is RECOMMENDED to be
   65537.  Implementations MAY support only 65537 and reject other
   exponent values.  Legacy RSA implementations that use other values
   for the exponent MAY be used within a composite, but need to be
   careful when interoperating with other implementations.

Given the interoperability issues there, I suggest to add relevant operational
considerations under the OPS Cons section to cover at least the following:

* Implems need to expose which sizes they support
* Implems need to offer a configuration knob to control the size.

# Section 10: Operational Considerations

## I suggest to rename “Implementation Considerations” to “Operational
Considerations” as that section is more about operational guidance. This would
be also consistent with the approach in 9881.

## Inherit ML-DSA Operational Considerations

There are important considerations that are inherited by the composite design.
Can we please add a note to increase awareness about considerations listed in
rfc9881#section-8?

## Hidden Operational Consideration

Section 1.3:
   Composite ML-DSA is NOT RECOMMENDED for use in
   applications where it is has not been shown that EUF-CMA is
   acceptable.

This reco is IMO hidden in the introduction part. Given the importance of this
reco for those who will deploy, I suggest we have this more visible as part of
the OPS Considerations section.

Hope this helps.

Cheers,
Med