lkml.org 
[lkml]   [2022]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v2 2/3] selftests/bpf: Add test_progs opts for sign-file and kernel priv key + cert
Date
> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> Sent: Thursday, June 9, 2022 5:38 PM
> On Thu, Jun 9, 2022 at 2:00 AM Roberto Sassu <roberto.sassu@huawei.com>
> wrote:
> >
> > > From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> > > Sent: Thursday, June 9, 2022 2:13 AM
> > > On Wed, Jun 8, 2022 at 4:15 AM Roberto Sassu
> <roberto.sassu@huawei.com>
> > > 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@huawei.com>
> > > > ---
> > > > 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
\
 
 \ /
  Last update: 2022-06-10 14:11    [W:0.105 / U:2.540 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site