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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP01T77d-79-1nGUC=i4mgExY+iKs0j2iiB5DitT_cTj7oXYHQ@mail.gmail.com>
Date:   Sun, 11 Sep 2022 23:08:05 +0200
From:   Kumar Kartikeya Dwivedi <memxor@...il.com>
To:     KP Singh <kpsingh@...nel.org>
Cc:     Roberto Sassu <roberto.sassu@...weicloud.com>, ast@...nel.org,
        daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev,
        song@...nel.org, yhs@...com, john.fastabend@...il.com,
        sdf@...gle.com, haoluo@...gle.com, jolsa@...nel.org,
        mykolal@...com, dhowells@...hat.com, jarkko@...nel.org,
        rostedt@...dmis.org, mingo@...hat.com, paul@...l-moore.com,
        jmorris@...ei.org, serge@...lyn.com, shuah@...nel.org,
        bpf@...r.kernel.org, keyrings@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
        deso@...teo.net, Roberto Sassu <roberto.sassu@...wei.com>
Subject: Re: [PATCH v17 07/12] bpf: Add bpf_verify_pkcs7_signature() kfunc

On Sun, 11 Sept 2022 at 13:41, KP Singh <kpsingh@...nel.org> wrote:
>
> On Fri, Sep 9, 2022 at 2:09 PM Roberto Sassu
> <roberto.sassu@...weicloud.com> wrote:
> >
> > From: Roberto Sassu <roberto.sassu@...wei.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@...wei.com>
> > Acked-by: KP Singh <kpsingh@...nel.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.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ