Skip to content
This repository has been archived by the owner on Jul 9, 2024. It is now read-only.

Issues Found in recent updates for the 10 release #112

Closed
opencrypto opened this issue Oct 24, 2023 · 2 comments · Fixed by #110
Closed

Issues Found in recent updates for the 10 release #112

opencrypto opened this issue Oct 24, 2023 · 2 comments · Fixed by #110
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Milestone

Comments

@opencrypto
Copy link
Collaborator

opencrypto commented Oct 24, 2023

Context

(last updated on Oct 23, 2023)

While reviewing -10, we noticed some things that can be improved. We created a new pull requests where we fixed the editorial changes (see logs in Pull Request #111 ).

Additionally, here's the list of more substantive issues that we found in the last release and that are NOT currently addressed by the PR referenced above since we have not discussed them.

Major Proposals/Discussion Points:

  • Section: OID Concatenation. In the section, there are many strings that are defined and their values are very similar. We noticed some errors in the list and we are worried that implementations may struggle to find a mis-configured string value. The proposed resolution is to use a single string for all combinations. Given that (a) we only support 2 keys, (b) the OIDs are already present in the structure, the use of a single string gives, practically, the same property (e.g., "id-composite-signature").
  • Section: Composite Verify. In the section (and throughout the document), we advocate for the order of signatures and signing keys to be the same. However, because of the fact that we allow only 2 keys, this requirement does not seem to provide any real security benefit. On top of this, the process of verifying composite signatures would be impossible when the input for the signing algorithm are provided via separate certificate chains, for example (how can you deterministically establish an order in that case? Do we still want to support this use-case behind the scenes?)
  • Section: KeyGen. I am a bit confused about why all the key definitions and the key gen has been moved to this document too. I understand that for the KEM document we needed something self-contained (well, to be honest... we did not need one, we could have updated the current one...), but this is not needed for the signature one. Is the intention here (but we did not discuss it!) to retire the keys document? I think this is a mistake since we would have to replicate the same sections for key generation across different documents. Why not updating the keys document that defines the format? I am just worried that having to re-write structures definitions across different documents might be way more difficult for implementers than having a base document that we can then specialize for the generic case (i.e., "The XX version of composite updates the definition Keys and Key Gen algorithm of [YYY] with the following modifications..."
  • Section: Pre Hashing The Message. While reading the section that explains the reasoning for choosing specific algorithms with specific combinations, I was a bit puzzled by the fact that there is no consistent approach. On one side, the section says that it wants to match the security level of the algorithms, but on the other hand it also wants to align with the internal hash used by the algorithm. This, I think, is a problem. It is our opinion that the second argument about aligning with the internally used hash algorithm is not valid in the sense that there is no advantage in using the same one (very weak code re-utilization argument) and it is plain simply confusing.

Here's more general issues we found:

  • Supported Combinations. As it is written today, the document bears thirteen (13) combinations for ML-DSA and only three (3) for FN-DSA. This does not seem correct to us. We would like to propose two possible resolutions here:
    • Reproduce the same number and types of combinations that we have for ML-DSA for FN-DSA. This makes the document symmetric with respect of the two algorithms. However, what will happen when the next algorithm is standardized (i.e., the new signature selection process identifies the next candidate... NIST Additional Signatures Round 1), or
    • (preferred) remove the OIDs for FN-DSA from the document altogether. This will make the document focused on ML-DSA (mentioned several times in our e-mails). An additional document can carry the OIDs and definitions for FN-DSA and references. This would also provide an example for how to add combinations to the Explicit via new documents. I would expect to find, at the end of the tunnel, an ML-DSA Explicit Composite Signatures and Keys, a FN-DSA Explicit Composite Signatures and Keys, etc..).

Question about naming conventions:

  • FN-DSA names for Level 1 and Level 5. When trying to update the draft to use FN-DSA instead of Falcon, I did not know what names to use for level 1 and level 5. We need some references to fix this issue. So far, we used FNDSA512 for the OID string (only level 1 is used in combinations, so far).
@opencrypto opencrypto added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Oct 24, 2023
@opencrypto opencrypto added this to the IETF118 milestone Oct 24, 2023
@opencrypto opencrypto linked a pull request Oct 24, 2023 that will close this issue
@opencrypto opencrypto pinned this issue Oct 24, 2023
@ounsworth ounsworth unpinned this issue Feb 8, 2024
@johngray-dev
Copy link
Collaborator

I wonder if we should just remove FN-DSA (Falcon) as it is going to be standardized a year out. We can always add algorithm update documents in the future...

@johngray-dev
Copy link
Collaborator

Group agreed that we have discussed and addressed these issues.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants