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 <[email protected]>
---
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;
case ARGP_KEY_ARG:
argp_usage(state);
break;
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 5fe1365c2bb1..9a860a59f06a 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -123,6 +123,9 @@ struct test_env {
pid_t *worker_pids; /* array of worker pids */
int *worker_socks; /* array of worker socks */
int *worker_current_test; /* array of current running test for each worker */
+
+ const char *sign_file_path; /* sign-file path */
+ const char *kernel_priv_cert_path; /* kernel priv key and cert path */
};
#define MAX_LOG_TRUNK_SIZE 8192
--
2.25.1
On Wed, Jun 8, 2022 at 4:15 AM Roberto Sassu <[email protected]> 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 <[email protected]>
> ---
> 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.
On Thu, Jun 9, 2022 at 2:00 AM Roberto Sassu <[email protected]> wrote:
>
> > From: Alexei Starovoitov [mailto:[email protected]]
> > Sent: Thursday, June 9, 2022 2:13 AM
> > On Wed, Jun 8, 2022 at 4:15 AM Roberto Sassu <[email protected]>
> > 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 <[email protected]>
> > > ---
> > > 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' ?
> 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