Add more test vectors to salsa20 ?

[oxarbitrage]

The salsa20 implementation has a few test vectors here, some of them seems to be from here.

I was wondering if this implementation should add some more test vectors in order to lock the functionality against future changes and have more coverage.

I am pretty sure this implementation will pass all these test vectors but will be nice to confirm it:

• https://github.com/das-labor/legacy/blob/master/microcontroller-2/arm-crypto-lib/testvectors/salsa20-256.64-verified.test-vectors
• https://github.com/das-labor/legacy/blob/master/microcontroller-2/arm-crypto-lib/testvectors/salsa20-128.64-verified.test-vectors
• https://github.com/das-labor/legacy/blob/master/microcontroller-2/arm-crypto-lib/testvectors/salsa20-full-verified.test-vectors

These are the only test vectors from some sort of reliable source that i was able to find online for salsa20.

This is a lot of vectors, an option will be to just implement some here instead. Another approach taken here is a shell script that generate the tests. The simpler one will be to just add an ecrypt module inside the tests, hardcode the vectors and run them.

I could create a PR for this if there is some interest.

[tarcieri]

Sure, that'd be appreciated

[oxarbitrage]

I was doing some research here and i ended up creating a little script to convert the data to JSON which i think it might be more useful in general than the old format used in the eSTREAM project:

https://github.com/oxarbitrage/salsa20-ecrypt-vectors-converter

So the files i was planning to use here are https://github.com/oxarbitrage/salsa20-ecrypt-vectors-converter/blob/main/test_vectors_128.json and https://github.com/oxarbitrage/salsa20-ecrypt-vectors-converter/blob/main/test_vectors_256.json

In order to use that i will have to add a few dev dependencies in https://github.com/RustCrypto/stream-ciphers/blob/master/salsa20/Cargo.toml#L18

The dependencies i will need are serde, seder-json and hex. I think that should be fine as they are only dev dependencies but that will mean a bit of more work in the repo as dependabot will submit pull requests to upgrade them as needed.

There might be other reasons for not adding more dev dependencies i am not seeing so i preferred to ask first if that will be ok or not. I can think on other alternatives if this is a problem.

Thanks!

[tarcieri]

We have a standard format for test vectors implemented by the blobby crate cc @newpavlov

[newpavlov]

I can do conversion into the blobby format. If we want to use the standard test macros from the cipher crate, I suggest we use only the vectors which end with 448..511 with blanks in streams filled by our implementation.

I am not sure how the xor-digest field works. Is it result of XORing every 64 byte chunk until last chunk provided in a vector?

[oxarbitrage]

The only thing i was able to find for the xor-digest is https://www.cosic.esat.kuleuven.be/nessie/testvectors/sc-title.html

Unsure if we will want to test it.

[oxarbitrage]

I was thinking on testing all the stream outputs similar to how it is done in https://github.com/RustCrypto/stream-ciphers/blob/master/salsa20/tests/mod.rs#L92-L99 but i will be happy to try whatever you guys think it will be the best.

[oxarbitrage]

I this this research in a separated branch, i wanted to figure out if this implementation pass the ecrypt tests and it does: oxarbitrage#1

But i was able to test only the 256 bit key sizes as the 128 ones are not supported.

[tarcieri]

Yeah, we don't currently support 128-bit Salsa20 or have plans to work on it. I'd consider it esoteric and not used in practice.