On Tue, Jan 21, 2020 at 02:48:52PM +0900, AKASHI Takahiro wrote:
On Fri, Jan 17, 2020 at 06:26:34AM +0100, Heinrich Schuchardt wrote:
On 1/17/20 2:53 AM, AKASHI Takahiro wrote:
On Tue, Jan 14, 2020 at 01:04:58PM +0100, Heinrich Schuchardt wrote:
On 1/14/20 8:33 AM, AKASHI Takahiro wrote:
On Wed, Jan 08, 2020 at 06:43:51PM +0100, Heinrich Schuchardt wrote:
On 11/21/19 1:11 AM, AKASHI Takahiro wrote: >In this patch, a very simple test is added to verify that rsa_verify() >using rsa_verify_with_pkey() work correctly. > >To keep the code simple, all the test data, either public key and >verified binary data, are embedded in the source. > >Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org >--- > test/Kconfig | 12 +++ > test/lib/Makefile | 1 + > test/lib/rsa.c | 206 ++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 219 insertions(+) > create mode 100644 test/lib/rsa.c > >diff --git a/test/Kconfig b/test/Kconfig >index cb7954041eda..64d76c3b20a5 100644 >--- a/test/Kconfig >+++ b/test/Kconfig >@@ -28,6 +28,18 @@ config UT_LIB_ASN1 > Enables a test which exercises asn1 compiler and decoder function > via various parsers. > >+config UT_LIB_RSA >+ bool "Unit test for rsa_verify() function" >+ imply RSA >+ imply ASYMMETRIC_KEY_TYPE >+ imply ASYMMETRIC_PUBLIC_KEY_SUBTYPE >+ imply RSA_PUBLIC_KEY_PARSER >+ imply RSA_VERIFY_WITH_PKEY
This test should depend on RSA_VERIFY_WITH_PKEY because is cannot be executed otherwise.
See below.
Best regards
Heinrich
>+ default y >+ help >+ Enables rsa_verify() test, currently rsa_verify_with_pkey only() >+ only, at the 'ut lib' command. >+ > endif > > config UT_TIME >diff --git a/test/lib/Makefile b/test/lib/Makefile >index 72d2ec74b5f4..2bf6ef3935bb 100644 >--- a/test/lib/Makefile >+++ b/test/lib/Makefile >@@ -8,3 +8,4 @@ obj-y += lmb.o > obj-y += string.o > obj-$(CONFIG_ERRNO_STR) += test_errno_str.o > obj-$(CONFIG_UT_LIB_ASN1) += asn1.o >+obj-$(CONFIG_UT_LIB_RSA) += rsa.o >diff --git a/test/lib/rsa.c b/test/lib/rsa.c >new file mode 100644 >index 000000000000..44f8ade226f4 >--- /dev/null >+++ b/test/lib/rsa.c >@@ -0,0 +1,206 @@ >+// SPDX-License-Identifier: GPL-2.0+ >+/* >+ * Copyright (c) 2019 Linaro Limited >+ * Author: AKASHI Takahiro >+ * >+ * Unit test for rsa_verify() function >+ */ >+ >+#include <common.h> >+#include <command.h> >+#include <image.h> >+#include <test/lib.h> >+#include <test/test.h> >+#include <test/ut.h> >+#include <u-boot/rsa.h> >+ >+#ifdef CONFIG_RSA_VERIFY_WITH_PKEY
Please, remove this ifdef. Simply do not build this module if CONFIG_RSA_VERIFY_WITH_PKEY is not defined.
That is why I added this #ifdef here to avoid a build error. I think it is a good idea to make test code as generic as possible to easily add more test cases.
It is preferable to move configuration dependencies to the Makefile.
You can create a new C file if you have tests needing a different configuration.
test/lib/rsa_verify.c might be a better name for the file.
In test/Kconfig, please, replace
config UT_LIB_RSA bool "Unit test for rsa_verify() function" imply RSA
by
config UT_LIB_RSA bool "Unit test for rsa_verify() function" default y depends on RSA
Initially in my development code, I did the exact same thing, but I didn't adopt this approach.
If we follow your proposal, UT_LIB_RSA won't be selected unless RSA is enabled. So UT_LIB_RSA won't be executed under most existing configurations in the current CI. This was not what I wanted.
What I wanted here is that, if UT (and UT_LIB) is selected, all the configurations needed for enabling all the test cases be also selected *automatically*.
What I want is that if I run 'make menuconfig' and select CONFIG_UT manually I end up with a configuration that compiles and that does what I have configured.
Hmm, that is what I intend to do here, but the code doesn't work as I expected. It works only if CONFIG_UT_LIB_RSA is on in *defconfig.
So I have no option other than changing reverse dependencies as follows, CONFIG_UT_LIB_RSA select IMAGE_SIGN_INFO select RSA imply RSA_VERIFY_WITH_PKEY
Using 'select' is still safe as CONFIG_RSA has no dependency. (Please note we can still use 'imply' for VERIFY_WITH_PKEY here.)
Tom, do you agree to this change?
OK, my fault for not spotting it before. Using imply (as we do today on UT_LIB_ASN1) is wrong. All of the tests should "depends on" what they need in order to function. Thanks!