* 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: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3
From: Konstantin Ryabitsev @ 2019-08-05 19:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: Micah Morton, linux-security-module, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wgdiiBVprEVoi8+mpicGnOVNZ4Lb9YUJVskOXahO50sXw@mail.gmail.com>
On Mon, Aug 05, 2019 at 11:20:59AM -0700, Linus Torvalds wrote:
>I don't know if it's worth changing the pr-tracker-bot rules. I *do*
>think that the whole unquoted
>
> for you to fetch changes up to [hex string]
>
>is by far the strongest single signal for a pull request, but it's not
>clear that it's worth spending a lot of CPU time looking for that
>unless you have a strong signal in the subject line.
The way we do it currently is by hooking into public-inbox where the
email subject is in the commit log. So for us to grab all new subjects
it's a single git call, whereas getting the message body requires a git
call per message. This is why we pre-filter by subject, as it's a cheap
way to avoid needing to issue hundreds of git calls looking for possible
matches in message bodies.
>So I consider this "solved", and maybe people should just realize that
>they won't get the automated responses unless they do everything just
>right.
Would you consider recording the message-id of the pull request as part
of the commit message? This would be a sure way for us to be able to
catch all possible cases. In fact, this would allow me to throw out most
of the bot logic, as it would become unnecessary. E.g. the merge commit
would look like:
Merge tag 'foo' of git://git.kernel.org/bar
Pull foo features
* foo
* bar
* baz
Link: https://lore.kernel.org/r/<message-id>
However, I suspect that getting message-ids for all your pull requests
would significantly complicate your workflow.
-K
^ permalink raw reply
* Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3
From: Linus Torvalds @ 2019-08-05 18:20 UTC (permalink / raw)
To: Konstantin Ryabitsev
Cc: Micah Morton, linux-security-module, Linux Kernel Mailing List
In-Reply-To: <20190805142756.GA4887@chatter.i7.local>
On Mon, Aug 5, 2019 at 7:28 AM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> On Sun, Aug 04, 2019 at 10:47:54AM -0700, Linus Torvalds wrote:
> > - maybe pr-tracker-bot ignores follow-up emails with "Re:" in the
> > subject?
>
> Yes, this is the culprit. Here are the matching regexes:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mricon/korg-helpers.git/tree/pr-tracker-bot.py#n41
>
> Normally, pull requests don't come in as replies -- this is the first
> one that got missed in months, to my knowledge.
Most pull requests certainly are proper starts of threads. And I
generally wish they were, because I know I myself tend to skim over
emails much more quickly if it's some old discussion that I either
consider solved or where somebody else is handling it, so if I see a
pull request in the middle of a thread, it's much more likely that I'd
miss it.
It does happen, though. Not just in situations like this, where I
replied to the original pull request with some reason for why I
wouldn't pull it, and then the fixed pull came in as part of the
thread.
Al Viro does that to me occasionally, for example. Some discussion
about a vfs problem, and then the pull request is in the middle of
that thread. You can see an example of that here:
https://lore.kernel.org/lkml/20180125002151.GR13338@ZenIV.linux.org.uk/
although in that case it was Davem who merged it (in merge commit 8ec59b44a006.
Of course, pr-tracker-bot wouldn't have noticed that one anyway,
because it also doesn't have "GIT PULL" or anything like that in the
subject line at all. So maybe it's not a great example.
I don't know if it's worth changing the pr-tracker-bot rules. I *do*
think that the whole unquoted
for you to fetch changes up to [hex string]
is by far the strongest single signal for a pull request, but it's not
clear that it's worth spending a lot of CPU time looking for that
unless you have a strong signal in the subject line.
So I consider this "solved", and maybe people should just realize that
they won't get the automated responses unless they do everything just
right.
Linus
^ permalink raw reply
* Re: [PATCH] security/tomoyo: convert put_page() to put_user_page*()
From: John Hubbard @ 2019-08-05 17:48 UTC (permalink / raw)
To: Tetsuo Handa, john.hubbard, Andrew Morton; +Cc: linux-security-module
In-Reply-To: <346e5a8e-57fe-b2ac-4069-50a8a83048b5@i-love.sakura.ne.jp>
On 8/5/19 3:01 AM, Tetsuo Handa wrote:
> On 2019/08/05 11:26, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
>>
>> For pages that were retained via get_user_pages*(), release those pages
>> via the new put_user_page*() routines, instead of via put_page() or
>> release_pages().
>>
>> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
>> ("mm: introduce put_user_page*(), placeholder versions").
>>
>> Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
>> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> Cc: linux-security-module@vger.kernel.org
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>
> Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>
> Sending to Andrew Morton means you expect this patch to go via -mm tree, don't you?
Yes, exactly.
thanks,
--
John Hubbard
NVIDIA
> Andrew, please take this patch. Thank you.
>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-08-05 17:23 UTC (permalink / raw)
To: Song Liu
Cc: Andy Lutomirski, Kees Cook, Networking, bpf, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
Linux API, LSM List
In-Reply-To: <EE7B7AE1-3D44-4561-94B9-E97A626A251D@fb.com>
On Mon, Aug 5, 2019 at 12:37 AM Song Liu <songliubraving@fb.com> wrote:
>
> Hi Andy,
>
> >
> > # mount -t bpf bpf /sys/fs/bpf
> > # cd /sys/fs/bpf
> > # mkdir luto
> > # chown luto: luto
> > # setpriv --euid=1000 --ruid=1000 bash
> > $ pwd
> > /sys/fs/bpf
> > bash-5.0$ ls -l
> > total 0
> > drwxr-xr-x 2 luto luto 0 Aug 4 22:41 luto
> > bash-5.0$ bpftool map create /sys/fs/bpf/luto/filename type hash key 8
> > value 8 entries 64 name mapname
> > bash-5.0$ bpftool map dump pinned /sys/fs/bpf/luto/filename
> > Found 0 elements
> >
> > # chown root: /sys/fs/bpf/luto/filename
> >
> > $ bpftool map dump pinned /sys/fs/bpf/luto/filename
> > Error: bpf obj get (/sys/fs/bpf/luto): Permission denied
> >
> > So I think it's possible to get a respectable subset of bpf()
> > functionality working without privilege in short order :)
>
> I think we have two key questions to answer:
> 1. What subset of bpf() functionality will the users need?
> 2. Who are the users?
>
> Different answers to these two questions lead to different directions.
>
>
> In our use case, the answers are
> 1) almost all bpf() functionality
> 2) highly trusted users (sudoers)
>
> So our initial approach of /dev/bpf allows all bpf() functionality
> in one bit in task_struct. (Yes, we can just sudo. But, we would
> rather not use sudo when possible.)
For this, I think some compelling evidence is needed that a new kernel
mechanism is actually better than sudo and better than making bpftool
privileged as previously discussed :)
>
>
> "cgroup management" use case may have answers like:
> 1) cgroup_bpf only
> 2) users in their own containers
>
> For this case, getting cgroup_bpf related features (cgroup_bpf progs;
> some map types, etc.) work with unprivileged users would be the right
> direction.
:)
>
>
> "USDT tracing" use case may have answers like:
> 1) uprobe, stockmap, histogram, etc.
> 2) unprivileged user, w/ or w/o containers
>
> For this case, the first step is likely hacking sys_perf_event_open().
>
This would be nice.
>
> I guess we will need more discussions to decide how to make bpf()
> work better for all these (and more) use cases.
>
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.
^ permalink raw reply
* Re: [PATCH v3] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Tyler Hicks @ 2019-08-05 17:20 UTC (permalink / raw)
To: Roberto Sassu
Cc: jarkko.sakkinen, jejb, zohar, jgg, linux-integrity,
linux-security-module, keyrings, linux-kernel, crazyt2019+lml,
nayna, silviu.vlasceanu
In-Reply-To: <20190805164427.17408-1-roberto.sassu@huawei.com>
On 2019-08-05 18:44:27, 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>
The approach changed a fair bit from v2 to v3 so I'll confirm that my
Reviewed-by still stands.
Also, we have positive test results from an affected user:
https://bugzilla.kernel.org/show_bug.cgi?id=203953#c10
Tyler
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> security/keys/trusted.c | 13 -------------
> 1 file changed, 13 deletions(-)
>
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 9a94672e7adc..ade699131065 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -1228,24 +1228,11 @@ static int __init trusted_shash_alloc(void)
>
> static int __init init_digests(void)
> {
> - u8 digest[TPM_MAX_DIGEST_SIZE];
> - int ret;
> - int i;
> -
> - ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
> - if (ret < 0)
> - return ret;
> - if (ret < TPM_MAX_DIGEST_SIZE)
> - return -EFAULT;
> -
> digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
> GFP_KERNEL);
> if (!digests)
> return -ENOMEM;
>
> - for (i = 0; i < chip->nr_allocated_banks; i++)
> - memcpy(digests[i].digest, digest, TPM_MAX_DIGEST_SIZE);
> -
> return 0;
> }
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Tyler Hicks @ 2019-08-05 16:53 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, Jarkko Sakkinen, jejb, jgg, linux-integrity,
linux-security-module, keyrings, linux-kernel, crazyt2019+lml,
nayna, silviu.vlasceanu
In-Reply-To: <0d9f2f6f-9a69-5169-a92d-9ca7de3c0d18@huawei.com>
On 2019-08-05 18:51:09, Roberto Sassu wrote:
> On 8/5/2019 6:04 PM, Tyler Hicks wrote:
> > On 2019-08-05 11:54:19, Mimi Zohar wrote:
> > > On Mon, 2019-08-05 at 16:50 +0200, Roberto Sassu wrote:
> > > > Regarding Mimi's proposal to avoid the issue by extending the PCR with
> > > > zeros, I think it also achieve the goal.
> > >
> > > Roberto, removing the following code from init_digests() would be the
> > > equivalent to the prior code, without needing to make any other
> > > changes. Let's keep it simple. Do you want to post the patch with
> > > the change, or should I?
> > >
> > > ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
> > > if (ret < 0)
> > > return ret;
> > > if (ret < TPM_MAX_DIGEST_SIZE)
> > > return -EFAULT;
> > >
> > > As I can't duplicate the problem, it would need to be tested by others
> > > experiencing the problem.
> >
> > The bug reporter tested Roberto's last patch:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=203953#c8
> >
> > We should Cc the reporter on future patches or at least leave another
> > testing request in the bugzilla.
>
> I don't see the reporter's email. Please ask him to test the new patch.
Done!
Tyler
>
> Thanks
>
> Roberto
>
> --
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Li Jian, Shi Yanli
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Roberto Sassu @ 2019-08-05 16:51 UTC (permalink / raw)
To: Tyler Hicks, Mimi Zohar
Cc: Jarkko Sakkinen, jejb, jgg, linux-integrity,
linux-security-module, keyrings, linux-kernel, crazyt2019+lml,
nayna, silviu.vlasceanu
In-Reply-To: <20190805160451.GB3449@elm>
On 8/5/2019 6:04 PM, Tyler Hicks wrote:
> On 2019-08-05 11:54:19, Mimi Zohar wrote:
>> On Mon, 2019-08-05 at 16:50 +0200, Roberto Sassu wrote:
>>> Regarding Mimi's proposal to avoid the issue by extending the PCR with
>>> zeros, I think it also achieve the goal.
>>
>> Roberto, removing the following code from init_digests() would be the
>> equivalent to the prior code, without needing to make any other
>> changes. Let's keep it simple. Do you want to post the patch with
>> the change, or should I?
>>
>> ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
>> if (ret < 0)
>> return ret;
>> if (ret < TPM_MAX_DIGEST_SIZE)
>> return -EFAULT;
>>
>> As I can't duplicate the problem, it would need to be tested by others
>> experiencing the problem.
>
> The bug reporter tested Roberto's last patch:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=203953#c8
>
> We should Cc the reporter on future patches or at least leave another
> testing request in the bugzilla.
I don't see the reporter's email. Please ask him to test the new patch.
Thanks
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
^ permalink raw reply
* [PATCH v3] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Roberto Sassu @ 2019-08-05 16:44 UTC (permalink / raw)
To: jarkko.sakkinen, jejb, zohar, jgg, tyhicks
Cc: linux-integrity, linux-security-module, keyrings, linux-kernel,
crazyt2019+lml, nayna, silviu.vlasceanu, Roberto Sassu
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>
---
security/keys/trusted.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 9a94672e7adc..ade699131065 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1228,24 +1228,11 @@ static int __init trusted_shash_alloc(void)
static int __init init_digests(void)
{
- u8 digest[TPM_MAX_DIGEST_SIZE];
- int ret;
- int i;
-
- ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
- if (ret < 0)
- return ret;
- if (ret < TPM_MAX_DIGEST_SIZE)
- return -EFAULT;
-
digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
GFP_KERNEL);
if (!digests)
return -ENOMEM;
- for (i = 0; i < chip->nr_allocated_banks; i++)
- memcpy(digests[i].digest, digest, TPM_MAX_DIGEST_SIZE);
-
return 0;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Tyler Hicks @ 2019-08-05 16:04 UTC (permalink / raw)
To: Mimi Zohar
Cc: Roberto Sassu, Jarkko Sakkinen, jejb, jgg, linux-integrity,
linux-security-module, keyrings, linux-kernel, crazyt2019+lml,
nayna, silviu.vlasceanu
In-Reply-To: <1565020459.11223.179.camel@linux.ibm.com>
On 2019-08-05 11:54:19, Mimi Zohar wrote:
> On Mon, 2019-08-05 at 16:50 +0200, Roberto Sassu wrote:
> > Regarding Mimi's proposal to avoid the issue by extending the PCR with
> > zeros, I think it also achieve the goal.
>
> Roberto, removing the following code from init_digests() would be the
> equivalent to the prior code, without needing to make any other
> changes. Let's keep it simple. Do you want to post the patch with
> the change, or should I?
>
> ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
> if (ret < 0)
> return ret;
> if (ret < TPM_MAX_DIGEST_SIZE)
> return -EFAULT;
>
> As I can't duplicate the problem, it would need to be tested by others
> experiencing the problem.
The bug reporter tested Roberto's last patch:
https://bugzilla.kernel.org/show_bug.cgi?id=203953#c8
We should Cc the reporter on future patches or at least leave another
testing request in the bugzilla.
Tyler
>
> thanks,
>
> Mimi
>
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Roberto Sassu @ 2019-08-05 16:04 UTC (permalink / raw)
To: Mimi Zohar, Tyler Hicks, Jarkko Sakkinen
Cc: jejb, jgg, linux-integrity, linux-security-module, keyrings,
linux-kernel, crazyt2019+lml, nayna, silviu.vlasceanu
In-Reply-To: <1565020459.11223.179.camel@linux.ibm.com>
On 8/5/2019 5:54 PM, Mimi Zohar wrote:
> On Mon, 2019-08-05 at 16:50 +0200, Roberto Sassu wrote:
>> Regarding Mimi's proposal to avoid the issue by extending the PCR with
>> zeros, I think it also achieve the goal.
>
> Roberto, removing the following code from init_digests() would be the
> equivalent to the prior code, without needing to make any other
> changes. Let's keep it simple. Do you want to post the patch with
> the change, or should I?
I will send the new patch.
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Mimi Zohar @ 2019-08-05 15:54 UTC (permalink / raw)
To: Roberto Sassu, Tyler Hicks, Jarkko Sakkinen
Cc: jejb, 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, 2019-08-05 at 16:50 +0200, Roberto Sassu wrote:
> Regarding Mimi's proposal to avoid the issue by extending the PCR with
> zeros, I think it also achieve the goal.
Roberto, removing the following code from init_digests() would be the
equivalent to the prior code, without needing to make any other
changes. Let's keep it simple. Do you want to post the patch with
the change, or should I?
ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
if (ret < 0)
return ret;
if (ret < TPM_MAX_DIGEST_SIZE)
return -EFAULT;
As I can't duplicate the problem, it would need to be tested by others
experiencing the problem.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: allow module init if TPM is inactive or deactivated
From: Roberto Sassu @ 2019-08-05 14:50 UTC (permalink / raw)
To: Tyler Hicks, Jarkko Sakkinen
Cc: jejb, zohar, jgg, linux-integrity, linux-security-module,
keyrings, linux-kernel, crazyt2019+lml, nayna, silviu.vlasceanu
In-Reply-To: <20190802202343.GE26616@elm>
On 8/2/2019 10:23 PM, Tyler Hicks wrote:
> On 2019-08-02 22:42:26, Jarkko Sakkinen wrote:
>> On Fri, Aug 02, 2019 at 09:27:22AM -0500, Tyler Hicks wrote:
>>> On 2019-08-02 10:21:16, Roberto Sassu wrote:
>>>> On 8/1/2019 6:32 PM, Jarkko Sakkinen wrote:
>>>>> On Mon, Jul 15, 2019 at 06:44:28PM +0200, Roberto Sassu wrote:
>>>>>> According to the bug report at https://bugs.archlinux.org/task/62678,
>>>>>> the trusted module is a dependency of the ecryptfs module. We should
>>>>>> load the trusted module even if the TPM is inactive or deactivated.
>>>>>>
>>>>>> Given that commit 782779b60faa ("tpm: Actually fail on TPM errors during
>>>>>> "get random"") changes the return code of tpm_get_random(), the patch
>>>>>> should be modified to ignore the -EIO error. I will send a new version.
>>>>>
>>>>> Do you have information where this dependency comes from?
>>>>
>>>> ecryptfs retrieves the encryption key from encrypted keys (see
>>>> ecryptfs_get_encrypted_key()).
>>>
>>> That has been there for many years with any problems. It was added
>>> in 2011:
>>>
>>> commit 1252cc3b232e582e887623dc5f70979418caaaa2
>>> Author: Roberto Sassu <roberto.sassu@polito.it>
>>> Date: Mon Jun 27 13:45:45 2011 +0200
>>>
>>> eCryptfs: added support for the encrypted key type
>>>
>>> What's recently changed the situation is this patch:
>>>
>>> commit 240730437deb213a58915830884e1a99045624dc
>>> Author: Roberto Sassu <roberto.sassu@huawei.com>
>>> Date: Wed Feb 6 17:24:51 2019 +0100
>>>
>>> KEYS: trusted: explicitly use tpm_chip structure from tpm_default_chip()
>>>
>>> Now eCryptfs has a hard dependency on a TPM chip that's working
>>> as expected even if eCryptfs (or the rest of the system) isn't utilizing
>>> the TPM. If the TPM behaves unexpectedly, you can't access your files.
>>> We need to get this straightened out soon.
>>
>> I agree with this conclusion that eCryptfs needs to be fixed, not
>> another workaround to trusted.ko.
>
> That wasn't the conclusion that I came to. I prefer Robert's proposed
> change to trusted.ko.
>
> How do you propose that this be fixed in eCryptfs?
>
> Removing encrypted_key support from eCryptfs is the only way that I can
> see to fix the bug in eCryptfs. That support has been there since 2011.
> I'm not sure of the number of users that would be broken by removing
> encrypted_key support. I don't think the number is high but I can't say
> that confidently.
>
> Roberto, what was your use case when you added encrypted_key support to
> eCryptfs back then? Are you aware of any users of eCryptfs +
> encrypted_keys?
Well, my use case at that time (I was at the university) was to provide
a secure storage based on TPM. I'm not aware of users of this
functionality, but I believe that it is reasonable (depending on how
trusted keys are used, the secure storage is available only on a
specific platform and if the software configuration is good). This
secure storage would be even more effective if it is coupled with
Mandatory Access Control, to release sensitive data only to the
legitimate applications.
> Jarkko, removing a long-standing feature is potentially more disruptive
> to users than adding a workaround to trusted.ko which already requires a
> similar workaround. I don't think that I agree with you on the proper
> fix here.
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.
The patch I proposed (https://lkml.org/lkml/2019/8/2/1282, I will apply
Tyler's comments) is not invasive and fixes the issue. If the digests
pointer is still NULL, pcrlock() returns an error.
Regarding Mimi's proposal to avoid the issue by extending the PCR with
zeros, I think it also achieve the goal. The purpose of pcrlock() is to
prevent unsealing of sealed data outside the kernel. If PCR values do
not change, sealed data can be unsealed and misused by a user space
process.
Extending the PCR with any value would be sufficient, as the extend
operation (the only way to update PCRs) was designed in a way that
finding the value that would be passed to the extend operation to obtain
again one of the previous PCR values is computationally infeasible.
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
^ permalink raw reply
* Re: [PATCH] ima: Allow to import the blacklisted cert signed by secondary CA cert
From: Mimi Zohar @ 2019-08-05 14:28 UTC (permalink / raw)
To: Jia Zhang, dhowells, dmitry.kasatkin
Cc: keyrings, linux-security-module, linux-integrity, linux-kernel,
Mark D. Baushke, Petko Manolov
In-Reply-To: <d6a27436-3822-3dda-a3be-4e2dfbe04390@linux.alibaba.com>
On Fri, 2019-08-02 at 09:42 +0800, Jia Zhang wrote:
>
> On 2019/8/2 上午6:57, Mimi Zohar wrote:
> > Hi Jia,
> >
> > On Thu, 2019-08-01 at 09:23 +0800, Jia Zhang wrote:
> >> Similar to .ima, the cert imported to .ima_blacklist is able to be
> >> authenticated by a secondary CA cert.
> >>
> >> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> >
> > The IMA blacklist, which is defined as experimental for a reason, was
> > upstreamed prior to the system blacklist. Any reason you're not using
> > the system blacklist? Before making this sort of change, I'd like
> > some input from others.
>
> In our trusted cloud service, the IMA private key is controlled by
> tenant for some reason. Some unprofessional operations made by tenant
> may lead to the leakage of IMA private key. So the need for importing
> the blacklisted is necessary，without system/kexec reboot, on the
> contrary, the system blacklist needs a kernel rebuild and system/kexec
> reboot, without runtime and fine-grained control.
>
> The secondary CA cert has a similar story, but it is not controlled by
> tenant. It is always imported during system/kexec boot to serve
> importing IMA trusted cert and IMA blacklisted cert.
Before expanding the set of keys permitted to blacklist a key on the
IMA keyring, there needs to be a way of differentiating how keys may
be used. The same certificate should not be allowed to be loaded onto
both the IMA and the IMA blacklist keyrings for obvious reasons.
The normal way of blacklisting a key is by using CRLs. I'm not
recommending adding full CRL support in the kernel, but using the key
usage extension to differentiate who may sign a certificate being
black listed. Please refer to section "4.2.1.3. Key Usage", in
particular the cRLSign bit.
thanks,
Mimi
^ permalink raw reply
* Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3
From: Konstantin Ryabitsev @ 2019-08-05 14:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: Micah Morton, linux-security-module, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wgT7Z3kCbKS9Q1rdA=OVxPL32CdBovX=eHvD2PppWCHpQ@mail.gmail.com>
On Sun, Aug 04, 2019 at 10:47:54AM -0700, Linus Torvalds wrote:
> - maybe pr-tracker-bot ignores follow-up emails with "Re:" in the
> subject?
Yes, this is the culprit. Here are the matching regexes:
https://git.kernel.org/pub/scm/linux/kernel/git/mricon/korg-helpers.git/tree/pr-tracker-bot.py#n41
Normally, pull requests don't come in as replies -- this is the first
one that got missed in months, to my knowledge.
I can change it to be more permissive -- the only concern would be that
we would be scanning more bodies, and there *might* be a chance where we
end up tracking the wrong message if someone uses Outlook-style replies
(fully quoted messages without '>') and those replies arrive before the
original message. Both of these are very unlikely to happen (in fact, I
believe using Outlook-style replies on LKML would result in mass
outrage).
So, just let me know if you'd like me to make this change and I'll
modify the regex to be something like '^\S*:?\s*\[GIT' that should match
most permutations of Re/Aw/etc.
-K
^ permalink raw reply
* Re: [PATCH v12 01/11] MODSIGN: Export module signature definitions
From: Mimi Zohar @ 2019-08-05 14:25 UTC (permalink / raw)
To: Philipp Rudo, Thiago Jung Bauermann
Cc: Jessica Yu, linux-integrity, linux-security-module, keyrings,
linux-crypto, linuxppc-dev, linux-doc, linux-kernel,
Dmitry Kasatkin, James Morris, Serge E. Hallyn, David Howells,
David Woodhouse, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Heiko Carstens, linux-s390
In-Reply-To: <20190805151123.12510d72@laptop-ibm>
On Mon, 2019-08-05 at 15:11 +0200, Philipp Rudo wrote:
> Hi Thiago,
>
> > > The patch looks good now.
> >
> > Thanks! Can I add your Reviewed-by?
>
> sorry, for the late answer, but I was on vacation the last two weeks. I hope
> it's not too late now.
>
> Reviewed-by: Philipp Rudo <prudo@linux.ibm.com>
Thanks! This patch set is still in the #next-queued-testing
branch. I'm still hoping for a few more tags, before pushing it out
to the #next-integrity branch later today.
Mimi
^ permalink raw reply
* Re: [PATCH v12 01/11] MODSIGN: Export module signature definitions
From: Philipp Rudo @ 2019-08-05 13:11 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Jessica Yu, linux-integrity, linux-security-module, keyrings,
linux-crypto, linuxppc-dev, linux-doc, linux-kernel, Mimi Zohar,
Dmitry Kasatkin, James Morris, Serge E. Hallyn, David Howells,
David Woodhouse, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Heiko Carstens, linux-s390
In-Reply-To: <8736iw9y00.fsf@morokweng.localdomain>
Hi Thiago,
> > The patch looks good now.
>
> Thanks! Can I add your Reviewed-by?
sorry, for the late answer, but I was on vacation the last two weeks. I hope
it's not too late now.
Reviewed-by: Philipp Rudo <prudo@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH] security/tomoyo: convert put_page() to put_user_page*()
From: Tetsuo Handa @ 2019-08-05 10:01 UTC (permalink / raw)
To: john.hubbard, Andrew Morton; +Cc: John Hubbard, linux-security-module
In-Reply-To: <20190805022626.13291-1-jhubbard@nvidia.com>
On 2019/08/05 11:26, john.hubbard@gmail.com wrote:
> From: John Hubbard <jhubbard@nvidia.com>
>
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
>
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
>
> Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: linux-security-module@vger.kernel.org
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Sending to Andrew Morton means you expect this patch to go via -mm tree, don't you?
Andrew, please take this patch. Thank you.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Song Liu @ 2019-08-05 7:36 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Kees Cook, Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, Lorenz Bauer, Jann Horn, Greg KH, Linux API,
LSM List
In-Reply-To: <CALCETrWtE2U4EvZVYeq8pSmQjBzF2PHH+KxYW8FSeF+W=1FYjw@mail.gmail.com>
Hi Andy,
> On Aug 4, 2019, at 10:47 PM, Andy Lutomirski <luto@kernel.org> wrote:
>
> On Sun, Aug 4, 2019 at 5:08 PM Andy Lutomirski <luto@kernel.org> wrote:
>>
>> On Sun, Aug 4, 2019 at 3:16 PM Andy Lutomirski <luto@kernel.org> wrote:
>>>
>>> On Fri, Aug 2, 2019 at 12:22 AM Song Liu <songliubraving@fb.com> wrote:
>>>>
>>>> Hi Andy,
>>>>
>>>>> I actually agree CAP_BPF_ADMIN makes sense. The hard part is to make
>>>>>> existing tools (setcap, getcap, etc.) and libraries aware of the new CAP.
>>>>>
>>>>> It's been done before -- it's not that hard. IMO the main tricky bit
>>>>> would be try be somewhat careful about defining exactly what
>>>>> CAP_BPF_ADMIN does.
>>>>
>>>> Agreed. I think defining CAP_BPF_ADMIN could be a good topic for the
>>>> Plumbers conference.
>>>>
>>>> OTOH, I don't think we have to wait for CAP_BPF_ADMIN to allow daemons
>>>> like systemd to do sys_bpf() without root.
>>>
>>> I don't understand the use case here. Are you talking about systemd
>>> --user? As far as I know, a user is expected to be able to fully
>>> control their systemd --user process, so giving it unrestricted bpf
>>> access is very close to giving it superuser access, and this doesn't
>>> sound like a good idea. I think that, if systemd --user needs bpf(),
>>> it either needs real unprivileged bpf() or it needs a privileged
>>> helper (SUID or a daemon) to intermediate this access.
>>>
>>>>
>>>>>
>>>>>>> I don't see why you need to invent a whole new mechanism for this.
>>>>>>> The entire cgroup ecosystem outside bpf() does just fine using the
>>>>>>> write permission on files in cgroupfs to control access. Why can't
>>>>>>> bpf() do the same thing?
>>>>>>
>>>>>> It is easier to use write permission for BPF_PROG_ATTACH. But it is
>>>>>> not easy to do the same for other bpf commands: BPF_PROG_LOAD and
>>>>>> BPF_MAP_*. A lot of these commands don't have target concept. Maybe
>>>>>> we should have target concept for all these commands. But that is a
>>>>>> much bigger project. OTOH, "all or nothing" model allows all these
>>>>>> commands at once.
>>>>>
>>>>> For BPF_PROG_LOAD, I admit I've never understood why permission is
>>>>> required at all. I think that CAP_SYS_ADMIN or similar should be
>>>>> needed to get is_priv in the verifier, but I think that should mainly
>>>>> be useful for tracing, and that requires lots of privilege anyway.
>>>>> BPF_MAP_* is probably the trickiest part. One solution would be some
>>>>> kind of bpffs, but I'm sure other solutions are possible.
>>>>
>>>> Improving permission management of cgroup_bpf is another good topic to
>>>> discuss. However, it is also an overkill for current use case.
>>>>
>>>
>>> I looked at the code some more, and I don't think this is so hard
>>> after all. As I understand it, all of the map..by_id stuff is, to
>>> some extent, deprecated in favor of persistent maps. As I see it, the
>>> map..by_id calls should require privilege forever, although I can
>>> imagine ways to scope that privilege to a namespace if the maps
>>> themselves were to be scoped to a namespace.
>>>
>>> Instead, unprivileged tools would use the persistent map interface
>>> roughly like this:
>>>
>>> $ bpftool map create /sys/fs/bpf/my_dir/filename type hash key 8 value
>>> 8 entries 64 name mapname
>>>
>>> This would require that the caller have either CAP_DAC_OVERRIDE or
>>> that the caller have permission to create files in /sys/fs/bpf/my_dir
>>> (using the same rules as for any filesystem), and the resulting map
>>> would end up owned by the creating user and have mode 0600 (or maybe
>>> 0666, or maybe a new bpf_attr parameter) modified by umask. Then all
>>> the various capable() checks that are currently involved in accessing
>>> a persistent map would instead check FMODE_READ or FMODE_WRITE on the
>>> map file as appropriate.
>>>
>>> Half of this stuff already works. I just set my system up like this:
>>>
>>> $ ls -l /sys/fs/bpf
>>> total 0
>>> drwxr-xr-x. 3 luto luto 0 Aug 4 15:10 luto
>>>
>>> $ mkdir /sys/fs/bpf/luto/test
>>>
>>> $ ls -l /sys/fs/bpf/luto
>>> total 0
>>> drwxrwxr-x. 2 luto luto 0 Aug 4 15:10 test
>>>
>>> I bet that making the bpf() syscalls work appropriately in this
>>> context without privilege would only be a couple of hours of work.
>>> The hard work, creating bpffs and making it function, is already done
>>> :)
>>>
>>> P.S. The docs for bpftool create are less than fantastic. The
>>> complete lack of any error message at all when the syscall returns
>>> -EACCES is also not fantastic.
>>
>> This isn't remotely finished, but I spent a bit of time fiddling with this:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=bpf/perms
>>
>> What do you think? (It's obviously not done. It doesn't compile, and
>> I haven't gotten to the permissions needed to do map operations. I
>> also haven't touched the capable() checks.)
>
> I updated the branch. It compiles, and basic map functionality works!
Thanks a lot for trying this out. This is a very interesting direction
that we will explore.
>
> # mount -t bpf bpf /sys/fs/bpf
> # cd /sys/fs/bpf
> # mkdir luto
> # chown luto: luto
> # setpriv --euid=1000 --ruid=1000 bash
> $ pwd
> /sys/fs/bpf
> bash-5.0$ ls -l
> total 0
> drwxr-xr-x 2 luto luto 0 Aug 4 22:41 luto
> bash-5.0$ bpftool map create /sys/fs/bpf/luto/filename type hash key 8
> value 8 entries 64 name mapname
> bash-5.0$ bpftool map dump pinned /sys/fs/bpf/luto/filename
> Found 0 elements
>
> # chown root: /sys/fs/bpf/luto/filename
>
> $ bpftool map dump pinned /sys/fs/bpf/luto/filename
> Error: bpf obj get (/sys/fs/bpf/luto): Permission denied
>
> So I think it's possible to get a respectable subset of bpf()
> functionality working without privilege in short order :)
I think we have two key questions to answer:
1. What subset of bpf() functionality will the users need?
2. Who are the users?
Different answers to these two questions lead to different directions.
In our use case, the answers are
1) almost all bpf() functionality
2) highly trusted users (sudoers)
So our initial approach of /dev/bpf allows all bpf() functionality
in one bit in task_struct. (Yes, we can just sudo. But, we would
rather not use sudo when possible.)
"cgroup management" use case may have answers like:
1) cgroup_bpf only
2) users in their own containers
For this case, getting cgroup_bpf related features (cgroup_bpf progs;
some map types, etc.) work with unprivileged users would be the right
direction.
"USDT tracing" use case may have answers like:
1) uprobe, stockmap, histogram, etc.
2) unprivileged user, w/ or w/o containers
For this case, the first step is likely hacking sys_perf_event_open().
I guess we will need more discussions to decide how to make bpf()
work better for all these (and more) use cases.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-08-05 5:47 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: <CALCETrUjh6DdgW1qSuSRd1_=0F9CqB8+sNj__e_6AHEvh_BaxQ@mail.gmail.com>
On Sun, Aug 4, 2019 at 5:08 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> On Sun, Aug 4, 2019 at 3:16 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On Fri, Aug 2, 2019 at 12:22 AM Song Liu <songliubraving@fb.com> wrote:
> > >
> > > Hi Andy,
> > >
> > >> I actually agree CAP_BPF_ADMIN makes sense. The hard part is to make
> > > >> existing tools (setcap, getcap, etc.) and libraries aware of the new CAP.
> > > >
> > > > It's been done before -- it's not that hard. IMO the main tricky bit
> > > > would be try be somewhat careful about defining exactly what
> > > > CAP_BPF_ADMIN does.
> > >
> > > Agreed. I think defining CAP_BPF_ADMIN could be a good topic for the
> > > Plumbers conference.
> > >
> > > OTOH, I don't think we have to wait for CAP_BPF_ADMIN to allow daemons
> > > like systemd to do sys_bpf() without root.
> >
> > I don't understand the use case here. Are you talking about systemd
> > --user? As far as I know, a user is expected to be able to fully
> > control their systemd --user process, so giving it unrestricted bpf
> > access is very close to giving it superuser access, and this doesn't
> > sound like a good idea. I think that, if systemd --user needs bpf(),
> > it either needs real unprivileged bpf() or it needs a privileged
> > helper (SUID or a daemon) to intermediate this access.
> >
> > >
> > > >
> > > >>> I don't see why you need to invent a whole new mechanism for this.
> > > >>> The entire cgroup ecosystem outside bpf() does just fine using the
> > > >>> write permission on files in cgroupfs to control access. Why can't
> > > >>> bpf() do the same thing?
> > > >>
> > > >> It is easier to use write permission for BPF_PROG_ATTACH. But it is
> > > >> not easy to do the same for other bpf commands: BPF_PROG_LOAD and
> > > >> BPF_MAP_*. A lot of these commands don't have target concept. Maybe
> > > >> we should have target concept for all these commands. But that is a
> > > >> much bigger project. OTOH, "all or nothing" model allows all these
> > > >> commands at once.
> > > >
> > > > For BPF_PROG_LOAD, I admit I've never understood why permission is
> > > > required at all. I think that CAP_SYS_ADMIN or similar should be
> > > > needed to get is_priv in the verifier, but I think that should mainly
> > > > be useful for tracing, and that requires lots of privilege anyway.
> > > > BPF_MAP_* is probably the trickiest part. One solution would be some
> > > > kind of bpffs, but I'm sure other solutions are possible.
> > >
> > > Improving permission management of cgroup_bpf is another good topic to
> > > discuss. However, it is also an overkill for current use case.
> > >
> >
> > I looked at the code some more, and I don't think this is so hard
> > after all. As I understand it, all of the map..by_id stuff is, to
> > some extent, deprecated in favor of persistent maps. As I see it, the
> > map..by_id calls should require privilege forever, although I can
> > imagine ways to scope that privilege to a namespace if the maps
> > themselves were to be scoped to a namespace.
> >
> > Instead, unprivileged tools would use the persistent map interface
> > roughly like this:
> >
> > $ bpftool map create /sys/fs/bpf/my_dir/filename type hash key 8 value
> > 8 entries 64 name mapname
> >
> > This would require that the caller have either CAP_DAC_OVERRIDE or
> > that the caller have permission to create files in /sys/fs/bpf/my_dir
> > (using the same rules as for any filesystem), and the resulting map
> > would end up owned by the creating user and have mode 0600 (or maybe
> > 0666, or maybe a new bpf_attr parameter) modified by umask. Then all
> > the various capable() checks that are currently involved in accessing
> > a persistent map would instead check FMODE_READ or FMODE_WRITE on the
> > map file as appropriate.
> >
> > Half of this stuff already works. I just set my system up like this:
> >
> > $ ls -l /sys/fs/bpf
> > total 0
> > drwxr-xr-x. 3 luto luto 0 Aug 4 15:10 luto
> >
> > $ mkdir /sys/fs/bpf/luto/test
> >
> > $ ls -l /sys/fs/bpf/luto
> > total 0
> > drwxrwxr-x. 2 luto luto 0 Aug 4 15:10 test
> >
> > I bet that making the bpf() syscalls work appropriately in this
> > context without privilege would only be a couple of hours of work.
> > The hard work, creating bpffs and making it function, is already done
> > :)
> >
> > P.S. The docs for bpftool create are less than fantastic. The
> > complete lack of any error message at all when the syscall returns
> > -EACCES is also not fantastic.
>
> This isn't remotely finished, but I spent a bit of time fiddling with this:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=bpf/perms
>
> What do you think? (It's obviously not done. It doesn't compile, and
> I haven't gotten to the permissions needed to do map operations. I
> also haven't touched the capable() checks.)
I updated the branch. It compiles, and basic map functionality works!
# mount -t bpf bpf /sys/fs/bpf
# cd /sys/fs/bpf
# mkdir luto
# chown luto: luto
# setpriv --euid=1000 --ruid=1000 bash
$ pwd
/sys/fs/bpf
bash-5.0$ ls -l
total 0
drwxr-xr-x 2 luto luto 0 Aug 4 22:41 luto
bash-5.0$ bpftool map create /sys/fs/bpf/luto/filename type hash key 8
value 8 entries 64 name mapname
bash-5.0$ bpftool map dump pinned /sys/fs/bpf/luto/filename
Found 0 elements
# chown root: /sys/fs/bpf/luto/filename
$ bpftool map dump pinned /sys/fs/bpf/luto/filename
Error: bpf obj get (/sys/fs/bpf/luto): Permission denied
So I think it's possible to get a respectable subset of bpf()
functionality working without privilege in short order :)
(FWIW, a decent fraction of this probably works even without my
patches, but it's going to have nonsensical semantics and may fail for
silly reasons.)
^ permalink raw reply
* Re: [RFC/RFT v2 1/2] KEYS: trusted: create trusted keys subsystem
From: Sumit Garg @ 2019-08-05 5:02 UTC (permalink / raw)
To: Jarkko Sakkinen
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: <20190802193802.jn56jhoz5crebggt@linux.intel.com>
On Sat, 3 Aug 2019 at 01:08, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Fri, Aug 02, 2019 at 11:20:09AM +0530, Sumit Garg wrote:
> > On Thu, 1 Aug 2019 at 22:54, Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Thu, Jul 18, 2019 at 04:54:45PM +0530, Sumit Garg wrote:
> > > > 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>
> > >
> > > What about TPM2 trusted keys code?
> >
> > Isn't TPM2 code located here: "drivers/char/tpm/"? Would you like to
> > move that code under trusted keys subsystem only?
>
> Yeah, we need a design decision here. What I care is that they should
> be in the same subsystem. I did trusted keys directly to TPM 2.0 subsys
> because the subsystem was not too robust back then.
>
> Right now I think it would be feasible to implement TPM2 trusted keys
> outside TPM driver since the whole transmit functionality is way more
> robust.
>
Okay, I will try to move TPM2 trusted keys code also.
-Sumit
> /Jarkko
^ permalink raw reply
* [PATCH] security/tomoyo: convert put_page() to put_user_page*()
From: john.hubbard @ 2019-08-05 2:26 UTC (permalink / raw)
To: Andrew Morton
Cc: Christoph Hellwig, Ira Weiny, Jan Kara, Jason Gunthorpe,
Jerome Glisse, LKML, linux-mm, linux-fsdevel, John Hubbard,
Kentaro Takeda, Tetsuo Handa, linux-security-module
From: John Hubbard <jhubbard@nvidia.com>
For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().
This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").
Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
security/tomoyo/domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c
index 8526a0a74023..6887beecfb6e 100644
--- a/security/tomoyo/domain.c
+++ b/security/tomoyo/domain.c
@@ -931,7 +931,7 @@ bool tomoyo_dump_page(struct linux_binprm *bprm, unsigned long pos,
}
/* Same with put_arg_page(page) in fs/exec.c */
#ifdef CONFIG_MMU
- put_page(page);
+ put_user_page(page);
#endif
return true;
}
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-08-05 0:08 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: <CALCETrU7NbBnXXsw1B+DvTkfTVRBFWXuJ8cZERCCNvdFG6KqRw@mail.gmail.com>
On Sun, Aug 4, 2019 at 3:16 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> On Fri, Aug 2, 2019 at 12:22 AM Song Liu <songliubraving@fb.com> wrote:
> >
> > Hi Andy,
> >
> >> I actually agree CAP_BPF_ADMIN makes sense. The hard part is to make
> > >> existing tools (setcap, getcap, etc.) and libraries aware of the new CAP.
> > >
> > > It's been done before -- it's not that hard. IMO the main tricky bit
> > > would be try be somewhat careful about defining exactly what
> > > CAP_BPF_ADMIN does.
> >
> > Agreed. I think defining CAP_BPF_ADMIN could be a good topic for the
> > Plumbers conference.
> >
> > OTOH, I don't think we have to wait for CAP_BPF_ADMIN to allow daemons
> > like systemd to do sys_bpf() without root.
>
> I don't understand the use case here. Are you talking about systemd
> --user? As far as I know, a user is expected to be able to fully
> control their systemd --user process, so giving it unrestricted bpf
> access is very close to giving it superuser access, and this doesn't
> sound like a good idea. I think that, if systemd --user needs bpf(),
> it either needs real unprivileged bpf() or it needs a privileged
> helper (SUID or a daemon) to intermediate this access.
>
> >
> > >
> > >>> I don't see why you need to invent a whole new mechanism for this.
> > >>> The entire cgroup ecosystem outside bpf() does just fine using the
> > >>> write permission on files in cgroupfs to control access. Why can't
> > >>> bpf() do the same thing?
> > >>
> > >> It is easier to use write permission for BPF_PROG_ATTACH. But it is
> > >> not easy to do the same for other bpf commands: BPF_PROG_LOAD and
> > >> BPF_MAP_*. A lot of these commands don't have target concept. Maybe
> > >> we should have target concept for all these commands. But that is a
> > >> much bigger project. OTOH, "all or nothing" model allows all these
> > >> commands at once.
> > >
> > > For BPF_PROG_LOAD, I admit I've never understood why permission is
> > > required at all. I think that CAP_SYS_ADMIN or similar should be
> > > needed to get is_priv in the verifier, but I think that should mainly
> > > be useful for tracing, and that requires lots of privilege anyway.
> > > BPF_MAP_* is probably the trickiest part. One solution would be some
> > > kind of bpffs, but I'm sure other solutions are possible.
> >
> > Improving permission management of cgroup_bpf is another good topic to
> > discuss. However, it is also an overkill for current use case.
> >
>
> I looked at the code some more, and I don't think this is so hard
> after all. As I understand it, all of the map..by_id stuff is, to
> some extent, deprecated in favor of persistent maps. As I see it, the
> map..by_id calls should require privilege forever, although I can
> imagine ways to scope that privilege to a namespace if the maps
> themselves were to be scoped to a namespace.
>
> Instead, unprivileged tools would use the persistent map interface
> roughly like this:
>
> $ bpftool map create /sys/fs/bpf/my_dir/filename type hash key 8 value
> 8 entries 64 name mapname
>
> This would require that the caller have either CAP_DAC_OVERRIDE or
> that the caller have permission to create files in /sys/fs/bpf/my_dir
> (using the same rules as for any filesystem), and the resulting map
> would end up owned by the creating user and have mode 0600 (or maybe
> 0666, or maybe a new bpf_attr parameter) modified by umask. Then all
> the various capable() checks that are currently involved in accessing
> a persistent map would instead check FMODE_READ or FMODE_WRITE on the
> map file as appropriate.
>
> Half of this stuff already works. I just set my system up like this:
>
> $ ls -l /sys/fs/bpf
> total 0
> drwxr-xr-x. 3 luto luto 0 Aug 4 15:10 luto
>
> $ mkdir /sys/fs/bpf/luto/test
>
> $ ls -l /sys/fs/bpf/luto
> total 0
> drwxrwxr-x. 2 luto luto 0 Aug 4 15:10 test
>
> I bet that making the bpf() syscalls work appropriately in this
> context without privilege would only be a couple of hours of work.
> The hard work, creating bpffs and making it function, is already done
> :)
>
> P.S. The docs for bpftool create are less than fantastic. The
> complete lack of any error message at all when the syscall returns
> -EACCES is also not fantastic.
This isn't remotely finished, but I spent a bit of time fiddling with this:
https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=bpf/perms
What do you think? (It's obviously not done. It doesn't compile, and
I haven't gotten to the permissions needed to do map operations. I
also haven't touched the capable() checks.)
^ permalink raw reply
* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
From: Andy Lutomirski @ 2019-08-04 23:55 UTC (permalink / raw)
To: Jan Kara, Song Liu, Alexei Starovoitov, Daniel Borkmann
Cc: Mickaël Salaün, LKML, Al Viro, James Morris,
Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
Shuah Khan, Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
Kernel Hardening, Linux API, LSM List, Linux FS Devel,
Steve Grubb, Matthew Bobrowski
In-Reply-To: <20181212144306.GA19945@quack2.suse.cz>
On Wed, Dec 12, 2018 at 6:43 AM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
> > When the O_MAYEXEC flag is passed, sys_open() may be subject to
> > additional restrictions depending on a security policy implemented by an
> > LSM through the inode_permission hook.
> >
> > The underlying idea is to be able to restrict scripts interpretation
> > according to a policy defined by the system administrator. For this to
> > be possible, script interpreters must use the O_MAYEXEC flag
> > appropriately. To be fully effective, these interpreters also need to
> > handle the other ways to execute code (for which the kernel can't help):
> > command line parameters (e.g., option -e for Perl), module loading
> > (e.g., option -m for Python), stdin, file sourcing, environment
> > variables, configuration files... According to the threat model, it may
> > be acceptable to allow some script interpreters (e.g. Bash) to interpret
> > commands from stdin, may it be a TTY or a pipe, because it may not be
> > enough to (directly) perform syscalls.
> >
> > A simple security policy implementation is available in a following
> > patch for Yama.
> >
> > This is an updated subset of the patch initially written by Vincent
> > Strubel for CLIP OS:
> > https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> > This patch has been used for more than 10 years with customized script
> > interpreters. Some examples can be found here:
> > https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> >
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> > Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
> > Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>
> ...
>
> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
> > if (flags & O_APPEND)
> > acc_mode |= MAY_APPEND;
> >
> > + /* Check execution permissions on open. */
> > + if (flags & O_MAYEXEC)
> > + acc_mode |= MAY_OPENEXEC;
> > +
> > op->acc_mode = acc_mode;
> >
> > op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...
>
I would really like to land this patch. I'm fiddling with making
bpffs handle permissions intelligently, and the lack of a way to say
"hey, I want to open this bpf program so that I can run it" is
annoying.
--Andy
^ permalink raw reply
* Re: [RFC PATCH v3 04/12] x86/sgx: Require userspace to define enclave pages' protection bits
From: Andy Lutomirski @ 2019-08-04 22:20 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: <20190801163839.wvcnq57hity4wwrk@linux.intel.com>
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.
^ 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