Hi Jork,
On Fri, 24 Jan 2025 at 01:00, Jork Loeser (he/him) via OP-TEE < op-tee@lists.trustedfirmware.org> wrote:
(Please provide a better way of reporting should this ML not be a good forum for bug-reports/fixes)
The prefered place is on the OP-TEE OS GitHub issues section: https://github.com/OP-TEE/optee_os/issues, but this ML is OK too, unless the discussion involves a lot of code changes.
Hello all,
While experimenting with OP-TEE OS on a non-ARM platform (x86, to be precise), I noticed xtest regression_4003 and regression_4017 to be failing. It appears that for AES-CTR encrypt/decrypt, optee_os/core/lib/libtomcrypt/src/modes/ctr/ctr_encrypt.c:s_ctr_encrypt()< https://github.com/OP-TEE/optee_os/blob/37de1791d97d0df56db0709b5d001f821598... fails to increment the counter on block boundaries when passed data covers a whole block. I suppose that it is a non-problem for architectures having crypto acceleration hooked up, since ctr_encrypt() bypasses s_ctr_encrypt() in that case.
Yes, and even without acceleration the platforms we routinely build for (arm, arm64) do not set LTC_FAST and therefore do not use the part of the code that has the problem.
The below fixes the behavior for my setup - xtest regression_4003 and regression_4017 pass as a result:
diff --git a/core/lib/libtomcrypt/src/modes/ctr/ctr_encrypt.c b/core/lib/libtomcrypt/src/modes/ctr/ctr_encrypt.c index a23e33c05..2019de2b6 100644 --- a/core/lib/libtomcrypt/src/modes/ctr/ctr_encrypt.c +++ b/core/lib/libtomcrypt/src/modes/ctr/ctr_encrypt.c @@ -61,6 +61,7 @@ static int s_ctr_encrypt(const unsigned char *pt, unsigned char *ct, unsigned lo ct += ctr->blocklen; len -= ctr->blocklen; ctr->padlen = ctr->blocklen;
s_ctr_increment_counter(ctr); continue; }#endif
This change looks totally correct to me, both in principle (given how CTR is supposed to work) and considering that the non-LTC_FAST part does call s_ctr_increment_counter() on a block boundary just a few lines below.
I understand that you might prefer a full PR instead. Please accept my apologies for not providing such at this time; perhaps one of the maintainers (Jerome Forissier?) can take the above into consideration instead. If in doubt, consider above change as BSD-2-Clause'd.
That's fine with me. I will create a PR. Let me know if I may credit you with a "Reported-by: Your Name <your.email>" tag.
Many thanks for reporting!