Adapt some test scripts for SM3 support - #320
Conversation
Add SM3 to the low-level hash test generator and avoid exercising SM3 as a component of key algorithms for the time being. Signed-off-by: Ronald Cron <ronald.cron@arm.com>
gilles-peskine-arm
left a comment
There was a problem hiding this comment.
There are two ticking bombs. The python version sensitivity is not likely to explode until way after SM3 lands, but it also seems unnecessary. The restriction on generated tests will explode while SM3 lands.
| def _hashlib_calculator(name: str) -> Optional[Callable[[bytes], str]]: | ||
| """Return a hashlib-based calculator if the algorithm is available.""" | ||
| try: | ||
| hashlib.new(name) |
There was a problem hiding this comment.
Having output that depends on the version of Python could be a problem. At one point, I'd like to clean up our Python testing, and one of the things I would test is that different versions of Python produce the same generated file.
| 'PSA_ALG_SHAKE128_256': None, #lambda data: hashlib.shake_128(data).hexdigest(32), | ||
| 'PSA_ALG_SHAKE256_256': None, #lambda data: hashlib.shake_256(data).hexdigest(32), | ||
| 'PSA_ALG_SHAKE256_512': None, #lambda data: hashlib.shake_256(data).hexdigest(64), | ||
| 'PSA_ALG_SM3': _hashlib_calculator('sm3'), |
There was a problem hiding this comment.
Why bother to look for it? SM3 is not in the current development version of Python.
I'd like to move to a model where only maintainers generate this test data, and all algorithms are mandatory. We now have this model for generate_mldsa_tests.py. We should do the same for hashes, but that's at least a size-s task of itself.
| # SM3 support is limited to hash operations. Do not exercise key | ||
| # algorithms that use SM3 as a component. |
There was a problem hiding this comment.
This is a temporary limitation in code that is not in the framework. At some point we're going to want to lift it, but then we'll need a migration path for the time period where the same framework commit is consumed both by TF-PSA-Crypto branches that have the limitation and branches that haven't.
One solution for this transition is to first add support for compound operations in TF-PSA-Crypto, then wait some time (at least one release, I would say) to flush the branches we care about, and finally lift the limitation here. But this takes time.
A better solution is to split the script into a small runner in the consuming project, and code in the framework that takes some arguments that determine what to generate. This is what I've done with generate_mldsa_tests.py and mldsa_test_generator.py. But migrating to this architecture would be a size-s task of itself.
Another possible solution would be to have the script detect the availability of the algorithms, like it detects the availability of the basic building blocks. I don't know if there's a plausible way to do that. The test case generator parses test_suite_psa_crypto_metadata.data to find combinations of algorithms, in addition to having rules to build combinations from constructors. That code is a bit of a mess, and I don't know offhand if there's a reasonable way to extract meaningful information.
Yet another solution would be to generate all the tests as usual, and make it the responsibility of TF-PSA-Crypto to skip some tests at runtime. It's a bit of a hack, but I think it's fine for a temporary solution. We'd have to list exceptions in analyze_outcomes.py, and we'd remember to stop skipping these tests thanks to the issue linked from the exception.
Finally, a clean solution would be to not add code to prevent algorithm combinations in the first place. This has my preference. Why should we go out of our way to block things that will naturally work? The only extra work is to define an HMAC block size. You don't have to make a big pull request with everything: you can add tests in further pull requests.
|
#316 is closer to what's needed. I think it would be easier for you both if one of you made a single PR that lists both Blake and SM3. |
Description
Needed for Mbed-TLS/TF-PSA-Crypto#895
PR checklist