Re: [PATCH bpf-next 0/3] bpf: add signature

Luca Boccassi
2021-12-09T13:40:20Z

[-- Attachment #1: Type: text/plain, Size: 4144 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]