[Skiboot] [RFC 5/8] secvar: add build-time mechanism to inject default variable data

[Daniel Axtens]

Eric Richter <erichte at linux.ibm.com> writes:

> To allow the implementation of a backend that uses static, built-in
> variables, this patch adds the ability to compile in a blob of data,
> accessible by symbol named similarly to the file in which the data was
> loaded from.
>
> To operate: place a file in the libstb/secvar/defaultvars directory named
> `secvar_default_NAME.var`, where NAME is the case-sensitive name of the
> variable the static-variable enabled backend will expect it to be.
>
> For example, to build in a db into the forthcoming edk2-compat-static
> driver, copy the db.esl file into the aforementioned directory as
> `secvar_default_db.var`. This will be compiled in, and exposed in the
> secvar_default_vars.h header, which will look something like:
>
> uint8_t secvar_default_db[] = { ... };
> \#define SECVAR_DEFAULT_DB
>
> The #define may be used by backends to determine if a default variable was
> provided for a specific variable it may expect.
>
> RFC NOTE: This is a rather janky first implementation, and ideally should
> be improved in some form. I am open to suggestions on how to improve this.
> Ideally at minimum, the backend should be able to "discover" which
> variables have been built in.

At runtime or at compile time? Why would we need to discover at runtime
rather than compile time? To put it another way, in what cases are the
defines you already build insufficient?

Anyway, yeah, building keys in is tricky. You could do tricks with
linkers but just converting the thing to a header is probably fine. (But
please make it a const!)

xxd also gives you a _len variable, doesn't it? That probably needs to
be documented in the coommit message.

>
> Furthermore, there should likely be some kind of trigger to detect whether
> or not default keys have been built in at all, but that is something that
> will come up in a later patch.

Relatedly, do you need to (or should you for the sake of sanity) enforce
something a bit less free-form?

To be more precise, should you require that there is either a complete
3-level trust store (a PK and a db and optionally dbx and KEK) or
strictly nothing?  That way you don't have to worry about the case where
someone compiles in just db and you don't have to think about what the
code should do if there's no compiled in PK.

>
> Signed-off-by: Eric Richter <erichte at linux.ibm.com>
> ---
>  Makefile.main                          |  1 +
>  libstb/secvar/Makefile.inc             |  3 ++-
>  libstb/secvar/defaultvars/Makefile.inc | 31 ++++++++++++++++++++++++++
>  3 files changed, 34 insertions(+), 1 deletion(-)
>  create mode 100644 libstb/secvar/defaultvars/Makefile.inc
>
> diff --git a/Makefile.main b/Makefile.main
> index c8a63e8b..d0b5d4eb 100644
> --- a/Makefile.main
> +++ b/Makefile.main
> @@ -409,6 +409,7 @@ clean:
>  	$(RM) include/asm-offsets.h version.c .version
>  	$(RM) skiboot.info external/gard/gard.info external/pflash/pflash.info
>  	$(RM) extract-gcov $(TARGET).lid.stb $(TARGET).lid.xz.stb
> +	$(RM) libstb/secvar/defaultvars/*.{c,d,o,h}
>  
>  distclean: clean
>  	$(RM) *~ $(SUBDIRS:%=%/*~) include/*~
> diff --git a/libstb/secvar/Makefile.inc b/libstb/secvar/Makefile.inc
> index 57c7cfb5..1d6f4c81 100644
> --- a/libstb/secvar/Makefile.inc
> +++ b/libstb/secvar/Makefile.inc
> @@ -8,5 +8,6 @@ SECVAR = libstb/secvar/built-in.a
>  
>  include $(SRC)/libstb/secvar/storage/Makefile.inc
>  include $(SRC)/libstb/secvar/backend/Makefile.inc
> +include $(SRC)/libstb/secvar/defaultvars/Makefile.inc
>  
> -$(SECVAR): $(SECVAR_OBJS:%=libstb/secvar/%) $(SECVAR_STORAGE) $(SECVAR_BACKEND)
> +$(SECVAR): $(SECVAR_OBJS:%=libstb/secvar/%) $(SECVAR_DEFAULTVARS) $(SECVAR_STORAGE) $(SECVAR_BACKEND)
> diff --git a/libstb/secvar/defaultvars/Makefile.inc b/libstb/secvar/defaultvars/Makefile.inc
> new file mode 100644
> index 00000000..f831413d
> --- /dev/null
> +++ b/libstb/secvar/defaultvars/Makefile.inc
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
> +# -*-Makefile-*-
> +
> +SECVAR_DEFAULTVARS_DIR = libstb/secvar/defaultvars
> +
> +SUBDIRS += $(SECVAR_DEFAULTVARS_DIR)
> +
> +SECVAR_DEFAULTVARS_FILES = $(wildcard $(SECVAR_DEFAULTVARS_DIR)/secvar_default_*.var)
> +SECVAR_DEFAULTVARS_C_FILES = $(patsubst %.var,%.c,$(notdir $(SECVAR_DEFAULTVARS_FILES)))
> +SECVAR_DEFAULTVARS_OBJS = $(patsubst %.c,%.o,$(SECVAR_DEFAULTVARS_C_FILES))
> +
> +$(SECVAR_DEFAULTVARS_DIR)/secvar_default_vars.h: $(SECVAR_DEFAULTVARS_FILES)
> +	echo -n "" > $@
> +	for var in $(subst .var,,$(notdir $^)) ; do \
> +		echo "#ifndef _SECVAR_DEFAULTVARS_H_" >> $@ ; \
> +		echo "#define _SECVAR_DEFAULTVARS_H_" >> $@ ; \
> +		echo "extern unsigned char *$$var;" >> $@ ; \
> +		echo -n "#define " >> $@ ; \
> +		echo $$var | tr [:lower:] [:upper:] >> $@ ; \
> +		echo "#endif" >> $@ ; \
> +	done;
> +
> +secvar_default_%.c: secvar_default_%.var $(SECVAR_DEFAULTVARS_DIR)/secvar_default_vars.h
> +	echo $^
> +	echo "unsigned char $(subst .var,,$(notdir $<))[] = {" > $@
> +	cat $^ | xxd -e -i >> $@

xxd isn't currently a builddep for skiboot. I don't mind making it so
but we should probably document that somewhere lest someone tries and
fails to build it on something without xxd.

Kind regards,
Daniel

> +	echo "};" >> $@
> +
> +SECVAR_DEFAULTVARS = $(SECVAR_DEFAULTVARS_DIR)/built-in.a
> +
> +$(SECVAR_DEFAULTVARS): $(SECVAR_DEFAULTVARS_OBJS:%=$(SECVAR_DEFAULTVARS_DIR)/%)
> -- 
> 2.33.0
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot