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.aka...@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! -- Tom
signature.asc
Description: PGP signature
