> From: Alexei Starovoitov [mailto:alexei.starovoitov@xxxxxxxxx] > Sent: Thursday, June 9, 2022 5:38 PM > On Thu, Jun 9, 2022 at 2:00 AM Roberto Sassu <roberto.sassu@xxxxxxxxxx> > wrote: > > > > > From: Alexei Starovoitov [mailto:alexei.starovoitov@xxxxxxxxx] > > > Sent: Thursday, June 9, 2022 2:13 AM > > > On Wed, Jun 8, 2022 at 4:15 AM Roberto Sassu > <roberto.sassu@xxxxxxxxxx> > > > wrote: > > > > > > > > According to the logs of the eBPF CI, built kernel and tests are copied to > > > > a virtual machine to run there. > > > > > > > > Since a test for a new helper to verify PKCS#7 signatures requires to sign > > > > data to be verified, extend test_progs to store in the test_env data > > > > structure (accessible by individual tests) the path of sign-file and of the > > > > kernel private key and cert. > > > > > > > > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > > > --- > > > > tools/testing/selftests/bpf/test_progs.c | 12 ++++++++++++ > > > > tools/testing/selftests/bpf/test_progs.h | 3 +++ > > > > 2 files changed, 15 insertions(+) > > > > > > > > diff --git a/tools/testing/selftests/bpf/test_progs.c > > > b/tools/testing/selftests/bpf/test_progs.c > > > > index c639f2e56fc5..90ce2c06a15e 100644 > > > > --- a/tools/testing/selftests/bpf/test_progs.c > > > > +++ b/tools/testing/selftests/bpf/test_progs.c > > > > @@ -707,6 +707,8 @@ enum ARG_KEYS { > > > > ARG_TEST_NAME_GLOB_DENYLIST = 'd', > > > > ARG_NUM_WORKERS = 'j', > > > > ARG_DEBUG = -1, > > > > + ARG_SIGN_FILE = 'S', > > > > + ARG_KERNEL_PRIV_CERT = 'C', > > > > }; > > > > > > > > static const struct argp_option opts[] = { > > > > @@ -732,6 +734,10 @@ static const struct argp_option opts[] = { > > > > "Number of workers to run in parallel, default to number of cpus." }, > > > > { "debug", ARG_DEBUG, NULL, 0, > > > > "print extra debug information for test_progs." }, > > > > + { "sign-file", ARG_SIGN_FILE, "PATH", 0, > > > > + "sign-file path " }, > > > > + { "kernel-priv-cert", ARG_KERNEL_PRIV_CERT, "PATH", 0, > > > > + "kernel private key and cert path " }, > > > > {}, > > > > }; > > > > > > > > @@ -862,6 +868,12 @@ static error_t parse_arg(int key, char *arg, struct > > > argp_state *state) > > > > case ARG_DEBUG: > > > > env->debug = true; > > > > break; > > > > + case ARG_SIGN_FILE: > > > > + env->sign_file_path = arg; > > > > + break; > > > > + case ARG_KERNEL_PRIV_CERT: > > > > + env->kernel_priv_cert_path = arg; > > > > + break; > > > > > > That's cumbersome approach to use to force CI and > > > users to pass these args on command line. > > > The test has to be self contained. > > > test_progs should execute it without any additional input. > > > For example by having test-only private/public key > > > that is used to sign and verify the signature. > > > > I thought a bit about this. Just generating a test key does not work, > > as it must be signed by the kernel signing key (otherwise, loading > > in the secondary keyring will be rejected). Having the test key around > > is as dangerous as having the kernel signing key around copied > > somewhere. > > > > Allowing users to specify a test keyring in the helper is possible. > > We shouldn't need to load into the secondary keyring. > The helper needs to support an arbitrary key ring. > The kernel shouldn't interfere with loading that test key into > a test ring. > > > But it would introduce unnecessary code, plus the keyring identifier > > What kind of 'unnecessary code' ? The code for handling eBPF-specific keyring identifiers. But at the end, it is not that much. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Yang Xi, Li He > > will be understood by eBPF only and not by verify_pkcs7_signature(), > > as it happens for other keyring identifiers. > > Maybe wrapping verify_pkcs7_signature as a helper is too high level? > > > We may have environment variables directly in the eBPF test, to > > specify the location of the signing key, but there is a risk of > > duplication, as other tests wanting the same information might > > not be aware of them. > > That's no go. > > > I would not introduce any code that handles the kernel signing > > key (in the Makefile, or in a separate script). This information is > > so sensible, that it must be responsibility of an external party > > to do the work of making that key available and tell where it is. > > > > Roberto > > > > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 > > Managing Director: Li Peng, Yang Xi, Li He
