| Message ID | 20220909120736.1027040-8-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | bpf: Add kfuncs for PKCS#7 signature verification | expand |
On Fri, Sep 9, 2022 at 1:09 PM Roberto Sassu <roberto.sassu@huaweicloud.com> wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security modules > the ability to check the validity of a signature against supplied data, by > using user-provided or system-provided keys as trust anchor. > > The new kfunc makes it possible to enforce mandatory policies, as eBPF > programs might be allowed to make security decisions only based on data > sources the system administrator approves. > > The caller should provide the data to be verified and the signature as eBPF > dynamic pointers (to minimize the number of parameters) and a bpf_key > structure containing a reference to the keyring with keys trusted for > signature verification, obtained from bpf_lookup_user_key() or > bpf_lookup_system_key(). > > For bpf_key structures obtained from the former lookup function, > bpf_verify_pkcs7_signature() completes the permission check deferred by > that function by calling key_validate(). key_task_permission() is already > called by the PKCS#7 code. > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > Acked-by: KP Singh <kpsingh@kernel.org> Acked-by: Song Liu <song@kernel.org> [...]
On Fri, Sep 9, 2022 at 2:09 PM Roberto Sassu <roberto.sassu@huaweicloud.com> wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security modules > the ability to check the validity of a signature against supplied data, by > using user-provided or system-provided keys as trust anchor. > > The new kfunc makes it possible to enforce mandatory policies, as eBPF > programs might be allowed to make security decisions only based on data > sources the system administrator approves. > > The caller should provide the data to be verified and the signature as eBPF > dynamic pointers (to minimize the number of parameters) and a bpf_key > structure containing a reference to the keyring with keys trusted for > signature verification, obtained from bpf_lookup_user_key() or > bpf_lookup_system_key(). > > For bpf_key structures obtained from the former lookup function, > bpf_verify_pkcs7_signature() completes the permission check deferred by > that function by calling key_validate(). key_task_permission() is already > called by the PKCS#7 code. > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > Acked-by: KP Singh <kpsingh@kernel.org> > --- > kernel/trace/bpf_trace.c | 45 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 45 insertions(+) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index ab183dbaa8d1..9df53c40cffd 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey) > kfree(bkey); > } > > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION > +/** > + * bpf_verify_pkcs7_signature - verify a PKCS#7 signature > + * @data_ptr: data to verify > + * @sig_ptr: signature of the data > + * @trusted_keyring: keyring with keys trusted for signature verification > + * > + * Verify the PKCS#7 signature *sig_ptr* against the supplied *data_ptr* > + * with keys in a keyring referenced by *trusted_keyring*. > + * > + * Return: 0 on success, a negative value on error. > + */ > +int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, > + struct bpf_dynptr_kern *sig_ptr, > + struct bpf_key *trusted_keyring) > +{ > + int ret; > + > + if (trusted_keyring->has_ref) { > + /* > + * Do the permission check deferred in bpf_lookup_user_key(). > + * See bpf_lookup_user_key() for more details. > + * > + * A call to key_task_permission() here would be redundant, as > + * it is already done by keyring_search() called by > + * find_asymmetric_key(). > + */ > + ret = key_validate(trusted_keyring->key); > + if (ret < 0) > + return ret; > + } > + > + return verify_pkcs7_signature(data_ptr->data, > + bpf_dynptr_get_size(data_ptr), > + sig_ptr->data, > + bpf_dynptr_get_size(sig_ptr), > + trusted_keyring->key, > + VERIFYING_UNSPECIFIED_SIGNATURE, NULL, > + NULL); > +} This seems to work if the data that needs to be verified and the signature is allocated onto the map. For BPF program signing, the signature will be void * pointer (and length) in a struct in the kernel +++ b/include/uapi/linux/bpf.h @@ -1383,6 +1383,8 @@ union bpf_attr { __aligned_u64 fd_array; /* array of FDs */ __aligned_u64 core_relos; __u32 core_relo_rec_size; /* sizeof(struct bpf_core_relo) */ + __aligned_u64 signature; + __u32 signature_size; }; Something like this in the bpf_prog_aux struct which is passed to security_bpf_prog_alloc. Now creating a dynptr to use with this kfunc does not work: bpf_dynptr_from_mem(aux->signature, aux->signature_size, 0, &sig_ptr); So one has to copy kernel data into a map and then create dynptrs. Would you be able to update the dynptr logic to handle this case too? (follow up is okay too). - KP > +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ > + > __diag_pop(); > > BTF_SET8_START(key_sig_kfunc_set) > BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE) > BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL) > BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE) > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION > +BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE) > +#endif > BTF_SET8_END(key_sig_kfunc_set) > > static const struct btf_kfunc_id_set bpf_key_sig_kfunc_set = { > -- > 2.25.1 >
On Sun, 11 Sept 2022 at 13:41, KP Singh <kpsingh@kernel.org> wrote: > > On Fri, Sep 9, 2022 at 2:09 PM Roberto Sassu > <roberto.sassu@huaweicloud.com> wrote: > > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security modules > > the ability to check the validity of a signature against supplied data, by > > using user-provided or system-provided keys as trust anchor. > > > > The new kfunc makes it possible to enforce mandatory policies, as eBPF > > programs might be allowed to make security decisions only based on data > > sources the system administrator approves. > > > > The caller should provide the data to be verified and the signature as eBPF > > dynamic pointers (to minimize the number of parameters) and a bpf_key > > structure containing a reference to the keyring with keys trusted for > > signature verification, obtained from bpf_lookup_user_key() or > > bpf_lookup_system_key(). > > > > For bpf_key structures obtained from the former lookup function, > > bpf_verify_pkcs7_signature() completes the permission check deferred by > > that function by calling key_validate(). key_task_permission() is already > > called by the PKCS#7 code. > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > Acked-by: KP Singh <kpsingh@kernel.org> > > --- > > kernel/trace/bpf_trace.c | 45 ++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 45 insertions(+) > > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index ab183dbaa8d1..9df53c40cffd 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey) > > kfree(bkey); > > } > > > > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION > > +/** > > + * bpf_verify_pkcs7_signature - verify a PKCS#7 signature > > + * @data_ptr: data to verify > > + * @sig_ptr: signature of the data > > + * @trusted_keyring: keyring with keys trusted for signature verification > > + * > > + * Verify the PKCS#7 signature *sig_ptr* against the supplied *data_ptr* > > + * with keys in a keyring referenced by *trusted_keyring*. > > + * > > + * Return: 0 on success, a negative value on error. > > + */ > > +int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, > > + struct bpf_dynptr_kern *sig_ptr, > > + struct bpf_key *trusted_keyring) > > +{ > > + int ret; > > + > > + if (trusted_keyring->has_ref) { > > + /* > > + * Do the permission check deferred in bpf_lookup_user_key(). > > + * See bpf_lookup_user_key() for more details. > > + * > > + * A call to key_task_permission() here would be redundant, as > > + * it is already done by keyring_search() called by > > + * find_asymmetric_key(). > > + */ > > + ret = key_validate(trusted_keyring->key); > > + if (ret < 0) > > + return ret; > > + } > > + > > + return verify_pkcs7_signature(data_ptr->data, > > + bpf_dynptr_get_size(data_ptr), > > + sig_ptr->data, > > + bpf_dynptr_get_size(sig_ptr), > > + trusted_keyring->key, > > + VERIFYING_UNSPECIFIED_SIGNATURE, NULL, > > + NULL); > > +} > > This seems to work if the data that needs to be verified > and the signature is allocated onto the map. > > For BPF program signing, the signature will be void * pointer (and length) > in a struct in the kernel > > +++ b/include/uapi/linux/bpf.h > @@ -1383,6 +1383,8 @@ union bpf_attr { > __aligned_u64 fd_array; /* array of FDs */ > __aligned_u64 core_relos; > __u32 core_relo_rec_size; /* sizeof(struct > bpf_core_relo) */ > + __aligned_u64 signature; > + __u32 signature_size; > }; > > Something like this in the bpf_prog_aux struct which is passed to > security_bpf_prog_alloc. > > Now creating a dynptr to use with this kfunc does not work: > > bpf_dynptr_from_mem(aux->signature, aux->signature_size, 0, &sig_ptr); > > So one has to copy kernel data into a map and then create dynptrs. > Would you be able to update > the dynptr logic to handle this case too? (follow up is okay too). > ISTM it needs the feature first before it can be added. To make it work like map_val, value_size(which is a constant) to pass to bpf_dynptr_from_mem, verifier will have to mark load of aux->signature as PTR_TO_MEM with the known constant size, and then mark_reg_known for scalar reg for aux->signature_size. Since we need to know that 0 <= r2 <= r1.mem_size. This would require some work on the btf_struct_access handling. It cannot be made to work in the general case of void * and len. There might also be other better options (like kernel itself preparing read only bpf_dynptr struct in bpf_prog_aux for the signature) so you can pass its address directly to the kfunc.
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index ab183dbaa8d1..9df53c40cffd 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey) kfree(bkey); } +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION +/** + * bpf_verify_pkcs7_signature - verify a PKCS#7 signature + * @data_ptr: data to verify + * @sig_ptr: signature of the data + * @trusted_keyring: keyring with keys trusted for signature verification + * + * Verify the PKCS#7 signature *sig_ptr* against the supplied *data_ptr* + * with keys in a keyring referenced by *trusted_keyring*. + * + * Return: 0 on success, a negative value on error. + */ +int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, + struct bpf_dynptr_kern *sig_ptr, + struct bpf_key *trusted_keyring) +{ + int ret; + + if (trusted_keyring->has_ref) { + /* + * Do the permission check deferred in bpf_lookup_user_key(). + * See bpf_lookup_user_key() for more details. + * + * A call to key_task_permission() here would be redundant, as + * it is already done by keyring_search() called by + * find_asymmetric_key(). + */ + ret = key_validate(trusted_keyring->key); + if (ret < 0) + return ret; + } + + return verify_pkcs7_signature(data_ptr->data, + bpf_dynptr_get_size(data_ptr), + sig_ptr->data, + bpf_dynptr_get_size(sig_ptr), + trusted_keyring->key, + VERIFYING_UNSPECIFIED_SIGNATURE, NULL, + NULL); +} +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ + __diag_pop(); BTF_SET8_START(key_sig_kfunc_set) BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL) BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE) +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION +BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE) +#endif BTF_SET8_END(key_sig_kfunc_set) static const struct btf_kfunc_id_set bpf_key_sig_kfunc_set = {