Duplicate symbol appears in alphabet for FF1 base85 test file

[dspdon]

The alphabet definition contains a duplicate symbol in the FF1 test file:
testfolders_v1/aes_ff1_base85_test.json

[dspdon]

Sorry, I believe I opened this in error.

[dspdon]

I paused on this issue due to some insight I was receiving from Daniel regarding a KW and a KWP issue I opened. However, I see a different basis for concern here. The base85 test file contains a duplicate underscore symbol in the alphabet. The same alphabet is defined for all test cases. Having a duplicate symbol seems appropropriate for some test cases, but it seems unlikely to have been intended for all test cases.

Do you confirm two underscore symbols in the alphabet? I wonder if it downloaded properly.

[bleichenbacher-daniel]

Yes, this is a mistake. aes_ff1_radix85_test.json contains the same test vectors in a different format.

[dspdon]

Thanks - I ran those radix85 test cases, they work fine. Will you be able to regenerate the base85 test with a unique alphabet? It would be nice to have that case for validation purposes.

[bleichenbacher-daniel]

Unfortunately, I don't have access to the generation code anymore since I'm no longer working at Google. This is sad because fixing this would probably require just 2 lines of change.
I'm working on it but regenerating the whole set of test vectors from scratch will take some time. FF1 is an algorithm that has many opportunities for mistakes. E.g., BouncyCastle had flaw that was caused by a floating point rounding error. After noticing this flaw, I did add quite a large number of additional test vectors for more edge cases. As far as I can see these test cases have not yet been uploaded.

[dspdon]

Hi Daniel Attached please find an updated version of aes_ff1_base85_test.json. It is a regeneration based on the radix85 file, and uses an updated alphabet to produce a new base85 file. The new file preserves the order of all test cases, and retains all fields and strings as the original. It only required mapping from radix digit to alphabet symbol; no FF1 cipher needed to be executed to create the updated file. The differences are as follows. *Difference 1: alphabet* The new base85 file has the alphabet updated to use a unique set of symbols, with the same alphabet utilized in every test case. I mentioned previously that the alphabet had two underscores; it also contained two t's. I removed the duplicate symbols, added a "w" (which was missing and likely a good/expected addition) and added ":" which fit into the alphabet nicely. Finally, I used "-" (hyphen) as the "invalid symbol" for test cases that need an invalid symbol in the plaintext. I notice your base85 file seemed to rotate through a set of invalid symbols. If you used an algorithm for this, let me know; I'd be happy to re-run the generator. *Difference 2: "ct" computation* Of course, the ciphertext "ct" does not remain the same with the change in alphabet. *Difference 3: no space before colon* Although it's valid pretty-print JSON, my generated JSON file has: "header": [ instead of "header" : [ Notice the colon immedately following the end quote in "header", versus a space after "header" in yours. I cannot coax my pretty-print to do what yours does. *Difference 4: scalar vs vector* Similarly, although it's valid pretty-print JSON, my generated file presents the "tests" structure without "vector" brackets when only a scalar child struct appears. In yours, every "tests" structure has vector brackets even when a scalar child struct appears. I cannot coax my pretty-print to force vector notation when scalar children are rendered. I ran my cipher suite with the new file and properly processed all FF1 radix85 tests. It is of course up to you if you choose to use this file in the test suite, or just use it to compare to what you may be producing on your own. Regards, --Don

…

On Mon, Jan 15, 2024 at 4:50 AM bleichenbacher-daniel < ***@***.***> wrote: Unfortunately, I don't have access to the generation code anymore since I'm no longer working at Google. This is sad because fixing this would probably require just 2 lines of change. I'm working on it but regenerating the whole set of test vectors from scratch will take some time. FF1 is an algorithm that has many opportunities for mistakes. E.g., BouncyCastle had flaw that was caused by a floating point rounding error. After noticing this flaw, I did add quite a large number of additional test vectors for more edge cases. As far as I can see these test cases have not yet been uploaded. — Reply to this email directly, view it on GitHub <#100 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACWSRZQNCKPRMEKIKWCMHDYOT3W7AVCNFSM6AAAAABBBUNZBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJRG42TIMRYGM> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[bleichenbacher-daniel]

Thanks a lot for looking into this issue.

Please note, that I can't upload any new files, since I don't have access. All the test vector files are generated and fixing them would best be done by fixing the generation code.

I was forced to write my own formatter for the test vector. Mainly, since the only pretty printer that generated reasonably looking output ran afoul with the style checkers and generated test vector files would trigger thousands of style violations. Personally, I don't care a lot about the amount of spaces used. Also, no spaces before ':' does indeed look better. More important is the format of the files, which could be checked with JSON schemas. Unfortunately, the schemas for testvector_v1 files are still missing on github. I would think that many test implementations would fail if "tests" just contain a single test vector instead of an array.

Thanks again, it is really sad that this issue cannot be fixed in reasonable time.