| Message ID | 20210922031129.4188386-6-erichte@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Implement secvar system mode and built-in key support | expand |
Eric Richter <erichte@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@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@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
>> >> 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@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@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/skiboot
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 >> $@ + echo "};" >> $@ + +SECVAR_DEFAULTVARS = $(SECVAR_DEFAULTVARS_DIR)/built-in.a + +$(SECVAR_DEFAULTVARS): $(SECVAR_DEFAULTVARS_OBJS:%=$(SECVAR_DEFAULTVARS_DIR)/%)
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. 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. Signed-off-by: Eric Richter <erichte@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