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? > > 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. 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 > > >
