[PATCH v2 11/13] bpftool: Add support for signing BPF programs
KP Singh
kpsingh at kernel.org
Mon Aug 11 14:23:29 UTC 2025
On Thu, Jul 24, 2025 at 7:07 PM KP Singh <kpsingh at kernel.org> wrote:
>
> On Tue, Jul 22, 2025 at 5:51 PM Quentin Monnet <qmo at kernel.org> wrote:
> >
> > 2025-07-21 23:19 UTC+0200 ~ KP Singh <kpsingh at kernel.org>
> > > Two modes of operation being added:
> > >
> > > Add two modes of operation:
> > >
> > > * For prog load, allow signing a program immediately before loading. This
> > > is essential for command-line testing and administration.
> > >
> > > bpftool prog load -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
> > >
> > > * For gen skeleton, embed a pre-generated signature into the C skeleton
> > > file. This supports the use of signed programs in compiled applications.
> > >
> > > bpftool gen skeleton -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
> > >
> > > Generation of the loader program and its metadata map is implemented in
> > > libbpf (bpf_obj__gen_loader). bpftool generates a skeleton that loads
> > > the program and automates the required steps: freezing the map, creating
> > > an exclusive map, loading, and running. Users can use standard libbpf
> > > APIs directly or integrate loader program generation into their own
> > > toolchains.
> >
> >
> > Thanks KP! Some bpftool-related comments below. Looks good overall, I
> > mostly have minor comments.
> >
> > One concern might be the license for the new file, GPL-2.0 in your
> > patch, whereas bpftool is dual-licensed. I hope this is simply an oversight?
>
> An oversight, fixed.
>
> >
> >
> > >
> > > Signed-off-by: KP Singh <kpsingh at kernel.org>
> > > ---
> > > .../bpf/bpftool/Documentation/bpftool-gen.rst | 12 +
> > > .../bpftool/Documentation/bpftool-prog.rst | 12 +
> > > tools/bpf/bpftool/Makefile | 6 +-
> > > tools/bpf/bpftool/cgroup.c | 5 +-
> > > tools/bpf/bpftool/gen.c | 58 ++++-
> > > tools/bpf/bpftool/main.c | 21 +-
> > > tools/bpf/bpftool/main.h | 11 +
> > > tools/bpf/bpftool/prog.c | 25 +++
> > > tools/bpf/bpftool/sign.c | 210 ++++++++++++++++++
> > > 9 files changed, 352 insertions(+), 8 deletions(-)
> > > create mode 100644 tools/bpf/bpftool/sign.c
> > >
> > > diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > > index ca860fd97d8d..2997313003b1 100644
> > > --- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > > +++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > > @@ -185,6 +185,18 @@ OPTIONS
> > > For skeletons, generate a "light" skeleton (also known as "loader"
> > > skeleton). A light skeleton contains a loader eBPF program. It does not use
> > > the majority of the libbpf infrastructure, and does not need libelf.
> >
> >
> > Blank line separator, please
>
> done
>
> >
> >
> > > +-S, --sign
> > > + For skeletons, generate a signed skeleton. This option must be used with
> > > + **-k** and **-i**. Using this flag implicitly enables **--use-loader**.
> > > + See the "Signed Skeletons" section in the description of the
> > > + **gen skeleton** command for more details.
> > > +
> > > +-k <private_key.pem>
> > > + Path to the private key file in PEM format, required for signing.
> > > +
> > > +-i <certificate.x509>
> > > + Path to the X.509 certificate file in PEM or DER format, required for
> > > + signing.
> >
> >
> > Please also update the options list in the SYNOPSIS section at the top
> > of the page; and the option list at the bottom of gen.c (just like for
> > "--use-loader").
>
> done also, isn't this the right formatting for the SYNOPSIS given that
> some of these are optional?
>
> **bpftool** [*OPTIONS*] **prog** *COMMAND*
> *OPTIONS* := { |COMMON_OPTIONS| [ { **-f** | **--bpffs** } ] [ {
> **-m** | **--mapcompat** } ]
> [ { **-n** | **--nomount** } ] [ { **-L** | **--use-loader** } ]
> [ { { **-S** | **--sign** } **-k** <private_key.pem> **-i**
> <certificate.x509> } ] }
>
> not an expert here but I vaguely remember.
>
> Also do you think we need to:
>
> { "use-loader", no_argument, NULL, 'L' },
> - { "sign", required_argument, NULL, 'S'},
> + { "sign", no_argument, NULL, 'S' },
>
>
> Now that we don't use an argument blob for --sign?
>
>
> >
> > Can you also please take a look at the bash completion update? It
> > shouldn't be too hard if you look at how it deals with other options, in
> > particular --base-btf that also takes one argument - and I can help if
> > necessary.
>
> I will give it a go.
>
> >
> >
> > >
> > > EXAMPLES
> > > ========
> > > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > > index f69fd92df8d8..dc2ca196137e 100644
> > > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > > @@ -248,6 +248,18 @@ OPTIONS
> > > creating the maps, and loading the programs (see **bpftool prog tracelog**
> > > as a way to dump those messages).
> > >
> > > +-S, --sign
> > > + Enable signing of the BPF program before loading. This option must be
> > > + used with **-k** and **-i**. Using this flag implicitly enables
> > > + **--use-loader**.
> > > +
> > > +-k <private_key.pem>
> > > + Path to the private key file in PEM format, required when signing.
> > > +
> > > +-i <certificate.x509>
> > > + Path to the X.509 certificate file in PEM or DER format, required when
> > > + signing.
> >
> >
> > Same as for skeletons: please update the list of options in the synopsis
> > and at the bottom of prog.c (bash completion for skeletons' options
> > should also cover this case, so no additional work required here).
> >
> >
> > > +
> > > EXAMPLES
> > > ========
> > > **# bpftool prog show**
> > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > > index 9e9a5f006cd2..586d1b2595d1 100644
> > > --- a/tools/bpf/bpftool/Makefile
> > > +++ b/tools/bpf/bpftool/Makefile
> > > @@ -130,8 +130,8 @@ include $(FEATURES_DUMP)
> > > endif
> > > endif
> > >
> > > -LIBS = $(LIBBPF) -lelf -lz
> > > -LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
> > > +LIBS = $(LIBBPF) -lelf -lz -lcrypto
> > > +LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz -lcrypto
> > >
> > > ifeq ($(feature-libelf-zstd),1)
> > > LIBS += -lzstd
> > > @@ -194,7 +194,7 @@ endif
> > >
> > > BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
> > >
> > > -BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o)
> > > +BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o sign.o)
> > > $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
> > >
> > > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> > > diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> > > index 944ebe21a216..90c9aa297806 100644
> > > --- a/tools/bpf/bpftool/cgroup.c
> > > +++ b/tools/bpf/bpftool/cgroup.c
> > > @@ -1,7 +1,10 @@
> > > // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > // Copyright (C) 2017 Facebook
> > > // Author: Roman Gushchin <guro at fb.com>
> > > -
> >
> >
> > Let's keep the blank line
>
> Done
>
> >
> >
> > > +#undef GCC_VERSION
> > > +#ifndef _GNU_SOURCE
> > > +#define _GNU_SOURCE
> > > +#endif
> >
> >
> > What are these for?
>
> kpsingh at kpsingh-genoa:~/projects/linux/tools/bpf/bpftool$ vmk
>
> Auto-detecting system features:
> ... clang-bpf-co-re: [ on ]
> ... llvm: [ on ]
> ... libcap: [ on ]
> ... libbfd: [ OFF ]
>
> In file included from cgroup.c:19:
> In file included from ./main.h:16:
> /home/kpsingh/projects/linux/tools/bpf/bpftool/libbpf/include/bpf/skel_internal.h:87:9:
> error: call to undeclared function 'syscall'; ISO C99 and later do not
> support implicit function declarations
> [-Wimplicit-function-declaration]
> 87 | return syscall(__NR_bpf, cmd, attr, size);
> | ^
> 1 error generated.
>
> >
> >
> > > #define _XOPEN_SOURCE 500
> > > #include <errno.h>
> > > #include <fcntl.h>
> >
> > [...]
> >
> > > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> > > index 2b7f2bd3a7db..fc25bb390ec7 100644
> > > --- a/tools/bpf/bpftool/main.c
> > > +++ b/tools/bpf/bpftool/main.c
> > > @@ -33,6 +33,9 @@ bool relaxed_maps;
> > > bool use_loader;
> > > struct btf *base_btf;
> > > struct hashmap *refs_table;
> > > +bool sign_progs;
> > > +const char *private_key_path;
> > > +const char *cert_path;
> > >
> > > static void __noreturn clean_and_exit(int i)
> > > {
> > > @@ -447,6 +450,7 @@ int main(int argc, char **argv)
> > > { "nomount", no_argument, NULL, 'n' },
> > > { "debug", no_argument, NULL, 'd' },
> > > { "use-loader", no_argument, NULL, 'L' },
> > > + { "sign", required_argument, NULL, 'S'},
> > > { "base-btf", required_argument, NULL, 'B' },
> > > { 0 }
> > > };
> > > @@ -473,7 +477,7 @@ int main(int argc, char **argv)
> > > bin_name = "bpftool";
> > >
> > > opterr = 0;
> > > - while ((opt = getopt_long(argc, argv, "VhpjfLmndB:l",
> > > + while ((opt = getopt_long(argc, argv, "VhpjfLmndSi:k:B:l",
> > > options, NULL)) >= 0) {
> > > switch (opt) {
> > > case 'V':
> > > @@ -519,6 +523,16 @@ int main(int argc, char **argv)
> > > case 'L':
> > > use_loader = true;
> > > break;
> > > + case 'S':
> > > + sign_progs = true;
> > > + use_loader = true;
> > > + break;
> > > + case 'k':
> > > + private_key_path = optarg;
> > > + break;
> > > + case 'i':
> > > + cert_path = optarg;
> > > + break;
> > > default:
> > > p_err("unrecognized option '%s'", argv[optind - 1]);
> > > if (json_output)
> > > @@ -533,6 +547,11 @@ int main(int argc, char **argv)
> > > if (argc < 0)
> > > usage();
> > >
> > > + if (sign_progs && (private_key_path == NULL || cert_path == NULL)) {
> > > + p_err("-i <identity_x509_cert> and -k <private> key must be supplied with -S for signing");
> > > + return -EINVAL;
> > > + }
> >
> >
> > What if -i and/or -k are passed without -S?
>
> We can either print a warning or error out
>
> A) User does not want to sign removes --sign and forgets to remove -i
> -k (better with warning)
> B) User wants to sign but forgets to --sign (better with error)
>
> I'd say we print an error so that we don't accidentally not sign, WDYT?
>
> The reason why I think we should keep an explicit --sign is because we
> can also extend this to have e.g. --verify.
if (!sign_progs && (private_key_path != NULL || cert_path != NULL)) {
p_err("-i <identity_x509_cert> and -k <private> also need --sign to be
used for sign programs");
return -EINVAL;
}
I will error out, I was waiting for Quentin's reply, we can fix it
later if needed.
- KP
>
> - KP
>
> >
> >
> > > +
> > > if (version_requested)
> > > ret = do_version(argc, argv);
> > > else
> > > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > > index 6db704fda5c0..f921af3cda87 100644
> > > --- a/tools/bpf/bpftool/main.h
> > > +++ b/tools/bpf/bpftool/main.h
> > > @@ -6,9 +6,14 @@
> > >
> > > /* BFD and kernel.h both define GCC_VERSION, differently */
> > > #undef GCC_VERSION
> > > +#ifndef _GNU_SOURCE
> > > +#define _GNU_SOURCE
> > > +#endif
> > > #include <stdbool.h>
> > > #include <stdio.h>
> > > +#include <errno.h>
> > > #include <stdlib.h>
> > > +#include <bpf/skel_internal.h>
> >
> >
> > Wnat do you need these includes (and _GNU_SOURCE) in main.h for?
>
> Explained above, let me know if you have better ideas on where to place these.
>
> >
> >
> > > #include <linux/bpf.h>
> > > #include <linux/compiler.h>
> > > #include <linux/kernel.h>
> >
> > [...]
> >
> > > diff --git a/tools/bpf/bpftool/sign.c b/tools/bpf/bpftool/sign.c
> > > new file mode 100644
> > > index 000000000000..f0b5dd10a46b
> > > --- /dev/null
> > > +++ b/tools/bpf/bpftool/sign.c
> > > @@ -0,0 +1,210 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> >
> >
> > Please consider making this file dual-licensed like the rest of
> > bpftool's source code, "(GPL-2.0-only OR BSD-2-Clause)".
>
> Done.
>
> >
> >
> > > +
> > > +/*
> > > + * Copyright (C) 2022 Google LLC.
> >
> >
> > 2025?
>
> Let's keep it 2022, nah just kidding :) Thanks.
>
> >
> >
> > > + */
> > > +#define _GNU_SOURCE
> >
> >
> > Please guard this:
> >
> > #ifndef _GNU_SOURCE
> > #define _GNU_SOURCE
> > #endif
> >
> > This is because "llvm-config --cflags" passes -D_GNU_SOURCE and we may
> > end up with a duplicate definition, otherwise.
>
> ack, done.
>
> >
> >
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <stdint.h>
> > > +#include <stdbool.h>
> > > +#include <string.h>
> > > +#include <string.h>
> > > +#include <getopt.h>
> > > +#include <err.h>
> > > +#include <openssl/opensslv.h>
> > > +#include <openssl/bio.h>
> > > +#include <openssl/evp.h>
> > > +#include <openssl/pem.h>
> > > +#include <openssl/err.h>
> > > +#include <openssl/cms.h>
> > > +#include <linux/keyctl.h>
> > > +#include <errno.h>
> > > +
> > > +#include <bpf/skel_internal.h>
> > > +
> > > +#include "main.h"
> > > +
> > > +#define OPEN_SSL_ERR_BUF_LEN 256
> > > +
> > > +static void display_openssl_errors(int l)
> > > +{
> > > + char buf[OPEN_SSL_ERR_BUF_LEN];
> > > + const char *file;
> > > + const char *data;
> > > + unsigned long e;
> > > + int flags;
> > > + int line;
> > > +
> > > + while ((e = ERR_get_error_all(&file, &line, NULL, &data, &flags))) {
> > > + ERR_error_string_n(e, buf, sizeof(buf));
> > > + if (data && (flags & ERR_TXT_STRING)) {
> > > + p_err("OpenSSL %s: %s:%d: %s\n", buf, file, line, data);
> >
> >
> > Please remove the trailing '\n', p_err() handles it already.
> >
> >
> > > + } else {
> > > + p_err("OpenSSL %s: %s:%d\n", buf, file, line);
> >
> >
> > Same here.
>
> done.
>
> - KP
>
> >
> > [...]
More information about the Linux-security-module-archive
mailing list