fix and cleanup des and 3des test cases #509
Conversation
|
Thanks for the PR!
Those 3 points are fine IMO
This part is misleading as they were never explicitly 3DES TV's but just "more" DES TV's and now you're (ab)using the fact that the same key used thrice results in the same result as a single DES round... I think this part should be reverted. @karel-m your thoughts? |
|
btw if you're already at it, feel free to re-factor the TV's in |
|
Thanks for the feedback! I started looking at the extra test vectors because they failed when I enabled them. Googling the test vector values found a golang test suite that uses them for 3DES, so I just copied that approach. However, I see now that the real bug in the current LTC code is that these vectors are marked as DES decryption tests when they should be DES encryption tests... just fixing that makes the tests pass. I'll send an updated PR. |
armcc
force-pushed
the
improve-des-tests
branch
from
aed1a1a to
8646c0c
Compare
armcc
force-pushed
the
improve-des-tests
branch
from
8646c0c to
d3f4398
Compare
|
Thanks! Looks very good now and thx for the TDES TV's! Now you only need to fix this travis error and we're good to go :-) https://travis-ci.org/libtom/libtomcrypt/jobs/595904038 btw. you can install the pre-commit hooks via |
- Drop unused 'num' field from struct des_test_case. - Fix the order of arguments passed to compare_testvector() (actual and expected buffers were swapped, leading to misleading error messages for failing tests). - Enable all DES test vectors by default and use them for both encrypt and decrypt. That allows the struct des_test_case 'mode' field (which was previously incorrect for the LTC_TEST_EXT tests) to be dropped. - Run the "encrypt / decrypt all zero's" tests once, instead of running repeatedly from within the test vectors loop. - Add minimal set of 128bit key 3DES test vectors. - Try to more closely align the des_test() and des3_test() functions (common flow, common variable names, etc). - Minor indent fixes. Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
armcc
force-pushed
the
improve-des-tests
branch
from
d3f4398 to
d85045e
Compare
|
The indent issue should now be fixed but it looks like the Travis build then failed for some other reason? Should I force push the latest version again to try to re-trigger Travis? |
yes, that's fixed
yeah, there's an issue with their infrastructure, c.f. [1] resp. [2]
no need, I've just restarted the failed builds [1] https://travis-ci.community/t/sometimes-build-fails-when-apt-is-updating-postgresql-apt-repository/4872 |
2 participants
"DES" tests (actual -vs- expected buffers were swapped, leading to
misleading error messages for failing tests).
des_test() into des3_test(). Note that these 3des test vectors
were previously guarded by LTC_TEST_EXT, so even though they
always failed when run as des tests, the failure was not seen by
default.
Signed-off-by: Andre McCurdy armccurdy@gmail.com
Checklist