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

[Eric Richter]

>>
>> 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?

Compile-time. I'd like to avoid run-time checks, as that'd probably introduce
more boot-time problems I don't want to debug.

My intent is to use predictable preprocessor symbols like SECVAR_BUILT_IN_VARNAME,
so that a driver can #ifdef them. See the later static keys edk2 driver.

> 
> 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!)
> 

Yeah, further research led me to using objcopy. Might experiment with that later
if it means I can get away from having so many intermediate files, and gross
Makefile hacks.

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

Original version used `xxd -i`. Problem is, it names the symbol with the whole dang
path. So instead of trying to write a sed line to fix `libstb_secvar_defaultkeys_db`,
I went this route to try to generate the file manually, which involved less sed.

The objcopy method may or may not solve this, I'm still looking into better options.

>>
>> 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.
> 

Noted, will consider options. Do we consider hexdump or other things in
binutils like objcopy as a build dep?

> 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