Date:   Thu, 09 Dec 2021 13:40:14 +0000
From:   Luca Boccassi <bluca@...ian.org>
To:     John Fastabend <john.fastabend@...il.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Matteo Croce <mcroce@...ux.microsoft.com>
Cc:     bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        KP Singh <kpsingh@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        keyrings@...r.kernel.org,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Lorenzo Bianconi <lorenzo@...nel.org>
Subject: Re: [PATCH bpf-next 0/3] bpf: add signature

On Wed, 2021-12-08 at 12:17 -0800, John Fastabend wrote:
> [...]
> 
> > > > > Hope this makes sense. Thanks!
> > > > 
> > > > I think I understand your use case. When done as BPF helper you
> > > > can get the behavior you want with a one line BPF program
> > > > loaded at boot.
> > > > 
> > > > int verify_all(struct bpf_prog **prog) {
> > > >         return verify_signature(prog->insn,
> > > >                                 prog->len * sizeof(struct bpf_insn),
> > > >                                 signature, KEYRING, BPF_SIGTYPE);
> > > > }
> > > > 
> > > > And I can write some more specific things as,
> > > > 
> > > > int verify_blobs(void data) {
> > > >   int reject = verify_signature(data, data_len, sig, KEYRING, TYPE);
> > > >   struct policy_key *key = map_get_key();
> > > > 
> > > >   return policy(key, reject);  
> > > > }
> > > > 
> > > > map_get_key() looks into some datastor with the policy likely using
> > > > 'current' to dig something up. It doesn't just apply to BPF progs
> > > > we can use it on other executables more generally. And I get more
> > > > interesting use cases like, allowing 'tc' programs unsigned, but
> > > > requiring kernel memory reads to require signatures or any N
> > > > other policies that may have value. Or only allowing my dbg user
> > > > to run read-only programs, because the dbg maybe shouldn't ever
> > > > be writing into packets, etc. Driving least privilege use cases
> > > > in fine detail.
> > > > 
> > > > By making it a BPF program we side step the debate where the kernel
> > > > tries to get the 'right' policy for you, me, everyone now and in
> > > > the future. The only way I can see to do this without getting N
> > > > policies baked into the kernel and at M different hook points is via
> > > > a BPF helper.
> > > > 
> > > > Thanks,
> > > > John
> > > 
> > > Now this sounds like something that could work - we can prove that this
> > > could be loaded before any writable fs comes up anywhere, so in
> > > principle I think it would be acceptable and free of races. Matteo, we
> > > should talk about this tomorrow.
> > > And this requires some infrastructure work right? Is there a WIP git
> > > tree somewhere that we can test out?
> > > 
> > > Thank you!
> > 
> 
> I don't have a WIP tree, but I believe it should be fairly easy.
> First I would add a wrapper BPF helper for verify_signature() so
> we can call it from fentry/freturn context. That can be done on
> its own IMO as its a generally useful operation.
> 
> Then I would stub a hook point into the BPF load path. The exact
> place to put this is going to have some debate I think, but I
> would place it immediately after the check_bpf call.
> 
> With above two you have enough to do sig verification iiuc.
> 
> Early boot loading I would have to check its current status. But I know
> folks have been working on it. Maybe its done?
> 
> > One question more question: with the signature + kconfig approach,
> > nothing can disable the signature check. But if the signature checker
> > is itself a bpf program, is there/can there be anything stopping root
> > from unloading it?
> 
> Interesting. Not that I'm aware of. Currently something with sufficient
> privileges could unload the program. Maybe we should have a flag so
> early boot programs can signal they shouldn't be unloaded ever. I would
> be OK with this and also seems generally useful. I have a case where
> I want to always set the socket cookie and we leave it running all the
> time. It would be nice if it came up and was pinned at boot.
> 
> Maybe slightly better than a flag would be to have a new CAP support
> that only early boot has like CAP_BPF_EARLY. From my point of view
> this both seems doable with just some smallish changes on BPF side.
> 
> Thanks,
> John

Thanks - again the means of enforcing this are not too important for
our use case, as long as there is something that works reliably and can
be attested.

-- 
Kind regards,
Luca Boccassi

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists