sign: Adding SLH-DSA signature #512
Conversation
Passes Test vectors from ACVP for internal functions.
|
there is a timeout happening on the ARM build because tests are running in parallel, but it doesn't seems to be related to a failure in the code. |
| } | ||
|
|
||
| // See FIPS 205 -- Section 5 -- Algorithm 5. | ||
| func (s *state) chain(x []byte, index, step uint32, addr address) (out []byte) { |
There was a problem hiding this comment.
The name step confused me, bc I thought it'd be the step size of the for loop, but it's not. I'd rename to steps or num_steps.
| } | ||
|
|
||
| // See FIPS 205 -- Section 5.2 -- Algorithm 7. | ||
| func (s *statePriv) wotsSign(sig wotsSignature, msg []byte, addr address) { |
There was a problem hiding this comment.
IMO it'd be more natural to return a wotsSignature instead of taking one in. It's not even immediately clear from the type signature that this is a ref.
| func (s *statePriv) wotsSign(sig wotsSignature, msg []byte, addr address) { | ||
| curSig := cursor(sig) | ||
| wotsLen1 := s.wotsLen1() | ||
| csum := wotsLen1 * (wotsW - 1) |
There was a problem hiding this comment.
I know this is an internal func, but there should be a sanity check here that msg is the correct length (I believe it's 2*wotsLen1)
| s.PRF.address.SetTypeAndClear(addressWotsPrf) | ||
| s.PRF.address.SetKeyPairAddress(addr.GetKeyPairAddress()) | ||
|
|
||
| for i := uint32(0); i < wotsLen1; i++ { |
There was a problem hiding this comment.
I found it hard to follow what this function was doing. It isn't structured like Alg. 7 at all, so I didn't have a map. More comments would be useful
| // See FIPS 205 -- Section 5.3 -- Algorithm 8. | ||
| func (s *state) wotsPkFromSig( | ||
| sig wotsSignature, msg []byte, addr address, | ||
| ) wotsPublicKey { |
There was a problem hiding this comment.
Same notes on previous func (minus the one about returning a sig) apply here
| "github.com/cloudflare/circl/internal/test" | ||
| ) | ||
|
|
||
| func testWotsPlus(t *testing.T, p *params) { |
There was a problem hiding this comment.
I'd rename testWotsPlusCorrectness, since that's the only property it tests. I agree that's enough tho
| } | ||
|
|
||
| // See FIPS 205 -- Section 6.1 -- Algorithm 9 -- Iterative version. | ||
| func (s *statePriv) xmssNodeIter( |
There was a problem hiding this comment.
Similar thing. I had trouble following along with this algorithm, since it differs so much from the paper. Also why is i a parameter at all if it's not a recursive algorithm?
|
|
||
| // See FIPS 205 -- Section 6.2 -- Algorithm 10. | ||
| func (s *statePriv) xmssSign( | ||
| stack stackNode, sig xmssSignature, msg []byte, idx uint32, addr address, |
There was a problem hiding this comment.
As before, I think it'd be clearer to return a sig than take one as input. It's also not clear it's a pointer. I think this applies to a lot of functions in this PR
| defer s.Clear() | ||
|
|
||
| s.forsSign(sig.forsSig, md, addr) | ||
| pkFors := s.forsPkFromSig(md, sig.forsSig, addr) |
There was a problem hiding this comment.
Would it be possible to compute pkFors as a side effect of forsSign? Can save a few hashes if so. Similarly might be able to do this in the htSign function
This implementation supports the twelve parameter sets approved at FIPS 205
Test vectors match the ones at ACVP-Server version 1.1.0.35. These test vectors only target the internal functions.
Pure and Prehash-based signatures are supported, but no known test vectors are available.
The last commit adds the twelve instances to the generic signing
Schemeinterface.Implementation makes a good effort to avoid heap allocations that usually add a significant overhead.