| Message ID | 20220809134603.1769279-8-roberto.sassu@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Add kfuncs for PKCS#7 signature verification | expand |
On 8/9/22 3:46 PM, Roberto Sassu wrote: [...] > 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> > --- > kernel/trace/bpf_trace.c | 56 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 33ca4cfe6e26..79ba8c96735a 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -21,6 +21,7 @@ > #include <linux/bsearch.h> > #include <linux/sort.h> > #include <linux/key.h> > +#include <linux/verification.h> > > #include <net/bpf_sk_storage.h> > > @@ -1290,6 +1291,47 @@ noinline __weak 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. > + */ > +noinline __weak int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, nit: see comment on prev patch for noinline and __weak > + struct bpf_dynptr_kern *sig_ptr, > + struct bpf_key *trusted_keyring) > +{ > + int ret; > + > + if (trusted_keyring->valid_ptr) { > + /* > + * Do the permission check deferred in bpf_lookup_user_key(). > + * > + * A call to key_task_permission() here would be redundant, as > + * it is already done by keyring_search() called by > + * find_asymmetric_key(). > + */ In this patch and previous one, you describe that calling key_validate() is deferred, but you should also provide the actual rationale for readers on "why" we need to do it. > + 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_kfunc_set) > @@ -1303,11 +1345,25 @@ static const struct btf_kfunc_id_set bpf_key_kfunc_set = { > .owner = THIS_MODULE, > .set = &key_kfunc_set, > };
On Tue, Aug 09, 2022 at 03:46:00PM +0200, Roberto Sassu wrote: > 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> > --- > kernel/trace/bpf_trace.c | 56 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 33ca4cfe6e26..79ba8c96735a 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -21,6 +21,7 @@ > #include <linux/bsearch.h> > #include <linux/sort.h> > #include <linux/key.h> > +#include <linux/verification.h> > > #include <net/bpf_sk_storage.h> > > @@ -1290,6 +1291,47 @@ noinline __weak 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. > + */ > +noinline __weak 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->valid_ptr) { > + /* > + * Do the permission check deferred in bpf_lookup_user_key(). > + * > + * 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_kfunc_set) > @@ -1303,11 +1345,25 @@ static const struct btf_kfunc_id_set bpf_key_kfunc_set = { > .owner = THIS_MODULE, > .set = &key_kfunc_set, > }; > + > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION > +BTF_SET8_START(verify_sig_kfunc_set) > +BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE) > +BTF_SET8_END(verify_sig_kfunc_set) > + > +static const struct btf_kfunc_id_set bpf_verify_sig_kfunc_set = { > + .owner = THIS_MODULE, > + .set = &verify_sig_kfunc_set, > +}; > +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ > #endif /* CONFIG_KEYS */ > > const struct btf_kfunc_id_set *kfunc_sets[] = { > #ifdef CONFIG_KEYS > &bpf_key_kfunc_set, > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION > + &bpf_verify_sig_kfunc_set, > +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ > #endif /* CONFIG_KEYS */ > }; Why different sets? The loop over the set from the previous patch can be removed if it's just one set. Each kfunc can be ifdef-ed independently.
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 33ca4cfe6e26..79ba8c96735a 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -21,6 +21,7 @@ #include <linux/bsearch.h> #include <linux/sort.h> #include <linux/key.h> +#include <linux/verification.h> #include <net/bpf_sk_storage.h> @@ -1290,6 +1291,47 @@ noinline __weak 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. + */ +noinline __weak 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->valid_ptr) { + /* + * Do the permission check deferred in bpf_lookup_user_key(). + * + * 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_kfunc_set) @@ -1303,11 +1345,25 @@ static const struct btf_kfunc_id_set bpf_key_kfunc_set = { .owner = THIS_MODULE, .set = &key_kfunc_set, }; + +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION +BTF_SET8_START(verify_sig_kfunc_set) +BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE) +BTF_SET8_END(verify_sig_kfunc_set) + +static const struct btf_kfunc_id_set bpf_verify_sig_kfunc_set = { + .owner = THIS_MODULE, + .set = &verify_sig_kfunc_set, +}; +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ #endif /* CONFIG_KEYS */ const struct btf_kfunc_id_set *kfunc_sets[] = { #ifdef CONFIG_KEYS &bpf_key_kfunc_set, +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION + &bpf_verify_sig_kfunc_set, +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ #endif /* CONFIG_KEYS */ };
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> --- kernel/trace/bpf_trace.c | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)