| Message ID | 20191011074200.30269-1-takahiro.akashi@linaro.org |
|---|---|
| Headers | show |
| Series | import x509/pkcs7 parsers from linux | expand |
I hope this patch set will be reviewed promptly as I'm aiming to push my "UEFI secure boot" patch for v2020.01. On Fri, Oct 11, 2019 at 04:41:49PM +0900, AKASHI Takahiro wrote: > Asn1 parsers of x509 certificates and pkcs7 messages are required > to implement image authentication and variable authentication as > part of UEFI secure boot feature. > > As we discussed before in the thread[1], most people insisted that > we should re-use corresponding source code from Linux repository > for this purpose. > > Here is my attempt to import all the necessary files from Linux; Those > will eventually be part of UEFI secure boot implementation, but I'd like > to get early feedback from other peoples before submitting the whole > patchset so that they will be better formatted for merging. > > My approach here is > * files from the latest Linux > * modify files as little as possible > * mark/protect unavoidable changes with "#if(n)def __UBOOT__" > so that future fixes/differences in Linux repository will easily > be applied to U-Boot. > > Please note that checkpatch.pl will complain with a bunch of > warnings/errors but I intentionally left them unchanged for the sake > of better maintainability I said above. > > Any comments will be appreciated. > -Takahiro Akashi > > [1] https://lists.denx.de/pipermail/u-boot/2019-April/366423.html > > Changes in v1 (Oct 11, 2019) from RFC > * change the kernel code base from v5.0 to v5.3 * add preparatory patches (#1, #2 and #3) -Takahiro Akashi > * comment off x509_check_for_self_signed() which is not useful > for UEFI secure boot (patch#9) > * improve usages of "#if(n)def __UBOOT__* to minimize differences > between U-Boot and linux kernel > > AKASHI Takahiro (11): > linux_compat: add kmemdup() > include: time.h: define time64_t > include: kernel.h: include printk.h > cmd: add asn1_compiler > Makefile: add build script for asn1 parsers > lib: add asn1 decoder > lib: add oid registry utility > lib: crypto: add public key utility > lib: crypto: add x509 parser > lib: crypto: add pkcs7 message parser > lib: crypto: add rsa public key parser > > cmd/Kconfig | 3 + > include/crypto/internal/rsa.h | 57 + > include/crypto/pkcs7.h | 47 + > include/crypto/public_key.h | 90 ++ > include/keys/asymmetric-type.h | 88 ++ > include/linux/asn1.h | 65 ++ > include/linux/asn1_ber_bytecode.h | 89 ++ > include/linux/asn1_decoder.h | 20 + > include/linux/compat.h | 4 +- > include/linux/kernel.h | 2 + > include/linux/oid_registry.h | 117 +++ > include/linux/time.h | 24 + > lib/Kconfig | 12 + > lib/Makefile | 18 + > lib/asn1_decoder.c | 527 ++++++++++ > lib/build_OID_registry | 203 ++++ > lib/crypto/Kconfig | 38 + > lib/crypto/Makefile | 46 + > lib/crypto/asymmetric_type.c | 668 ++++++++++++ > lib/crypto/pkcs7.asn1 | 135 +++ > lib/crypto/pkcs7_parser.c | 693 +++++++++++++ > lib/crypto/pkcs7_parser.h | 65 ++ > lib/crypto/public_key.c | 376 +++++++ > lib/crypto/rsa_helper.c | 198 ++++ > lib/crypto/rsapubkey.asn1 | 4 + > lib/crypto/x509.asn1 | 60 ++ > lib/crypto/x509_akid.asn1 | 35 + > lib/crypto/x509_cert_parser.c | 697 +++++++++++++ > lib/crypto/x509_parser.h | 57 + > lib/crypto/x509_public_key.c | 292 ++++++ > lib/linux_compat.c | 11 + > lib/oid_registry.c | 179 ++++ > scripts/Makefile | 3 + > scripts/Makefile.build | 2 +- > scripts/asn1_compiler.c | 1611 +++++++++++++++++++++++++++++ > 35 files changed, 6533 insertions(+), 3 deletions(-) > create mode 100644 include/crypto/internal/rsa.h > create mode 100644 include/crypto/pkcs7.h > create mode 100644 include/crypto/public_key.h > create mode 100644 include/keys/asymmetric-type.h > create mode 100644 include/linux/asn1.h > create mode 100644 include/linux/asn1_ber_bytecode.h > create mode 100644 include/linux/asn1_decoder.h > create mode 100644 include/linux/oid_registry.h > create mode 100644 lib/asn1_decoder.c > create mode 100755 lib/build_OID_registry > create mode 100644 lib/crypto/Kconfig > create mode 100644 lib/crypto/Makefile > create mode 100644 lib/crypto/asymmetric_type.c > create mode 100644 lib/crypto/pkcs7.asn1 > create mode 100644 lib/crypto/pkcs7_parser.c > create mode 100644 lib/crypto/pkcs7_parser.h > create mode 100644 lib/crypto/public_key.c > create mode 100644 lib/crypto/rsa_helper.c > create mode 100644 lib/crypto/rsapubkey.asn1 > create mode 100644 lib/crypto/x509.asn1 > create mode 100644 lib/crypto/x509_akid.asn1 > create mode 100644 lib/crypto/x509_cert_parser.c > create mode 100644 lib/crypto/x509_parser.h > create mode 100644 lib/crypto/x509_public_key.c > create mode 100644 lib/oid_registry.c > create mode 100644 scripts/asn1_compiler.c > > -- > 2.21.0 >
On 10/11/19 9:55 AM, AKASHI Takahiro wrote: > I hope this patch set will be reviewed promptly as I'm aiming to > push my "UEFI secure boot" patch for v2020.01. > How can I make all of these new files being built to check for build warnings? Please, provide unit tests for the patch series. Please, provide a documentation how these different tools and files work together. Best regards Heinrich
On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: > On 10/11/19 9:55 AM, AKASHI Takahiro wrote: > >I hope this patch set will be reviewed promptly as I'm aiming to > >push my "UEFI secure boot" patch for v2020.01. > > > > How can I make all of these new files being built to check for build > warnings? As always in my case of UEFI secure boot, they have gone through build and run/tests as part of UEFI secure boot. This is also true for RSA extension as UEFI secure boot is the only user of those features. Please note that almost of all the code here come from the latest linux code without any changes. A few changes that I made are quite U-Boot/UEFI-secure-boot specific and is *best* tested through UEFI secure boot test. That said, you can at least build the code by enabling Library routines Security support Asymmetric Key Support CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE CONFIG_X509_CERTIFICATE_PARSER CONFIG_PKCS7_MESSAGE_PARSER > Please, provide unit tests for the patch series. As I said above, it will be exercised and tested under UEFI secure boot test. > Please, provide a documentation how these different tools and files work > together. What do you mean by different tools? Asn1 compiler and what? Do you want to have doc/README.asn1compiler? Thanks, -Takahiro Akashi > Best regards > > Heinrich
On 10/15/19 5:18 AM, AKASHI Takahiro wrote: > On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: >> On 10/11/19 9:55 AM, AKASHI Takahiro wrote: >>> I hope this patch set will be reviewed promptly as I'm aiming to >>> push my "UEFI secure boot" patch for v2020.01. >>> >> >> How can I make all of these new files being built to check for build >> warnings? > > As always in my case of UEFI secure boot, they have gone through build and > run/tests as part of UEFI secure boot. This is also true for RSA > extension as UEFI secure boot is the only user of those features. Did you run them through Travis? > > Please note that almost of all the code here come from the latest > linux code without any changes. A few changes that I made are > quite U-Boot/UEFI-secure-boot specific and is *best* tested through > UEFI secure boot test. > > That said, you can at least build the code by enabling > Library routines > Security support > Asymmetric Key Support > CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE > CONFIG_X509_CERTIFICATE_PARSER > CONFIG_PKCS7_MESSAGE_PARSER > >> Please, provide unit tests for the patch series. > > As I said above, it will be exercised and tested under UEFI secure boot > test. If there is nothing I can test now, I would not know how to evaluate these patches. There is good reason that we have unit tests and don't simply say U-Boot can be tested by booting Linux. > >> Please, provide a documentation how these different tools and files work >> together. > > What do you mean by different tools? > Asn1 compiler and what? > Do you want to have doc/README.asn1compiler? This patch series provides some puzzle pieces but doesn't tell me how they fit together. Maybe a README describing the different elements provided for UEFI secure boot would be most appropriate. Best regards Heinrich > > Thanks, > -Takahiro Akashi > > >> Best regards >> >> Heinrich >
On Tue, Oct 15, 2019 at 07:33:18AM +0200, Heinrich Schuchardt wrote: > On 10/15/19 5:18 AM, AKASHI Takahiro wrote: > >On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: > >>On 10/11/19 9:55 AM, AKASHI Takahiro wrote: > >>>I hope this patch set will be reviewed promptly as I'm aiming to > >>>push my "UEFI secure boot" patch for v2020.01. > >>> > >> > >>How can I make all of these new files being built to check for build > >>warnings? > > > >As always in my case of UEFI secure boot, they have gone through build and > >run/tests as part of UEFI secure boot. This is also true for RSA > >extension as UEFI secure boot is the only user of those features. > > Did you run them through Travis? > > > > >Please note that almost of all the code here come from the latest > >linux code without any changes. A few changes that I made are > >quite U-Boot/UEFI-secure-boot specific and is *best* tested through > >UEFI secure boot test. > > > >That said, you can at least build the code by enabling > > Library routines > > Security support > > Asymmetric Key Support > > CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE > > CONFIG_X509_CERTIFICATE_PARSER > > CONFIG_PKCS7_MESSAGE_PARSER > > > >>Please, provide unit tests for the patch series. > > > >As I said above, it will be exercised and tested under UEFI secure boot > >test. > > If there is nothing I can test now, I would not know how to evaluate > these patches. Do you ask me to write "unit tests" to test all the aspects of asn1 compiler and parsers that I have not developed any part of and that are not changed from the original? Doesn't make sense. > There is good reason that we have unit tests and don't simply say U-Boot > can be tested by booting Linux. There are lots of examples, one is the original RSA routines, as I said, which have not direct-linked tests and are only tested by vboot.py. > > > >>Please, provide a documentation how these different tools and files work > >>together. > > > >What do you mean by different tools? > >Asn1 compiler and what? > >Do you want to have doc/README.asn1compiler? > > This patch series provides some puzzle pieces but doesn't tell me how > they fit together. Maybe a README describing the different elements > provided for UEFI secure boot would be most appropriate. All what you need to know is that the patch set will generate and provide x509 parser and pkcs7 parser as a result of build process. I will a few lines of README.asn1compiler to describe that. -Takahiro Akashi > Best regards > > Heinrich > > > > >Thanks, > >-Takahiro Akashi > > > > > >>Best regards > >> > >>Heinrich > > >
On Tue, Oct 15, 2019 at 07:33:18AM +0200, Heinrich Schuchardt wrote: > On 10/15/19 5:18 AM, AKASHI Takahiro wrote: > >On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: > >>On 10/11/19 9:55 AM, AKASHI Takahiro wrote: > >>>I hope this patch set will be reviewed promptly as I'm aiming to > >>>push my "UEFI secure boot" patch for v2020.01. > >>> > >> > >>How can I make all of these new files being built to check for build > >>warnings? > > > >As always in my case of UEFI secure boot, they have gone through build and > >run/tests as part of UEFI secure boot. This is also true for RSA > >extension as UEFI secure boot is the only user of those features. > > Did you run them through Travis? As far as this patch set is concerned, no configuration enables any of kconfig options listed below and running Travis doesn't make sense. -Takahiro Akashi > > > >Please note that almost of all the code here come from the latest > >linux code without any changes. A few changes that I made are > >quite U-Boot/UEFI-secure-boot specific and is *best* tested through > >UEFI secure boot test. > > > >That said, you can at least build the code by enabling > > Library routines > > Security support > > Asymmetric Key Support > > CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE > > CONFIG_X509_CERTIFICATE_PARSER > > CONFIG_PKCS7_MESSAGE_PARSER > > > >>Please, provide unit tests for the patch series. > > > >As I said above, it will be exercised and tested under UEFI secure boot > >test. > > If there is nothing I can test now, I would not know how to evaluate > these patches. > > There is good reason that we have unit tests and don't simply say U-Boot > can be tested by booting Linux. > > > > >>Please, provide a documentation how these different tools and files work > >>together. > > > >What do you mean by different tools? > >Asn1 compiler and what? > >Do you want to have doc/README.asn1compiler? > > This patch series provides some puzzle pieces but doesn't tell me how > they fit together. Maybe a README describing the different elements > provided for UEFI secure boot would be most appropriate. > > Best regards > > Heinrich > > > > >Thanks, > >-Takahiro Akashi > > > > > >>Best regards > >> > >>Heinrich > > >
On 10/15/19 10:56 AM, AKASHI Takahiro wrote: > On Tue, Oct 15, 2019 at 07:33:18AM +0200, Heinrich Schuchardt wrote: >> On 10/15/19 5:18 AM, AKASHI Takahiro wrote: >>> On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: >>>> On 10/11/19 9:55 AM, AKASHI Takahiro wrote: >>>>> I hope this patch set will be reviewed promptly as I'm aiming to >>>>> push my "UEFI secure boot" patch for v2020.01. >>>>> >>>> >>>> How can I make all of these new files being built to check for build >>>> warnings? >>> >>> As always in my case of UEFI secure boot, they have gone through build and >>> run/tests as part of UEFI secure boot. This is also true for RSA >>> extension as UEFI secure boot is the only user of those features. >> >> Did you run them through Travis? >> >>> >>> Please note that almost of all the code here come from the latest >>> linux code without any changes. A few changes that I made are >>> quite U-Boot/UEFI-secure-boot specific and is *best* tested through >>> UEFI secure boot test. >>> >>> That said, you can at least build the code by enabling >>> Library routines >>> Security support >>> Asymmetric Key Support >>> CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE >>> CONFIG_X509_CERTIFICATE_PARSER >>> CONFIG_PKCS7_MESSAGE_PARSER >>> >>>> Please, provide unit tests for the patch series. >>> >>> As I said above, it will be exercised and tested under UEFI secure boot >>> test. >> >> If there is nothing I can test now, I would not know how to evaluate >> these patches. > > Do you ask me to write "unit tests" to test all the aspects of > asn1 compiler and parsers that I have not developed any part of > and that are not changed from the original? > Doesn't make sense. Wouldn't it be enough to ASN1-compile one file and check the MD5 hash of the result file? Regards Heinrich > >> There is good reason that we have unit tests and don't simply say U-Boot >> can be tested by booting Linux. > > There are lots of examples, one is the original RSA routines, as I said, > which have not direct-linked tests and are only tested by vboot.py. > >>> >>>> Please, provide a documentation how these different tools and files work >>>> together. >>> >>> What do you mean by different tools? >>> Asn1 compiler and what? >>> Do you want to have doc/README.asn1compiler? >> >> This patch series provides some puzzle pieces but doesn't tell me how >> they fit together. Maybe a README describing the different elements >> provided for UEFI secure boot would be most appropriate. > > All what you need to know is that the patch set will generate > and provide x509 parser and pkcs7 parser as a result of build process. > > I will a few lines of README.asn1compiler to describe that. > > -Takahiro Akashi > >> Best regards >> >> Heinrich >> >>> >>> Thanks, >>> -Takahiro Akashi >>> >>> >>>> Best regards >>>> >>>> Heinrich >>> >> >
On Tue, Oct 15, 2019 at 06:25:19PM +0900, AKASHI Takahiro wrote: > On Tue, Oct 15, 2019 at 07:33:18AM +0200, Heinrich Schuchardt wrote: > > On 10/15/19 5:18 AM, AKASHI Takahiro wrote: > > >On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: > > >>On 10/11/19 9:55 AM, AKASHI Takahiro wrote: > > >>>I hope this patch set will be reviewed promptly as I'm aiming to > > >>>push my "UEFI secure boot" patch for v2020.01. > > >>> > > >> > > >>How can I make all of these new files being built to check for build > > >>warnings? > > > > > >As always in my case of UEFI secure boot, they have gone through build and > > >run/tests as part of UEFI secure boot. This is also true for RSA > > >extension as UEFI secure boot is the only user of those features. > > > > Did you run them through Travis? > > As far as this patch set is concerned, no configuration enables > any of kconfig options listed below and running Travis doesn't make sense. That's a problem that needs to be fixed. I am not OK with the idea of adding a new feature that will not be put through our CI and so when things break it (which will happen, inadvertently) it won't be caught until much later. Figuring out how to extend our CI to test this is a must. Thanks!
Hi Tom, On Thu, Oct 17, 2019 at 11:23:21AM -0400, Tom Rini wrote: > On Tue, Oct 15, 2019 at 06:25:19PM +0900, AKASHI Takahiro wrote: > > On Tue, Oct 15, 2019 at 07:33:18AM +0200, Heinrich Schuchardt wrote: > > > On 10/15/19 5:18 AM, AKASHI Takahiro wrote: > > > >On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: > > > >>On 10/11/19 9:55 AM, AKASHI Takahiro wrote: > > > >>>I hope this patch set will be reviewed promptly as I'm aiming to > > > >>>push my "UEFI secure boot" patch for v2020.01. > > > >>> > > > >> > > > >>How can I make all of these new files being built to check for build > > > >>warnings? > > > > > > > >As always in my case of UEFI secure boot, they have gone through build and > > > >run/tests as part of UEFI secure boot. This is also true for RSA > > > >extension as UEFI secure boot is the only user of those features. > > > > > > Did you run them through Travis? > > > > As far as this patch set is concerned, no configuration enables > > any of kconfig options listed below and running Travis doesn't make sense. > > That's a problem that needs to be fixed. I am not OK with the idea of > adding a new feature that will not be put through our CI and so when > things break it (which will happen, inadvertently) it won't be caught > until much later. Figuring out how to extend our CI to test this is a > must. Thanks! I added a simple *unit* test under "test/lib." As I said before, however, no existing configuration enables either CONFIG_X509_CERTIFICATE_PARSER, nor CONFIG_PKCS7_MESSAGE_PARSER and the related code won't be built or exercised in any way. So I made a small trick to Kconfig: === 8< === config UT_LIB bool "Unit tests for library functions" depends on UNIT_TEST default y help Enables the 'ut lib' command which tests library functions like memcat(), memcyp(), memmove(). if UT_LIB config UT_LIB_ASN1 bool "Unit test for asn1 compiler and decoder function" default y imply ASYMMETRIC_KEY_TYPE imply ASYMMETRIC_PUBLIC_KEY_SUBTYPE imply X509_CERTIFICATE_PARSER imply PKCS7_MESSAGE_PARSER imply RSA_PUBLIC_KEY_PARSER help Enables a test which exercises asn1 compiler and decoder function via various parsers. endif === >8 === So as long as UT_LIB is enabled and run by one of Travis CI test cases, a new test for ASN1 will also be exercised. (I don't know which one will invoke "ut" command.) Do you agree to this approach? Thanks, -Takahiro Akashi > -- > Tom
On Fri, Oct 18, 2019 at 05:36:28PM +0900, AKASHI Takahiro wrote: > Hi Tom, > > On Thu, Oct 17, 2019 at 11:23:21AM -0400, Tom Rini wrote: > > On Tue, Oct 15, 2019 at 06:25:19PM +0900, AKASHI Takahiro wrote: > > > On Tue, Oct 15, 2019 at 07:33:18AM +0200, Heinrich Schuchardt wrote: > > > > On 10/15/19 5:18 AM, AKASHI Takahiro wrote: > > > > >On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: > > > > >>On 10/11/19 9:55 AM, AKASHI Takahiro wrote: > > > > >>>I hope this patch set will be reviewed promptly as I'm aiming to > > > > >>>push my "UEFI secure boot" patch for v2020.01. > > > > >>> > > > > >> > > > > >>How can I make all of these new files being built to check for build > > > > >>warnings? > > > > > > > > > >As always in my case of UEFI secure boot, they have gone through build and > > > > >run/tests as part of UEFI secure boot. This is also true for RSA > > > > >extension as UEFI secure boot is the only user of those features. > > > > > > > > Did you run them through Travis? > > > > > > As far as this patch set is concerned, no configuration enables > > > any of kconfig options listed below and running Travis doesn't make sense. > > > > That's a problem that needs to be fixed. I am not OK with the idea of > > adding a new feature that will not be put through our CI and so when > > things break it (which will happen, inadvertently) it won't be caught > > until much later. Figuring out how to extend our CI to test this is a > > must. Thanks! > > I added a simple *unit* test under "test/lib." > As I said before, however, no existing configuration enables either > CONFIG_X509_CERTIFICATE_PARSER, nor > CONFIG_PKCS7_MESSAGE_PARSER > > and the related code won't be built or exercised in any way. > So I made a small trick to Kconfig: > > === 8< === > config UT_LIB > bool "Unit tests for library functions" > depends on UNIT_TEST > default y > help > Enables the 'ut lib' command which tests library functions like > memcat(), memcyp(), memmove(). > > if UT_LIB > > config UT_LIB_ASN1 > bool "Unit test for asn1 compiler and decoder function" > default y > imply ASYMMETRIC_KEY_TYPE > imply ASYMMETRIC_PUBLIC_KEY_SUBTYPE > imply X509_CERTIFICATE_PARSER > imply PKCS7_MESSAGE_PARSER > imply RSA_PUBLIC_KEY_PARSER > help > Enables a test which exercises asn1 compiler and decoder function > via various parsers. > > endif > === >8 === > > So as long as UT_LIB is enabled and run by one of Travis CI test cases, > a new test for ASN1 will also be exercised. > (I don't know which one will invoke "ut" command.) > > Do you agree to this approach? I think you're going to need to get a bit more familiar with some aspects of testing and building. Today, CONFIG_UNIT_TEST is enabled for sandbox. And we want as much as possible enabled on sandbox as that's also where coverity scan is performed. So that will get us part of the way forward here longer term. I think however you're going to also need to enable some tests on the QEMU platforms so that we can have all of this new secure boot code put through CI there. Thanks!
Tom, On Fri, Oct 18, 2019 at 08:35:23AM -0400, Tom Rini wrote: > On Fri, Oct 18, 2019 at 05:36:28PM +0900, AKASHI Takahiro wrote: > > Hi Tom, > > > > On Thu, Oct 17, 2019 at 11:23:21AM -0400, Tom Rini wrote: > > > On Tue, Oct 15, 2019 at 06:25:19PM +0900, AKASHI Takahiro wrote: > > > > On Tue, Oct 15, 2019 at 07:33:18AM +0200, Heinrich Schuchardt wrote: > > > > > On 10/15/19 5:18 AM, AKASHI Takahiro wrote: > > > > > >On Sat, Oct 12, 2019 at 03:02:09PM +0200, Heinrich Schuchardt wrote: > > > > > >>On 10/11/19 9:55 AM, AKASHI Takahiro wrote: > > > > > >>>I hope this patch set will be reviewed promptly as I'm aiming to > > > > > >>>push my "UEFI secure boot" patch for v2020.01. > > > > > >>> > > > > > >> > > > > > >>How can I make all of these new files being built to check for build > > > > > >>warnings? > > > > > > > > > > > >As always in my case of UEFI secure boot, they have gone through build and > > > > > >run/tests as part of UEFI secure boot. This is also true for RSA > > > > > >extension as UEFI secure boot is the only user of those features. > > > > > > > > > > Did you run them through Travis? > > > > > > > > As far as this patch set is concerned, no configuration enables > > > > any of kconfig options listed below and running Travis doesn't make sense. > > > > > > That's a problem that needs to be fixed. I am not OK with the idea of > > > adding a new feature that will not be put through our CI and so when > > > things break it (which will happen, inadvertently) it won't be caught > > > until much later. Figuring out how to extend our CI to test this is a > > > must. Thanks! > > > > I added a simple *unit* test under "test/lib." > > As I said before, however, no existing configuration enables either > > CONFIG_X509_CERTIFICATE_PARSER, nor > > CONFIG_PKCS7_MESSAGE_PARSER > > > > and the related code won't be built or exercised in any way. > > So I made a small trick to Kconfig: > > > > === 8< === > > config UT_LIB > > bool "Unit tests for library functions" > > depends on UNIT_TEST > > default y > > help > > Enables the 'ut lib' command which tests library functions like > > memcat(), memcyp(), memmove(). > > > > if UT_LIB > > > > config UT_LIB_ASN1 > > bool "Unit test for asn1 compiler and decoder function" > > default y > > imply ASYMMETRIC_KEY_TYPE > > imply ASYMMETRIC_PUBLIC_KEY_SUBTYPE > > imply X509_CERTIFICATE_PARSER > > imply PKCS7_MESSAGE_PARSER > > imply RSA_PUBLIC_KEY_PARSER > > help > > Enables a test which exercises asn1 compiler and decoder function > > via various parsers. > > > > endif > > === >8 === > > > > So as long as UT_LIB is enabled and run by one of Travis CI test cases, > > a new test for ASN1 will also be exercised. > > (I don't know which one will invoke "ut" command.) > > > > Do you agree to this approach? I'd like to confirm exactly what you suggested here: > I think you're going to need to get a bit more familiar with some > aspects of testing and building. Today, CONFIG_UNIT_TEST is enabled for > sandbox. And we want as much as possible enabled on sandbox as that's > also where coverity scan is performed. Is this ("as much as possible") true? I wonder how it should be achieved. As far as my RSA test approach above is concerned, 1) If UT_TEST is enabled, UT_LIB and then UT_LIB_ASN1 are also enabled *by default* and expected to be run automatically through Travis's sandbox build with test/py even if, say, X509_CERTIRFFICATE_PARSER is *not* enabled in any of sandbox_*_defconfig. Is this approach is OK for you? Or, 2) Should we add ASYMMETRIC_*/X509_CERTIFICATE_PARSER/PKCS7_MESSAGE_PARSER to one (or all) of sandbox_*_defconfig (even though there is no explicit user of these features before my secure boot patch will be merged)? Or, 3) Should we create a new sandbox_*_defconfig for any further tests? Or, 4) Would we better set ASYMMETRIC_*/X509_CERTIFICATE_PARSER/PKCS7_MESSAGE_PARSER enabled by default on Sandbox? I mean, === 8< === menuconfig ASYMMETRIC_KEY_TYPE bool "Asymmetric (public-key cryptographic) key Support" default y if SANDBOX <== added if ASYMMETRIC_KEY_TYPE ... config X509_CERTIFICATE_PARSER bool "RSA public key parser" depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE default y if SANDBOX <== added ... === >8 === > So that will get us part of the > way forward here longer term. I think however you're going to also need > to enable some tests on the QEMU platforms so that we can have all of > this new secure boot code put through CI there. Thanks! Are you talking about this patch set(asn1 parsers) or is this your general comment on my overall "UEFI secure boot" patch? Please note that my secure boot patch is architecture agnostic and will perfectly work on Sandbox and all the related py tests will also be done on Sandbox. Thanks, -Takahiro Akashi > -- > Tom