lamps-wg / draft-composite-sigs

IETF Internet-Draft about X.509 certificates with composite keys and signatures.
Other
1 stars 1 forks source link

Carl Wallace's comments on composite signatures v13 #1

Closed johngray-dev closed 2 months ago

johngray-dev commented 3 months ago

From: https://github.com/EntrustCorporation/draft-ounsworth-composite-sigs/issues/147

Below are some comments, suggestions, and questions based on a brief review of draft-ounsworth-pq-composite-sigs-13.

General

The draft defines a set of 13 algorithms. Is LAMPS the right forum to vet algorithms? Generally, LAMPS deals with specs that describe how to use a given algorithm relative to PKIX and S/MIME artifacts, not algorithm specifications. Is the "Internet PKI" really the target? Is this kind of designation appropriate for what would appear to be a general-purpose algorithm specification? The draft needs a thorough review to sharpen the fact that it now defines a set of 13 algorithms, not a framework for using composite key and signature combinations. Statements like "should be regarded as a single key" muddy the water. Even the term "composite key" is unhelpful. It's likely that most instances of the word "composite" could be written "Composite ML-DSA." Given that this spec defines a means of generating and using keys and signatures, there's no need for all the "drop-in replacement" verbiage. Offhand, I think all of the relevant structures could use templates. Assuming this is correct, it may be worth including a section that notes ASN.1 decoders/encoders are not required. All references to "explicit composites" should be reviewed and either deleted or amended. Abstract

Is a "X.509, PKIX, and CMS" target appropriate for an algorithm specification? Is there some reason any of these 13 algorithms could not be used in a different context? s/Signaturem/Signature Sections 2.3.1 - 2.3.3

These sections should include references to the relevant sections of each possible component algorithm. There is a list of algorithms in Section 5 (with a broken link for ML-DSA) but, given this is an algorithm specification, including more detail in the key generation, signature generation and signature verification sections would be appropriate and helpful. s/sketched/defined There are two references to "recursive composite public keys." These should be removed given they are not supported by definition. Section 2.3.1

s/The KeyGen() -> (pk, sk) of a composite signature algorithm will perform the KeyGen() of the respective component signature algorithms / KeyGen() will perform the KeyGen() of the respective component signature algorithms It's probably worth including pseudocode as done for Sign() and Verify() Rewrite the last sentence to be Composite ML-DSA specific: The component keys MUST be uniquely generated for use in a Composite ML-DSA key. Section 2.3.2

The second and third steps could use some work. In step 2, S1 and S2 are raw signatures from a component algorithm. In step 3, they are BIT STRINGs containing the raw signatures. Additionally, it is unlikely the "algorithm specifications" address encoding as a BIT STRING. Suggest rewriting bullet 3 as below:

Encode each component signature S1 and S2 as BIT STRINGs B1 and B2 then as a SEQUENCE

signature ::= Sequence { B1, B2 }

s/A composite signature MUST produce/Sign() MUST produce

Section 2.4

Drop the word concatenation. No OIDs are being concatenated. There are other references to OID concatenation throughout the document that should be reviewed as well. Section 2.6

Depending on the answer to the first question in General above, the following sentence may need to be removed or amended: "If other combinations are needed, a separate specification should be submitted to the IETF LAMPS working group." The statement that this "also does not preclude future specification from extending these structures" is false. The definitions are fixed at length 2. That someone could define a new specification with other lengths need not be commented on here. Suggest removing this sentence. Section 3

Replace "multiple algorithms" with "algorithm pairs" or similar. Same comment applies throughout the draft. FWIW, I did not test compile the ASN.1 or work with the examples yet. Given ECPoint is an alias for OCTET STRING do we need to ability to define types for each component, or can the object class be simplified to assert all component keys are simply OCTET STRINGs? Section 3.2

The reference to Table 3 is not altogether apt. The table lists signature algorithms. There should probably be a similar table for use in reconstituting SubjectPublicKeyInfo values (i.e., including curve name for parameters). Section 3.4

Why is this section necessary? The public key format defined by this specification is CompositeSignaturePublicKey. If someone wants to encode this in an OCTET STRING or BIT STRING that's their business. I would delete the entire section, but if the section remains, I would definitely delete the last sentence. The definitions provided call for DER encoding. Immediately following that with allowance for BER in the “interests of simplicity and avoiding compatibility issues” is odd in that it provides for neither. If you want to support BER and DER, cite BER encoding. But why do that? Amongst other things, that would break ability to templatize the artifacts. Section 5

Remove "for explicit combinations" from the first sentence. Remove the second paragraph given the list is exhaustive for this specification. Remove the third paragraph since only 13 algorithms are defined. There is no flexibility provided for in this specification to define other combinations. Security Considerations

This section should incorporate security considerations from component algorithms by reference. The section still reflects the earlier versions before the scope was limited to focus on 13 algorithms. It could use some pruning. Maybe move some of the stripping discussion to the signature generation section. All considerations are listed under the same heading. These should be broken up into new subsections (or the heading removed). Section B.2

This section (and subsections) should probably be deleted. At a minimum, they aren't really "Implementation considerations" for the 13 algorithms that are the subject of the draft.

johngray-dev commented 3 months ago

June 5th - Lets all look at Carl's comments and discussion on June 19th.