* Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3
From: Linus Torvalds @ 2019-08-05 19:17 UTC (permalink / raw)
To: Konstantin Ryabitsev
Cc: Micah Morton, linux-security-module, Linux Kernel Mailing List
In-Reply-To: <20190805191136.GB4887@chatter.i7.local>
On Mon, Aug 5, 2019 at 12:11 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> However, I suspect that getting message-ids for all your pull requests
> would significantly complicate your workflow.
Yeah, that would be a noticeable annoyance. If I were to process pull
requests the way I used to process emailed patches (ie "git am -s" on
a mailbox) that would be a natural thing to perhaps do, but it's not
at all how it ends up working. Having to save the pull request email
to then process it with some script would turn it into a chore.
I think the pr-tracker-bot clearly catches most cases as it is, and
it's only the occasional "somebody did something odd" that then misses
an automated response. Not a huge deal. For me it was actually more
the "I didn't understand why the response didn't happen", not so much
"I really want to always see responses".
Linus
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Alexei Starovoitov @ 2019-08-05 19:21 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Song Liu, Kees Cook, Networking, bpf, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
Linux API, LSM List
In-Reply-To: <CALCETrXX-Jeb4wiQuL6FUai4wNMmMiUxuLLh_Lb9mT7h=0GgAw@mail.gmail.com>
On Mon, Aug 05, 2019 at 10:23:10AM -0700, Andy Lutomirski wrote:
>
> I refreshed the branch again. I had a giant hole in my previous idea
> that we could deprivilege program loading: some BPF functions need
> privilege. Now I have a changelog comment to that effect and a patch
> that sketches out a way to addressing this.
>
> I don't think I'm going to have time soon to actually get any of this
> stuff mergeable, and it would be fantastic if you or someone else who
> likes working of bpf were to take this code and run with it. Feel
> free to add my Signed-off-by, and I'd be happy to help review.
Thanks a lot for working on patches and helping us with the design!
Can you resend the patches to the mailing list?
It's kinda hard to reply/review to patches that are somewhere in the web.
I'm still trying to understand the main idea.
If I'm reading things correctly:
patch 1 "add access permissions to bpf fds"
just passes the flags ?
patch 2 "Don't require mknod() permission to pin an object"
makes sense in isolation.
patch 3 "Allow creating all program types without privilege"
is not right.
patch 4 "Add a way to mark functions as requiring privilege"
is an interesting idea, but I don't think it helps that much.
So the main thing we're trying to solve with augmented bpf syscall
and/or /dev/bpf is to be able to use root-only features of bpf when
trused process already dropped root permissions.
These features include bpf2bpf calls, bounded loops, special maps (like LPM), etc.
Attaching to a cgroup already has file based permission checks.
The user needs to open cgroup directory to attach.
acls on cgroup dir can already be used to prevent attaching to
certain parts of cgroup hierarchy.
It seems this discussion is centered around making /dev/bpf to
let unpriv (and not trusted) users (humans) to do bpf.
That's not quite the case.
It's a good use case, but not the one we're after at the moment.
In our enviroment bpftrace, bpftool, all bcc tools are pre-installed
and the users (humans) can simply 'sudo' to run them.
Adding suid bit to installed bpftool binary is doable, but there is no need.
'sudo' works just fine.
What we need is to drop privileges sooner in daemons like systemd.
Container management daemon runs in the nested containers.
These trusted daemons need to have access to full bpf, but they
don't want to be root all the time.
They cannot flip back and forth via seteuid to root every time they
need to do bpf.
Hence the idea is to have a file that this daemon can open,
then drop privileges and still keep doing bpf things because FD is held.
Outer container daemon can pass this /dev/bpf's FD to inner daemon, etc.
This /dev/bpf would be accessible to root only.
There is no desire to open it up to non-root.
It seems there is concern that /dev/bpf is unnecessary special.
How about we combine bpffs and /dev/bpf ideas?
Like we can have a special file name in bpffs.
The root would do 'touch /sys/fs/bpf/privileges' and it would behave
just like /dev/bpf, but now it can be in any bpffs directory and acls
to bpffs mount would work as-is.
CAP_BPF is also good idea. I think for the enviroment where untrusted
and unprivileged users want to run 'bpftrace' that would be perfect mechanism.
getcap /bin/bpftrace would have cap_bpf, cap_kprobe and whatever else.
Sort of like /bin/ping.
But I don't see how cap_bpf helps to solve our trusted root daemon problem.
imo open ("/sys/fs/bpf/privileges") and pass that FD into bpf syscall
is the only viable mechanism.
Note the verifier does very different amount of work for unpriv vs root.
It does speculative execution analysis, pointer leak checks for unpriv.
So we gotta pass special flag to the verifier to make it act like it's
loading a program for root.
^ permalink raw reply
* Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3
From: Konstantin Ryabitsev @ 2019-08-05 19:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: Micah Morton, linux-security-module, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wg4xMXMM3EfW=NV6YuScA4zvcvaCAPou3bxegjGy6r-qA@mail.gmail.com>
On Mon, Aug 05, 2019 at 12:17:49PM -0700, Linus Torvalds wrote:
>> However, I suspect that getting message-ids for all your pull
>> requests
>> would significantly complicate your workflow.
>
>Yeah, that would be a noticeable annoyance. If I were to process pull
>requests the way I used to process emailed patches (ie "git am -s" on
>a mailbox) that would be a natural thing to perhaps do, but it's not
>at all how it ends up working. Having to save the pull request email
>to then process it with some script would turn it into a chore.
>
>I think the pr-tracker-bot clearly catches most cases as it is, and
>it's only the occasional "somebody did something odd" that then misses
>an automated response. Not a huge deal. For me it was actually more
>the "I didn't understand why the response didn't happen", not so much
>"I really want to always see responses".
Ok, let me add a fix for Re: at the start -- this won't make things
significantly more expensive, but will catch this particular corner
case.
Best regards,
-K
^ permalink raw reply
* Re: [RFC PATCH v3 04/12] x86/sgx: Require userspace to define enclave pages' protection bits
From: Jarkko Sakkinen @ 2019-08-05 20:51 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Sean Christopherson, Schaufler, Casey, James Morris, linux-sgx,
Dave Hansen, Cedric Xing, Jethro Beekman, Dr . Greg Wettstein,
Stephen Smalley, LSM List
In-Reply-To: <CALCETrWBewZyqVUf3cPDj11eSWXqfDG0z50vVw+Yg0Z8r0AtCg@mail.gmail.com>
On Sun, Aug 04, 2019 at 03:20:24PM -0700, Andy Lutomirski wrote:
> On Thu, Aug 1, 2019 at 9:38 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Mon, Jul 15, 2019 at 03:29:23PM -0700, Andy Lutomirski wrote:
> > > I would say it differently: regardless of exactly how /dev/sgx/enclave
> > > is wired up under the hood, we want a way that a process can be
> > > granted permission to usefully run enclaves without being granted
> > > permission to execute whatever bytes of code it wants. Preferably
> > > without requiring LSMs to maintain some form of enclave signature
> > > whitelist.
> >
> > Would it be better to have a signer whitelist instead or some
> > combination? E.g. you could whiteliste either by signer or
> > enclave signature.
> >
>
> I'm not sure, and also don't really think we need to commit to an
> answer right now. I do think that the eventual solution should be
> more flexible than just whitelisting the signers. In particular, it
> should be possible to make secure enclaves, open-source or otherwise,
> that are reproducibly buildable. This more or less requires that the
> signing private key not be a secret, which means that no one would
> want to whitelist the signing key. The enclave would be trusted, and
> would seal data, on the basis of its MRENCLAVE, and the policy, if
> any, would want to whitelist the MRENCLAVE or perhaps the whole
> SIGSTRUCT.
>
> But my overall point is that it should be possible to have a conherent
> policy that allows any enclave whatsoever to run but that still
> respects EXECMEM and such.
So could kernel embed a fixed signing key that would be made available
through sysfs for signing? Already have one for my selftest.
/Jarkko
^ permalink raw reply
* Re: [RFC/RFT v2 1/2] KEYS: trusted: create trusted keys subsystem
From: Jarkko Sakkinen @ 2019-08-05 20:59 UTC (permalink / raw)
To: Sumit Garg
Cc: keyrings, linux-integrity,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-security-module, dhowells, Herbert Xu, davem, jejb,
Mimi Zohar, James Morris, Serge E. Hallyn, Casey Schaufler,
Ard Biesheuvel, Daniel Thompson, Linux Kernel Mailing List,
tee-dev @ lists . linaro . org
In-Reply-To: <CAFA6WYOMXc2y=vXOwRv+PYyF8oBV70G7CrJ81jvD5yJT41zLZw@mail.gmail.com>
On Mon, Aug 05, 2019 at 10:32:59AM +0530, Sumit Garg wrote:
> Okay, I will try to move TPM2 trusted keys code also.
I'm definitely for extending trusted keys beyond TPMs. Before that can be
done, however, the current mess needs to be cleaned up.
I did a lot of work recently [1] to clean up TPM transmit code to better
suited to be used outside of the TPM drivers (remove recursive calls,
put the whole stack use tpm_buf for everything).
What still needs to be done is to move tpm_buf stuff to include/linux in
order to be usable in the keyring code. Also for TPM 2.0 trusted keys,
TPM2 constants need to be moved to include/linux. For the latter, I'd
suggest to move all protocol constants there and not just what is
required for trusted keys. Better to have them in one place.
[1] https://lkml.org/lkml/2019/2/13/176
/Jarkko
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-08-05 21:25 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Song Liu, Kees Cook, Networking, bpf,
Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
Jann Horn, Greg KH, Linux API, LSM List
In-Reply-To: <20190805192122.laxcaz75k4vxdspn@ast-mbp>
On Mon, Aug 5, 2019 at 12:21 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Aug 05, 2019 at 10:23:10AM -0700, Andy Lutomirski wrote:
> >
> > I refreshed the branch again. I had a giant hole in my previous idea
> > that we could deprivilege program loading: some BPF functions need
> > privilege. Now I have a changelog comment to that effect and a patch
> > that sketches out a way to addressing this.
> >
> > I don't think I'm going to have time soon to actually get any of this
> > stuff mergeable, and it would be fantastic if you or someone else who
> > likes working of bpf were to take this code and run with it. Feel
> > free to add my Signed-off-by, and I'd be happy to help review.
>
> Thanks a lot for working on patches and helping us with the design!
>
> Can you resend the patches to the mailing list?
> It's kinda hard to reply/review to patches that are somewhere in the web.
Will do.
> I'm still trying to understand the main idea.
> If I'm reading things correctly:
The series doesn't, strictly speaking, have an overall problem that it
solves. It's a series of steps in the direction of making bpf() make
more sense without privilege and toward reducing the required
privilege.
> patch 1 "add access permissions to bpf fds"
> just passes the flags ?
It tries to make the kernel respect the access modes for fds. Without
this patch, there seem to be some holes: nothing looked at program fds
and, unless I missed something, you could take a readonly fd for a
program, pin the program, and reopen it RW.
> patch 2 "Don't require mknod() permission to pin an object"
> makes sense in isolation.
It makes even more sense now :)
> patch 3 "Allow creating all program types without privilege"
> is not right.
I think it can be made right, which is the point.
> patch 4 "Add a way to mark functions as requiring privilege"
> is an interesting idea, but I don't think it helps that much.
Other than the issue that this patch partially fixes, can you see any
reason that loading a program should require privilege? Obviously the
verifier is weakened a bit when called by privileged users, but a lot
of that is about excessive resource usage and various less-well-tested
features. It seems to me that most of the value of bpf() should be
available to programs that should not need privilege to load. Are
there things I'm missing?
>
> So the main thing we're trying to solve with augmented bpf syscall
> and/or /dev/bpf is to be able to use root-only features of bpf when
> trused process already dropped root permissions.
> These features include bpf2bpf calls, bounded loops, special maps (like LPM), etc.
Can you elaborate on all these:
I see nothing inherently wrong with bpf2bpf for unprivileged users as
long as they have appropriate access to the called program. Patch 1
improves that.
Bounded loops: if they are adequately well verified, then the only
damage is that they can make bpf progs that run slowly, right? It
seems like some kind of capability or sysctl for "allow using lots of
bpf resources" would do the trick. This could even be a cgroup
setting -- bpf resources aren't all that different from any other
resource.
LPM: I don't see why this requires privilege at all. It indeed checks
capable(CAP_SYS_ADMIN), but I don't see why.
>
> Attaching to a cgroup already has file based permission checks.
> The user needs to open cgroup directory to attach.
> acls on cgroup dir can already be used to prevent attaching to
> certain parts of cgroup hierarchy.
The current checks seem inadequate.
$ echo 'yay' </sys/fs/cgroup/systemd/system.slice/
The ability to obtain an fd to a cgroup does *not* imply any right to
modify that cgroup. The ability to write to a cgroup directory
already means something else -- it's the ability to create cgroups
under the group in question. I'm suggesting that a new API be added
that allows attaching a bpf program to a cgroup without capabilities
and that instead requires write access to a new file in the cgroup
directory. (It could be a single file for all bpf types or one file
per type. I prefer the latter -- it gives the admin finer-grained
control.)
> What we need is to drop privileges sooner in daemons like systemd.
This is doable right now: systemd could fork off a subprocess and
delegate its cgroup operations to it. It would be maybe a couple
hundred lines of code. As an added benefit, that subprocess could
verify that the bpf operations in question are reasonable.
Alternatively, if there was a CAP_BPF_ADMIN, systemd could retain that
capability and flip it on and off as needed.
> Container management daemon runs in the nested containers.
> These trusted daemons need to have access to full bpf, but they
> don't want to be root all the time.
> They cannot flip back and forth via seteuid to root every time they
> need to do bpf.
> Hence the idea is to have a file that this daemon can open,
> then drop privileges and still keep doing bpf things because FD is held.
> Outer container daemon can pass this /dev/bpf's FD to inner daemon, etc.
> This /dev/bpf would be accessible to root only.
> There is no desire to open it up to non-root.
This seems extremely dangerous right now. A program that can bypass
*all* of the capable() checks in bpf() can do a whole lot. Among
other things, it can read all of kernel memory. It can very likely
gain full system root by appropriate installation of malicious
programs in a cgroup that contains fully privileged programs. In this
regard, bpf() is like most of the Linux capabilities -- it seems
somewhat limited, but it really implies a lot of privilege. There was
a little paper awhile back pointing out that, on a normal system, most
of the Linux capabilities were functionally equivalent.
>
> It seems there is concern that /dev/bpf is unnecessary special.
> How about we combine bpffs and /dev/bpf ideas?
> Like we can have a special file name in bpffs.
> The root would do 'touch /sys/fs/bpf/privileges' and it would behave
> just like /dev/bpf, but now it can be in any bpffs directory and acls
> to bpffs mount would work as-is.
This seems to have most of the same problems. My main point is that
it conflates a whole lot of different permissions, and I really don't
think it's that much work to mostly disentangle the permissions in
question. My little series (if completed) plus a patch to allow
unprivileged cgroup attach operations if you have an FMODE_WRITE fd to
an appropriate file should get most of the way there.
Also, be careful about your bpffs idea: bpffs is (sort of) namespaced,
and it would make sense to allow new bpf instances to be created
inside unprivileged user namespaces. Such instances should not be
able to create magical privilege-granting files. In that respect,
/dev/bpf is better.
>
> CAP_BPF is also good idea. I think for the enviroment where untrusted
> and unprivileged users want to run 'bpftrace' that would be perfect mechanism.
> getcap /bin/bpftrace would have cap_bpf, cap_kprobe and whatever else.
> Sort of like /bin/ping.
> But I don't see how cap_bpf helps to solve our trusted root daemon problem.
> imo open ("/sys/fs/bpf/privileges") and pass that FD into bpf syscall
> is the only viable mechanism.
>
As above, I think that forking before dropping privileges and asking
the child to do the bpf() operations is safer and more flexible.
> Note the verifier does very different amount of work for unpriv vs root.
> It does speculative execution analysis, pointer leak checks for unpriv.
> So we gotta pass special flag to the verifier to make it act like it's
> loading a program for root.
>
Indeed. And programs in untrusted containers should not be able to do this.
^ permalink raw reply
* [WIP 1/4] bpf: Respect persistent map and prog access modes
From: Andy Lutomirski @ 2019-08-05 21:29 UTC (permalink / raw)
To: LKML, Alexei Starovoitov
Cc: Song Liu, Kees Cook, Networking, bpf, Daniel Borkmann,
Alexei Starovoitov, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
Linux API, LSM List, Andy Lutomirski
In-Reply-To: <cover.1565040372.git.luto@kernel.org>
In the interest of making bpf() more useful by unprivileged users,
this patch teaches bpf to respect access modes on map and prog
inodes. The permissions are:
R on a map: read the map
W on a map: write the map
Referencing a map from a program should require RW.
R on a prog: Read or introspect the prog
W on a prog: Attach the prog to something
Test-running a prog is a form of introspection, so it requires RW.
Detaching a prog merely uses the fd for identification, so neither R
nor W is needed.
This is likely incomplete, and it has some comments that should be
removed.
This patch uses WRITE instead of EXEC as the permission needed to
run (by attaching or test-running) a program. EXEC seems nicer, but
O_MAYEXEC isn't merged, which makes using X awkward.
---
include/linux/bpf.h | 15 +++++++------
kernel/bpf/arraymap.c | 8 ++++++-
kernel/bpf/cgroup.c | 6 ++++-
kernel/bpf/inode.c | 25 ++++++++++++++-------
kernel/bpf/syscall.c | 51 ++++++++++++++++++++++++++++++------------
kernel/events/core.c | 5 +++--
net/core/dev.c | 4 +++-
net/core/filter.c | 8 ++++---
net/netfilter/xt_bpf.c | 5 +++--
net/packet/af_packet.c | 2 +-
10 files changed, 89 insertions(+), 40 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 18f4cc2c6acd..2d5e1a4dff6c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -630,9 +630,9 @@ extern const struct bpf_prog_ops bpf_offload_prog_ops;
extern const struct bpf_verifier_ops tc_cls_act_analyzer_ops;
extern const struct bpf_verifier_ops xdp_analyzer_ops;
-struct bpf_prog *bpf_prog_get(u32 ufd);
+struct bpf_prog *bpf_prog_get(u32 ufd, int mask);
struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
- bool attach_drv);
+ bool attach_drv, int mask);
struct bpf_prog * __must_check bpf_prog_add(struct bpf_prog *prog, int i);
void bpf_prog_sub(struct bpf_prog *prog, int i);
struct bpf_prog * __must_check bpf_prog_inc(struct bpf_prog *prog);
@@ -662,7 +662,7 @@ void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr);
extern int sysctl_unprivileged_bpf_disabled;
int bpf_map_new_fd(struct bpf_map *map, int flags);
-int bpf_prog_new_fd(struct bpf_prog *prog);
+int bpf_prog_new_fd(struct bpf_prog *prog, int flags);
int bpf_obj_pin_user(u32 ufd, const char __user *pathname);
int bpf_obj_get_user(const char __user *pathname, int flags);
@@ -733,7 +733,7 @@ static inline int bpf_map_attr_numa_node(const union bpf_attr *attr)
attr->numa_node : NUMA_NO_NODE;
}
-struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type type);
+struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type type, int mask);
int array_map_alloc_check(union bpf_attr *attr);
int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
@@ -850,7 +850,7 @@ static inline int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu,
}
static inline struct bpf_prog *bpf_prog_get_type_path(const char *name,
- enum bpf_prog_type type)
+ enum bpf_prog_type type, int mask)
{
return ERR_PTR(-EOPNOTSUPP);
}
@@ -878,9 +878,10 @@ static inline int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
#endif /* CONFIG_BPF_SYSCALL */
static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
- enum bpf_prog_type type)
+ enum bpf_prog_type type,
+ int mask)
{
- return bpf_prog_get_type_dev(ufd, type, false);
+ return bpf_prog_get_type_dev(ufd, type, false, mask);
}
bool bpf_prog_get_ok(struct bpf_prog *, enum bpf_prog_type *, bool);
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 1c65ce0098a9..7e17a5d42110 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -522,6 +522,10 @@ int bpf_fd_array_map_lookup_elem(struct bpf_map *map, void *key, u32 *value)
}
/* only called from syscall */
+/*
+ * XXX: it's totally unclear to me what this ends up doing with the fd
+ * in general.
+ */
int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
void *key, void *value, u64 map_flags)
{
@@ -569,7 +573,9 @@ static void *prog_fd_array_get_ptr(struct bpf_map *map,
struct file *map_file, int fd)
{
struct bpf_array *array = container_of(map, struct bpf_array, map);
- struct bpf_prog *prog = bpf_prog_get(fd);
+
+ /* XXX: what, exactly, does this end up doing to the prog in question? */
+ struct bpf_prog *prog = bpf_prog_get(fd, FMODE_READ | FMODE_WRITE);
if (IS_ERR(prog))
return prog;
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 0a00eaca6fae..1450c3bdab82 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -562,7 +562,11 @@ int cgroup_bpf_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype)
if (IS_ERR(cgrp))
return PTR_ERR(cgrp);
- prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
+ /*
+ * No particular access required -- this only uses the fd to identify
+ * a program, not to do anything with the program.
+ */
+ prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype, 0);
if (IS_ERR(prog))
prog = NULL;
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index cc0d0cf114e3..cb07736b33ae 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -58,7 +58,7 @@ static void bpf_any_put(void *raw, enum bpf_type type)
}
}
-static void *bpf_fd_probe_obj(u32 ufd, enum bpf_type *type)
+static void *bpf_fd_probe_obj(u32 ufd, enum bpf_type *type, int mask)
{
void *raw;
@@ -66,7 +66,7 @@ static void *bpf_fd_probe_obj(u32 ufd, enum bpf_type *type)
raw = bpf_map_get_with_uref(ufd);
if (IS_ERR(raw)) {
*type = BPF_TYPE_PROG;
- raw = bpf_prog_get(ufd);
+ raw = bpf_prog_get(ufd, mask);
}
return raw;
@@ -430,7 +430,12 @@ int bpf_obj_pin_user(u32 ufd, const char __user *pathname)
if (IS_ERR(pname))
return PTR_ERR(pname);
- raw = bpf_fd_probe_obj(ufd, &type);
+ /*
+ * Pinning an object effectively grants the caller all access, because
+ * the caller ends up owning the inode. So require all access.
+ * XXX: If we use FMODE_EXEC, we should require FMODE_EXEC too.
+ */
+ raw = bpf_fd_probe_obj(ufd, &type, FMODE_READ | FMODE_WRITE);
if (IS_ERR(raw)) {
ret = PTR_ERR(raw);
goto out;
@@ -456,6 +461,10 @@ static void *bpf_obj_do_get(const struct filename *pathname,
if (ret)
return ERR_PTR(ret);
+ /*
+ * XXX: O_MAYEXEC doesn't exist, which is problematic here if we
+ * want to use FMODE_EXEC.
+ */
inode = d_backing_inode(path.dentry);
ret = inode_permission(inode, ACC_MODE(flags));
if (ret)
@@ -499,7 +508,7 @@ int bpf_obj_get_user(const char __user *pathname, int flags)
}
if (type == BPF_TYPE_PROG)
- ret = bpf_prog_new_fd(raw);
+ ret = bpf_prog_new_fd(raw, f_flags);
else if (type == BPF_TYPE_MAP)
ret = bpf_map_new_fd(raw, f_flags);
else
@@ -512,10 +521,10 @@ int bpf_obj_get_user(const char __user *pathname, int flags)
return ret;
}
-static struct bpf_prog *__get_prog_inode(struct inode *inode, enum bpf_prog_type type)
+static struct bpf_prog *__get_prog_inode(struct inode *inode, enum bpf_prog_type type, int mask)
{
struct bpf_prog *prog;
- int ret = inode_permission(inode, MAY_READ);
+ int ret = inode_permission(inode, mask);
if (ret)
return ERR_PTR(ret);
@@ -536,14 +545,14 @@ static struct bpf_prog *__get_prog_inode(struct inode *inode, enum bpf_prog_type
return bpf_prog_inc(prog);
}
-struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type type)
+struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type type, int mask)
{
struct bpf_prog *prog;
struct path path;
int ret = kern_path(name, LOOKUP_FOLLOW, &path);
if (ret)
return ERR_PTR(ret);
- prog = __get_prog_inode(d_backing_inode(path.dentry), type);
+ prog = __get_prog_inode(d_backing_inode(path.dentry), type, mask);
if (!IS_ERR(prog))
touch_atime(&path);
path_put(&path);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 5d141f16f6fa..23f8f89d2a86 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -447,6 +447,7 @@ int bpf_map_new_fd(struct bpf_map *map, int flags)
int bpf_get_file_flag(int flags)
{
+ /* XXX: What about exec? */
if ((flags & BPF_F_RDONLY) && (flags & BPF_F_WRONLY))
return -EINVAL;
if (flags & BPF_F_RDONLY)
@@ -556,6 +557,10 @@ static int map_create(union bpf_attr *attr)
if (err)
return -EINVAL;
+ /*
+ * XXX: I'm a bit confused. Why would you ever create a map and
+ * grant *yourself* less than full permission?
+ */
f_flags = bpf_get_file_flag(attr->map_flags);
if (f_flags < 0)
return f_flags;
@@ -1411,7 +1416,7 @@ const struct file_operations bpf_prog_fops = {
.write = bpf_dummy_write,
};
-int bpf_prog_new_fd(struct bpf_prog *prog)
+int bpf_prog_new_fd(struct bpf_prog *prog, int flags)
{
int ret;
@@ -1420,10 +1425,10 @@ int bpf_prog_new_fd(struct bpf_prog *prog)
return ret;
return anon_inode_getfd("bpf-prog", &bpf_prog_fops, prog,
- O_RDWR | O_CLOEXEC);
+ flags | O_CLOEXEC);
}
-static struct bpf_prog *____bpf_prog_get(struct fd f)
+static struct bpf_prog *____bpf_prog_get(struct fd f, int mask)
{
if (!f.file)
return ERR_PTR(-EBADF);
@@ -1431,6 +1436,10 @@ static struct bpf_prog *____bpf_prog_get(struct fd f)
fdput(f);
return ERR_PTR(-EINVAL);
}
+ if ((f.file->f_mode & mask) != mask) {
+ fdput(f);
+ return ERR_PTR(-EACCES);
+ }
return f.file->private_data;
}
@@ -1497,12 +1506,12 @@ bool bpf_prog_get_ok(struct bpf_prog *prog,
}
static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type,
- bool attach_drv)
+ bool attach_drv, int mask)
{
struct fd f = fdget(ufd);
struct bpf_prog *prog;
- prog = ____bpf_prog_get(f);
+ prog = ____bpf_prog_get(f, mask);
if (IS_ERR(prog))
return prog;
if (!bpf_prog_get_ok(prog, attach_type, attach_drv)) {
@@ -1516,15 +1525,15 @@ static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type,
return prog;
}
-struct bpf_prog *bpf_prog_get(u32 ufd)
+struct bpf_prog *bpf_prog_get(u32 ufd, int mask)
{
- return __bpf_prog_get(ufd, NULL, false);
+ return __bpf_prog_get(ufd, NULL, false, mask);
}
struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
- bool attach_drv)
+ bool attach_drv, int mask)
{
- return __bpf_prog_get(ufd, &type, attach_drv);
+ return __bpf_prog_get(ufd, &type, attach_drv, mask);
}
EXPORT_SYMBOL_GPL(bpf_prog_get_type_dev);
@@ -1707,7 +1716,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
if (err)
goto free_used_maps;
- err = bpf_prog_new_fd(prog);
+ err = bpf_prog_new_fd(prog, O_RDWR /* | O_MAYEXEC */);
if (err < 0) {
/* failed to allocate fd.
* bpf_prog_put() is needed because the above
@@ -1808,7 +1817,7 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
}
raw_tp->btp = btp;
- prog = bpf_prog_get(attr->raw_tracepoint.prog_fd);
+ prog = bpf_prog_get(attr->raw_tracepoint.prog_fd, MAY_EXEC);
if (IS_ERR(prog)) {
err = PTR_ERR(prog);
goto out_free_tp;
@@ -1929,7 +1938,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
return -EINVAL;
}
- prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
+ prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype, MAY_EXEC);
if (IS_ERR(prog))
return PTR_ERR(prog);
@@ -2083,7 +2092,11 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
(!attr->test.ctx_size_out && attr->test.ctx_out))
return -EINVAL;
- prog = bpf_prog_get(attr->test.prog_fd);
+ /*
+ * A test run is is a form of query, so require RW. Using W as a proxy for
+ * X, since X is awkward due to a lack of O_MAYEXEC.
+ */
+ prog = bpf_prog_get(attr->test.prog_fd, MAY_READ | MAY_WRITE);
if (IS_ERR(prog))
return PTR_ERR(prog);
@@ -2147,7 +2160,11 @@ static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
if (IS_ERR(prog))
return PTR_ERR(prog);
- fd = bpf_prog_new_fd(prog);
+ /*
+ * We have all permissions. This is okay, since we also require
+ * CAP_SYS_ADMIN to do this at all.
+ */
+ fd = bpf_prog_new_fd(prog, O_RDWR /* | O_MAYEXEC */);
if (fd < 0)
bpf_prog_put(prog);
@@ -2638,6 +2655,11 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
if (!f.file)
return -EBADFD;
+ if (!(f.file->f_mode & FMODE_READ)) {
+ err = -EACCES;
+ goto out;
+ }
+
if (f.file->f_op == &bpf_prog_fops)
err = bpf_prog_get_info_by_fd(f.file->private_data, attr,
uattr);
@@ -2649,6 +2671,7 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
else
err = -EINVAL;
+out:
fdput(f);
return err;
}
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 026a14541a38..f2e3973b28f2 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8876,7 +8876,8 @@ static int perf_event_set_bpf_handler(struct perf_event *event, u32 prog_fd)
if (event->prog)
return -EEXIST;
- prog = bpf_prog_get_type(prog_fd, BPF_PROG_TYPE_PERF_EVENT);
+ /* Should maybe be FMODE_EXEC? */
+ prog = bpf_prog_get_type(prog_fd, BPF_PROG_TYPE_PERF_EVENT, FMODE_WRITE);
if (IS_ERR(prog))
return PTR_ERR(prog);
@@ -8942,7 +8943,7 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
/* bpf programs can only be attached to u/kprobe or tracepoint */
return -EINVAL;
- prog = bpf_prog_get(prog_fd);
+ prog = bpf_prog_get(prog_fd, FMODE_WRITE);
if (IS_ERR(prog))
return PTR_ERR(prog);
diff --git a/net/core/dev.c b/net/core/dev.c
index fc676b2610e3..3fcaeae693bb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8093,8 +8093,10 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
return -EBUSY;
}
+ /* XXX: FMODE_EXEC? */
prog = bpf_prog_get_type_dev(fd, BPF_PROG_TYPE_XDP,
- bpf_op == ops->ndo_bpf);
+ bpf_op == ops->ndo_bpf,
+ FMODE_WRITE);
if (IS_ERR(prog))
return PTR_ERR(prog);
diff --git a/net/core/filter.c b/net/core/filter.c
index 4e2a79b2fd77..9282462678fd 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1544,7 +1544,8 @@ static struct bpf_prog *__get_bpf(u32 ufd, struct sock *sk)
if (sock_flag(sk, SOCK_FILTER_LOCKED))
return ERR_PTR(-EPERM);
- return bpf_prog_get_type(ufd, BPF_PROG_TYPE_SOCKET_FILTER);
+ /* FMODE_EXEC? */
+ return bpf_prog_get_type(ufd, BPF_PROG_TYPE_SOCKET_FILTER, FMODE_WRITE);
}
int sk_attach_bpf(u32 ufd, struct sock *sk)
@@ -1572,9 +1573,10 @@ int sk_reuseport_attach_bpf(u32 ufd, struct sock *sk)
if (sock_flag(sk, SOCK_FILTER_LOCKED))
return -EPERM;
- prog = bpf_prog_get_type(ufd, BPF_PROG_TYPE_SOCKET_FILTER);
+ prog = bpf_prog_get_type(ufd, BPF_PROG_TYPE_SOCKET_FILTER, FMODE_WRITE);
if (IS_ERR(prog) && PTR_ERR(prog) == -EINVAL)
- prog = bpf_prog_get_type(ufd, BPF_PROG_TYPE_SK_REUSEPORT);
+ prog = bpf_prog_get_type(ufd, BPF_PROG_TYPE_SK_REUSEPORT,
+ FMODE_WRITE);
if (IS_ERR(prog))
return PTR_ERR(prog);
diff --git a/net/netfilter/xt_bpf.c b/net/netfilter/xt_bpf.c
index 13cf3f9b5938..34e5c08ee1f3 100644
--- a/net/netfilter/xt_bpf.c
+++ b/net/netfilter/xt_bpf.c
@@ -44,7 +44,7 @@ static int __bpf_mt_check_fd(int fd, struct bpf_prog **ret)
{
struct bpf_prog *prog;
- prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_SOCKET_FILTER);
+ prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_SOCKET_FILTER, MAY_EXEC);
if (IS_ERR(prog))
return PTR_ERR(prog);
@@ -57,7 +57,8 @@ static int __bpf_mt_check_path(const char *path, struct bpf_prog **ret)
if (strnlen(path, XT_BPF_PATH_MAX) == XT_BPF_PATH_MAX)
return -EINVAL;
- *ret = bpf_prog_get_type_path(path, BPF_PROG_TYPE_SOCKET_FILTER);
+ *ret = bpf_prog_get_type_path(path, BPF_PROG_TYPE_SOCKET_FILTER,
+ MAY_EXEC);
return PTR_ERR_OR_ZERO(*ret);
}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 8d54f3047768..5b8c5e5d94bf 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1563,7 +1563,7 @@ static int fanout_set_data_ebpf(struct packet_sock *po, char __user *data,
if (copy_from_user(&fd, data, len))
return -EFAULT;
- new = bpf_prog_get_type(fd, BPF_PROG_TYPE_SOCKET_FILTER);
+ new = bpf_prog_get_type(fd, BPF_PROG_TYPE_SOCKET_FILTER, FMODE_WRITE);
if (IS_ERR(new))
return PTR_ERR(new);
--
2.21.0
^ permalink raw reply related
* [WIP 4/4] bpf: Allow creating all program types without privilege
From: Andy Lutomirski @ 2019-08-05 21:29 UTC (permalink / raw)
To: LKML, Alexei Starovoitov
Cc: Song Liu, Kees Cook, Networking, bpf, Daniel Borkmann,
Alexei Starovoitov, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
Linux API, LSM List, Andy Lutomirski
In-Reply-To: <cover.1565040372.git.luto@kernel.org>
This doesn't let you *run* the programs except in test mode, so it should
be safe. Famous last words.
This assumes that the check-privilege-to-call-privileged-functions
patch actually catches all the cases and that there's nothing else
that should need privilege lurking in the type-specific verifiers.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
kernel/bpf/syscall.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 23f8f89d2a86..730afa2be786 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1649,8 +1649,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
attr->insn_cnt > (capable(CAP_SYS_ADMIN) ? BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
return -E2BIG;
if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
- type != BPF_PROG_TYPE_CGROUP_SKB &&
- !capable(CAP_SYS_ADMIN))
+ type != BPF_PROG_TYPE_CGROUP_SKB)
return -EPERM;
bpf_prog_load_fixup_attach_type(attr);
--
2.21.0
^ permalink raw reply related
* [WIP 2/4] bpf: Don't require mknod() permission to pin an object
From: Andy Lutomirski @ 2019-08-05 21:29 UTC (permalink / raw)
To: LKML, Alexei Starovoitov
Cc: Song Liu, Kees Cook, Networking, bpf, Daniel Borkmann,
Alexei Starovoitov, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
Linux API, LSM List, Andy Lutomirski
In-Reply-To: <cover.1565040372.git.luto@kernel.org>
security_path_mknod() seems excessive for pinning an object --
pinning an object is effectively just creating a file. It's also
redundant, as vfs_mkobj() calls security_inode_create() by itself.
This isn't strictly required -- mknod(path, S_IFREG, unused) works
to create regular files, but bpf is currently the only user in the
kernel outside of mknod() itself that uses it to create regular
(i.e. S_IFREG) files.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
kernel/bpf/inode.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index cb07736b33ae..14304609003a 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -394,10 +394,6 @@ static int bpf_obj_do_pin(const struct filename *pathname, void *raw,
mode = S_IFREG | ((S_IRUSR | S_IWUSR) & ~current_umask());
- ret = security_path_mknod(&path, dentry, mode, 0);
- if (ret)
- goto out;
-
dir = d_inode(path.dentry);
if (dir->i_op != &bpf_dir_iops) {
ret = -EPERM;
--
2.21.0
^ permalink raw reply related
* [WIP 3/4] bpf: Add a way to mark functions as requiring privilege
From: Andy Lutomirski @ 2019-08-05 21:29 UTC (permalink / raw)
To: LKML, Alexei Starovoitov
Cc: Song Liu, Kees Cook, Networking, bpf, Daniel Borkmann,
Alexei Starovoitov, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
Linux API, LSM List, Andy Lutomirski
In-Reply-To: <cover.1565040372.git.luto@kernel.org>
This is horribly incomplete:
- I only marked one function as requiring privilege, and there are
surely more.
- Checking is_priv is probably not the right thing to do. This should
probably do something more clever. At the very lease, it needs to
integrate with the upcoming lockdown LSM infrastructure.
- The seen_privileged_funcs mechanism is probably not a good solution.
Instead we should check something while we still have enough context
to give a good error message. But we *don't* want to check for
capabilities up front before even seeing a function call, since we
don't want to inadvertently generate audit events for privileges that
are never used.
So it's the idea that counts :)
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
include/linux/bpf.h | 15 +++++++++++++++
include/linux/bpf_verifier.h | 1 +
kernel/bpf/verifier.c | 8 ++++++++
kernel/trace/bpf_trace.c | 1 +
4 files changed, 25 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2d5e1a4dff6c..de31b9888b6c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -229,6 +229,7 @@ struct bpf_func_proto {
u64 (*func)(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
bool gpl_only;
bool pkt_access;
+ u16 privilege;
enum bpf_return_type ret_type;
enum bpf_arg_type arg1_type;
enum bpf_arg_type arg2_type;
@@ -237,6 +238,20 @@ struct bpf_func_proto {
enum bpf_arg_type arg5_type;
};
+/*
+ * Some functions should require privilege to call at all, even in a test
+ * run. These flags indicate why privilege is required. The core BPF
+ * code will verify that the creator of such a program has the requisite
+ * privilege.
+ *
+ * NB: This means that anyone who creates a privileged program (due to
+ * such a call or due to a privilege-requiring pointer-to-integer conversion)
+ * is responsible for restricting access to the program in an appropriate
+ * manner.
+ */
+#define BPF_FUNC_PRIV_READ_KERNEL_MEMORY BIT(0)
+#define BPT_FUNC_PRIV_WRITE_GLOBAL_LOGS BIT(1)
+
/* bpf_context is intentionally undefined structure. Pointer to bpf_context is
* the first argument to eBPF programs.
* For socket filters: 'struct bpf_context *' == 'struct sk_buff *'
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 5fe99f322b1c..9877f5753cf4 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -363,6 +363,7 @@ struct bpf_verifier_env {
u32 id_gen; /* used to generate unique reg IDs */
bool allow_ptr_leaks;
bool seen_direct_write;
+ u16 seen_privileged_funcs;
struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
const struct bpf_line_info *prev_linfo;
struct bpf_verifier_log log;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5900cbb966b1..5e048688fd8d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4129,6 +4129,9 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
if (changes_data)
clear_all_pkt_pointers(env);
+
+ env->seen_privileged_funcs |= fn->privilege;
+
return 0;
}
@@ -9371,6 +9374,11 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
if (ret == 0)
adjust_btf_func(env);
+ if (env->seen_privileged_funcs && !is_priv) {
+ ret = -EPERM;
+ goto err_release_maps;
+ }
+
err_release_maps:
if (!env->prog->aux->used_maps)
/* if we didn't copy map pointers into bpf_prog_info, release
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index ca1255d14576..d9454588d9e8 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -152,6 +152,7 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
static const struct bpf_func_proto bpf_probe_read_proto = {
.func = bpf_probe_read,
.gpl_only = true,
+ .privilege = BPF_FUNC_PRIV_READ_KERNEL_MEMORY,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_UNINIT_MEM,
.arg2_type = ARG_CONST_SIZE_OR_ZERO,
--
2.21.0
^ permalink raw reply related
* [WIP 0/4] bpf: A bit of progress toward unprivileged use
From: Andy Lutomirski @ 2019-08-05 21:29 UTC (permalink / raw)
To: LKML, Alexei Starovoitov
Cc: Song Liu, Kees Cook, Networking, bpf, Daniel Borkmann,
Alexei Starovoitov, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
Linux API, LSM List, Andy Lutomirski
Other than the mknod() patch, this is not ready for prime time. These
patches try to make progress toward making bpf() more useful without
privilege
Andy Lutomirski (4):
bpf: Respect persistent map and prog access modes
bpf: Don't require mknod() permission to pin an object
bpf: Add a way to mark functions as requiring privilege
bpf: Allow creating all program types without privilege
include/linux/bpf.h | 30 +++++++++++++++-----
include/linux/bpf_verifier.h | 1 +
kernel/bpf/arraymap.c | 8 +++++-
kernel/bpf/cgroup.c | 6 +++-
kernel/bpf/inode.c | 29 +++++++++++--------
kernel/bpf/syscall.c | 54 +++++++++++++++++++++++++-----------
kernel/bpf/verifier.c | 8 ++++++
kernel/events/core.c | 5 ++--
kernel/trace/bpf_trace.c | 1 +
net/core/dev.c | 4 ++-
net/core/filter.c | 8 ++++--
net/netfilter/xt_bpf.c | 5 ++--
net/packet/af_packet.c | 2 +-
13 files changed, 115 insertions(+), 46 deletions(-)
--
2.21.0
^ permalink raw reply
* Re: [RFC PATCH v3 04/12] x86/sgx: Require userspace to define enclave pages' protection bits
From: Andy Lutomirski @ 2019-08-05 21:30 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Andy Lutomirski, Sean Christopherson, Schaufler, Casey,
James Morris, linux-sgx, Dave Hansen, Cedric Xing, Jethro Beekman,
Dr . Greg Wettstein, Stephen Smalley, LSM List
In-Reply-To: <20190805205104.6ylxomqq37deqb3v@linux.intel.com>
On Mon, Aug 5, 2019 at 1:51 PM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Sun, Aug 04, 2019 at 03:20:24PM -0700, Andy Lutomirski wrote:
> > On Thu, Aug 1, 2019 at 9:38 AM Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Mon, Jul 15, 2019 at 03:29:23PM -0700, Andy Lutomirski wrote:
> > > > I would say it differently: regardless of exactly how /dev/sgx/enclave
> > > > is wired up under the hood, we want a way that a process can be
> > > > granted permission to usefully run enclaves without being granted
> > > > permission to execute whatever bytes of code it wants. Preferably
> > > > without requiring LSMs to maintain some form of enclave signature
> > > > whitelist.
> > >
> > > Would it be better to have a signer whitelist instead or some
> > > combination? E.g. you could whiteliste either by signer or
> > > enclave signature.
> > >
> >
> > I'm not sure, and also don't really think we need to commit to an
> > answer right now. I do think that the eventual solution should be
> > more flexible than just whitelisting the signers. In particular, it
> > should be possible to make secure enclaves, open-source or otherwise,
> > that are reproducibly buildable. This more or less requires that the
> > signing private key not be a secret, which means that no one would
> > want to whitelist the signing key. The enclave would be trusted, and
> > would seal data, on the basis of its MRENCLAVE, and the policy, if
> > any, would want to whitelist the MRENCLAVE or perhaps the whole
> > SIGSTRUCT.
> >
> > But my overall point is that it should be possible to have a conherent
> > policy that allows any enclave whatsoever to run but that still
> > respects EXECMEM and such.
>
> So could kernel embed a fixed signing key that would be made available
> through sysfs for signing? Already have one for my selftest.
>
Do you mean a public and private key? I was imagining that someone
would just create a key pair and publish it for the case of SGX
programs that don't depend on MRSIGNER. There doesn't have to be just
one.
But I may be misunderstanding you.
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Jarkko Sakkinen @ 2019-08-05 22:11 UTC (permalink / raw)
To: Roberto Sassu
Cc: Tyler Hicks, jejb, zohar, jgg, linux-integrity,
linux-security-module, keyrings, linux-kernel, crazyt2019+lml,
nayna, silviu.vlasceanu
In-Reply-To: <e10f7b04-3d63-435e-180e-72a084ac4bab@huawei.com>
On Mon, Aug 05, 2019 at 04:50:35PM +0200, Roberto Sassu wrote:
> I also don't think it is a good idea to remove this functionality.
>
> Jarkko, we were discussing about this issue in another thread, and your
> answer then (https://lkml.org/lkml/2019/3/21/396) was that it is a
> priority to fix the regression.
OK, I think I'm now in the same page. I guess vacation did the job.
/Jarkko
^ permalink raw reply
* Re: [PATCH v3] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Jarkko Sakkinen @ 2019-08-05 22:12 UTC (permalink / raw)
To: Roberto Sassu
Cc: jejb, zohar, jgg, tyhicks, linux-integrity, linux-security-module,
keyrings, linux-kernel, crazyt2019+lml, nayna, silviu.vlasceanu
In-Reply-To: <20190805164427.17408-1-roberto.sassu@huawei.com>
On Mon, Aug 05, 2019 at 06:44:27PM +0200, Roberto Sassu wrote:
> Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to initialize w/o a
> TPM") allows the trusted module to be loaded even if a TPM is not found, to
> avoid module dependency problems.
>
> However, trusted module initialization can still fail if the TPM is
> inactive or deactivated. tpm_get_random() returns an error.
>
> This patch removes the call to tpm_get_random() and instead extends the PCR
> specified by the user with zeros. The security of this alternative is
> equivalent to the previous one, as either option prevents with a PCR update
> unsealing and misuse of sealed data by a user space process.
>
> Even if a PCR is extended with zeros, instead of random data, it is still
> computationally infeasible to find a value as input for a new PCR extend
> operation, to obtain again the PCR value that would allow unsealing.
>
> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-08-05 22:21 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Song Liu, Kees Cook, Networking, bpf,
Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
Jann Horn, Greg KH, Linux API, LSM List
In-Reply-To: <CALCETrVtPs8gY-H4gmzSqPboid3CB++n50SvYd6RU9YVde_-Ow@mail.gmail.com>
> On Aug 5, 2019, at 2:25 PM, Andy Lutomirski <luto@kernel.org> wrote:
>
> On Mon, Aug 5, 2019 at 12:21 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>
>> What we need is to drop privileges sooner in daemons like systemd.
>
> This is doable right now: systemd could fork off a subprocess and
> delegate its cgroup operations to it. It would be maybe a couple
> hundred lines of code. As an added benefit, that subprocess could
> verify that the bpf operations in question are reasonable.
> Alternatively, if there was a CAP_BPF_ADMIN, systemd could retain that
> capability and flip it on and off as needed.
I tried to look at the code and I couldn’t find it. Does systemd drop privileges at all? Can you point me at the code you’re thinking of
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Alexei Starovoitov @ 2019-08-06 1:11 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Song Liu, Kees Cook, Networking, bpf, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
Linux API, LSM List
In-Reply-To: <CALCETrVtPs8gY-H4gmzSqPboid3CB++n50SvYd6RU9YVde_-Ow@mail.gmail.com>
On Mon, Aug 05, 2019 at 02:25:35PM -0700, Andy Lutomirski wrote:
> It tries to make the kernel respect the access modes for fds. Without
> this patch, there seem to be some holes: nothing looked at program fds
> and, unless I missed something, you could take a readonly fd for a
> program, pin the program, and reopen it RW.
I think it's by design. iirc Daniel had a use case for something like this.
The key to understand is that bpf is not about security. It's about safety.
All features are geared to safety.
The users are trusted most of the time.
The number of unprivileged bpf use cases is tiny compared to trusted.
Hence unprivileged bpf is actually something that can be deprecated.
> Other than the issue that this patch partially fixes, can you see any
> reason that loading a program should require privilege? Obviously the
> verifier is weakened a bit when called by privileged users, but a lot
> of that is about excessive resource usage and various less-well-tested
> features. It seems to me that most of the value of bpf() should be
> available to programs that should not need privilege to load. Are
> there things I'm missing?
see below.
> LPM: I don't see why this requires privilege at all. It indeed checks
> capable(CAP_SYS_ADMIN), but I don't see why.
see below.
>
> >
> > Attaching to a cgroup already has file based permission checks.
> > The user needs to open cgroup directory to attach.
> > acls on cgroup dir can already be used to prevent attaching to
> > certain parts of cgroup hierarchy.
>
> The current checks seem inadequate.
>
> $ echo 'yay' </sys/fs/cgroup/systemd/system.slice/
>
> The ability to obtain an fd to a cgroup does *not* imply any right to
> modify that cgroup. The ability to write to a cgroup directory
> already means something else -- it's the ability to create cgroups
> under the group in question. I'm suggesting that a new API be added
> that allows attaching a bpf program to a cgroup without capabilities
> and that instead requires write access to a new file in the cgroup
> directory. (It could be a single file for all bpf types or one file
> per type. I prefer the latter -- it gives the admin finer-grained
> control.)
This is something to discuss. I don't mind something like this,
but in general bpf is not for untrusted users.
Hence I don't want to overdesign.
>
> > What we need is to drop privileges sooner in daemons like systemd.
>
> This is doable right now: systemd could fork off a subprocess and
> delegate its cgroup operations to it. It would be maybe a couple
> hundred lines of code. As an added benefit, that subprocess could
> verify that the bpf operations in question are reasonable.
> Alternatively, if there was a CAP_BPF_ADMIN, systemd could retain that
> capability and flip it on and off as needed.
See https://github.com/systemd/systemd/blob/01234e1fe777602265ffd25ab6e73823c62b0efe/src/core/bpf-firewall.c#L671-L674
bpf based IP sandboxing doesn't work in 'systemd --user'.
That is just one of the problems that people complained about.
Note that systemd bpf usage is basic. There is ongoing work to
adopt libbpf in systemd, so more features and more use cases will open up.
Inside containers and inside nested containers we need to start processes
that will use bpf. All of the processes are trusted.
We need to drop root not to be secure, but to be safe.
Consider a bug in a code that accidently did sys_kill(-1).
Dropping root is a mitigation for bugs like this.
>
> > Container management daemon runs in the nested containers.
> > These trusted daemons need to have access to full bpf, but they
> > don't want to be root all the time.
> > They cannot flip back and forth via seteuid to root every time they
> > need to do bpf.
> > Hence the idea is to have a file that this daemon can open,
> > then drop privileges and still keep doing bpf things because FD is held.
> > Outer container daemon can pass this /dev/bpf's FD to inner daemon, etc.
> > This /dev/bpf would be accessible to root only.
> > There is no desire to open it up to non-root.
>
> This seems extremely dangerous right now. A program that can bypass
> *all* of the capable() checks in bpf() can do a whole lot. Among
> other things, it can read all of kernel memory.
It's 'dangerous' only if you think about it from security point of view.
The tracing (and sometimes networking) bpf progs need to read all of kernel
memory without being root.
That is the whole point of the /dev/bpf.
> This seems to have most of the same problems. My main point is that
> it conflates a whole lot of different permissions, and I really don't
> think it's that much work to mostly disentangle the permissions in
> question. My little series (if completed) plus a patch to allow
> unprivileged cgroup attach operations if you have an FMODE_WRITE fd to
> an appropriate file should get most of the way there.
I think I understand your concern. One /dev/bpf magic to by-pass
all of capable() checks in bpf doesn't look nice indeed.
Now to answer your question about capable(sys_admin) in the verifier.
See this bugfix:
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=7c2e988f400e83501e0a3568250780609b7c8263
It's a bug in the JIT that was there pretty much forever,
but it got exposed due to two new bpf features.
Initially syzbot complained that bounded loops allow jmp to 1st insn.
syzbot reproducer contained 'never taken' branch, so buggy JIT
wouldn't have caused a kernel panic for this reproducer though
JITed x86 code is broken.
It took me few days to realize that with bpf2bpf calls _and_
bounded loops I can craft a test that will expose this JIT bug
and will crash the kernel.
So a combination of two recent verifier features exposed old JIT bug
that would have been a security issue if we didn't gate these verifier
features for root only.
Now it's simply a kernel bugfix.
Such subtle bugs happen all the time when new verifier features are
introduced. Hence we always start them with root only.
bpf2bpf calls, bounded loops, precision tracking, dead code elimination,
LPM maps, many programs type are root only.
We don't want cve-s to be filed for every bug like this.
Even if we start relaxing features (like dropping root from LPM map)
at any given time there will be a lot of useful verifier features,
maps and program types that are root only.
For root == for trusted users only.
Unfortunately this approach creates adoption problem.
The trusted users don't want to be root to use bpf.
Hence this /dev/bpf.
To solve your concern of bypassing all capable checks...
How about we do /dev/bpf/full_verifier first?
It will replace capable() checks in the verifier only.
How to delegate cgroup attach permission is a follow up discussion.
Could be special files in cgroup dir as you proposed or something else.
Let's table that and focus on the verifier first.
^ permalink raw reply
* Re: [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around
From: Tetsuo Handa @ 2019-08-06 10:43 UTC (permalink / raw)
To: Al Viro
Cc: John Johansen, Eric W. Biederman, David Howells, linux-api,
linux-fsdevel, torvalds, linux-security-module
In-Reply-To: <7eb7378e-2eb8-c1ba-4e1f-ea8f5611f42b@i-love.sakura.ne.jp>
This 5.2-stable/5.3 patch is stalling. Should I ask different route?
On 2019/07/23 22:45, Tetsuo Handa wrote:
> Al, will you send this patch to 5.3-rcX via vfs.git tree?
^ permalink raw reply
* [RFC/RFT v3 0/3] KEYS: trusted: Add generic trusted keys framework
From: Sumit Garg @ 2019-08-06 13:37 UTC (permalink / raw)
To: keyrings, linux-integrity, linux-crypto, linux-security-module
Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
daniel.thompson, linux-kernel, tee-dev, Sumit Garg
This patch-set is an outcome of discussion here [1].
I have tested this framework with trusted keys support provided via TEE
but I wasn't able to test it with a TPM device as I don't possess one. It
would be really helpful if others could test this patch-set using a TPM
device.
[1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg30591.html
Changes in v3:
Move TPM2 trusted keys code to trusted keys subsystem.
Changes in v2:
Split trusted keys abstraction patch for ease of review.
Sumit Garg (3):
KEYS: trusted: create trusted keys subsystem
KEYS: trusted: move tpm2 trusted keys code
KEYS: trusted: Add generic trusted keys framework
crypto/asymmetric_keys/asym_tpm.c | 2 +-
drivers/char/tpm/tpm-interface.c | 56 ---
drivers/char/tpm/tpm.h | 224 ------------
drivers/char/tpm/tpm2-cmd.c | 307 -----------------
include/keys/trusted-type.h | 45 +++
include/keys/{trusted.h => trusted_tpm.h} | 42 +--
include/linux/tpm.h | 264 +++++++++++++-
security/keys/Makefile | 2 +-
security/keys/trusted-keys/Makefile | 8 +
.../keys/{trusted.c => trusted-keys/trusted-tpm.c} | 363 ++++----------------
security/keys/trusted-keys/trusted-tpm2.c | 378 +++++++++++++++++++++
security/keys/trusted-keys/trusted.c | 343 +++++++++++++++++++
12 files changed, 1109 insertions(+), 925 deletions(-)
rename include/keys/{trusted.h => trusted_tpm.h} (72%)
create mode 100644 security/keys/trusted-keys/Makefile
rename security/keys/{trusted.c => trusted-keys/trusted-tpm.c} (76%)
create mode 100644 security/keys/trusted-keys/trusted-tpm2.c
create mode 100644 security/keys/trusted-keys/trusted.c
--
2.7.4
^ permalink raw reply
* [RFC/RFT v3 1/3] KEYS: trusted: create trusted keys subsystem
From: Sumit Garg @ 2019-08-06 13:37 UTC (permalink / raw)
To: keyrings, linux-integrity, linux-crypto, linux-security-module
Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
daniel.thompson, linux-kernel, tee-dev, Sumit Garg
In-Reply-To: <1565098640-12536-1-git-send-email-sumit.garg@linaro.org>
Move existing code to trusted keys subsystem. Also, rename files with
"tpm" as suffix which provides the underlying implementation.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
crypto/asymmetric_keys/asym_tpm.c | 2 +-
include/keys/{trusted.h => trusted_tpm.h} | 4 ++--
security/keys/Makefile | 2 +-
security/keys/trusted-keys/Makefile | 6 ++++++
security/keys/{trusted.c => trusted-keys/trusted-tpm.c} | 2 +-
5 files changed, 11 insertions(+), 5 deletions(-)
rename include/keys/{trusted.h => trusted_tpm.h} (98%)
create mode 100644 security/keys/trusted-keys/Makefile
rename security/keys/{trusted.c => trusted-keys/trusted-tpm.c} (99%)
diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c
index 76d2ce3..ec3f309 100644
--- a/crypto/asymmetric_keys/asym_tpm.c
+++ b/crypto/asymmetric_keys/asym_tpm.c
@@ -13,7 +13,7 @@
#include <crypto/sha.h>
#include <asm/unaligned.h>
#include <keys/asymmetric-subtype.h>
-#include <keys/trusted.h>
+#include <keys/trusted_tpm.h>
#include <crypto/asym_tpm_subtype.h>
#include <crypto/public_key.h>
diff --git a/include/keys/trusted.h b/include/keys/trusted_tpm.h
similarity index 98%
rename from include/keys/trusted.h
rename to include/keys/trusted_tpm.h
index 0071298..7d7b108 100644
--- a/include/keys/trusted.h
+++ b/include/keys/trusted_tpm.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __TRUSTED_KEY_H
-#define __TRUSTED_KEY_H
+#ifndef __TRUSTED_TPM_H
+#define __TRUSTED_TPM_H
/* implementation specific TPM constants */
#define MAX_BUF_SIZE 1024
diff --git a/security/keys/Makefile b/security/keys/Makefile
index 9cef540..074f275 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -28,5 +28,5 @@ obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += keyctl_pkey.o
# Key types
#
obj-$(CONFIG_BIG_KEYS) += big_key.o
-obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
+obj-$(CONFIG_TRUSTED_KEYS) += trusted-keys/
obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
new file mode 100644
index 0000000..ad34d17
--- /dev/null
+++ b/security/keys/trusted-keys/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for trusted keys
+#
+
+obj-$(CONFIG_TRUSTED_KEYS) += trusted-tpm.o
diff --git a/security/keys/trusted.c b/security/keys/trusted-keys/trusted-tpm.c
similarity index 99%
rename from security/keys/trusted.c
rename to security/keys/trusted-keys/trusted-tpm.c
index 9a94672..b7e53a3 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted-keys/trusted-tpm.c
@@ -27,7 +27,7 @@
#include <linux/tpm.h>
#include <linux/tpm_command.h>
-#include <keys/trusted.h>
+#include <keys/trusted_tpm.h>
static const char hmac_alg[] = "hmac(sha1)";
static const char hash_alg[] = "sha1";
--
2.7.4
^ permalink raw reply related
* [RFC/RFT v3 2/3] KEYS: trusted: move tpm2 trusted keys code
From: Sumit Garg @ 2019-08-06 13:37 UTC (permalink / raw)
To: keyrings, linux-integrity, linux-crypto, linux-security-module
Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
daniel.thompson, linux-kernel, tee-dev, Sumit Garg
In-Reply-To: <1565098640-12536-1-git-send-email-sumit.garg@linaro.org>
Move TPM2 trusted keys code to trusted keys subsystem.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
drivers/char/tpm/tpm-interface.c | 56 -----
drivers/char/tpm/tpm.h | 224 ------------------
drivers/char/tpm/tpm2-cmd.c | 307 ------------------------
include/keys/trusted_tpm.h | 23 +-
include/linux/tpm.h | 264 +++++++++++++++++++--
security/keys/trusted-keys/Makefile | 3 +-
security/keys/trusted-keys/trusted-tpm.c | 16 +-
security/keys/trusted-keys/trusted-tpm2.c | 378 ++++++++++++++++++++++++++++++
8 files changed, 652 insertions(+), 619 deletions(-)
create mode 100644 security/keys/trusted-keys/trusted-tpm2.c
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1b4f95c..208e5ba 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -454,62 +454,6 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
}
EXPORT_SYMBOL_GPL(tpm_get_random);
-/**
- * tpm_seal_trusted() - seal a trusted key payload
- * @chip: a &struct tpm_chip instance, %NULL for the default chip
- * @options: authentication values and other options
- * @payload: the key data in clear and encrypted form
- *
- * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
- * the keyring subsystem.
- *
- * Return: same as with tpm_transmit_cmd()
- */
-int tpm_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload,
- struct trusted_key_options *options)
-{
- int rc;
-
- chip = tpm_find_get_ops(chip);
- if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
- return -ENODEV;
-
- rc = tpm2_seal_trusted(chip, payload, options);
-
- tpm_put_ops(chip);
- return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_seal_trusted);
-
-/**
- * tpm_unseal_trusted() - unseal a trusted key
- * @chip: a &struct tpm_chip instance, %NULL for the default chip
- * @options: authentication values and other options
- * @payload: the key data in clear and encrypted form
- *
- * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
- * the keyring subsystem.
- *
- * Return: same as with tpm_transmit_cmd()
- */
-int tpm_unseal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options)
-{
- int rc;
-
- chip = tpm_find_get_ops(chip);
- if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
- return -ENODEV;
-
- rc = tpm2_unseal_trusted(chip, payload, options);
-
- tpm_put_ops(chip);
-
- return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_unseal_trusted);
-
static int __init tpm_init(void)
{
int rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index e503ffc..0705d44 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -25,7 +25,6 @@
#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/tpm.h>
-#include <linux/highmem.h>
#include <linux/tpm_eventlog.h>
#ifdef CONFIG_X86
@@ -58,123 +57,12 @@ enum tpm_addr {
#define TPM_ERR_DISABLED 0x7
#define TPM_ERR_INVALID_POSTINIT 38
-#define TPM_HEADER_SIZE 10
-
-enum tpm2_const {
- TPM2_PLATFORM_PCR = 24,
- TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8),
-};
-
-enum tpm2_timeouts {
- TPM2_TIMEOUT_A = 750,
- TPM2_TIMEOUT_B = 2000,
- TPM2_TIMEOUT_C = 200,
- TPM2_TIMEOUT_D = 30,
- TPM2_DURATION_SHORT = 20,
- TPM2_DURATION_MEDIUM = 750,
- TPM2_DURATION_LONG = 2000,
- TPM2_DURATION_LONG_LONG = 300000,
- TPM2_DURATION_DEFAULT = 120000,
-};
-
-enum tpm2_structures {
- TPM2_ST_NO_SESSIONS = 0x8001,
- TPM2_ST_SESSIONS = 0x8002,
-};
-
-/* Indicates from what layer of the software stack the error comes from */
-#define TSS2_RC_LAYER_SHIFT 16
-#define TSS2_RESMGR_TPM_RC_LAYER (11 << TSS2_RC_LAYER_SHIFT)
-
-enum tpm2_return_codes {
- TPM2_RC_SUCCESS = 0x0000,
- TPM2_RC_HASH = 0x0083, /* RC_FMT1 */
- TPM2_RC_HANDLE = 0x008B,
- TPM2_RC_INITIALIZE = 0x0100, /* RC_VER1 */
- TPM2_RC_FAILURE = 0x0101,
- TPM2_RC_DISABLED = 0x0120,
- TPM2_RC_COMMAND_CODE = 0x0143,
- TPM2_RC_TESTING = 0x090A, /* RC_WARN */
- TPM2_RC_REFERENCE_H0 = 0x0910,
- TPM2_RC_RETRY = 0x0922,
-};
-
-enum tpm2_command_codes {
- TPM2_CC_FIRST = 0x011F,
- TPM2_CC_HIERARCHY_CONTROL = 0x0121,
- TPM2_CC_HIERARCHY_CHANGE_AUTH = 0x0129,
- TPM2_CC_CREATE_PRIMARY = 0x0131,
- TPM2_CC_SEQUENCE_COMPLETE = 0x013E,
- TPM2_CC_SELF_TEST = 0x0143,
- TPM2_CC_STARTUP = 0x0144,
- TPM2_CC_SHUTDOWN = 0x0145,
- TPM2_CC_NV_READ = 0x014E,
- TPM2_CC_CREATE = 0x0153,
- TPM2_CC_LOAD = 0x0157,
- TPM2_CC_SEQUENCE_UPDATE = 0x015C,
- TPM2_CC_UNSEAL = 0x015E,
- TPM2_CC_CONTEXT_LOAD = 0x0161,
- TPM2_CC_CONTEXT_SAVE = 0x0162,
- TPM2_CC_FLUSH_CONTEXT = 0x0165,
- TPM2_CC_VERIFY_SIGNATURE = 0x0177,
- TPM2_CC_GET_CAPABILITY = 0x017A,
- TPM2_CC_GET_RANDOM = 0x017B,
- TPM2_CC_PCR_READ = 0x017E,
- TPM2_CC_PCR_EXTEND = 0x0182,
- TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
- TPM2_CC_HASH_SEQUENCE_START = 0x0186,
- TPM2_CC_CREATE_LOADED = 0x0191,
- TPM2_CC_LAST = 0x0193, /* Spec 1.36 */
-};
-
-enum tpm2_permanent_handles {
- TPM2_RS_PW = 0x40000009,
-};
-
-enum tpm2_capabilities {
- TPM2_CAP_HANDLES = 1,
- TPM2_CAP_COMMANDS = 2,
- TPM2_CAP_PCRS = 5,
- TPM2_CAP_TPM_PROPERTIES = 6,
-};
-
-enum tpm2_properties {
- TPM_PT_TOTAL_COMMANDS = 0x0129,
-};
-
-enum tpm2_startup_types {
- TPM2_SU_CLEAR = 0x0000,
- TPM2_SU_STATE = 0x0001,
-};
-
-enum tpm2_cc_attrs {
- TPM2_CC_ATTR_CHANDLES = 25,
- TPM2_CC_ATTR_RHANDLE = 28,
-};
-
#define TPM_VID_INTEL 0x8086
#define TPM_VID_WINBOND 0x1050
#define TPM_VID_STM 0x104A
-enum tpm_chip_flags {
- TPM_CHIP_FLAG_TPM2 = BIT(1),
- TPM_CHIP_FLAG_IRQ = BIT(2),
- TPM_CHIP_FLAG_VIRTUAL = BIT(3),
- TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
- TPM_CHIP_FLAG_ALWAYS_POWERED = BIT(5),
-};
-
#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
-struct tpm_header {
- __be16 tag;
- __be32 length;
- union {
- __be32 ordinal;
- __be32 return_code;
- };
-} __packed;
-
#define TPM_TAG_RQU_COMMAND 193
struct stclear_flags_t {
@@ -274,102 +162,6 @@ enum tpm_sub_capabilities {
* compiler warnings about stack frame size. */
#define TPM_MAX_RNG_DATA 128
-/* A string buffer type for constructing TPM commands. This is based on the
- * ideas of string buffer code in security/keys/trusted.h but is heap based
- * in order to keep the stack usage minimal.
- */
-
-enum tpm_buf_flags {
- TPM_BUF_OVERFLOW = BIT(0),
-};
-
-struct tpm_buf {
- struct page *data_page;
- unsigned int flags;
- u8 *data;
-};
-
-static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
-{
- struct tpm_header *head = (struct tpm_header *)buf->data;
-
- head->tag = cpu_to_be16(tag);
- head->length = cpu_to_be32(sizeof(*head));
- head->ordinal = cpu_to_be32(ordinal);
-}
-
-static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
-{
- buf->data_page = alloc_page(GFP_HIGHUSER);
- if (!buf->data_page)
- return -ENOMEM;
-
- buf->flags = 0;
- buf->data = kmap(buf->data_page);
- tpm_buf_reset(buf, tag, ordinal);
- return 0;
-}
-
-static inline void tpm_buf_destroy(struct tpm_buf *buf)
-{
- kunmap(buf->data_page);
- __free_page(buf->data_page);
-}
-
-static inline u32 tpm_buf_length(struct tpm_buf *buf)
-{
- struct tpm_header *head = (struct tpm_header *)buf->data;
-
- return be32_to_cpu(head->length);
-}
-
-static inline u16 tpm_buf_tag(struct tpm_buf *buf)
-{
- struct tpm_header *head = (struct tpm_header *)buf->data;
-
- return be16_to_cpu(head->tag);
-}
-
-static inline void tpm_buf_append(struct tpm_buf *buf,
- const unsigned char *new_data,
- unsigned int new_len)
-{
- struct tpm_header *head = (struct tpm_header *)buf->data;
- u32 len = tpm_buf_length(buf);
-
- /* Return silently if overflow has already happened. */
- if (buf->flags & TPM_BUF_OVERFLOW)
- return;
-
- if ((len + new_len) > PAGE_SIZE) {
- WARN(1, "tpm_buf: overflow\n");
- buf->flags |= TPM_BUF_OVERFLOW;
- return;
- }
-
- memcpy(&buf->data[len], new_data, new_len);
- head->length = cpu_to_be32(len + new_len);
-}
-
-static inline void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
-{
- tpm_buf_append(buf, &value, 1);
-}
-
-static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
-{
- __be16 value2 = cpu_to_be16(value);
-
- tpm_buf_append(buf, (u8 *) &value2, 2);
-}
-
-static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
-{
- __be32 value2 = cpu_to_be32(value);
-
- tpm_buf_append(buf, (u8 *) &value2, 4);
-}
-
extern struct class *tpm_class;
extern struct class *tpmrm_class;
extern dev_t tpm_devt;
@@ -378,8 +170,6 @@ extern const struct file_operations tpmrm_fops;
extern struct idr dev_nums_idr;
ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
- size_t min_rsp_body_length, const char *desc);
int tpm_get_timeouts(struct tpm_chip *);
int tpm_auto_startup(struct tpm_chip *chip);
@@ -406,9 +196,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
int tpm_chip_start(struct tpm_chip *chip);
void tpm_chip_stop(struct tpm_chip *chip);
-struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
__must_check int tpm_try_get_ops(struct tpm_chip *chip);
-void tpm_put_ops(struct tpm_chip *chip);
struct tpm_chip *tpm_chip_alloc(struct device *dev,
const struct tpm_class_ops *ops);
@@ -428,24 +216,12 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
}
#endif
-static inline u32 tpm2_rc_value(u32 rc)
-{
- return (rc & BIT(7)) ? rc & 0xff : rc;
-}
-
int tpm2_get_timeouts(struct tpm_chip *chip);
int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest, u16 *digest_size_ptr);
int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digests);
int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
-void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
-int tpm2_seal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options);
-int tpm2_unseal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options);
ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
u32 *value, const char *desc);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index d103545..0a319e1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -13,20 +13,6 @@
#include "tpm.h"
#include <crypto/hash_info.h>
-#include <keys/trusted-type.h>
-
-enum tpm2_object_attributes {
- TPM2_OA_USER_WITH_AUTH = BIT(6),
-};
-
-enum tpm2_session_attributes {
- TPM2_SA_CONTINUE_SESSION = BIT(0),
-};
-
-struct tpm2_hash {
- unsigned int crypto_id;
- unsigned int tpm_id;
-};
static struct tpm2_hash tpm2_hash_map[] = {
{HASH_ALGO_SHA1, TPM_ALG_SHA1},
@@ -377,299 +363,6 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
tpm_buf_destroy(&buf);
}
-/**
- * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
- *
- * @buf: an allocated tpm_buf instance
- * @session_handle: session handle
- * @nonce: the session nonce, may be NULL if not used
- * @nonce_len: the session nonce length, may be 0 if not used
- * @attributes: the session attributes
- * @hmac: the session HMAC or password, may be NULL if not used
- * @hmac_len: the session HMAC or password length, maybe 0 if not used
- */
-static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
- const u8 *nonce, u16 nonce_len,
- u8 attributes,
- const u8 *hmac, u16 hmac_len)
-{
- tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
- tpm_buf_append_u32(buf, session_handle);
- tpm_buf_append_u16(buf, nonce_len);
-
- if (nonce && nonce_len)
- tpm_buf_append(buf, nonce, nonce_len);
-
- tpm_buf_append_u8(buf, attributes);
- tpm_buf_append_u16(buf, hmac_len);
-
- if (hmac && hmac_len)
- tpm_buf_append(buf, hmac, hmac_len);
-}
-
-/**
- * tpm2_seal_trusted() - seal the payload of a trusted key
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- *
- * Return: < 0 on error and 0 on success.
- */
-int tpm2_seal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options)
-{
- unsigned int blob_len;
- struct tpm_buf buf;
- u32 hash;
- int i;
- int rc;
-
- for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
- if (options->hash == tpm2_hash_map[i].crypto_id) {
- hash = tpm2_hash_map[i].tpm_id;
- break;
- }
- }
-
- if (i == ARRAY_SIZE(tpm2_hash_map))
- return -EINVAL;
-
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
- if (rc)
- return rc;
-
- tpm_buf_append_u32(&buf, options->keyhandle);
- tpm2_buf_append_auth(&buf, TPM2_RS_PW,
- NULL /* nonce */, 0,
- 0 /* session_attributes */,
- options->keyauth /* hmac */,
- TPM_DIGEST_SIZE);
-
- /* sensitive */
- tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
-
- tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
- tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
- tpm_buf_append_u16(&buf, payload->key_len + 1);
- tpm_buf_append(&buf, payload->key, payload->key_len);
- tpm_buf_append_u8(&buf, payload->migratable);
-
- /* public */
- tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
- tpm_buf_append_u16(&buf, TPM_ALG_KEYEDHASH);
- tpm_buf_append_u16(&buf, hash);
-
- /* policy */
- if (options->policydigest_len) {
- tpm_buf_append_u32(&buf, 0);
- tpm_buf_append_u16(&buf, options->policydigest_len);
- tpm_buf_append(&buf, options->policydigest,
- options->policydigest_len);
- } else {
- tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
- tpm_buf_append_u16(&buf, 0);
- }
-
- /* public parameters */
- tpm_buf_append_u16(&buf, TPM_ALG_NULL);
- tpm_buf_append_u16(&buf, 0);
-
- /* outside info */
- tpm_buf_append_u16(&buf, 0);
-
- /* creation PCR */
- tpm_buf_append_u32(&buf, 0);
-
- if (buf.flags & TPM_BUF_OVERFLOW) {
- rc = -E2BIG;
- goto out;
- }
-
- rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
- if (rc)
- goto out;
-
- blob_len = be32_to_cpup((__be32 *) &buf.data[TPM_HEADER_SIZE]);
- if (blob_len > MAX_BLOB_SIZE) {
- rc = -E2BIG;
- goto out;
- }
- if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
- rc = -EFAULT;
- goto out;
- }
-
- memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
- payload->blob_len = blob_len;
-
-out:
- tpm_buf_destroy(&buf);
-
- if (rc > 0) {
- if (tpm2_rc_value(rc) == TPM2_RC_HASH)
- rc = -EINVAL;
- else
- rc = -EPERM;
- }
-
- return rc;
-}
-
-/**
- * tpm2_load_cmd() - execute a TPM2_Load command
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: returned blob handle
- *
- * Return: 0 on success.
- * -E2BIG on wrong payload size.
- * -EPERM on tpm error status.
- * < 0 error from tpm_transmit_cmd.
- */
-static int tpm2_load_cmd(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options,
- u32 *blob_handle)
-{
- struct tpm_buf buf;
- unsigned int private_len;
- unsigned int public_len;
- unsigned int blob_len;
- int rc;
-
- private_len = be16_to_cpup((__be16 *) &payload->blob[0]);
- if (private_len > (payload->blob_len - 2))
- return -E2BIG;
-
- public_len = be16_to_cpup((__be16 *) &payload->blob[2 + private_len]);
- blob_len = private_len + public_len + 4;
- if (blob_len > payload->blob_len)
- return -E2BIG;
-
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
- if (rc)
- return rc;
-
- tpm_buf_append_u32(&buf, options->keyhandle);
- tpm2_buf_append_auth(&buf, TPM2_RS_PW,
- NULL /* nonce */, 0,
- 0 /* session_attributes */,
- options->keyauth /* hmac */,
- TPM_DIGEST_SIZE);
-
- tpm_buf_append(&buf, payload->blob, blob_len);
-
- if (buf.flags & TPM_BUF_OVERFLOW) {
- rc = -E2BIG;
- goto out;
- }
-
- rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
- if (!rc)
- *blob_handle = be32_to_cpup(
- (__be32 *) &buf.data[TPM_HEADER_SIZE]);
-
-out:
- tpm_buf_destroy(&buf);
-
- if (rc > 0)
- rc = -EPERM;
-
- return rc;
-}
-
-/**
- * tpm2_unseal_cmd() - execute a TPM2_Unload command
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: blob handle
- *
- * Return: 0 on success
- * -EPERM on tpm error status
- * < 0 error from tpm_transmit_cmd
- */
-static int tpm2_unseal_cmd(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options,
- u32 blob_handle)
-{
- struct tpm_buf buf;
- u16 data_len;
- u8 *data;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
- if (rc)
- return rc;
-
- tpm_buf_append_u32(&buf, blob_handle);
- tpm2_buf_append_auth(&buf,
- options->policyhandle ?
- options->policyhandle : TPM2_RS_PW,
- NULL /* nonce */, 0,
- TPM2_SA_CONTINUE_SESSION,
- options->blobauth /* hmac */,
- TPM_DIGEST_SIZE);
-
- rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
- if (rc > 0)
- rc = -EPERM;
-
- if (!rc) {
- data_len = be16_to_cpup(
- (__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
- if (data_len < MIN_KEY_SIZE || data_len > MAX_KEY_SIZE + 1) {
- rc = -EFAULT;
- goto out;
- }
-
- if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
- rc = -EFAULT;
- goto out;
- }
- data = &buf.data[TPM_HEADER_SIZE + 6];
-
- memcpy(payload->key, data, data_len - 1);
- payload->key_len = data_len - 1;
- payload->migratable = data[data_len - 1];
- }
-
-out:
- tpm_buf_destroy(&buf);
- return rc;
-}
-
-/**
- * tpm2_unseal_trusted() - unseal the payload of a trusted key
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- *
- * Return: Same as with tpm_transmit_cmd.
- */
-int tpm2_unseal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options)
-{
- u32 blob_handle;
- int rc;
-
- rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
- if (rc)
- return rc;
-
- rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
- tpm2_flush_context(chip, blob_handle);
- return rc;
-}
-
struct tpm2_get_cap_out {
u8 more_data;
__be32 subcap_id;
diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
index 7d7b108..77a2b5c 100644
--- a/include/keys/trusted_tpm.h
+++ b/include/keys/trusted_tpm.h
@@ -17,7 +17,7 @@
#define LOAD32N(buffer, offset) (*(uint32_t *)&buffer[offset])
#define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset]))
-struct tpm_buf {
+struct tpm1_buf {
int len;
unsigned char data[MAX_BUF_SIZE];
};
@@ -46,7 +46,14 @@ int TSS_checkhmac1(unsigned char *buffer,
unsigned int keylen, ...);
int trusted_tpm_send(unsigned char *cmd, size_t buflen);
-int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce);
+int oiap(struct tpm1_buf *tb, uint32_t *handle, unsigned char *nonce);
+
+int tpm_seal_trusted(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options);
+int tpm_unseal_trusted(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options);
#define TPM_DEBUG 0
@@ -84,7 +91,7 @@ static inline void dump_sess(struct osapsess *s)
16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
}
-static inline void dump_tpm_buf(unsigned char *buf)
+static inline void dump_tpm1_buf(unsigned char *buf)
{
int len;
@@ -105,29 +112,29 @@ static inline void dump_sess(struct osapsess *s)
{
}
-static inline void dump_tpm_buf(unsigned char *buf)
+static inline void dump_tpm1_buf(unsigned char *buf)
{
}
#endif
-static inline void store8(struct tpm_buf *buf, const unsigned char value)
+static inline void store8(struct tpm1_buf *buf, const unsigned char value)
{
buf->data[buf->len++] = value;
}
-static inline void store16(struct tpm_buf *buf, const uint16_t value)
+static inline void store16(struct tpm1_buf *buf, const uint16_t value)
{
*(uint16_t *) & buf->data[buf->len] = htons(value);
buf->len += sizeof value;
}
-static inline void store32(struct tpm_buf *buf, const uint32_t value)
+static inline void store32(struct tpm1_buf *buf, const uint32_t value)
{
*(uint32_t *) & buf->data[buf->len] = htonl(value);
buf->len += sizeof value;
}
-static inline void storebytes(struct tpm_buf *buf, const unsigned char *in,
+static inline void storebytes(struct tpm1_buf *buf, const unsigned char *in,
const int len)
{
memcpy(buf->data + buf->len, in, len);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 53c0ea9..5c43b41 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -21,6 +21,7 @@
#include <linux/acpi.h>
#include <linux/cdev.h>
#include <linux/fs.h>
+#include <linux/highmem.h>
#include <crypto/hash_info.h>
#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
@@ -161,6 +162,231 @@ struct tpm_chip {
int locality;
};
+#define TPM_HEADER_SIZE 10
+
+struct tpm_header {
+ __be16 tag;
+ __be32 length;
+ union {
+ __be32 ordinal;
+ __be32 return_code;
+ };
+} __packed;
+
+enum tpm2_const {
+ TPM2_PLATFORM_PCR = 24,
+ TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8),
+};
+
+enum tpm2_timeouts {
+ TPM2_TIMEOUT_A = 750,
+ TPM2_TIMEOUT_B = 2000,
+ TPM2_TIMEOUT_C = 200,
+ TPM2_TIMEOUT_D = 30,
+ TPM2_DURATION_SHORT = 20,
+ TPM2_DURATION_MEDIUM = 750,
+ TPM2_DURATION_LONG = 2000,
+ TPM2_DURATION_LONG_LONG = 300000,
+ TPM2_DURATION_DEFAULT = 120000,
+};
+
+enum tpm2_structures {
+ TPM2_ST_NO_SESSIONS = 0x8001,
+ TPM2_ST_SESSIONS = 0x8002,
+};
+
+/* Indicates from what layer of the software stack the error comes from */
+#define TSS2_RC_LAYER_SHIFT 16
+#define TSS2_RESMGR_TPM_RC_LAYER (11 << TSS2_RC_LAYER_SHIFT)
+
+enum tpm2_return_codes {
+ TPM2_RC_SUCCESS = 0x0000,
+ TPM2_RC_HASH = 0x0083, /* RC_FMT1 */
+ TPM2_RC_HANDLE = 0x008B,
+ TPM2_RC_INITIALIZE = 0x0100, /* RC_VER1 */
+ TPM2_RC_FAILURE = 0x0101,
+ TPM2_RC_DISABLED = 0x0120,
+ TPM2_RC_COMMAND_CODE = 0x0143,
+ TPM2_RC_TESTING = 0x090A, /* RC_WARN */
+ TPM2_RC_REFERENCE_H0 = 0x0910,
+ TPM2_RC_RETRY = 0x0922,
+};
+
+enum tpm2_command_codes {
+ TPM2_CC_FIRST = 0x011F,
+ TPM2_CC_HIERARCHY_CONTROL = 0x0121,
+ TPM2_CC_HIERARCHY_CHANGE_AUTH = 0x0129,
+ TPM2_CC_CREATE_PRIMARY = 0x0131,
+ TPM2_CC_SEQUENCE_COMPLETE = 0x013E,
+ TPM2_CC_SELF_TEST = 0x0143,
+ TPM2_CC_STARTUP = 0x0144,
+ TPM2_CC_SHUTDOWN = 0x0145,
+ TPM2_CC_NV_READ = 0x014E,
+ TPM2_CC_CREATE = 0x0153,
+ TPM2_CC_LOAD = 0x0157,
+ TPM2_CC_SEQUENCE_UPDATE = 0x015C,
+ TPM2_CC_UNSEAL = 0x015E,
+ TPM2_CC_CONTEXT_LOAD = 0x0161,
+ TPM2_CC_CONTEXT_SAVE = 0x0162,
+ TPM2_CC_FLUSH_CONTEXT = 0x0165,
+ TPM2_CC_VERIFY_SIGNATURE = 0x0177,
+ TPM2_CC_GET_CAPABILITY = 0x017A,
+ TPM2_CC_GET_RANDOM = 0x017B,
+ TPM2_CC_PCR_READ = 0x017E,
+ TPM2_CC_PCR_EXTEND = 0x0182,
+ TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
+ TPM2_CC_HASH_SEQUENCE_START = 0x0186,
+ TPM2_CC_CREATE_LOADED = 0x0191,
+ TPM2_CC_LAST = 0x0193, /* Spec 1.36 */
+};
+
+enum tpm2_permanent_handles {
+ TPM2_RS_PW = 0x40000009,
+};
+
+enum tpm2_capabilities {
+ TPM2_CAP_HANDLES = 1,
+ TPM2_CAP_COMMANDS = 2,
+ TPM2_CAP_PCRS = 5,
+ TPM2_CAP_TPM_PROPERTIES = 6,
+};
+
+enum tpm2_properties {
+ TPM_PT_TOTAL_COMMANDS = 0x0129,
+};
+
+enum tpm2_startup_types {
+ TPM2_SU_CLEAR = 0x0000,
+ TPM2_SU_STATE = 0x0001,
+};
+
+enum tpm2_cc_attrs {
+ TPM2_CC_ATTR_CHANDLES = 25,
+ TPM2_CC_ATTR_RHANDLE = 28,
+};
+
+enum tpm_chip_flags {
+ TPM_CHIP_FLAG_TPM2 = BIT(1),
+ TPM_CHIP_FLAG_IRQ = BIT(2),
+ TPM_CHIP_FLAG_VIRTUAL = BIT(3),
+ TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
+ TPM_CHIP_FLAG_ALWAYS_POWERED = BIT(5),
+};
+
+enum tpm2_object_attributes {
+ TPM2_OA_USER_WITH_AUTH = BIT(6),
+};
+
+enum tpm2_session_attributes {
+ TPM2_SA_CONTINUE_SESSION = BIT(0),
+};
+
+struct tpm2_hash {
+ unsigned int crypto_id;
+ unsigned int tpm_id;
+};
+
+/* A string buffer type for constructing TPM commands. This is based on the
+ * ideas of string buffer code in security/keys/trusted_tpm.h but is heap based
+ * in order to keep the stack usage minimal.
+ */
+
+enum tpm_buf_flags {
+ TPM_BUF_OVERFLOW = BIT(0),
+};
+
+struct tpm_buf {
+ struct page *data_page;
+ unsigned int flags;
+ u8 *data;
+};
+
+static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+ struct tpm_header *head = (struct tpm_header *)buf->data;
+
+ head->tag = cpu_to_be16(tag);
+ head->length = cpu_to_be32(sizeof(*head));
+ head->ordinal = cpu_to_be32(ordinal);
+}
+
+static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+ buf->data_page = alloc_page(GFP_HIGHUSER);
+ if (!buf->data_page)
+ return -ENOMEM;
+
+ buf->flags = 0;
+ buf->data = kmap(buf->data_page);
+ tpm_buf_reset(buf, tag, ordinal);
+ return 0;
+}
+
+static inline void tpm_buf_destroy(struct tpm_buf *buf)
+{
+ kunmap(buf->data_page);
+ __free_page(buf->data_page);
+}
+
+static inline u32 tpm_buf_length(struct tpm_buf *buf)
+{
+ struct tpm_header *head = (struct tpm_header *)buf->data;
+
+ return be32_to_cpu(head->length);
+}
+
+static inline u16 tpm_buf_tag(struct tpm_buf *buf)
+{
+ struct tpm_header *head = (struct tpm_header *)buf->data;
+
+ return be16_to_cpu(head->tag);
+}
+
+static inline void tpm_buf_append(struct tpm_buf *buf,
+ const unsigned char *new_data,
+ unsigned int new_len)
+{
+ struct tpm_header *head = (struct tpm_header *)buf->data;
+ u32 len = tpm_buf_length(buf);
+
+ /* Return silently if overflow has already happened. */
+ if (buf->flags & TPM_BUF_OVERFLOW)
+ return;
+
+ if ((len + new_len) > PAGE_SIZE) {
+ WARN(1, "tpm_buf: overflow\n");
+ buf->flags |= TPM_BUF_OVERFLOW;
+ return;
+ }
+
+ memcpy(&buf->data[len], new_data, new_len);
+ head->length = cpu_to_be32(len + new_len);
+}
+
+static inline void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
+{
+ tpm_buf_append(buf, &value, 1);
+}
+
+static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
+{
+ __be16 value2 = cpu_to_be16(value);
+
+ tpm_buf_append(buf, (u8 *) &value2, 2);
+}
+
+static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
+{
+ __be32 value2 = cpu_to_be32(value);
+
+ tpm_buf_append(buf, (u8 *) &value2, 4);
+}
+
+static inline u32 tpm2_rc_value(u32 rc)
+{
+ return (rc & BIT(7)) ? rc & 0xff : rc;
+}
+
#if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
extern int tpm_is_tpm2(struct tpm_chip *chip);
@@ -170,13 +396,12 @@ extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digests);
extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
-extern int tpm_seal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options);
-extern int tpm_unseal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options);
extern struct tpm_chip *tpm_default_chip(void);
+extern struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
+extern void tpm_put_ops(struct tpm_chip *chip);
+extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
+ size_t min_rsp_body_length, const char *desc);
+extern void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
#else
static inline int tpm_is_tpm2(struct tpm_chip *chip)
{
@@ -204,21 +429,30 @@ static inline int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max)
return -ENODEV;
}
-static inline int tpm_seal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options)
+static inline struct tpm_chip *tpm_default_chip(void)
{
- return -ENODEV;
+ return NULL;
}
-static inline int tpm_unseal_trusted(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options)
+
+static inline struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip)
+{
+ return NULL;
+}
+
+static inline void tpm_put_ops(struct tpm_chip *chip)
+{
+}
+
+static inline ssize_t tpm_transmit_cmd(struct tpm_chip *chip,
+ struct tpm_buf *buf,
+ size_t min_rsp_body_length,
+ const char *desc)
{
return -ENODEV;
}
-static inline struct tpm_chip *tpm_default_chip(void)
+
+static inline void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
{
- return NULL;
}
#endif
#endif
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index ad34d17..590a694 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -3,4 +3,5 @@
# Makefile for trusted keys
#
-obj-$(CONFIG_TRUSTED_KEYS) += trusted-tpm.o
+obj-$(CONFIG_TRUSTED_KEYS) += trusted-tpm.o \
+ trusted-tpm2.o
diff --git a/security/keys/trusted-keys/trusted-tpm.c b/security/keys/trusted-keys/trusted-tpm.c
index b7e53a3..d09893b 100644
--- a/security/keys/trusted-keys/trusted-tpm.c
+++ b/security/keys/trusted-keys/trusted-tpm.c
@@ -368,9 +368,9 @@ int trusted_tpm_send(unsigned char *cmd, size_t buflen)
if (!chip)
return -ENODEV;
- dump_tpm_buf(cmd);
+ dump_tpm1_buf(cmd);
rc = tpm_send(chip, cmd, buflen);
- dump_tpm_buf(cmd);
+ dump_tpm1_buf(cmd);
if (rc > 0)
/* Can't return positive return codes values to keyctl */
rc = -EPERM;
@@ -395,7 +395,7 @@ static int pcrlock(const int pcrnum)
/*
* Create an object specific authorisation protocol (OSAP) session
*/
-static int osap(struct tpm_buf *tb, struct osapsess *s,
+static int osap(struct tpm1_buf *tb, struct osapsess *s,
const unsigned char *key, uint16_t type, uint32_t handle)
{
unsigned char enonce[TPM_NONCE_SIZE];
@@ -430,7 +430,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
/*
* Create an object independent authorisation protocol (oiap) session
*/
-int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
+int oiap(struct tpm1_buf *tb, uint32_t *handle, unsigned char *nonce)
{
int ret;
@@ -464,7 +464,7 @@ struct tpm_digests {
* Have the TPM seal(encrypt) the trusted key, possibly based on
* Platform Configuration Registers (PCRs). AUTH1 for sealing key.
*/
-static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
+static int tpm_seal(struct tpm1_buf *tb, uint16_t keytype,
uint32_t keyhandle, const unsigned char *keyauth,
const unsigned char *data, uint32_t datalen,
unsigned char *blob, uint32_t *bloblen,
@@ -579,7 +579,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
/*
* use the AUTH2_COMMAND form of unseal, to authorize both key and blob
*/
-static int tpm_unseal(struct tpm_buf *tb,
+static int tpm_unseal(struct tpm1_buf *tb,
uint32_t keyhandle, const unsigned char *keyauth,
const unsigned char *blob, int bloblen,
const unsigned char *blobauth,
@@ -670,7 +670,7 @@ static int tpm_unseal(struct tpm_buf *tb,
static int key_seal(struct trusted_key_payload *p,
struct trusted_key_options *o)
{
- struct tpm_buf *tb;
+ struct tpm1_buf *tb;
int ret;
tb = kzalloc(sizeof *tb, GFP_KERNEL);
@@ -696,7 +696,7 @@ static int key_seal(struct trusted_key_payload *p,
static int key_unseal(struct trusted_key_payload *p,
struct trusted_key_options *o)
{
- struct tpm_buf *tb;
+ struct tpm1_buf *tb;
int ret;
tb = kzalloc(sizeof *tb, GFP_KERNEL);
diff --git a/security/keys/trusted-keys/trusted-tpm2.c b/security/keys/trusted-keys/trusted-tpm2.c
new file mode 100644
index 0000000..98892ed7
--- /dev/null
+++ b/security/keys/trusted-keys/trusted-tpm2.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * Trusted Keys code for TCG/TCPA TPM2 (trusted platform module).
+ */
+
+#include <linux/string.h>
+#include <linux/err.h>
+#include <linux/tpm.h>
+#include <linux/tpm_command.h>
+
+#include <keys/trusted-type.h>
+#include <keys/trusted_tpm.h>
+
+static struct tpm2_hash tpm2_hash_map[] = {
+ {HASH_ALGO_SHA1, TPM_ALG_SHA1},
+ {HASH_ALGO_SHA256, TPM_ALG_SHA256},
+ {HASH_ALGO_SHA384, TPM_ALG_SHA384},
+ {HASH_ALGO_SHA512, TPM_ALG_SHA512},
+ {HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+};
+
+/**
+ * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
+ *
+ * @buf: an allocated tpm_buf instance
+ * @session_handle: session handle
+ * @nonce: the session nonce, may be NULL if not used
+ * @nonce_len: the session nonce length, may be 0 if not used
+ * @attributes: the session attributes
+ * @hmac: the session HMAC or password, may be NULL if not used
+ * @hmac_len: the session HMAC or password length, maybe 0 if not used
+ */
+static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
+ const u8 *nonce, u16 nonce_len,
+ u8 attributes,
+ const u8 *hmac, u16 hmac_len)
+{
+ tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
+ tpm_buf_append_u32(buf, session_handle);
+ tpm_buf_append_u16(buf, nonce_len);
+
+ if (nonce && nonce_len)
+ tpm_buf_append(buf, nonce, nonce_len);
+
+ tpm_buf_append_u8(buf, attributes);
+ tpm_buf_append_u16(buf, hmac_len);
+
+ if (hmac && hmac_len)
+ tpm_buf_append(buf, hmac, hmac_len);
+}
+
+/**
+ * tpm2_seal_trusted() - seal the payload of a trusted key
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ *
+ * Return: < 0 on error and 0 on success.
+ */
+int tpm2_seal_trusted(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options)
+{
+ unsigned int blob_len;
+ struct tpm_buf buf;
+ u32 hash;
+ int i;
+ int rc;
+
+ for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+ if (options->hash == tpm2_hash_map[i].crypto_id) {
+ hash = tpm2_hash_map[i].tpm_id;
+ break;
+ }
+ }
+
+ if (i == ARRAY_SIZE(tpm2_hash_map))
+ return -EINVAL;
+
+ rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(&buf, options->keyhandle);
+ tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+ NULL /* nonce */, 0,
+ 0 /* session_attributes */,
+ options->keyauth /* hmac */,
+ TPM_DIGEST_SIZE);
+
+ /* sensitive */
+ tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
+
+ tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
+ tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
+ tpm_buf_append_u16(&buf, payload->key_len + 1);
+ tpm_buf_append(&buf, payload->key, payload->key_len);
+ tpm_buf_append_u8(&buf, payload->migratable);
+
+ /* public */
+ tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
+ tpm_buf_append_u16(&buf, TPM_ALG_KEYEDHASH);
+ tpm_buf_append_u16(&buf, hash);
+
+ /* policy */
+ if (options->policydigest_len) {
+ tpm_buf_append_u32(&buf, 0);
+ tpm_buf_append_u16(&buf, options->policydigest_len);
+ tpm_buf_append(&buf, options->policydigest,
+ options->policydigest_len);
+ } else {
+ tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
+ tpm_buf_append_u16(&buf, 0);
+ }
+
+ /* public parameters */
+ tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+ tpm_buf_append_u16(&buf, 0);
+
+ /* outside info */
+ tpm_buf_append_u16(&buf, 0);
+
+ /* creation PCR */
+ tpm_buf_append_u32(&buf, 0);
+
+ if (buf.flags & TPM_BUF_OVERFLOW) {
+ rc = -E2BIG;
+ goto out;
+ }
+
+ rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
+ if (rc)
+ goto out;
+
+ blob_len = be32_to_cpup((__be32 *) &buf.data[TPM_HEADER_SIZE]);
+ if (blob_len > MAX_BLOB_SIZE) {
+ rc = -E2BIG;
+ goto out;
+ }
+ if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
+ rc = -EFAULT;
+ goto out;
+ }
+
+ memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
+ payload->blob_len = blob_len;
+
+out:
+ tpm_buf_destroy(&buf);
+
+ if (rc > 0) {
+ if (tpm2_rc_value(rc) == TPM2_RC_HASH)
+ rc = -EINVAL;
+ else
+ rc = -EPERM;
+ }
+
+ return rc;
+}
+
+/**
+ * tpm_seal_trusted() - seal a trusted key payload
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
+ * @options: authentication values and other options
+ * @payload: the key data in clear and encrypted form
+ *
+ * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
+ * the keyring subsystem.
+ *
+ * Return: same as with tpm_transmit_cmd()
+ */
+int tpm_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload,
+ struct trusted_key_options *options)
+{
+ int rc;
+
+ chip = tpm_find_get_ops(chip);
+ if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+ return -ENODEV;
+
+ rc = tpm2_seal_trusted(chip, payload, options);
+
+ tpm_put_ops(chip);
+ return rc;
+}
+
+/**
+ * tpm2_load_cmd() - execute a TPM2_Load command
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ * @blob_handle: returned blob handle
+ *
+ * Return: 0 on success.
+ * -E2BIG on wrong payload size.
+ * -EPERM on tpm error status.
+ * < 0 error from tpm_transmit_cmd.
+ */
+static int tpm2_load_cmd(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options,
+ u32 *blob_handle)
+{
+ struct tpm_buf buf;
+ unsigned int private_len;
+ unsigned int public_len;
+ unsigned int blob_len;
+ int rc;
+
+ private_len = be16_to_cpup((__be16 *) &payload->blob[0]);
+ if (private_len > (payload->blob_len - 2))
+ return -E2BIG;
+
+ public_len = be16_to_cpup((__be16 *) &payload->blob[2 + private_len]);
+ blob_len = private_len + public_len + 4;
+ if (blob_len > payload->blob_len)
+ return -E2BIG;
+
+ rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(&buf, options->keyhandle);
+ tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+ NULL /* nonce */, 0,
+ 0 /* session_attributes */,
+ options->keyauth /* hmac */,
+ TPM_DIGEST_SIZE);
+
+ tpm_buf_append(&buf, payload->blob, blob_len);
+
+ if (buf.flags & TPM_BUF_OVERFLOW) {
+ rc = -E2BIG;
+ goto out;
+ }
+
+ rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
+ if (!rc)
+ *blob_handle = be32_to_cpup(
+ (__be32 *) &buf.data[TPM_HEADER_SIZE]);
+
+out:
+ tpm_buf_destroy(&buf);
+
+ if (rc > 0)
+ rc = -EPERM;
+
+ return rc;
+}
+
+/**
+ * tpm2_unseal_cmd() - execute a TPM2_Unload command
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ * @blob_handle: blob handle
+ *
+ * Return: 0 on success
+ * -EPERM on tpm error status
+ * < 0 error from tpm_transmit_cmd
+ */
+static int tpm2_unseal_cmd(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options,
+ u32 blob_handle)
+{
+ struct tpm_buf buf;
+ u16 data_len;
+ u8 *data;
+ int rc;
+
+ rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(&buf, blob_handle);
+ tpm2_buf_append_auth(&buf,
+ options->policyhandle ?
+ options->policyhandle : TPM2_RS_PW,
+ NULL /* nonce */, 0,
+ TPM2_SA_CONTINUE_SESSION,
+ options->blobauth /* hmac */,
+ TPM_DIGEST_SIZE);
+
+ rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
+ if (rc > 0)
+ rc = -EPERM;
+
+ if (!rc) {
+ data_len = be16_to_cpup(
+ (__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
+ if (data_len < MIN_KEY_SIZE || data_len > MAX_KEY_SIZE + 1) {
+ rc = -EFAULT;
+ goto out;
+ }
+
+ if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
+ rc = -EFAULT;
+ goto out;
+ }
+ data = &buf.data[TPM_HEADER_SIZE + 6];
+
+ memcpy(payload->key, data, data_len - 1);
+ payload->key_len = data_len - 1;
+ payload->migratable = data[data_len - 1];
+ }
+
+out:
+ tpm_buf_destroy(&buf);
+ return rc;
+}
+
+/**
+ * tpm2_unseal_trusted() - unseal the payload of a trusted key
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ *
+ * Return: Same as with tpm_transmit_cmd.
+ */
+int tpm2_unseal_trusted(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options)
+{
+ u32 blob_handle;
+ int rc;
+
+ rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
+ if (rc)
+ return rc;
+
+ rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+ tpm2_flush_context(chip, blob_handle);
+ return rc;
+}
+
+/**
+ * tpm_unseal_trusted() - unseal a trusted key
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
+ * @options: authentication values and other options
+ * @payload: the key data in clear and encrypted form
+ *
+ * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
+ * the keyring subsystem.
+ *
+ * Return: same as with tpm_transmit_cmd()
+ */
+int tpm_unseal_trusted(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options)
+{
+ int rc;
+
+ chip = tpm_find_get_ops(chip);
+ if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+ return -ENODEV;
+
+ rc = tpm2_unseal_trusted(chip, payload, options);
+
+ tpm_put_ops(chip);
+
+ return rc;
+}
--
2.7.4
^ permalink raw reply related
* [RFC/RFT v3 3/3] KEYS: trusted: Add generic trusted keys framework
From: Sumit Garg @ 2019-08-06 13:37 UTC (permalink / raw)
To: keyrings, linux-integrity, linux-crypto, linux-security-module
Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
daniel.thompson, linux-kernel, tee-dev, Sumit Garg
In-Reply-To: <1565098640-12536-1-git-send-email-sumit.garg@linaro.org>
Current trusted keys framework is tightly coupled to use TPM device as
an underlying implementation which makes it difficult for implementations
like Trusted Execution Environment (TEE) etc. to provide trusked keys
support in case platform doesn't posses a TPM device.
So this patch tries to add generic trusted keys framework where underlying
implemtations like TPM, TEE etc. could be easily plugged-in.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
include/keys/trusted-type.h | 45 ++++
include/keys/trusted_tpm.h | 15 --
security/keys/trusted-keys/Makefile | 3 +-
security/keys/trusted-keys/trusted-tpm.c | 345 ++++++-------------------------
security/keys/trusted-keys/trusted.c | 343 ++++++++++++++++++++++++++++++
5 files changed, 448 insertions(+), 303 deletions(-)
create mode 100644 security/keys/trusted-keys/trusted.c
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a..5559010 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -40,6 +40,51 @@ struct trusted_key_options {
uint32_t policyhandle;
};
+struct trusted_key_ops {
+ /*
+ * flag to indicate if trusted key implementation supports migration
+ * or not.
+ */
+ unsigned char migratable;
+
+ /* trusted key init */
+ int (*init)(void);
+
+ /* seal a trusted key */
+ int (*seal)(struct trusted_key_payload *p, char *datablob);
+
+ /* unseal a trusted key */
+ int (*unseal)(struct trusted_key_payload *p, char *datablob);
+
+ /* get random trusted key */
+ int (*get_random)(unsigned char *key, size_t key_len);
+
+ /* trusted key cleanup */
+ void (*cleanup)(void);
+};
+
extern struct key_type key_type_trusted;
+#if defined(CONFIG_TCG_TPM)
+extern struct trusted_key_ops tpm_trusted_key_ops;
+#endif
+
+#define TRUSTED_DEBUG 0
+
+#if TRUSTED_DEBUG
+static inline void dump_payload(struct trusted_key_payload *p)
+{
+ pr_info("trusted_key: key_len %d\n", p->key_len);
+ print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
+ 16, 1, p->key, p->key_len, 0);
+ pr_info("trusted_key: bloblen %d\n", p->blob_len);
+ print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
+ 16, 1, p->blob, p->blob_len, 0);
+ pr_info("trusted_key: migratable %d\n", p->migratable);
+}
+#else
+static inline void dump_payload(struct trusted_key_payload *p)
+{
+}
+#endif
#endif /* _KEYS_TRUSTED_TYPE_H */
diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
index 77a2b5c..ec54516 100644
--- a/include/keys/trusted_tpm.h
+++ b/include/keys/trusted_tpm.h
@@ -68,17 +68,6 @@ static inline void dump_options(struct trusted_key_options *o)
16, 1, o->pcrinfo, o->pcrinfo_len, 0);
}
-static inline void dump_payload(struct trusted_key_payload *p)
-{
- pr_info("trusted_key: key_len %d\n", p->key_len);
- print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
- 16, 1, p->key, p->key_len, 0);
- pr_info("trusted_key: bloblen %d\n", p->blob_len);
- print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
- 16, 1, p->blob, p->blob_len, 0);
- pr_info("trusted_key: migratable %d\n", p->migratable);
-}
-
static inline void dump_sess(struct osapsess *s)
{
print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
@@ -104,10 +93,6 @@ static inline void dump_options(struct trusted_key_options *o)
{
}
-static inline void dump_payload(struct trusted_key_payload *p)
-{
-}
-
static inline void dump_sess(struct osapsess *s)
{
}
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index 590a694..c2e6b9a 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -3,5 +3,6 @@
# Makefile for trusted keys
#
-obj-$(CONFIG_TRUSTED_KEYS) += trusted-tpm.o \
+obj-$(CONFIG_TRUSTED_KEYS) += trusted.o \
+ trusted-tpm.o \
trusted-tpm2.o
diff --git a/security/keys/trusted-keys/trusted-tpm.c b/security/keys/trusted-keys/trusted-tpm.c
index d09893b..fe7abc1 100644
--- a/security/keys/trusted-keys/trusted-tpm.c
+++ b/security/keys/trusted-keys/trusted-tpm.c
@@ -1,29 +1,26 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (C) 2010 IBM Corporation
+ * Copyright (c) 2019, Linaro Limited
*
* Author:
* David Safford <safford@us.ibm.com>
+ * Switch to generic trusted key framework: Sumit Garg <sumit.garg@linaro.org>
*
* See Documentation/security/keys/trusted-encrypted.rst
*/
#include <crypto/hash_info.h>
-#include <linux/uaccess.h>
-#include <linux/module.h>
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/parser.h>
#include <linux/string.h>
#include <linux/err.h>
-#include <keys/user-type.h>
#include <keys/trusted-type.h>
#include <linux/key-type.h>
-#include <linux/rcupdate.h>
#include <linux/crypto.h>
#include <crypto/hash.h>
#include <crypto/sha.h>
-#include <linux/capability.h>
#include <linux/tpm.h>
#include <linux/tpm_command.h>
@@ -717,7 +714,6 @@ static int key_unseal(struct trusted_key_payload *p,
enum {
Opt_err,
- Opt_new, Opt_load, Opt_update,
Opt_keyhandle, Opt_keyauth, Opt_blobauth,
Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
Opt_hash,
@@ -726,9 +722,6 @@ enum {
};
static const match_table_t key_tokens = {
- {Opt_new, "new"},
- {Opt_load, "load"},
- {Opt_update, "update"},
{Opt_keyhandle, "keyhandle=%s"},
{Opt_keyauth, "keyauth=%s"},
{Opt_blobauth, "blobauth=%s"},
@@ -855,71 +848,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
return 0;
}
-/*
- * datablob_parse - parse the keyctl data and fill in the
- * payload and options structures
- *
- * On success returns 0, otherwise -EINVAL.
- */
-static int datablob_parse(char *datablob, struct trusted_key_payload *p,
- struct trusted_key_options *o)
-{
- substring_t args[MAX_OPT_ARGS];
- long keylen;
- int ret = -EINVAL;
- int key_cmd;
- char *c;
-
- /* main command */
- c = strsep(&datablob, " \t");
- if (!c)
- return -EINVAL;
- key_cmd = match_token(c, key_tokens, args);
- switch (key_cmd) {
- case Opt_new:
- /* first argument is key size */
- c = strsep(&datablob, " \t");
- if (!c)
- return -EINVAL;
- ret = kstrtol(c, 10, &keylen);
- if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
- return -EINVAL;
- p->key_len = keylen;
- ret = getoptions(datablob, p, o);
- if (ret < 0)
- return ret;
- ret = Opt_new;
- break;
- case Opt_load:
- /* first argument is sealed blob */
- c = strsep(&datablob, " \t");
- if (!c)
- return -EINVAL;
- p->blob_len = strlen(c) / 2;
- if (p->blob_len > MAX_BLOB_SIZE)
- return -EINVAL;
- ret = hex2bin(p->blob, c, p->blob_len);
- if (ret < 0)
- return -EINVAL;
- ret = getoptions(datablob, p, o);
- if (ret < 0)
- return ret;
- ret = Opt_load;
- break;
- case Opt_update:
- /* all arguments are options */
- ret = getoptions(datablob, p, o);
- if (ret < 0)
- return ret;
- ret = Opt_update;
- break;
- case Opt_err:
- return -EINVAL;
- break;
- }
- return ret;
-}
-
static struct trusted_key_options *trusted_options_alloc(void)
{
struct trusted_key_options *options;
@@ -940,258 +868,99 @@ static struct trusted_key_options *trusted_options_alloc(void)
return options;
}
-static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
+static int tpm_tk_seal(struct trusted_key_payload *p, char *datablob)
{
- struct trusted_key_payload *p = NULL;
- int ret;
-
- ret = key_payload_reserve(key, sizeof *p);
- if (ret < 0)
- return p;
- p = kzalloc(sizeof *p, GFP_KERNEL);
- if (p)
- p->migratable = 1; /* migratable by default */
- return p;
-}
-
-/*
- * trusted_instantiate - create a new trusted key
- *
- * Unseal an existing trusted blob or, for a new key, get a
- * random key, then seal and create a trusted key-type key,
- * adding it to the specified keyring.
- *
- * On success, return 0. Otherwise return errno.
- */
-static int trusted_instantiate(struct key *key,
- struct key_preparsed_payload *prep)
-{
- struct trusted_key_payload *payload = NULL;
struct trusted_key_options *options = NULL;
- size_t datalen = prep->datalen;
- char *datablob;
int ret = 0;
- int key_cmd;
- size_t key_len;
int tpm2;
tpm2 = tpm_is_tpm2(chip);
if (tpm2 < 0)
return tpm2;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
- return -EINVAL;
-
- datablob = kmalloc(datalen + 1, GFP_KERNEL);
- if (!datablob)
- return -ENOMEM;
- memcpy(datablob, prep->data, datalen);
- datablob[datalen] = '\0';
-
options = trusted_options_alloc();
- if (!options) {
- ret = -ENOMEM;
- goto out;
- }
- payload = trusted_payload_alloc(key);
- if (!payload) {
- ret = -ENOMEM;
- goto out;
- }
+ if (!options)
+ return -ENOMEM;
- key_cmd = datablob_parse(datablob, payload, options);
- if (key_cmd < 0) {
- ret = key_cmd;
+ ret = getoptions(datablob, p, options);
+ if (ret < 0)
goto out;
- }
+ dump_options(options);
if (!options->keyhandle) {
ret = -EINVAL;
goto out;
}
- dump_payload(payload);
- dump_options(options);
+ if (tpm2)
+ ret = tpm_seal_trusted(chip, p, options);
+ else
+ ret = key_seal(p, options);
+ if (ret < 0) {
+ pr_info("tpm_trusted_key: key_seal failed (%d)\n", ret);
+ goto out;
+ }
- switch (key_cmd) {
- case Opt_load:
- if (tpm2)
- ret = tpm_unseal_trusted(chip, payload, options);
- else
- ret = key_unseal(payload, options);
- dump_payload(payload);
- dump_options(options);
- if (ret < 0)
- pr_info("trusted_key: key_unseal failed (%d)\n", ret);
- break;
- case Opt_new:
- key_len = payload->key_len;
- ret = tpm_get_random(chip, payload->key, key_len);
- if (ret != key_len) {
- pr_info("trusted_key: key_create failed (%d)\n", ret);
+ if (options->pcrlock) {
+ ret = pcrlock(options->pcrlock);
+ if (ret < 0) {
+ pr_info("tpm_trusted_key: pcrlock failed (%d)\n", ret);
goto out;
}
- if (tpm2)
- ret = tpm_seal_trusted(chip, payload, options);
- else
- ret = key_seal(payload, options);
- if (ret < 0)
- pr_info("trusted_key: key_seal failed (%d)\n", ret);
- break;
- default:
- ret = -EINVAL;
- goto out;
}
- if (!ret && options->pcrlock)
- ret = pcrlock(options->pcrlock);
out:
- kzfree(datablob);
kzfree(options);
- if (!ret)
- rcu_assign_keypointer(key, payload);
- else
- kzfree(payload);
return ret;
}
-static void trusted_rcu_free(struct rcu_head *rcu)
-{
- struct trusted_key_payload *p;
-
- p = container_of(rcu, struct trusted_key_payload, rcu);
- kzfree(p);
-}
-
-/*
- * trusted_update - reseal an existing key with new PCR values
- */
-static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
+static int tpm_tk_unseal(struct trusted_key_payload *p, char *datablob)
{
- struct trusted_key_payload *p;
- struct trusted_key_payload *new_p;
- struct trusted_key_options *new_o;
- size_t datalen = prep->datalen;
- char *datablob;
+ struct trusted_key_options *options = NULL;
int ret = 0;
+ int tpm2;
- if (key_is_negative(key))
- return -ENOKEY;
- p = key->payload.data[0];
- if (!p->migratable)
- return -EPERM;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
- return -EINVAL;
+ tpm2 = tpm_is_tpm2(chip);
+ if (tpm2 < 0)
+ return tpm2;
- datablob = kmalloc(datalen + 1, GFP_KERNEL);
- if (!datablob)
+ options = trusted_options_alloc();
+ if (!options)
return -ENOMEM;
- new_o = trusted_options_alloc();
- if (!new_o) {
- ret = -ENOMEM;
- goto out;
- }
- new_p = trusted_payload_alloc(key);
- if (!new_p) {
- ret = -ENOMEM;
- goto out;
- }
- memcpy(datablob, prep->data, datalen);
- datablob[datalen] = '\0';
- ret = datablob_parse(datablob, new_p, new_o);
- if (ret != Opt_update) {
- ret = -EINVAL;
- kzfree(new_p);
+ ret = getoptions(datablob, p, options);
+ if (ret < 0)
goto out;
- }
+ dump_options(options);
- if (!new_o->keyhandle) {
+ if (!options->keyhandle) {
ret = -EINVAL;
- kzfree(new_p);
goto out;
}
- /* copy old key values, and reseal with new pcrs */
- new_p->migratable = p->migratable;
- new_p->key_len = p->key_len;
- memcpy(new_p->key, p->key, p->key_len);
- dump_payload(p);
- dump_payload(new_p);
+ if (tpm2)
+ ret = tpm_unseal_trusted(chip, p, options);
+ else
+ ret = key_unseal(p, options);
+ if (ret < 0)
+ pr_info("tpm_trusted_key: key_unseal failed (%d)\n", ret);
- ret = key_seal(new_p, new_o);
- if (ret < 0) {
- pr_info("trusted_key: key_seal failed (%d)\n", ret);
- kzfree(new_p);
- goto out;
- }
- if (new_o->pcrlock) {
- ret = pcrlock(new_o->pcrlock);
+ if (options->pcrlock) {
+ ret = pcrlock(options->pcrlock);
if (ret < 0) {
- pr_info("trusted_key: pcrlock failed (%d)\n", ret);
- kzfree(new_p);
+ pr_info("tpm_trusted_key: pcrlock failed (%d)\n", ret);
goto out;
}
}
- rcu_assign_keypointer(key, new_p);
- call_rcu(&p->rcu, trusted_rcu_free);
out:
- kzfree(datablob);
- kzfree(new_o);
+ kzfree(options);
return ret;
}
-/*
- * trusted_read - copy the sealed blob data to userspace in hex.
- * On success, return to userspace the trusted key datablob size.
- */
-static long trusted_read(const struct key *key, char __user *buffer,
- size_t buflen)
-{
- const struct trusted_key_payload *p;
- char *ascii_buf;
- char *bufp;
- int i;
-
- p = dereference_key_locked(key);
- if (!p)
- return -EINVAL;
-
- if (buffer && buflen >= 2 * p->blob_len) {
- ascii_buf = kmalloc_array(2, p->blob_len, GFP_KERNEL);
- if (!ascii_buf)
- return -ENOMEM;
-
- bufp = ascii_buf;
- for (i = 0; i < p->blob_len; i++)
- bufp = hex_byte_pack(bufp, p->blob[i]);
- if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) {
- kzfree(ascii_buf);
- return -EFAULT;
- }
- kzfree(ascii_buf);
- }
- return 2 * p->blob_len;
-}
-
-/*
- * trusted_destroy - clear and free the key's payload
- */
-static void trusted_destroy(struct key *key)
+int tpm_tk_get_random(unsigned char *key, size_t key_len)
{
- kzfree(key->payload.data[0]);
+ return tpm_get_random(chip, key, key_len);
}
-struct key_type key_type_trusted = {
- .name = "trusted",
- .instantiate = trusted_instantiate,
- .update = trusted_update,
- .destroy = trusted_destroy,
- .describe = user_describe,
- .read = trusted_read,
-};
-
-EXPORT_SYMBOL_GPL(key_type_trusted);
-
static void trusted_shash_release(void)
{
if (hashalg)
@@ -1206,14 +975,14 @@ static int __init trusted_shash_alloc(void)
hmacalg = crypto_alloc_shash(hmac_alg, 0, 0);
if (IS_ERR(hmacalg)) {
- pr_info("trusted_key: could not allocate crypto %s\n",
+ pr_info("tpm_trusted_key: could not allocate crypto %s\n",
hmac_alg);
return PTR_ERR(hmacalg);
}
hashalg = crypto_alloc_shash(hash_alg, 0, 0);
if (IS_ERR(hashalg)) {
- pr_info("trusted_key: could not allocate crypto %s\n",
+ pr_info("tpm_trusted_key: could not allocate crypto %s\n",
hash_alg);
ret = PTR_ERR(hashalg);
goto hashalg_fail;
@@ -1249,16 +1018,13 @@ static int __init init_digests(void)
return 0;
}
-static int __init init_trusted(void)
+static int __init init_tpm_trusted(void)
{
int ret;
- /* encrypted_keys.ko depends on successful load of this module even if
- * TPM is not used.
- */
chip = tpm_default_chip();
if (!chip)
- return 0;
+ return -ENODEV;
ret = init_digests();
if (ret < 0)
@@ -1279,7 +1045,7 @@ static int __init init_trusted(void)
return ret;
}
-static void __exit cleanup_trusted(void)
+static void __exit cleanup_tpm_trusted(void)
{
if (chip) {
put_device(&chip->dev);
@@ -1289,7 +1055,12 @@ static void __exit cleanup_trusted(void)
}
}
-late_initcall(init_trusted);
-module_exit(cleanup_trusted);
-
-MODULE_LICENSE("GPL");
+struct trusted_key_ops tpm_trusted_key_ops = {
+ .migratable = 1, /* migratable by default */
+ .init = init_tpm_trusted,
+ .seal = tpm_tk_seal,
+ .unseal = tpm_tk_unseal,
+ .get_random = tpm_tk_get_random,
+ .cleanup = cleanup_tpm_trusted,
+};
+EXPORT_SYMBOL_GPL(tpm_trusted_key_ops);
diff --git a/security/keys/trusted-keys/trusted.c b/security/keys/trusted-keys/trusted.c
new file mode 100644
index 0000000..8f00fde
--- /dev/null
+++ b/security/keys/trusted-keys/trusted.c
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2010 IBM Corporation
+ * Copyright (c) 2019, Linaro Limited
+ *
+ * Author:
+ * David Safford <safford@us.ibm.com>
+ * Added generic trusted key framework: Sumit Garg <sumit.garg@linaro.org>
+ *
+ * See Documentation/security/keys/trusted-encrypted.rst
+ */
+
+#include <keys/user-type.h>
+#include <keys/trusted-type.h>
+#include <linux/capability.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/key-type.h>
+#include <linux/module.h>
+#include <linux/parser.h>
+#include <linux/rcupdate.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+
+static struct trusted_key_ops *available_tk_ops[] = {
+#if defined(CONFIG_TCG_TPM)
+ &tpm_trusted_key_ops,
+#endif
+};
+static struct trusted_key_ops *tk_ops;
+
+enum {
+ Opt_err,
+ Opt_new, Opt_load, Opt_update,
+};
+
+static const match_table_t key_tokens = {
+ {Opt_new, "new"},
+ {Opt_load, "load"},
+ {Opt_update, "update"},
+ {Opt_err, NULL}
+};
+
+/*
+ * datablob_parse - parse the keyctl data and fill in the
+ * payload structure
+ *
+ * On success returns 0, otherwise -EINVAL.
+ */
+static int datablob_parse(char *datablob, struct trusted_key_payload *p)
+{
+ substring_t args[MAX_OPT_ARGS];
+ long keylen;
+ int ret = -EINVAL;
+ int key_cmd;
+ char *c;
+
+ /* main command */
+ c = strsep(&datablob, " \t");
+ if (!c)
+ return -EINVAL;
+ key_cmd = match_token(c, key_tokens, args);
+ switch (key_cmd) {
+ case Opt_new:
+ /* first argument is key size */
+ c = strsep(&datablob, " \t");
+ if (!c)
+ return -EINVAL;
+ ret = kstrtol(c, 10, &keylen);
+ if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
+ return -EINVAL;
+ p->key_len = keylen;
+ ret = Opt_new;
+ break;
+ case Opt_load:
+ /* first argument is sealed blob */
+ c = strsep(&datablob, " \t");
+ if (!c)
+ return -EINVAL;
+ p->blob_len = strlen(c) / 2;
+ if (p->blob_len > MAX_BLOB_SIZE)
+ return -EINVAL;
+ ret = hex2bin(p->blob, c, p->blob_len);
+ if (ret < 0)
+ return -EINVAL;
+ ret = Opt_load;
+ break;
+ case Opt_update:
+ ret = Opt_update;
+ break;
+ case Opt_err:
+ return -EINVAL;
+ }
+ return ret;
+}
+
+static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
+{
+ struct trusted_key_payload *p = NULL;
+ int ret;
+
+ ret = key_payload_reserve(key, sizeof(*p));
+ if (ret < 0)
+ return p;
+ p = kzalloc(sizeof(*p), GFP_KERNEL);
+
+ p->migratable = tk_ops->migratable;
+
+ return p;
+}
+
+/*
+ * trusted_instantiate - create a new trusted key
+ *
+ * Unseal an existing trusted blob or, for a new key, get a
+ * random key, then seal and create a trusted key-type key,
+ * adding it to the specified keyring.
+ *
+ * On success, return 0. Otherwise return errno.
+ */
+static int trusted_instantiate(struct key *key,
+ struct key_preparsed_payload *prep)
+{
+ struct trusted_key_payload *payload = NULL;
+ size_t datalen = prep->datalen;
+ char *datablob;
+ int ret = 0;
+ int key_cmd;
+ size_t key_len;
+
+ if (datalen <= 0 || datalen > 32767 || !prep->data)
+ return -EINVAL;
+
+ datablob = kmalloc(datalen + 1, GFP_KERNEL);
+ if (!datablob)
+ return -ENOMEM;
+ memcpy(datablob, prep->data, datalen);
+ datablob[datalen] = '\0';
+
+ payload = trusted_payload_alloc(key);
+ if (!payload) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ key_cmd = datablob_parse(datablob, payload);
+ if (key_cmd < 0) {
+ ret = key_cmd;
+ goto out;
+ }
+
+ dump_payload(payload);
+
+ switch (key_cmd) {
+ case Opt_load:
+ ret = tk_ops->unseal(payload, datablob);
+ dump_payload(payload);
+ if (ret < 0)
+ pr_info("trusted_key: key_unseal failed (%d)\n", ret);
+ break;
+ case Opt_new:
+ key_len = payload->key_len;
+ ret = tk_ops->get_random(payload->key, key_len);
+ if (ret != key_len) {
+ pr_info("trusted_key: key_create failed (%d)\n", ret);
+ goto out;
+ }
+
+ ret = tk_ops->seal(payload, datablob);
+ if (ret < 0)
+ pr_info("trusted_key: key_seal failed (%d)\n", ret);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+out:
+ kzfree(datablob);
+ if (!ret)
+ rcu_assign_keypointer(key, payload);
+ else
+ kzfree(payload);
+ return ret;
+}
+
+static void trusted_rcu_free(struct rcu_head *rcu)
+{
+ struct trusted_key_payload *p;
+
+ p = container_of(rcu, struct trusted_key_payload, rcu);
+ kzfree(p);
+}
+
+/*
+ * trusted_update - reseal an existing key with new PCR values
+ */
+static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
+{
+ struct trusted_key_payload *p;
+ struct trusted_key_payload *new_p;
+ size_t datalen = prep->datalen;
+ char *datablob;
+ int ret = 0;
+
+ if (key_is_negative(key))
+ return -ENOKEY;
+ p = key->payload.data[0];
+ if (!p->migratable)
+ return -EPERM;
+ if (datalen <= 0 || datalen > 32767 || !prep->data)
+ return -EINVAL;
+
+ datablob = kmalloc(datalen + 1, GFP_KERNEL);
+ if (!datablob)
+ return -ENOMEM;
+
+ new_p = trusted_payload_alloc(key);
+ if (!new_p) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ memcpy(datablob, prep->data, datalen);
+ datablob[datalen] = '\0';
+ ret = datablob_parse(datablob, new_p);
+ if (ret != Opt_update) {
+ ret = -EINVAL;
+ kzfree(new_p);
+ goto out;
+ }
+
+ /* copy old key values, and reseal with new pcrs */
+ new_p->migratable = p->migratable;
+ new_p->key_len = p->key_len;
+ memcpy(new_p->key, p->key, p->key_len);
+ dump_payload(p);
+ dump_payload(new_p);
+
+ ret = tk_ops->seal(new_p, datablob);
+ if (ret < 0) {
+ pr_info("trusted_key: key_seal failed (%d)\n", ret);
+ kzfree(new_p);
+ goto out;
+ }
+
+ rcu_assign_keypointer(key, new_p);
+ call_rcu(&p->rcu, trusted_rcu_free);
+out:
+ kzfree(datablob);
+ return ret;
+}
+
+/*
+ * trusted_read - copy the sealed blob data to userspace in hex.
+ * On success, return to userspace the trusted key datablob size.
+ */
+static long trusted_read(const struct key *key, char __user *buffer,
+ size_t buflen)
+{
+ const struct trusted_key_payload *p;
+ char *ascii_buf;
+ char *bufp;
+ int i;
+
+ p = dereference_key_locked(key);
+ if (!p)
+ return -EINVAL;
+
+ if (buffer && buflen >= 2 * p->blob_len) {
+ ascii_buf = kmalloc_array(2, p->blob_len, GFP_KERNEL);
+ if (!ascii_buf)
+ return -ENOMEM;
+
+ bufp = ascii_buf;
+ for (i = 0; i < p->blob_len; i++)
+ bufp = hex_byte_pack(bufp, p->blob[i]);
+ if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) {
+ kzfree(ascii_buf);
+ return -EFAULT;
+ }
+ kzfree(ascii_buf);
+ }
+ return 2 * p->blob_len;
+}
+
+/*
+ * trusted_destroy - clear and free the key's payload
+ */
+static void trusted_destroy(struct key *key)
+{
+ kzfree(key->payload.data[0]);
+}
+
+struct key_type key_type_trusted = {
+ .name = "trusted",
+ .instantiate = trusted_instantiate,
+ .update = trusted_update,
+ .destroy = trusted_destroy,
+ .describe = user_describe,
+ .read = trusted_read,
+};
+EXPORT_SYMBOL_GPL(key_type_trusted);
+
+static int __init init_trusted(void)
+{
+ int i, ret = 0;
+
+ for (i = 0; i < sizeof(available_tk_ops); i++) {
+ tk_ops = available_tk_ops[i];
+
+ if (!(tk_ops && tk_ops->init && tk_ops->seal &&
+ tk_ops->unseal && tk_ops->get_random))
+ continue;
+
+ ret = tk_ops->init();
+ if (ret) {
+ if (tk_ops->cleanup)
+ tk_ops->cleanup();
+ } else {
+ break;
+ }
+ }
+
+ /*
+ * encrypted_keys.ko depends on successful load of this module even if
+ * trusted key implementation is not found.
+ */
+ if (ret == -ENODEV)
+ return 0;
+
+ return ret;
+}
+
+static void __exit cleanup_trusted(void)
+{
+ if (tk_ops->cleanup)
+ tk_ops->cleanup();
+}
+
+late_initcall(init_trusted);
+module_exit(cleanup_trusted);
+
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related
* Re: [RFC/RFT v3 2/3] KEYS: trusted: move tpm2 trusted keys code
From: Greg KH @ 2019-08-06 13:43 UTC (permalink / raw)
To: Sumit Garg
Cc: keyrings, linux-integrity, linux-crypto, linux-security-module,
dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
arnd, zohar, jmorris, serge, casey, ard.biesheuvel,
daniel.thompson, linux-kernel, tee-dev
In-Reply-To: <1565098640-12536-3-git-send-email-sumit.garg@linaro.org>
On Tue, Aug 06, 2019 at 07:07:19PM +0530, Sumit Garg wrote:
> Move TPM2 trusted keys code to trusted keys subsystem.
>
> Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
> drivers/char/tpm/tpm-interface.c | 56 -----
> drivers/char/tpm/tpm.h | 224 ------------------
> drivers/char/tpm/tpm2-cmd.c | 307 ------------------------
> include/keys/trusted_tpm.h | 23 +-
> include/linux/tpm.h | 264 +++++++++++++++++++--
> security/keys/trusted-keys/Makefile | 3 +-
> security/keys/trusted-keys/trusted-tpm.c | 16 +-
> security/keys/trusted-keys/trusted-tpm2.c | 378 ++++++++++++++++++++++++++++++
> 8 files changed, 652 insertions(+), 619 deletions(-)
> create mode 100644 security/keys/trusted-keys/trusted-tpm2.c
'git format-patch -M' will create a patch that shows the rename, and
then any tiny differences that happened after that. A patch like this
is hard to see what changed in the move.
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC/RFT v3 2/3] KEYS: trusted: move tpm2 trusted keys code
From: Sumit Garg @ 2019-08-06 13:55 UTC (permalink / raw)
To: Greg KH
Cc: keyrings, linux-integrity,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-security-module, dhowells, Herbert Xu, davem, peterhuewe,
jgg, jejb, Jarkko Sakkinen, Arnd Bergmann, Mimi Zohar,
James Morris, Serge E. Hallyn, Casey Schaufler, Ard Biesheuvel,
Daniel Thompson, Linux Kernel Mailing List,
tee-dev @ lists . linaro . org
In-Reply-To: <20190806134322.GA10783@kroah.com>
On Tue, 6 Aug 2019 at 19:13, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Aug 06, 2019 at 07:07:19PM +0530, Sumit Garg wrote:
> > Move TPM2 trusted keys code to trusted keys subsystem.
> >
> > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> > drivers/char/tpm/tpm-interface.c | 56 -----
> > drivers/char/tpm/tpm.h | 224 ------------------
> > drivers/char/tpm/tpm2-cmd.c | 307 ------------------------
> > include/keys/trusted_tpm.h | 23 +-
> > include/linux/tpm.h | 264 +++++++++++++++++++--
> > security/keys/trusted-keys/Makefile | 3 +-
> > security/keys/trusted-keys/trusted-tpm.c | 16 +-
> > security/keys/trusted-keys/trusted-tpm2.c | 378 ++++++++++++++++++++++++++++++
> > 8 files changed, 652 insertions(+), 619 deletions(-)
> > create mode 100644 security/keys/trusted-keys/trusted-tpm2.c
>
> 'git format-patch -M' will create a patch that shows the rename, and
> then any tiny differences that happened after that. A patch like this
> is hard to see what changed in the move.
>
I used this option only to generate the patch-set. Following is the
command I used:
git format-patch -M -3 --cover-letter --subject-prefix="RFC/RFT v3"
It seems like for this patch I need to collect pieces from
"drivers/char/tpm/" and aggregate them under
"security/keys/trusted-keys/trusted-tpm2.c" and "include/linux/tpm.h"
files. So that could be the reason for such patch view.
-Sumit
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH bpf-next v10 06/10] bpf,landlock: Add a new map type: inode
From: Mickaël Salaün @ 2019-08-06 16:24 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Mickaël Salaün, LKML, Alexander Viro,
Alexei Starovoitov, Andrew Morton, Andy Lutomirski,
Arnaldo Carvalho de Melo, Casey Schaufler, Daniel Borkmann,
David Drysdale, David S . Miller, Eric W . Biederman,
James Morris, Jann Horn, John Johansen, Jonathan Corbet,
Kees Cook, Michael Kerrisk, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
Kernel Hardening, Linux API, Linux-Fsdevel, LSM List,
Network Development
In-Reply-To: <20190801173534.etfls5ltixp5hfrh@ast-mbp.dhcp.thefacebook.com>
On 01/08/2019 19:35, Alexei Starovoitov wrote:
> On Wed, Jul 31, 2019 at 09:11:10PM +0200, Mickaël Salaün wrote:
>>
>>
>> On 31/07/2019 20:58, Alexei Starovoitov wrote:
>>> On Wed, Jul 31, 2019 at 11:46 AM Mickaël Salaün
>>> <mickael.salaun@ssi.gouv.fr> wrote:
>>>>>> + for (i = 0; i < htab->n_buckets; i++) {
>>>>>> + head = select_bucket(htab, i);
>>>>>> + hlist_nulls_for_each_entry_safe(l, n, head, hash_node) {
>>>>>> + landlock_inode_remove_map(*((struct inode **)l->key), map);
>>>>>> + }
>>>>>> + }
>>>>>> + htab_map_free(map);
>>>>>> +}
>>>>>
>>>>> user space can delete the map.
>>>>> that will trigger inode_htab_map_free() which will call
>>>>> landlock_inode_remove_map().
>>>>> which will simply itereate the list and delete from the list.
>>>>
>>>> landlock_inode_remove_map() removes the reference to the map (being
>>>> freed) from the inode (with an RCU lock).
>>>
>>> I'm going to ignore everything else for now and focus only on this bit,
>>> since it's fundamental issue to address before this discussion can
>>> go any further.
>>> rcu_lock is not a spin_lock. I'm pretty sure you know this.
>>> But you're arguing that it's somehow protecting from the race
>>> I mentioned above?
>>>
>>
>> I was just clarifying your comment to avoid misunderstanding about what
>> is being removed.
>>
>> As said in the full response, there is currently a race but, if I add a
>> bpf_map_inc() call when the map is referenced by inode->security, then I
>> don't see how a race could occur because such added map could only be
>> freed in a security_inode_free() (as long as it retains a reference to
>> this inode).
>
> then it will be a cycle and a map will never be deleted?
> closing map_fd should delete a map. It cannot be alive if it's not
> pinned in bpffs, there are no FDs that are holding it, and no progs using it.
> So the map deletion will iterate over inodes that belong to this map.
> In parallel security_inode_free() will be called that will iterate
> over its link list that contains elements from different maps.
> So the same link list is modified by two cpus.
> Where is a lock that protects from concurrent links list manipulations?
Ok, I think I got it. What about this fix?
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 4fc7755042f0..3226e50b6211 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1708,10 +1708,16 @@ static void inode_htab_map_free(struct bpf_map *map)
for (i = 0; i < htab->n_buckets; i++) {
head = select_bucket(htab, i);
- hlist_nulls_for_each_entry_safe(l, n, head, hash_node) {
+ rcu_read_lock();
+ hlist_nulls_for_each_entry_rcu(l, n, head, hash_node) {
landlock_inode_remove_map(*((struct inode **)l->key), map);
}
+ rcu_read_unlock();
}
+ /*
+ * The last pending put_landlock_inode_map() may be called here, before
+ * the rcu_barrier() from htab_map_free().
+ */
htab_map_free(map);
}
diff --git a/security/landlock/common.h b/security/landlock/common.h
index b0ba3f31ac7d..535c6a4292b9 100644
--- a/security/landlock/common.h
+++ b/security/landlock/common.h
@@ -58,6 +58,11 @@ struct landlock_prog_set {
refcount_t usage;
};
+struct landlock_inode_security {
+ struct list_head list;
+ spinlock_t lock;
+};
+
struct landlock_inode_map {
struct list_head list;
struct rcu_head rcu_put;
diff --git a/security/landlock/hooks_fs.c b/security/landlock/hooks_fs.c
index 8c9d6a333111..b9bfd558f8b8 100644
--- a/security/landlock/hooks_fs.c
+++ b/security/landlock/hooks_fs.c
@@ -10,6 +10,7 @@
#include <linux/kernel.h> /* ARRAY_SIZE */
#include <linux/lsm_hooks.h>
#include <linux/rcupdate.h> /* synchronize_rcu() */
+#include <linux/spinlock.h>
#include <linux/stat.h> /* S_ISDIR */
#include <linux/stddef.h> /* offsetof */
#include <linux/types.h> /* uintptr_t */
@@ -251,13 +252,16 @@ static int hook_sb_pivotroot(const struct path *old_path,
/* inode helpers */
-static inline struct list_head *inode_landlock(const struct inode *inode)
+static inline struct landlock_inode_security *inode_landlock(
+ const struct inode *inode)
{
return inode->i_security + landlock_blob_sizes.lbs_inode;
}
int landlock_inode_add_map(struct inode *inode, struct bpf_map *map)
{
+ unsigned long flags;
+ struct landlock_inode_security *inode_sec = inode_landlock(inode);
struct landlock_inode_map *inode_map;
inode_map = kzalloc(sizeof(*inode_map), GFP_ATOMIC);
@@ -266,60 +270,66 @@ int landlock_inode_add_map(struct inode *inode, struct bpf_map *map)
INIT_LIST_HEAD(&inode_map->list);
inode_map->map = map;
inode_map->inode = inode;
- list_add_tail(&inode_map->list, inode_landlock(inode));
+ spin_lock_irqsave(&inode_sec->lock, flags);
+ list_add_tail_rcu(&inode_map->list, &inode_sec->list);
+ spin_unlock_irqrestore(&inode_sec->lock, flags);
return 0;
}
static void put_landlock_inode_map(struct rcu_head *head)
{
struct landlock_inode_map *inode_map;
- int err;
inode_map = container_of(head, struct landlock_inode_map, rcu_put);
- err = bpf_inode_ptr_unlocked_htab_map_delete_elem(inode_map->map,
+ bpf_inode_ptr_unlocked_htab_map_delete_elem(inode_map->map,
&inode_map->inode, false);
- bpf_map_put(inode_map->map);
kfree(inode_map);
}
void landlock_inode_remove_map(struct inode *inode, const struct bpf_map *map)
{
+ unsigned long flags;
+ struct landlock_inode_security *inode_sec = inode_landlock(inode);
struct landlock_inode_map *inode_map;
- bool found = false;
+ spin_lock_irqsave(&inode_sec->lock, flags);
rcu_read_lock();
- list_for_each_entry_rcu(inode_map, inode_landlock(inode), list) {
+ list_for_each_entry_rcu(inode_map, &inode_sec->list, list) {
if (inode_map->map == map) {
- found = true;
list_del_rcu(&inode_map->list);
kfree_rcu(inode_map, rcu_put);
break;
}
}
rcu_read_unlock();
- WARN_ON(!found);
+ spin_unlock_irqrestore(&inode_sec->lock, flags);
}
/* inode hooks */
static int hook_inode_alloc_security(struct inode *inode)
{
- struct list_head *ll_inode = inode_landlock(inode);
+ struct landlock_inode_security *inode_sec = inode_landlock(inode);
- INIT_LIST_HEAD(ll_inode);
+ INIT_LIST_HEAD(&inode_sec->list);
+ spin_lock_init(&inode_sec->lock);
return 0;
}
static void hook_inode_free_security(struct inode *inode)
{
+ unsigned long flags;
+ struct landlock_inode_security *inode_sec = inode_landlock(inode);
struct landlock_inode_map *inode_map;
+ spin_lock_irqsave(&inode_sec->lock, flags);
rcu_read_lock();
- list_for_each_entry_rcu(inode_map, inode_landlock(inode), list) {
+ list_for_each_entry_rcu(inode_map, &inode_sec->list, list) {
list_del_rcu(&inode_map->list);
call_rcu(&inode_map->rcu_put, put_landlock_inode_map);
}
rcu_read_unlock();
+ spin_unlock_irqrestore(&inode_sec->lock, flags);
}
/* a directory inode contains only one dentry */
diff --git a/security/landlock/init.c b/security/landlock/init.c
index 35165fc8a595..1305255f5d2e 100644
--- a/security/landlock/init.c
+++ b/security/landlock/init.c
@@ -137,7 +137,7 @@ static int __init landlock_init(void)
}
struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
- .lbs_inode = sizeof(struct list_head),
+ .lbs_inode = sizeof(struct landlock_inode_security),
};
DEFINE_LSM(LANDLOCK_NAME) = {
>
>> Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter contact.rgpd@sgdsn.gouv.fr. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: contact.rgpd@sgdsn.gouv.fr. If you have received this message in error, we thank you for informing the sender and destroying the message.
>
> Please get rid of this. It's absolutely not appropriate on public mailing list.
> Next time I'd have to ignore emails that contain such disclaimers.
Unfortunately this message is automatically appended (server-side) to all my
professional emails...
^ permalink raw reply related
* Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3
From: Micah Morton @ 2019-08-06 16:32 UTC (permalink / raw)
To: Konstantin Ryabitsev
Cc: Linus Torvalds, linux-security-module, Linux Kernel Mailing List
In-Reply-To: <20190805192727.GA15470@chatter.i7.local>
On Mon, Aug 5, 2019 at 12:27 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> On Mon, Aug 05, 2019 at 12:17:49PM -0700, Linus Torvalds wrote:
> >> However, I suspect that getting message-ids for all your pull
> >> requests
> >> would significantly complicate your workflow.
> >
> >Yeah, that would be a noticeable annoyance. If I were to process pull
> >requests the way I used to process emailed patches (ie "git am -s" on
> >a mailbox) that would be a natural thing to perhaps do, but it's not
> >at all how it ends up working. Having to save the pull request email
> >to then process it with some script would turn it into a chore.
> >
> >I think the pr-tracker-bot clearly catches most cases as it is, and
> >it's only the occasional "somebody did something odd" that then misses
> >an automated response. Not a huge deal. For me it was actually more
> >the "I didn't understand why the response didn't happen", not so much
> >"I really want to always see responses".
>
> Ok, let me add a fix for Re: at the start -- this won't make things
> significantly more expensive, but will catch this particular corner
> case.
>
> Best regards,
> -K
Linus, thanks for the tips earlier about gitk. I'll use that in the future.
Unfortunately I didn't have the mental model quite right of what
happens during the pull request. I was thinking along the lines of my
commits being cherry picked onto your tree, rather than how it
actually happens with git merge where my tree's commit history needs
to match yours perfectly.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox