crypto.sha1, crypto.sha256, crypto.sha512: add Secure Hash Algorithm Validation System tests

[kimshrier]

These test vectors come from the Secure Hash Algorithm Validation System as described at the NIST web site at secure-hashing.

I did not find any bugs in the v crypto routines with these test vectors. But, I do feel better about the accuracy of our sha1, sha256, and sha512 code since it passes all these tests. And, in the future, if optimized versions of these hash functions are implemented, we will have a much more rigorous set of unit tests.

[kimshrier]

What would be the preferred way to conditionally compile the really long test messages. I realized that I should have tested for $if !msvc instead of $if !windows, but I can't put a test like this around a const declaration.

[kimshrier]

My latest attempt to get around a Microsoft MSVC shortcoming, has failed. Recommendations for what I should attempt would be appreciated. I do not have any Windows environment here to test on so I am just guessing about what to do.

[JalonSolov]

Current failures on Linux are because you need to run v fmt -w . to format all the files. Looks like it might be the same for Windows.

[kimshrier]

I did do a v fmt -w on all the files before committing them. I am going to undo my latest commit since it was just a hack to see if it would placate msvc.

[spytheman]

Imho put the long messages in .txt files in a testdata/ folder, then modify the tests, to just iterate over samples := os.walk_ext('testdata/', '.txt') and read the files at runtime with os.read_file().

That will make them usable with msvc too.

[spytheman]

Another alternative is:
const abc = $if msvc { []SomeType{} } $else { ... }

[spytheman]

The external .txt files are better though for tooling reasons. Github struggles to show very long source files (it is also made by MS, like msvc 🤔).

[kimshrier]

I'm working on the .txt file approach.

[spytheman]

Excellent work.

I am a bit torn about merging this PR 🤔 .

The tests are valuable, and I agree with the description in the PR.

On the negative side, they are also relatively big.
Adding them will increase the size of the checkout by ~14% (8.8MB) in a single commit:

They also have high entropy, so they will increase the size of the released .zip files too, more than ordinary sources that compress a lot better (the increase for the linux .zip is ~4MB):

I guess that in the future, we could change the .zip release script, so that it could remove more files, like _test.v or testdata/ folders, that will not be needed by most V users, that only use the V releases, for compiling their own programs, while the repo clone could remain more full featured, and bigger, for V module developers 🤔 .

[spytheman]

Or we could have 2 .zip release variants - one "full" and one "light", so that people could choose whatever variant suits them.

[kimshrier]

I apologize for potentially creating a problem. I was wondering if these tests should be in a separate repository.

[JalonSolov]

Actually, that would solve the problem. The CI could clone that other repo then run the tests. Anyone who wanted to run them locally could do the same.

[medvednikov]

Great work, but yes it would be better if they were in a separate repository.

A separate CI job can pull from this repo and run the test.

[kimshrier]

For moving these tests and their associated data to a different repo, I assume that someone with the right authorization would need to create the new repo and than I could push the tests there and remove them from this repo. I do not have any experience with configuring the CI stuff on github so it would probably be in the best interest of the project to have someone more knowledgeable then me handle that part of the change. I could look in to it but I would need a serious review before committing anything.

[JalonSolov]

You could create the repo with your own id, but since this is something that we really should keep as part of the whole V ecosystem, @medvednikov can you create a repo for him under vlang? Perhaps something like... vlang/hash_validation_tests.

Other tests could perhaps be moved there as well, if we want to do more thorough testing like this for the other hash algorithms.

[medvednikov]

Done

https://github.com/vlang/hash_validation_tests

[medvednikov]

Entire dev team has write access.

[kimshrier]

i'll take on the task of moving my test code over. I may not be able to get to it today.

[kimshrier]

I don't see an option for forking https://github.com/vlang/hash_validation_tests. Is that still the preferred way to contribute? By pushing to my fork, that is?

Another question. Should I create a directory structure that follows what is in vlang/v. For instance, should I create vlib/crypto/sha1 , vlib/crypto/sha256 , and vlib/crypto/sha512 directories and put my xxx_test.v and testdata directories under them?

[spytheman]

No, there is no need to mirror the directory structure, although you can, if you prefer.

I've added a description for the new repo, @kimshrier . Can you try cloning it again?

@medvednikov can you please check, if @kimshrier is a member of the dev team on github, and add him, if not?

The new repo may also have to be public, so that other people, could check it out too. It will not contain any secrets/private info, just tests that take a lot of space and time to run.

[spytheman]

Hm, it is already public:

I do not know why it is not clone-able then 🤔 .

[spytheman]

Can it be renamed to more_tests_for_the_crypto_modules?

[spytheman]

I've just added .github/workflows/more_extensive_but_slower_tests_ci.yml that will clone and run v test . over a clone of https://github.com/vlang/hash_validation_tests in b2e8b2d .

[spytheman]

It runs fine now (although there are no tests yet there): https://github.com/vlang/v/actions/runs/10827196676/job/30039815599 :

[spytheman]

Hm, the new repo can be useful to extract not slow crypto tests, but vlib/v/slow_tests/ too.

Perhaps just slower_tests or even just more_tests would be a better, more general name?

[StunxFS]

@spytheman

Hm, the new repo can be useful to extract not slow crypto tests, but vlib/v/slow_tests/ too.

Perhaps just slower_tests or even just more_tests would be a better, more general name?

slower_tests is a better name to represent the contents of that repository (of course, if it's going to contain pure slow tests)

[medvednikov]

Done:

https://github.com/vlang/slower_tests

[JalonSolov]

Heh. I would have renamed it hash_tests. The tests that are being moved are large tests, not slow tests. Slow tests could be moved there as well, but so could speed tests.

Doesn't really matter what it's named. It is a place for extra tests of the hash functions. Or at least it was. Now it looks like it can be used for slower tests of everything.

[medvednikov]

It's going to be used for any kind of slow tests, not just has tests.

[JalonSolov]

The repo wasn't cloneable before because although Alex added the repo in github, that just adds the project name. The git repo under it wasn't initialized until @spytheman add the README.md file. Should have been cloneable/forkable from that point onward.

I've since added .gitattributes and .gitignore, and updated the README.md for the repo name change.