| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACYkzJ7L-fE740t91amu4uiDA5dnDMU1D+c0vhb-sFHyQK08kA@mail.gmail.com>
Date: Fri, 3 Jun 2022 14:07:35 +0200
From: KP Singh <kpsingh@...nel.org>
To: Roberto Sassu <roberto.sassu@...wei.com>
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
bpf@...r.kernel.org, netdev@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] bpf: Add BPF_F_VERIFY_ELEM to require signature
verification on map values
On Wed, May 25, 2022 at 3:21 PM Roberto Sassu <roberto.sassu@...wei.com> wrote:
>
> In some cases, it is desirable to ensure that a map contains data from
> authenticated sources, for example if map data are used for making security
> decisions.
I am guessing this comes from the discussion we had about digilim.
I remember we discussed a BPF helper that could verify signatures.
Why would that approach not work?
>
>
> Such restriction is achieved by verifying the signature of map values, at
> the time those values are added to the map with the bpf() system call (more
> specifically, when the commands passed to bpf() are BPF_MAP_UPDATE_ELEM or
> BPF_MAP_UPDATE_BATCH). Mmappable maps are not allowed in this case.
>
> Signature verification is initially done with keys in the primary and
> secondary kernel keyrings, similarly to kernel modules. This allows system
> owners to enforce a system-wide policy based on the keys they trust.
> Support for additional keyrings could be added later, based on use case
> needs.
>
> Signature verification is done only for those maps for which the new map
> flag BPF_F_VERIFY_ELEM is set. When the flag is set, the kernel expects map
> values to be in the following format:
>
> +-------------------------------+---------------+-----+-----------------+
> | verified data+sig size (be32) | verified data | sig | unverified data |
> +-------------------------------+---------------+-----+-----------------+
>
> where sig is a module-style appended signature as generated by the
> sign-file tool. The verified data+sig size (in big endian) must be
> explicitly provided (it is not generated by sign-file), as it cannot be
> determined in other ways (currently, the map value size is fixed). It can
> be obtained from the size of the file created by sign-file.
>
> Introduce the new map flag BPF_F_VERIFY_ELEM, and additionally call the new
> function bpf_map_verify_value_sig() from bpf_map_update_value() if the flag
> is set. bpf_map_verify_value_sig(), declared as global for a new helper, is
> basically equivalent to mod_verify_sig(). It additionally does the marker
> check, that for kernel modules is done in module_sig_check(), and the
> parsing of the verified data+sig size.
>
> Currently, enable the usage of the flag only for the array map. Support for
> more map types can be added later.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> ---
> include/linux/bpf.h | 7 ++++
> include/uapi/linux/bpf.h | 3 ++
> kernel/bpf/arraymap.c | 2 +-
> kernel/bpf/syscall.c | 70 ++++++++++++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 3 ++
> 5 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index a7080c86fa76..8f5c042e70a7 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1825,6 +1825,8 @@ static inline bool unprivileged_ebpf_enabled(void)
> return !sysctl_unprivileged_bpf_disabled;
> }
>
> +int bpf_map_verify_value_sig(const void *mod, size_t modlen, bool verify);
> +
> #else /* !CONFIG_BPF_SYSCALL */
> static inline struct bpf_prog *bpf_prog_get(u32 ufd)
> {
> @@ -2034,6 +2036,11 @@ static inline bool unprivileged_ebpf_enabled(void)
> return false;
> }
>
> +static inline int bpf_map_verify_value_sig(const void *mod, size_t modlen,
> + bool verify)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif /* CONFIG_BPF_SYSCALL */
>
> void __bpf_free_used_btfs(struct bpf_prog_aux *aux,
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index f4009dbdf62d..a8e7803d2593 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1226,6 +1226,9 @@ enum {
>
> /* Create a map that is suitable to be an inner map with dynamic max entries */
> BPF_F_INNER_MAP = (1U << 12),
> +
> +/* Verify map value (fmt: ver data+sig size(be32), ver data, sig, unver data) */
> + BPF_F_VERIFY_ELEM = (1U << 13)
> };
>
> /* Flags for BPF_PROG_QUERY. */
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index fe40d3b9458f..b430fdd0e892 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -17,7 +17,7 @@
>
> #define ARRAY_CREATE_FLAG_MASK \
> (BPF_F_NUMA_NODE | BPF_F_MMAPABLE | BPF_F_ACCESS_MASK | \
> - BPF_F_PRESERVE_ELEMS | BPF_F_INNER_MAP)
> + BPF_F_PRESERVE_ELEMS | BPF_F_INNER_MAP | BPF_F_VERIFY_ELEM)
>
> static void bpf_array_free_percpu(struct bpf_array *array)
> {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 2b69306d3c6e..ca9e4a284120 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -35,6 +35,8 @@
> #include <linux/rcupdate_trace.h>
> #include <linux/memcontrol.h>
> #include <linux/trace_events.h>
> +#include <linux/verification.h>
> +#include <linux/module_signature.h>
>
> #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
> (map)->map_type == BPF_MAP_TYPE_CGROUP_ARRAY || \
> @@ -180,6 +182,13 @@ static int bpf_map_update_value(struct bpf_map *map, struct fd f, void *key,
> {
> int err;
>
> + if (map->map_flags & BPF_F_VERIFY_ELEM) {
> + err = bpf_map_verify_value_sig(value, bpf_map_value_size(map),
> + true);
> + if (err < 0)
> + return err;
> + }
> +
> /* Need to create a kthread, thus must support schedule */
> if (bpf_map_is_dev_bound(map)) {
> return bpf_map_offload_update_elem(map, key, value, flags);
> @@ -1057,6 +1066,11 @@ static int map_create(union bpf_attr *attr)
> if (err)
> return -EINVAL;
>
> + /* Allow signed data to go through update/push methods only. */
> + if ((attr->map_flags & BPF_F_VERIFY_ELEM) &&
> + (attr->map_flags & BPF_F_MMAPABLE))
> + return -EINVAL;
> +
> if (attr->btf_vmlinux_value_type_id) {
> if (attr->map_type != BPF_MAP_TYPE_STRUCT_OPS ||
> attr->btf_key_type_id || attr->btf_value_type_id)
> @@ -1353,6 +1367,62 @@ static int map_lookup_elem(union bpf_attr *attr)
> return err;
> }
>
> +int bpf_map_verify_value_sig(const void *mod, size_t modlen, bool verify)
> +{
> + const size_t marker_len = strlen(MODULE_SIG_STRING);
> + struct module_signature ms;
> + size_t sig_len;
> + u32 _modlen;
> + int ret;
> +
> + /*
> + * Format of mod:
> + *
> + * verified data+sig size (be32), verified data, sig, unverified data
> + */
> + if (modlen <= sizeof(u32))
> + return -ENOENT;
> +
> + _modlen = be32_to_cpu(*(u32 *)(mod));
> +
> + if (_modlen > modlen - sizeof(u32))
> + return -EINVAL;
> +
> + modlen = _modlen;
> + mod += sizeof(u32);
> +
> + if (modlen <= marker_len)
> + return -ENOENT;
> +
> + if (memcmp(mod + modlen - marker_len, MODULE_SIG_STRING, marker_len))
> + return -ENOENT;
> +
> + modlen -= marker_len;
> +
> + if (modlen <= sizeof(ms))
> + return -EBADMSG;
> +
> + memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
> +
> + ret = mod_check_sig(&ms, modlen, "bpf_map_value");
> + if (ret)
> + return ret;
> +
> + sig_len = be32_to_cpu(ms.sig_len);
> + modlen -= sig_len + sizeof(ms);
> +
> + if (verify) {
> + ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
> + VERIFY_USE_SECONDARY_KEYRING,
> + VERIFYING_UNSPECIFIED_SIGNATURE,
> + NULL, NULL);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return modlen;
> +}
> +EXPORT_SYMBOL_GPL(bpf_map_verify_value_sig);
>
> #define BPF_MAP_UPDATE_ELEM_LAST_FIELD flags
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index f4009dbdf62d..a8e7803d2593 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1226,6 +1226,9 @@ enum {
>
> /* Create a map that is suitable to be an inner map with dynamic max entries */
> BPF_F_INNER_MAP = (1U << 12),
> +
> +/* Verify map value (fmt: ver data+sig size(be32), ver data, sig, unver data) */
> + BPF_F_VERIFY_ELEM = (1U << 13)
> };
>
> /* Flags for BPF_PROG_QUERY. */
> --
> 2.25.1
>
Powered by blists - more mailing lists