[DI] Move files from tests/scripts to mbedtls-framework #12
Labels
No labels
approved
bug
dependencies
DO-NOT-MERGE
enhancement
good-first-issue
help-wanted
historical-reviewed
historical-reviewing
needs-adoption
needs-ci
needs-design-approval
needs-info
needs-preceding-pr
needs-review
needs-reviewer
needs-work
priority-high
priority-low
priority-medium
priority-scheduled
priority-very-high
single-reviewer
size-l
size-m
size-s
size-xs
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-Mbed-TLS/mbedtls-framework#12
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Definition of Done for the work on a test script that must be move to mbedtls-framework (see table below) as part of this issue:
tf-psa-crypto/tests/scriptsusing that script and the component is run as part of the mbedtls CI (see #40 for more information abouttf-psa-crypto/tests/scriptscomponents).When the PR https://github.com/Mbed-TLS/mbedtls/pull/9445 is merged, the work on the generate_*.py files that in the table below have a green tick in their "F" column will be completed: files in mbedtls-framework, used to build and test mbedtls 3.6 and development, used to build and test the
tf-psa-cryptocode in thetest_cmake_tf_psa_crypto_out_of_source()all.sh code (component not yet in the right place though but this will be addressed by #40).Before to move a test script to mbedtls-framework, it has to be the same in 3.6 and development branches as otherwise it is
very likely to not work properly with one of these branches.
The following table lists the files in tests/scripts/ as of
898066b851.Besides the work on
depends.pythat will be covered by other issues, the duration for the estimated work (not fully estimated but what is not estimated does not seem to be necessary for TF-PSA-Crypto split) is:Necessary for the split (CI): 4M + 3S + 16XS ~ 8M
Done: 7XS
Remaining: 4M + 3S + 9XS ~ 7M
Not necessary for the split: 2XS
Table columns legend:
CI: Directly or indirectly used by the CI
F: To be moved to mbedtls-framework (y/n)
C: Used in TF-PSA-Crypto
I: Currently identical between 3.6 and development? (y/n/36/40/-) (36/40 =
present only in that branch, - = no longer present, presumably moved already)
S: Work size for the move and potentially changes needed for TF-PSA-Crypto
That's used on the CI - not in the PR job, and not via
all.sh, but it's invoked directly in the nightly and release jobs (that's what provides numbers for the testing part of the release report). So I'm moving it to "CI: yes".Edit: I think it should be in the framework because the 3.6 and 4.0 versions should be pretty close. The tf-psa-crypto version may differ significantly, so perhaps it will be more efficient for tf-psa-crypto to have its own copy. TBD.
I see no reason for moving those to the framework: those were used to generateEdit: wrong: https://github.com/Mbed-TLS/mbedtls-framework/issues/12#issuecomment-2388288623.datafile content for crypto test suites. As such, they belong in tf-psa-crypto only - they are not used and will never be used on the mbedtls side.(Note that all 4 of these scripts are for mostly for historical purposes, I don't really expect we'll have to run them again at any point. But if we feel the need to run them again at some point, the output will only be used in tf-psa-crypto.)
(The input files were downloaded from the NIST website. It's unfortunate that the scripts don't give more details, like a speficic URL, or actually we could have just committed the input files as well. I we want to find the input files again, I guess we should start by looking for CAVS (cryptographic algorithm validation suite).
So, I'm moving them to a new section "move to tf-psa-crypto".Edit: moving them back.That's not correct, due to the existence of Mbed TLS 3.6, which also has crypto. (Also, there might be LTS branches in tf-psa-crypto in the future and then the files would be shared between multiple branches on the crypto side.)
So, moving those files back to their original classification.
It's not really my place to interfere, but I'd be in favour of us avoiding moving things to the framework unless absolutely necessary, for the following reasons:
developmentand LTS versions just by backporting changes anyway.Is sharing between
developmentand LTS branches a sufficient reason to put something in the framework?I think it is. Historically, there haven't been a lot of changes in test case generation scripts that couldn't be backported. There are many that we didn't backport, but we could have, we just didn't bother because there weren't needed.
As to those particular scripts though (gen_gcm_decrypt.pl, gen_gcm_encrypt.pl, gen_pkcs1_v21_sign_verify.pl and gen_ctr_drbg.pl) — they're only of historical interest, since we aren't running them actively, so I don't think moving them is important. (But I think moving them is trivial, since we aren't running them actively.)
Currently I think that's the agreed plan, yes. The primary purpose of the framework is to allow sharing between
mbedtlsandtf-psa-crypto, but a secondary goal is to allow sharing between branches in the same (or a different) repo. (In the specific case above, which is honestly not that interesting as the files are mostly historical relics, so we're fine either way, they would be shared betweentf-psa-crypto/developmentandmbedtls/mbedtls-3.6after the split.)I wrote the following in the EPIC description (internal ref: SECLIB-1357):
I believe it reflects the current plan (though the wording of (2) is mine). However, I don't think you're the only one having doubts about the scope. We might want to have another team-wide discussion about that to make sure we're all on the same page and check if we're still good with the current plan. (And yes, I think it is your place, and any team member's place, to express doubts about current plans.)
Regardless, I intend to prioritize moving files that will be used by tf-psa-crypto. I'm in the process of re-organizing the table above and in #16 in order to dispatch between stages 2 and 3, and prioritize according to whether it's used by tf-psa-crypto within each stage.
Closing as superseded by #52 and #53 - initially same content, but grouped differently.