Re: [PATCH 12/12] selftests/bpf: Enable signature verification for all lskel tests

[Blaise Boscaccy <bboscaccy@linux.microsoft.com>]

KP Singh <kpsingh@kernel.org> writes:

> Convert the kernel's generated verification certificate into a C header
> file using xxd.  Finally, update the main test runner to load this
> certificate into the session keyring via the add_key() syscall before
> executing any tests.
>
> The kernel's module signing verification certificate is converted to a
> headerfile and loaded as a session key and all light skeleton tests are
> updated to be signed.
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>
> ---
>  tools/testing/selftests/bpf/.gitignore   |  1 +
>  tools/testing/selftests/bpf/Makefile     | 13 +++++++++++--
>  tools/testing/selftests/bpf/test_progs.c | 13 +++++++++++++
>  3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index e2a2c46c008b..5ab96f8ab1c9 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -45,3 +45,4 @@ xdp_redirect_multi
>  xdp_synproxy
>  xdp_hw_metadata
>  xdp_features
> +verification_cert.h
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index cf5ed3bee573..778b54be7ef4 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -7,6 +7,7 @@ CXX ?= $(CROSS_COMPILE)g++
>  
>  CURDIR := $(abspath .)
>  TOOLSDIR := $(abspath ../../..)
> +CERTSDIR := $(abspath ../../../../certs)
>  LIBDIR := $(TOOLSDIR)/lib
>  BPFDIR := $(LIBDIR)/bpf
>  TOOLSINCDIR := $(TOOLSDIR)/include
> @@ -534,7 +535,7 @@ HEADERS_FOR_BPF_OBJS := $(wildcard $(BPFDIR)/*.bpf.h)		\
>  # $1 - test runner base binary name (e.g., test_progs)
>  # $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, bpf_gcc, etc)
>  define DEFINE_TEST_RUNNER
> -
> +LSKEL_SIGN := -S -k $(CERTSDIR)/signing_key.pem -i $(CERTSDIR)/signing_key.x509
>  TRUNNER_OUTPUT := $(OUTPUT)$(if $2,/)$2
>  TRUNNER_BINARY := $1$(if $2,-)$2
>  TRUNNER_TEST_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.test.o,	\
> @@ -601,7 +602,7 @@ $(TRUNNER_BPF_LSKELS): %.lskel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
>  	$(Q)$$(BPFTOOL) gen object $$(<:.o=.llinked2.o) $$(<:.o=.llinked1.o)
>  	$(Q)$$(BPFTOOL) gen object $$(<:.o=.llinked3.o) $$(<:.o=.llinked2.o)
>  	$(Q)diff $$(<:.o=.llinked2.o) $$(<:.o=.llinked3.o)
> -	$(Q)$$(BPFTOOL) gen skeleton -L $$(<:.o=.llinked3.o) name $$(notdir $$(<:.bpf.o=_lskel)) > $$@
> +	$(Q)$$(BPFTOOL) gen skeleton $(LSKEL_SIGN) $$(<:.o=.llinked3.o) name $$(notdir $$(<:.bpf.o=_lskel)) > $$@
>  	$(Q)rm -f $$(<:.o=.llinked1.o) $$(<:.o=.llinked2.o) $$(<:.o=.llinked3.o)
>  
>  $(LINKED_BPF_OBJS): %: $(TRUNNER_OUTPUT)/%
> @@ -697,6 +698,13 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS)			\
>  
>  endef
>  
> +CERT_HEADER := verification_cert.h
> +CERT_SOURCE := $(CERTSDIR)/signing_key.x509
> +
> +$(CERT_HEADER): $(CERT_SOURCE)
> +	@echo "GEN-CERT-HEADER: $(CERT_HEADER) from $<"
> +	$(Q)xxd -i -n test_progs_verification_cert $< > $@
> +
>  # Define test_progs test runner.
>  TRUNNER_TESTS_DIR := prog_tests
>  TRUNNER_BPF_PROGS_DIR := progs
> @@ -716,6 +724,7 @@ TRUNNER_EXTRA_SOURCES := test_progs.c		\
>  			 disasm.c		\
>  			 disasm_helpers.c	\
>  			 json_writer.c 		\
> +			 $(CERT_HEADER)		\
>  			 flow_dissector_load.h	\
>  			 ip_check_defrag_frags.h
>  TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read				\
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 309d9d4a8ace..02a85dda30e6 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -14,12 +14,14 @@
>  #include <netinet/in.h>
>  #include <sys/select.h>
>  #include <sys/socket.h>
> +#include <linux/keyctl.h>
>  #include <sys/un.h>
>  #include <bpf/btf.h>
>  #include <time.h>
>  #include "json_writer.h"
>  
>  #include "network_helpers.h"
> +#include "verification_cert.h"
>  
>  /* backtrace() and backtrace_symbols_fd() are glibc specific,
>   * use header file when glibc is available and provide stub
> @@ -1928,6 +1930,13 @@ static void free_test_states(void)
>  	}
>  }
>  
> +static __u32 register_session_key(const char *key_data, size_t key_data_size)
> +{
> +	return syscall(__NR_add_key, "asymmetric", "libbpf_session_key",
> +			(const void *)key_data, key_data_size,
> +			KEY_SPEC_SESSION_KEYRING);
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	static const struct argp argp = {
> @@ -1961,6 +1970,10 @@ int main(int argc, char **argv)
>  	/* Use libbpf 1.0 API mode */
>  	libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
>  	libbpf_set_print(libbpf_print_fn);
> +	err = register_session_key((const char *)test_progs_verification_cert,
> +				   test_progs_verification_cert_len);
> +	if (err < 0)
> +		return err;
>  
>  	traffic_monitor_set_print(traffic_monitor_print_fn);
>  
> -- 
> 2.43.0


There aren't any test cases showing the "trusted" loader doing any sort
of enforcement of blocking invalid programs or maps.

-blaise