| Message ID | 20210303034418.106762-1-eric.snowberg@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | certs: Fix wrong kconfig option used for x509_revocation_list | expand |
> On Mar 3, 2021, at 2:24 AM, David Howells <dhowells@redhat.com> wrote: > > Eric Snowberg <eric.snowberg@oracle.com> wrote: > >> +ifeq ($(CONFIG_SYSTEM_REVOCATION_LIST),y) >> +obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += revocation_certificates.o >> +endif > > Should the ifeq be referring to CONFIG_SYSTEM_REVOCATION_KEYS rather than > CONFIG_SYSTEM_REVOCATION_LIST? In fact, since S_R_K depends indirectly on > S_B_K, you should be able to just do: > > +obj-$(CONFIG_SYSTEM_REVOCATION_KEYS) += revocation_certificates.o Since S_R_K is a string, I could not get that to work. I could get this to work: obj-$(CONFIG_SYSTEM_REVOCATION_LIST) += revocation_certificates.o If there is another way of doing this with S_R_K instead, let me know. >> +#ifdef CONFIG_SYSTEM_REVOCATION_LIST > > Here also? When S_R_L is defined, S_R_K will also always be defined too. Either as an empty string or a path to a file. With my change, it works the same as the current code in CONFIG_SYSTEM_TRUSTED_KEYS and CONFIG_SYSTEM_TRUSTED_KEYRING, which also uses the extract_certs script. It can properly handle a NULL string. If I changed it to S_R_K here, it seems confusing to me, since one might assume it is only defined when someone adds a string to S_R_K. But, I can change it if you’d like. >> + hostprogs-always-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += extract-cert > > And here too?
On Tue, Mar 02, 2021 at 10:44:18PM -0500, Eric Snowberg wrote: > This fixes a build issue when x509_revocation_list is not defined. "Fix a" Let's stick to the imperative form in commit messages. > > $ make ARCH=x86_64 O=build64 all > > EXTRACT_CERTS ../ > At main.c:154: > - SSL error:0909006C:PEM routines:get_name:no start line: crypto/pem/pem_lib.c:745 > extract-cert: ../: Is a directory > make[2]: [../certs/Makefile:119: certs/x509_revocation_list] Error 1 (ignored) > > When the new CONFIG_SYSTEM_REVOCATION_LIST was added [1], it was not carried > into the code for preloading the revocation certificates [2]. Change from > using the original CONFIG_SYSTEM_BLACKLIST_KEYRING to the new > CONFIG_SYSTEM_REVOCATION_LIST. > > [1] https://lore.kernel.org/keyrings/EDA280F9-F72D-4181-93C7-CDBE95976FF7@oracle.com/T/#m562c1b27bf402190e7bb573ad20eff5b6310d08f > [2] https://lore.kernel.org/keyrings/EDA280F9-F72D-4181-93C7-CDBE95976FF7@oracle.com/T/#m07e258bf019ccbac23820fad5192ceffa74fc6ab > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com> > --- > certs/Makefile | 7 +++++-- > certs/blacklist.c | 4 ++++ > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/certs/Makefile b/certs/Makefile > index e3f4926fd21e..3bc43c88a6d2 100644 > --- a/certs/Makefile > +++ b/certs/Makefile > @@ -4,7 +4,10 @@ > # > > obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o common.o > -obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o revocation_certificates.o common.o > +obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o common.o > +ifeq ($(CONFIG_SYSTEM_REVOCATION_LIST),y) > +obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += revocation_certificates.o > +endif > ifneq ($(CONFIG_SYSTEM_BLACKLIST_HASH_LIST),"") > obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o > else > @@ -105,7 +108,7 @@ $(obj)/signing_key.x509: scripts/extract-cert $(X509_DEP) FORCE > $(call if_changed,extract_certs,$(MODULE_SIG_KEY_SRCPREFIX)$(CONFIG_MODULE_SIG_KEY)) > endif # CONFIG_MODULE_SIG > > -ifeq ($(CONFIG_SYSTEM_BLACKLIST_KEYRING),y) > +ifeq ($(CONFIG_SYSTEM_REVOCATION_LIST),y) > > $(eval $(call config_filename,SYSTEM_REVOCATION_KEYS)) > > diff --git a/certs/blacklist.c b/certs/blacklist.c > index 723b19c96256..c9a435b15af4 100644 > --- a/certs/blacklist.c > +++ b/certs/blacklist.c > @@ -21,8 +21,10 @@ > > static struct key *blacklist_keyring; > > +#ifdef CONFIG_SYSTEM_REVOCATION_LIST > extern __initconst const u8 revocation_certificate_list[]; > extern __initconst const unsigned long revocation_certificate_list_size; > +#endif > > /* > * The description must be a type prefix, a colon and then an even number of > @@ -225,6 +227,7 @@ static int __init blacklist_init(void) > */ > device_initcall(blacklist_init); > > +#ifdef CONFIG_SYSTEM_REVOCATION_LIST > /* > * Load the compiled-in list of revocation X.509 certificates. > */ > @@ -237,3 +240,4 @@ static __init int load_revocation_certificate_list(void) > blacklist_keyring); > } > late_initcall(load_revocation_certificate_list); > +#endif > -- > 2.18.4 > > Code change looks good. /Jarkko
Eric Snowberg <eric.snowberg@oracle.com> wrote: > Since S_R_K is a string, I could not get that to work. Okay. > >> + hostprogs-always-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += extract-cert I think this should then refer to CONFIG_SYSTEM_REVOCATION_LIST. David
diff --git a/certs/Makefile b/certs/Makefile index e3f4926fd21e..3bc43c88a6d2 100644 --- a/certs/Makefile +++ b/certs/Makefile @@ -4,7 +4,10 @@ # obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o common.o -obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o revocation_certificates.o common.o +obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o common.o +ifeq ($(CONFIG_SYSTEM_REVOCATION_LIST),y) +obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += revocation_certificates.o +endif ifneq ($(CONFIG_SYSTEM_BLACKLIST_HASH_LIST),"") obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o else @@ -105,7 +108,7 @@ $(obj)/signing_key.x509: scripts/extract-cert $(X509_DEP) FORCE $(call if_changed,extract_certs,$(MODULE_SIG_KEY_SRCPREFIX)$(CONFIG_MODULE_SIG_KEY)) endif # CONFIG_MODULE_SIG -ifeq ($(CONFIG_SYSTEM_BLACKLIST_KEYRING),y) +ifeq ($(CONFIG_SYSTEM_REVOCATION_LIST),y) $(eval $(call config_filename,SYSTEM_REVOCATION_KEYS)) diff --git a/certs/blacklist.c b/certs/blacklist.c index 723b19c96256..c9a435b15af4 100644 --- a/certs/blacklist.c +++ b/certs/blacklist.c @@ -21,8 +21,10 @@ static struct key *blacklist_keyring; +#ifdef CONFIG_SYSTEM_REVOCATION_LIST extern __initconst const u8 revocation_certificate_list[]; extern __initconst const unsigned long revocation_certificate_list_size; +#endif /* * The description must be a type prefix, a colon and then an even number of @@ -225,6 +227,7 @@ static int __init blacklist_init(void) */ device_initcall(blacklist_init); +#ifdef CONFIG_SYSTEM_REVOCATION_LIST /* * Load the compiled-in list of revocation X.509 certificates. */ @@ -237,3 +240,4 @@ static __init int load_revocation_certificate_list(void) blacklist_keyring); } late_initcall(load_revocation_certificate_list); +#endif
This fixes a build issue when x509_revocation_list is not defined. $ make ARCH=x86_64 O=build64 all EXTRACT_CERTS ../ At main.c:154: - SSL error:0909006C:PEM routines:get_name:no start line: crypto/pem/pem_lib.c:745 extract-cert: ../: Is a directory make[2]: [../certs/Makefile:119: certs/x509_revocation_list] Error 1 (ignored) When the new CONFIG_SYSTEM_REVOCATION_LIST was added [1], it was not carried into the code for preloading the revocation certificates [2]. Change from using the original CONFIG_SYSTEM_BLACKLIST_KEYRING to the new CONFIG_SYSTEM_REVOCATION_LIST. [1] https://lore.kernel.org/keyrings/EDA280F9-F72D-4181-93C7-CDBE95976FF7@oracle.com/T/#m562c1b27bf402190e7bb573ad20eff5b6310d08f [2] https://lore.kernel.org/keyrings/EDA280F9-F72D-4181-93C7-CDBE95976FF7@oracle.com/T/#m07e258bf019ccbac23820fad5192ceffa74fc6ab Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com> --- certs/Makefile | 7 +++++-- certs/blacklist.c | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-)