> From: KP Singh [mailto:kpsingh@xxxxxxxxxx] > Sent: Friday, June 3, 2022 5:18 PM > On Fri, Jun 3, 2022 at 3:11 PM Roberto Sassu <roberto.sassu@xxxxxxxxxx> > wrote: > > > > > From: KP Singh [mailto:kpsingh@xxxxxxxxxx] > > > Sent: Friday, June 3, 2022 2:08 PM > > > On Wed, May 25, 2022 at 3:21 PM Roberto Sassu > <roberto.sassu@xxxxxxxxxx> > > > 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? > > > > The main reason is that signature verification can be done also > > for non-sleepable hooks. For example, one is fexit/array_map_update_elem. > > For your use-case, why is it not possible to hook the LSM hook "bpf" > i.e security_bpf and then check if there is a MAP_UPDATE_ELEM operation? It would require the following: a new helper to compare the user space fd with the address of the map in the eBPF program; copy data from user space to kernel space (verify_pkcs7_signatureI() expects kernel memory). That copy would happen twice. > > Currently the helper in patch 2 just returns the size of verified data. > > With an additional parameter, it could also be used as a helper for > > signature verification by any eBPF programs. > > > > Your bpf_map_verify_value_sig hard codes the type of signature > (bpf_map_verify_value_sig as verify_pkcs7_signature) > its implementation. This is not extensible. It is hardcoded now, but it wouldn't if there are more verification functions. For example, if 'id_type' of module_signature is set to PKEY_ID_PGP, bpf_map_verify_value_sig() would call verify_pgp_signature() (assuming that support for PGP keys and signatures is added to the kernel). Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Yang Xi, Li He > What we discussed was an extensible helper that can be used for > different signature types. > > > To be honest, I like more the idea of a map flag, as it is more > > clear that signature verification is being done. Otherwise, > > we would need to infer it from the eBPF program code. > > > > Thanks > > > > Roberto > > > > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 > > Managing Director: Li Peng, Yang Xi, Li He > > > > > > 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@xxxxxxxxxx> > > > > --- > > [...] > > > > > + 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 > > > >
