Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add one-time signature scheme from XMSS #169

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Add one-time signature scheme from XMSS #169

wants to merge 13 commits into from

Conversation

marsella
Copy link
Contributor

@marsella marsella commented Nov 4, 2024

Closes #162.

This adds the WOTS+ signature scheme that's used in XMSS. It's very slow! About 50s for each public function (keygen, sign, verify).

This will be unusably slow in the context of XMSS -- e.g. XMSS key generation calls WOTS+ key generation 2^h times, where $h \in [10, 16, 20]$ -- for the largest parameter, this would take 1048576 minutes, or roughly 728 days, without accounting for the additional hashing that happens in XMSS.

Anyway I optimized base_w a bit but I think the main cost is coming from the recursive hashing; if we want this to be faster we will probably need to go look at SHA2 (the required parameter sets) and SHA3 (the optional parameter sets). In general, the goal of these specs is not to be performant -- it's just to look exactly like the written NIST specs -- but this appears to be getting so slow that we won't even be able to check basic properties of XMSS.

Another point of interest: this takes longer than usual to load (~10 - 30s), I suspect because the type constraints have a lot of computation in them.

Aside from performance, I think this is fairly straightforwardly representative of the spec. I'll add some inline questions and notes.

also adds a missing citation.
I learned that you can put type definitions in an interface even if you
don't redefine them for each instantiation! So now the constraint on
`len` is coherent.
Adds the function that composes most of verification (but doesn't
actually do the final check).
This also moves around some type constraints to allow actual calling of
the functions.
@marsella
Copy link
Contributor Author

marsella commented Nov 4, 2024

Here's some initial benchmarks I ran. sign has a couple of measurements because it turns out all-zero input circumvents most of the function in a way that is not representative of real data.

> :time genPK zero zero zero
Measuring for 300 seconds
Avg time: 51.98 s    Avg CPU time: 52.03 s    Avg cycles: 1247554038
> 
> :time WOTS_sign zero zero zero zero
Measuring for 300 seconds
Avg time: 2.598 ms    Avg CPU time: 2.600 ms    Avg cycles: 62349
> :time WOTS_sign (zero # ADRS) zero SEED ADRS 
Measuring for 300 seconds
Avg time: 22.73 s    Avg CPU time: 22.73 s    Avg cycles: 545567978
>
> :time WOTS_pkFromSig  zero zero zero zero
Measuring for 300 seconds
Avg time: 51.43 s    Avg CPU time: 51.45 s    Avg cycles: 1234365157

Comment on lines 8 to 9
F KEY M = split (SHA256::hash (join ((zero : [32][8]) # KEY # M)))
PRF KEY M = split (SHA256::hash (join ((zero : [31][8]) # [(3 : [8])] # KEY # M)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nb: I think I need to add a hashBytes function to all the hashes because this is starting to be a really typical pattern (split (hash (join x))).

Comment on lines +73 to +74
* checksum. The checksum has an upper bound of 32 bits, so the padding
* must be at least 32 bits.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: I'm still kind of confused about this; the spec doesn't clarify what to do if the padding is shorter than the checksum, so I assume it needs to be at least as long. However in my instantiation it was shorter (only 2 bytes). Maybe I filled in the instantiation incorrectly?

Anyway I took out this type constraint but I need to figure out what's going on here.

Comment on lines +114 to +115
| m <= 8 * y => split (zext x)
| m > 8 * y => split (take x)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: this relates to the comment above -- the spec doesn't say anything about the m > 8y case. I need to try to figure out whether it should be here or not.

* Note that this isn't defined in terms of the `Word` type because it's
* operated on by things that require arrays of bytes.
*/
type OTSHashAddress = [8 * 4]Byte
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: open to suggestions on whether to represent this differently. E.g. it could be a record with named fields. This "looks" the most like the ascii representation of the type in Section 2.5 but I'm not sure that's the obvious best way to represent it.

* distribution or selected using a cryptographically secure pseudorandom
* process! Cryptol cannot verify that a `PrivateKey` was chosen suitably!
* Implementors must independently audit private key generation!
*
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: There's a bit about pseudorandom key generation in the spec; it calls out to a separate doc with a pseudorandom method. Consider adding that function to get a key from a seed or at least describing it in more detail here.

Comment on lines +167 to +168
genPK : PrivateKey -> OTSHashAddress -> [n]Byte -> PublicKey
genPK sk ADRS SEED = pk where
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (naming): In the spec this is WOTS_genPK. Preference on using the prefix (here and in the other functions) vs relying on the namespace to "fill in" the prefix in the calling spots (e.g. call it genPK and invoke it using WOTS::genPK).

Primitive/Asymmetric/Signature/XMSS/Wots.cry Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement WOTS+ signature scheme
1 participant