* Re: [Tee-dev] [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Sumit Garg @ 2019-08-01 10:27 UTC (permalink / raw)
To: Janne Karhunen
Cc: Rouven Czerwinski, tee-dev @ lists . linaro . org,
Daniel Thompson, Jonathan Corbet, jejb, Ard Biesheuvel,
Linux Doc Mailing List, Jarkko Sakkinen,
Linux Kernel Mailing List, dhowells, linux-security-module,
keyrings, Mimi Zohar, Casey Schaufler, linux-integrity,
linux-arm-kernel, Serge E. Hallyn
In-Reply-To: <CAE=NcrbujsM8wYJXq+s=o5Vy1xY1b0uKYBGvp6UP5ex70HrB2Q@mail.gmail.com>
On Thu, 1 Aug 2019 at 14:00, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Thu, Aug 1, 2019 at 10:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > > Anyway, just my .02c. I guess having any new support in the kernel for
> > > new trust sources is good and improvement from the current state. I
> > > can certainly make my stuff work with your setup as well, what ever
> > > people think is the best.
> >
> > Yes your implementation can very well fit under trusted keys
> > abstraction framework without creating a new keytype: "ext-trusted".
>
> The fundamental problem with the 'standardized kernel tee' still
> exists - it will never be generic in real life. Getting all this in
> the kernel will solve your problem and sell this particular product,
> but it is quite unlikely to help that many users. If the security is
> truly important to you, would you really trust any of this code to
> someone else? In this day and age, I really doubt many do.
There are already multiple platforms supported by OP-TEE [1] which
could benefit from this trusted keys interface.
> Everyone
> does their own thing, so this is why I really see all that as a
> userspace problem.
>
IMO, we should try to use standardized interfaces which are well
thought off rather than implementing your own.
[1] https://optee.readthedocs.io/general/platforms.html
-Sumit
>
> --
> Janne
^ permalink raw reply
* Re: [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Sumit Garg @ 2019-08-01 10:00 UTC (permalink / raw)
To: Janne Karhunen
Cc: keyrings, linux-integrity, linux-security-module, Jens Wiklander,
Jonathan Corbet, dhowells, jejb, Jarkko Sakkinen, Mimi Zohar,
James Morris, Serge E. Hallyn, Casey Schaufler, Ard Biesheuvel,
Daniel Thompson, Linux Doc Mailing List,
Linux Kernel Mailing List, linux-arm-kernel,
tee-dev @ lists . linaro . org
In-Reply-To: <CAE=Ncrae6pM+WBDu9eJ7Fw2Fkvf3_YqH5tj9Tt938D4RtWcdSQ@mail.gmail.com>
On Thu, 1 Aug 2019 at 13:30, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Thu, Aug 1, 2019 at 10:40 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > > I chose the userspace plugin due to this, you can use userspace aids
> > > to provide any type of service. Use the crypto library you desire to
> > > do the magic you want.
> >
> > Here TEE isn't similar to a user-space crypto library. In our case TEE
> > is based on ARM TrustZone which only allows TEE communications to be
> > initiated from privileged mode. So why would you like to route
> > communications via user-mode (which is less secure) when we have
> > standardised TEE interface available in kernel?
>
> The physical access guards for reading/writing the involved critical
> memory are identical as far as I know? Layered security is generally a
> good thing, and the userspace pass actually adds a layer, so not sure
> which is really safer?
>
AFAIK, layered security is better in case we move from lower privilege
level to higher privilege level rather than in reverse order.
-Sumit
> In my case the rerouting was to done generalize it. Any type of trust
> source, anywhere.
>
>
> > > > Isn't actual purpose to have trusted keys is to protect user-space
> > > > from access to kernel keys in plain format? Doesn't user mode helper
> > > > defeat that purpose in one way or another?
> > >
> > > Not really. CPU is in the user mode while running the code, but the
> > > code or the secure keydata being is not available to the 'normal'
> > > userspace. It's like microkernel service/driver this way. The usermode
> > > driver is part of the kernel image and it runs on top of a invisible
> > > rootfs.
> >
> > Can you elaborate here with an example regarding how this user-mode
> > helper will securely communicate with a hardware based trust source
> > with other user-space processes denied access to that trust source?
>
> The other user mode processes will never see the device node to open.
> There is none in existence for them; it only exists in the ramfs based
> root for the user mode helper.
>
>
> --
> Janne
^ permalink raw reply
* Re: [Tee-dev] [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Janne Karhunen @ 2019-08-01 8:30 UTC (permalink / raw)
To: Sumit Garg
Cc: Rouven Czerwinski, tee-dev @ lists . linaro . org,
Daniel Thompson, Jonathan Corbet, jejb, Ard Biesheuvel,
Linux Doc Mailing List, Jarkko Sakkinen,
Linux Kernel Mailing List, dhowells, linux-security-module,
keyrings, Mimi Zohar, Casey Schaufler, linux-integrity,
linux-arm-kernel, Serge E. Hallyn
In-Reply-To: <CAFA6WYPt4q+jaJbaoauXKr2qKgBHvtQ663s4t=W3nuPJPe2xpw@mail.gmail.com>
On Thu, Aug 1, 2019 at 10:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> > Anyway, just my .02c. I guess having any new support in the kernel for
> > new trust sources is good and improvement from the current state. I
> > can certainly make my stuff work with your setup as well, what ever
> > people think is the best.
>
> Yes your implementation can very well fit under trusted keys
> abstraction framework without creating a new keytype: "ext-trusted".
The fundamental problem with the 'standardized kernel tee' still
exists - it will never be generic in real life. Getting all this in
the kernel will solve your problem and sell this particular product,
but it is quite unlikely to help that many users. If the security is
truly important to you, would you really trust any of this code to
someone else? In this day and age, I really doubt many do. Everyone
does their own thing, so this is why I really see all that as a
userspace problem.
--
Janne
^ permalink raw reply
* Re: [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Janne Karhunen @ 2019-08-01 7:59 UTC (permalink / raw)
To: Sumit Garg
Cc: keyrings, linux-integrity, linux-security-module, Jens Wiklander,
Jonathan Corbet, dhowells, jejb, Jarkko Sakkinen, Mimi Zohar,
James Morris, Serge E. Hallyn, Casey Schaufler, Ard Biesheuvel,
Daniel Thompson, Linux Doc Mailing List,
Linux Kernel Mailing List, linux-arm-kernel,
tee-dev @ lists . linaro . org
In-Reply-To: <CAFA6WYMOXQbL5OeheFUFpTr8gte8XHHr-71-h8+qX0+R_sekDQ@mail.gmail.com>
On Thu, Aug 1, 2019 at 10:40 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> > I chose the userspace plugin due to this, you can use userspace aids
> > to provide any type of service. Use the crypto library you desire to
> > do the magic you want.
>
> Here TEE isn't similar to a user-space crypto library. In our case TEE
> is based on ARM TrustZone which only allows TEE communications to be
> initiated from privileged mode. So why would you like to route
> communications via user-mode (which is less secure) when we have
> standardised TEE interface available in kernel?
The physical access guards for reading/writing the involved critical
memory are identical as far as I know? Layered security is generally a
good thing, and the userspace pass actually adds a layer, so not sure
which is really safer?
In my case the rerouting was to done generalize it. Any type of trust
source, anywhere.
> > > Isn't actual purpose to have trusted keys is to protect user-space
> > > from access to kernel keys in plain format? Doesn't user mode helper
> > > defeat that purpose in one way or another?
> >
> > Not really. CPU is in the user mode while running the code, but the
> > code or the secure keydata being is not available to the 'normal'
> > userspace. It's like microkernel service/driver this way. The usermode
> > driver is part of the kernel image and it runs on top of a invisible
> > rootfs.
>
> Can you elaborate here with an example regarding how this user-mode
> helper will securely communicate with a hardware based trust source
> with other user-space processes denied access to that trust source?
The other user mode processes will never see the device node to open.
There is none in existence for them; it only exists in the ramfs based
root for the user mode helper.
--
Janne
^ permalink raw reply
* Re: [Tee-dev] [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Sumit Garg @ 2019-08-01 7:58 UTC (permalink / raw)
To: Janne Karhunen
Cc: Rouven Czerwinski, tee-dev @ lists . linaro . org,
Daniel Thompson, Jonathan Corbet, jejb, Ard Biesheuvel,
Linux Doc Mailing List, Jarkko Sakkinen,
Linux Kernel Mailing List, dhowells, linux-security-module,
keyrings, Mimi Zohar, Casey Schaufler, linux-integrity,
linux-arm-kernel, Serge E. Hallyn
In-Reply-To: <CAE=NcraqD9FNM0Gk9UGhPGi3heVzZrAKGc1gNZxoe1OnDaQ=pA@mail.gmail.com>
On Thu, 1 Aug 2019 at 13:00, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Thu, Aug 1, 2019 at 9:50 AM Rouven Czerwinski
> <r.czerwinski@pengutronix.de> wrote:
>
> > > I'm aware of it - I have implemented a large part of the GP TEE APIs
> > > earlier (primarily the crypto functions). Does the TEE you work with
> > > actually support GP properly? Can I take a look at the code?
> >
> > AFAIK Sumit is working with the OP-TEE implementation, which can be
> > found on github: https://github.com/op-tee/optee_os
>
> Thanks, I will take a look.
For documentation, refer to: https://optee.readthedocs.io/
> The fundamental problem with these things
> is that there are infinite amount of ways how TEEs and ROTs can be
> done in terms of the hardware and software. I really doubt there are 2
> implementations in existence that are even remotely compatible in real
> life.
I agree with you regarding implementation specific nature of TEE but
having a standardized client interface does solves the problem.
> As such, all things TEE/ROT would logically really belong in the
> userland and thanks to the bpfilter folks now the umh logic really
> makes that possible ... I think. The key implementation I did was just
> an RFC on the concept, what if we start to move the stuff that really
> belongs in the userspace to this pseudo-userland. It's not kernel, but
> it's not commonly accessible userland either. The shared memory would
> also work without any modifications between the umh based TEE/ROT
> driver and the userland if needed.
>
> Anyway, just my .02c. I guess having any new support in the kernel for
> new trust sources is good and improvement from the current state. I
> can certainly make my stuff work with your setup as well, what ever
> people think is the best.
Yes your implementation can very well fit under trusted keys
abstraction framework without creating a new keytype: "ext-trusted".
-Sumit
>
>
> --
> Janne
^ permalink raw reply
* Re: [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Sumit Garg @ 2019-08-01 7:40 UTC (permalink / raw)
To: Janne Karhunen
Cc: keyrings, linux-integrity, linux-security-module, Jens Wiklander,
Jonathan Corbet, dhowells, jejb, Jarkko Sakkinen, Mimi Zohar,
James Morris, Serge E. Hallyn, Casey Schaufler, Ard Biesheuvel,
Daniel Thompson, Linux Doc Mailing List,
Linux Kernel Mailing List, linux-arm-kernel,
tee-dev @ lists . linaro . org
In-Reply-To: <CAE=NcraDkm5cxE=ceq_9XkQz=NZ6KdVXkNUsdD4G2LrWz-bpDw@mail.gmail.com>
On Thu, 1 Aug 2019 at 11:51, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Wed, Jul 31, 2019 at 4:58 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > > To clarify a bit further - my thought was to support any type of trust
> > > source.
> >
> > That could be very well accomplished via Trusted Keys abstraction
> > framework [1]. A trust source just need to implement following APIs:
> >
> > struct trusted_key_ops ts_trusted_key_ops = {
> > .migratable = 0, /* non-migratable */
> > .init = init_ts_trusted,
> > .seal = ts_key_seal,
> > .unseal = ts_key_unseal,
> > .get_random = ts_get_random,
> > .cleanup = cleanup_ts_trusted,
> > };
>
> Which is basically the same as implementing a new keytype in the
> kernel; abstraction is not raised in any considerable manner this way?
>
It doesn't create a new keytype. There is only single keytype:
"trusted" which could be implemented via one of the trust source
available in the system like TPM, TEE etc.
> I chose the userspace plugin due to this, you can use userspace aids
> to provide any type of service. Use the crypto library you desire to
> do the magic you want.
Here TEE isn't similar to a user-space crypto library. In our case TEE
is based on ARM TrustZone which only allows TEE communications to be
initiated from privileged mode. So why would you like to route
communications via user-mode (which is less secure) when we have
standardised TEE interface available in kernel?
>
>
> > > With the
> > > user mode helper in between anyone can easily add their own thing in
> > > there.
> >
> > Isn't actual purpose to have trusted keys is to protect user-space
> > from access to kernel keys in plain format? Doesn't user mode helper
> > defeat that purpose in one way or another?
>
> Not really. CPU is in the user mode while running the code, but the
> code or the secure keydata being is not available to the 'normal'
> userspace. It's like microkernel service/driver this way. The usermode
> driver is part of the kernel image and it runs on top of a invisible
> rootfs.
>
Can you elaborate here with an example regarding how this user-mode
helper will securely communicate with a hardware based trust source
with other user-space processes denied access to that trust source?
-Sumit
>
> --
> Janne
^ permalink raw reply
* Re: [Tee-dev] [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Janne Karhunen @ 2019-08-01 7:30 UTC (permalink / raw)
To: Rouven Czerwinski
Cc: Sumit Garg, tee-dev @ lists . linaro . org, Daniel Thompson,
Jonathan Corbet, jejb, Ard Biesheuvel, Linux Doc Mailing List,
Jarkko Sakkinen, Linux Kernel Mailing List, dhowells,
linux-security-module, keyrings, Mimi Zohar, Casey Schaufler,
linux-integrity, linux-arm-kernel, Serge E. Hallyn
In-Reply-To: <19d9be198619e951750dedeb4d0a7f372083b42c.camel@pengutronix.de>
On Thu, Aug 1, 2019 at 9:50 AM Rouven Czerwinski
<r.czerwinski@pengutronix.de> wrote:
> > I'm aware of it - I have implemented a large part of the GP TEE APIs
> > earlier (primarily the crypto functions). Does the TEE you work with
> > actually support GP properly? Can I take a look at the code?
>
> AFAIK Sumit is working with the OP-TEE implementation, which can be
> found on github: https://github.com/op-tee/optee_os
Thanks, I will take a look. The fundamental problem with these things
is that there are infinite amount of ways how TEEs and ROTs can be
done in terms of the hardware and software. I really doubt there are 2
implementations in existence that are even remotely compatible in real
life. As such, all things TEE/ROT would logically really belong in the
userland and thanks to the bpfilter folks now the umh logic really
makes that possible ... I think. The key implementation I did was just
an RFC on the concept, what if we start to move the stuff that really
belongs in the userspace to this pseudo-userland. It's not kernel, but
it's not commonly accessible userland either. The shared memory would
also work without any modifications between the umh based TEE/ROT
driver and the userland if needed.
Anyway, just my .02c. I guess having any new support in the kernel for
new trust sources is good and improvement from the current state. I
can certainly make my stuff work with your setup as well, what ever
people think is the best.
--
Janne
^ permalink raw reply
* Re: [Tee-dev] [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Rouven Czerwinski @ 2019-08-01 6:50 UTC (permalink / raw)
To: Janne Karhunen, Sumit Garg
Cc: tee-dev @ lists . linaro . org, Daniel Thompson, Jonathan Corbet,
jejb, Ard Biesheuvel, Linux Doc Mailing List, Jarkko Sakkinen,
Linux Kernel Mailing List, dhowells, linux-security-module,
keyrings, Mimi Zohar, Casey Schaufler, linux-integrity,
linux-arm-kernel, Serge E. Hallyn
In-Reply-To: <CAE=NcrYz8bT9zDhS_ZcvY84fpeTDxZ-KhJKeQGGyf=o4pG2J-Q@mail.gmail.com>
On Thu, 2019-08-01 at 09:36 +0300, Janne Karhunen wrote:
> On Wed, Jul 31, 2019 at 5:23 PM Sumit Garg <sumit.garg@linaro.org>
> wrote:
>
> > > I guess my wording was wrong, tried to say that physical TEEs in
> > > the
> > > wild vary massively hardware wise. Generalizing these things is
> > > rough.
> > >
> >
> > There are already well defined GlobalPlatform Standards to
> > generalize
> > the TEE interface. One of them is GlobalPlatform TEE Client API [1]
> > which provides the basis for this TEE interface.
>
> I'm aware of it - I have implemented a large part of the GP TEE APIs
> earlier (primarily the crypto functions). Does the TEE you work with
> actually support GP properly? Can I take a look at the code?
AFAIK Sumit is working with the OP-TEE implementation, which can be
found on github: https://github.com/op-tee/optee_os
Regards,
Rouven
^ permalink raw reply
* Re: [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Janne Karhunen @ 2019-08-01 6:36 UTC (permalink / raw)
To: Sumit Garg
Cc: keyrings, linux-integrity, linux-security-module, Jens Wiklander,
Jonathan Corbet, dhowells, jejb, Jarkko Sakkinen, Mimi Zohar,
James Morris, Serge E. Hallyn, Casey Schaufler, Ard Biesheuvel,
Daniel Thompson, Linux Doc Mailing List,
Linux Kernel Mailing List, linux-arm-kernel,
tee-dev @ lists . linaro . org
In-Reply-To: <CAFA6WYOKcOzSwakHhgshZcebD8ZBMSi7xQdjWYFS79=Xc+odOg@mail.gmail.com>
On Wed, Jul 31, 2019 at 5:23 PM Sumit Garg <sumit.garg@linaro.org> wrote:
> > I guess my wording was wrong, tried to say that physical TEEs in the
> > wild vary massively hardware wise. Generalizing these things is rough.
> >
>
> There are already well defined GlobalPlatform Standards to generalize
> the TEE interface. One of them is GlobalPlatform TEE Client API [1]
> which provides the basis for this TEE interface.
I'm aware of it - I have implemented a large part of the GP TEE APIs
earlier (primarily the crypto functions). Does the TEE you work with
actually support GP properly? Can I take a look at the code?
Normally the TEE implementations are well-guarded secrets and the
state of the implementation is quite random. In many cases keeping
things secret is fine from my point of view, given that it is a RoT
after all. The secrecy is the core business here. So, this is why I
opted the userspace 'secret' route - no secrets in the kernel, but
it's fine for the userspace. Umh was a logical fit to implement it.
--
Janne
^ permalink raw reply
* Re: [RFC v2 0/6] Introduce TEE based Trusted Keys support
From: Janne Karhunen @ 2019-08-01 6:21 UTC (permalink / raw)
To: Sumit Garg
Cc: keyrings, linux-integrity, linux-security-module, Jens Wiklander,
Jonathan Corbet, dhowells, jejb, Jarkko Sakkinen, Mimi Zohar,
James Morris, Serge E. Hallyn, Casey Schaufler, Ard Biesheuvel,
Daniel Thompson, Linux Doc Mailing List,
Linux Kernel Mailing List, linux-arm-kernel,
tee-dev @ lists . linaro . org
In-Reply-To: <CAFA6WYOEqe1a1DCyVYKA+oZaZ0n5hnjxdubstUnrwdUW1-4xHw@mail.gmail.com>
On Wed, Jul 31, 2019 at 4:58 PM Sumit Garg <sumit.garg@linaro.org> wrote:
> > To clarify a bit further - my thought was to support any type of trust
> > source.
>
> That could be very well accomplished via Trusted Keys abstraction
> framework [1]. A trust source just need to implement following APIs:
>
> struct trusted_key_ops ts_trusted_key_ops = {
> .migratable = 0, /* non-migratable */
> .init = init_ts_trusted,
> .seal = ts_key_seal,
> .unseal = ts_key_unseal,
> .get_random = ts_get_random,
> .cleanup = cleanup_ts_trusted,
> };
Which is basically the same as implementing a new keytype in the
kernel; abstraction is not raised in any considerable manner this way?
I chose the userspace plugin due to this, you can use userspace aids
to provide any type of service. Use the crypto library you desire to
do the magic you want.
> > With the
> > user mode helper in between anyone can easily add their own thing in
> > there.
>
> Isn't actual purpose to have trusted keys is to protect user-space
> from access to kernel keys in plain format? Doesn't user mode helper
> defeat that purpose in one way or another?
Not really. CPU is in the user mode while running the code, but the
code or the secure keydata being is not available to the 'normal'
userspace. It's like microkernel service/driver this way. The usermode
driver is part of the kernel image and it runs on top of a invisible
rootfs.
--
Janne
^ permalink raw reply
* [PATCH] ima: Allow to import the blacklisted cert signed by secondary CA cert
From: Jia Zhang @ 2019-08-01 1:23 UTC (permalink / raw)
To: dhowells, zohar, dmitry.kasatkin
Cc: keyrings, linux-security-module, linux-integrity, linux-kernel,
zhang.jia
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>
---
include/keys/system_keyring.h | 6 ++++++
security/integrity/digsig.c | 6 ------
security/integrity/ima/ima_mok.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index c1a96fd..7dc91db 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -31,6 +31,12 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
#define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
#endif
+#ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
+#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
+#else
+#define restrict_link_to_ima restrict_link_by_builtin_trusted
+#endif
+
#ifdef CONFIG_SYSTEM_BLACKLIST_KEYRING
extern int mark_hash_blacklisted(const char *hash);
extern int is_hash_blacklisted(const u8 *hash, size_t hash_len,
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 868ade3..c6f3384 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -33,12 +33,6 @@
".platform",
};
-#ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
-#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
-#else
-#define restrict_link_to_ima restrict_link_by_builtin_trusted
-#endif
-
int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
const char *digest, int digestlen)
{
diff --git a/security/integrity/ima/ima_mok.c b/security/integrity/ima/ima_mok.c
index 36cadad..6d0b12d 100644
--- a/security/integrity/ima/ima_mok.c
+++ b/security/integrity/ima/ima_mok.c
@@ -31,7 +31,7 @@ __init int ima_mok_init(void)
if (!restriction)
panic("Can't allocate IMA blacklist restriction.");
- restriction->check = restrict_link_by_builtin_trusted;
+ restriction->check = restrict_link_to_ima;
ima_blacklist_keyring = keyring_alloc(".ima_blacklist",
KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
From: Paul Moore @ 2019-08-01 0:27 UTC (permalink / raw)
To: Casey Schaufler
Cc: Aaron Goidel, selinux, linux-security-module, linux-fsdevel,
dhowells, jack, amir73il, James Morris, Stephen Smalley,
linux-kernel
In-Reply-To: <1c62c931-9441-4264-c119-d038b2d0c9b9@schaufler-ca.com>
On Wed, Jul 31, 2019 at 1:26 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/31/2019 8:34 AM, Aaron Goidel wrote:
> > As of now, setting watches on filesystem objects has, at most, applied a
> > check for read access to the inode, and in the case of fanotify, requires
> > CAP_SYS_ADMIN. No specific security hook or permission check has been
> > provided to control the setting of watches. Using any of inotify, dnotify,
> > or fanotify, it is possible to observe, not only write-like operations, but
> > even read access to a file. Modeling the watch as being merely a read from
> > the file is insufficient for the needs of SELinux. This is due to the fact
> > that read access should not necessarily imply access to information about
> > when another process reads from a file. Furthermore, fanotify watches grant
> > more power to an application in the form of permission events. While
> > notification events are solely, unidirectional (i.e. they only pass
> > information to the receiving application), permission events are blocking.
> > Permission events make a request to the receiving application which will
> > then reply with a decision as to whether or not that action may be
> > completed. This causes the issue of the watching application having the
> > ability to exercise control over the triggering process. Without drawing a
> > distinction within the permission check, the ability to read would imply
> > the greater ability to control an application. Additionally, mount and
> > superblock watches apply to all files within the same mount or superblock.
> > Read access to one file should not necessarily imply the ability to watch
> > all files accessed within a given mount or superblock.
> >
> > In order to solve these issues, a new LSM hook is implemented and has been
> > placed within the system calls for marking filesystem objects with inotify,
> > fanotify, and dnotify watches. These calls to the hook are placed at the
> > point at which the target path has been resolved and are provided with the
> > path struct, the mask of requested notification events, and the type of
> > object on which the mark is being set (inode, superblock, or mount). The
> > mask and obj_type have already been translated into common FS_* values
> > shared by the entirety of the fs notification infrastructure. The path
> > struct is passed rather than just the inode so that the mount is available,
> > particularly for mount watches. This also allows for use of the hook by
> > pathname-based security modules. However, since the hook is intended for
> > use even by inode based security modules, it is not placed under the
> > CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security
> > modules would need to enable all of the path hooks, even though they do not
> > use any of them.
> >
> > This only provides a hook at the point of setting a watch, and presumes
> > that permission to set a particular watch implies the ability to receive
> > all notification about that object which match the mask. This is all that
> > is required for SELinux. If other security modules require additional hooks
> > or infrastructure to control delivery of notification, these can be added
> > by them. It does not make sense for us to propose hooks for which we have
> > no implementation. The understanding that all notifications received by the
> > requesting application are all strictly of a type for which the application
> > has been granted permission shows that this implementation is sufficient in
> > its coverage.
> >
> > Security modules wishing to provide complete control over fanotify must
> > also implement a security_file_open hook that validates that the access
> > requested by the watching application is authorized. Fanotify has the issue
> > that it returns a file descriptor with the file mode specified during
> > fanotify_init() to the watching process on event. This is already covered
> > by the LSM security_file_open hook if the security module implements
> > checking of the requested file mode there. Otherwise, a watching process
> > can obtain escalated access to a file for which it has not been authorized.
> >
> > The selinux_path_notify hook implementation works by adding five new file
> > permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm
> > (descriptions about which will follow), and one new filesystem permission:
> > watch (which is applied to superblock checks). The hook then decides which
> > subset of these permissions must be held by the requesting application
> > based on the contents of the provided mask and the obj_type. The
> > selinux_file_open hook already checks the requested file mode and therefore
> > ensures that a watching process cannot escalate its access through
> > fanotify.
> >
> > The watch, watch_mount, and watch_sb permissions are the baseline
> > permissions for setting a watch on an object and each are a requirement for
> > any watch to be set on a file, mount, or superblock respectively. It should
> > be noted that having either of the other two permissions (watch_reads and
> > watch_with_perm) does not imply the watch, watch_mount, or watch_sb
> > permission. Superblock watches further require the filesystem watch
> > permission to the superblock. As there is no labeled object in view for
> > mounts, there is no specific check for mount watches beyond watch_mount to
> > the inode. Such a check could be added in the future, if a suitable labeled
> > object existed representing the mount.
> >
> > The watch_reads permission is required to receive notifications from
> > read-exclusive events on filesystem objects. These events include accessing
> > a file for the purpose of reading and closing a file which has been opened
> > read-only. This distinction has been drawn in order to provide a direct
> > indication in the policy for this otherwise not obvious capability. Read
> > access to a file should not necessarily imply the ability to observe read
> > events on a file.
> >
> > Finally, watch_with_perm only applies to fanotify masks since it is the
> > only way to set a mask which allows for the blocking, permission event.
> > This permission is needed for any watch which is of this type. Though
> > fanotify requires CAP_SYS_ADMIN, this is insufficient as it gives implicit
> > trust to root, which we do not do, and does not support least privilege.
> >
> > Signed-off-by: Aaron Goidel <acgoide@tycho.nsa.gov>
>
> I can't say that I accept your arguments that this is sufficient,
> but as you point out, the SELinux team does, and if I want more
> for Smack that's my fish to fry.
>
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Thanks Aaron. Thanks Casey.
I think we also want an ACK from the other LSMs, what say all of you?
Can you live with the new security_path_notify() hook?
Aaron, you'll also need to put together a test for the
selinux-testsuite to exercise this code. If you already sent it to
the list, my apologies but I don't see it anywhere. If you get stuck
on the test, let me know and I'll try to help out.
Oh, one more thing ...
> > +static int selinux_path_notify(const struct path *path, u64 mask,
> > + unsigned int obj_type)
> > +{
> > + int ret;
> > + u32 perm;
> > +
> > + struct common_audit_data ad;
> > +
> > + ad.type = LSM_AUDIT_DATA_PATH;
> > + ad.u.path = *path;
> > +
> > + /*
> > + * Set permission needed based on the type of mark being set.
> > + * Performs an additional check for sb watches.
> > + */
> > + switch (obj_type) {
> > + case FSNOTIFY_OBJ_TYPE_VFSMOUNT:
> > + perm = FILE__WATCH_MOUNT;
> > + break;
> > + case FSNOTIFY_OBJ_TYPE_SB:
> > + perm = FILE__WATCH_SB;
> > + ret = superblock_has_perm(current_cred(), path->dentry->d_sb,
> > + FILESYSTEM__WATCH, &ad);
> > + if (ret)
> > + return ret;
> > + break;
> > + case FSNOTIFY_OBJ_TYPE_INODE:
> > + perm = FILE__WATCH;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + // check if the mask is requesting ability to set a blocking watch
... in the future please don't use "// XXX", use "/* XXX */" instead :)
Don't respin the patch just for this, but if you have to do it for
some other reason please fix the C++ style comments. Thanks.
> > + if (mask & (ALL_FSNOTIFY_PERM_EVENTS))
> > + perm |= FILE__WATCH_WITH_PERM; // if so, check that permission
> > +
> > + // is the mask asking to watch file reads?
> > + if (mask & (FS_ACCESS | FS_ACCESS_PERM | FS_CLOSE_NOWRITE))
> > + perm |= FILE__WATCH_READS; // check that permission as well
> > +
> > + return path_has_perm(current_cred(), path, perm);
> > +}
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH V37 00/29] security: Add support for locking down the kernel
From: Matthew Garrett @ 2019-07-31 22:15 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, linux-api
A minor fix to the tracefs patch, some Acks and reviews added to the SOB
chain, and rebased on next/master (there were a couple of minor fixes needed to
align that).
^ permalink raw reply
* [PATCH V37 02/29] security: Add a "locked down" LSM hook
From: Matthew Garrett @ 2019-07-31 22:15 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Matthew Garrett,
Matthew Garrett, Kees Cook, Casey Schaufler
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
Add a mechanism to allow LSMs to make a policy decision around whether
kernel functionality that would allow tampering with or examining the
runtime state of the kernel should be permitted.
Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/lsm_hooks.h | 2 ++
include/linux/security.h | 32 ++++++++++++++++++++++++++++++++
security/security.c | 6 ++++++
3 files changed, 40 insertions(+)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index aebb0e032072..29c22cf40113 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1807,6 +1807,7 @@ union security_list_options {
int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
#endif /* CONFIG_BPF_SYSCALL */
+ int (*locked_down)(enum lockdown_reason what);
};
struct security_hook_heads {
@@ -2046,6 +2047,7 @@ struct security_hook_heads {
struct hlist_head bpf_prog_alloc_security;
struct hlist_head bpf_prog_free_security;
#endif /* CONFIG_BPF_SYSCALL */
+ struct hlist_head locked_down;
} __randomize_layout;
/*
diff --git a/include/linux/security.h b/include/linux/security.h
index 66a2fcbe6ab0..c2b1204e8e26 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -77,6 +77,33 @@ enum lsm_event {
LSM_POLICY_CHANGE,
};
+/*
+ * These are reasons that can be passed to the security_locked_down()
+ * LSM hook. Lockdown reasons that protect kernel integrity (ie, the
+ * ability for userland to modify kernel code) are placed before
+ * LOCKDOWN_INTEGRITY_MAX. Lockdown reasons that protect kernel
+ * confidentiality (ie, the ability for userland to extract
+ * information from the running kernel that would otherwise be
+ * restricted) are placed before LOCKDOWN_CONFIDENTIALITY_MAX.
+ *
+ * LSM authors should note that the semantics of any given lockdown
+ * reason are not guaranteed to be stable - the same reason may block
+ * one set of features in one kernel release, and a slightly different
+ * set of features in a later kernel release. LSMs that seek to expose
+ * lockdown policy at any level of granularity other than "none",
+ * "integrity" or "confidentiality" are responsible for either
+ * ensuring that they expose a consistent level of functionality to
+ * userland, or ensuring that userland is aware that this is
+ * potentially a moving target. It is easy to misuse this information
+ * in a way that could break userspace. Please be careful not to do
+ * so.
+ */
+enum lockdown_reason {
+ LOCKDOWN_NONE,
+ LOCKDOWN_INTEGRITY_MAX,
+ LOCKDOWN_CONFIDENTIALITY_MAX,
+};
+
/* These functions are in security/commoncap.c */
extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
int cap, unsigned int opts);
@@ -393,6 +420,7 @@ void security_inode_invalidate_secctx(struct inode *inode);
int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
+int security_locked_down(enum lockdown_reason what);
#else /* CONFIG_SECURITY */
static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
@@ -1205,6 +1233,10 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
{
return -EOPNOTSUPP;
}
+static inline int security_locked_down(enum lockdown_reason what)
+{
+ return 0;
+}
#endif /* CONFIG_SECURITY */
#ifdef CONFIG_SECURITY_NETWORK
diff --git a/security/security.c b/security/security.c
index 90f1e291c800..ce6c945bf347 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2392,3 +2392,9 @@ void security_bpf_prog_free(struct bpf_prog_aux *aux)
call_void_hook(bpf_prog_free_security, aux);
}
#endif /* CONFIG_BPF_SYSCALL */
+
+int security_locked_down(enum lockdown_reason what)
+{
+ return call_int_hook(locked_down, 0, what);
+}
+EXPORT_SYMBOL(security_locked_down);
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 05/29] Restrict /dev/{mem,kmem,port} when the kernel is locked down
From: Matthew Garrett @ 2019-07-31 22:15 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Matthew Garrett,
David Howells, Matthew Garrett, Kees Cook, x86
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Matthew Garrett <mjg59@srcf.ucam.org>
Allowing users to read and write to core kernel memory makes it possible
for the kernel to be subverted, avoiding module loading restrictions, and
also to steal cryptographic information.
Disallow /dev/mem and /dev/kmem from being opened this when the kernel has
been locked down to prevent this.
Also disallow /dev/port from being opened to prevent raw ioport access and
thus DMA from being used to accomplish the same thing.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: x86@kernel.org
---
drivers/char/mem.c | 7 +++++--
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index b08dc50f9f26..d0148aee1aab 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -29,8 +29,8 @@
#include <linux/export.h>
#include <linux/io.h>
#include <linux/uio.h>
-
#include <linux/uaccess.h>
+#include <linux/security.h>
#ifdef CONFIG_IA64
# include <linux/efi.h>
@@ -786,7 +786,10 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)
static int open_port(struct inode *inode, struct file *filp)
{
- return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
+ if (!capable(CAP_SYS_RAWIO))
+ return -EPERM;
+
+ return security_locked_down(LOCKDOWN_DEV_MEM);
}
#define zero_lseek null_lseek
diff --git a/include/linux/security.h b/include/linux/security.h
index 8e70063074a1..9458152601b5 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -104,6 +104,7 @@ enum lsm_event {
enum lockdown_reason {
LOCKDOWN_NONE,
LOCKDOWN_MODULE_SIGNATURE,
+ LOCKDOWN_DEV_MEM,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 2c53fd9f5c9b..d2ef29d9f0b2 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -19,6 +19,7 @@ static enum lockdown_reason kernel_locked_down;
static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_NONE] = "none",
[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
+ [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 06/29] kexec_load: Disable at runtime if the kernel is locked down
From: Matthew Garrett @ 2019-07-31 22:15 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Matthew Garrett,
David Howells, Matthew Garrett, Dave Young, Kees Cook, kexec
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Matthew Garrett <mjg59@srcf.ucam.org>
The kexec_load() syscall permits the loading and execution of arbitrary
code in ring 0, which is something that lock-down is meant to prevent. It
makes sense to disable kexec_load() in this situation.
This does not affect kexec_file_load() syscall which can check for a
signature on the image to be booted.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Dave Young <dyoung@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
cc: kexec@lists.infradead.org
---
include/linux/security.h | 1 +
kernel/kexec.c | 8 ++++++++
security/lockdown/lockdown.c | 1 +
3 files changed, 10 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index 9458152601b5..69c5de539e9a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -105,6 +105,7 @@ enum lockdown_reason {
LOCKDOWN_NONE,
LOCKDOWN_MODULE_SIGNATURE,
LOCKDOWN_DEV_MEM,
+ LOCKDOWN_KEXEC,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 1b018f1a6e0d..bc933c0db9bf 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -205,6 +205,14 @@ static inline int kexec_load_check(unsigned long nr_segments,
if (result < 0)
return result;
+ /*
+ * kexec can be used to circumvent module loading restrictions, so
+ * prevent loading in that case
+ */
+ result = security_locked_down(LOCKDOWN_KEXEC);
+ if (result)
+ return result;
+
/*
* Verify we have a legal set of flags
* This leaves us room for future extensions.
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index d2ef29d9f0b2..6f302c156bc8 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -20,6 +20,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_NONE] = "none",
[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
+ [LOCKDOWN_KEXEC] = "kexec of unsigned images",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 07/29] Copy secure_boot flag in boot params across kexec reboot
From: Matthew Garrett @ 2019-07-31 22:15 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Dave Young,
David Howells, Matthew Garrett, Kees Cook, kexec
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Dave Young <dyoung@redhat.com>
Kexec reboot in case secure boot being enabled does not keep the secure
boot mode in new kernel, so later one can load unsigned kernel via legacy
kexec_load. In this state, the system is missing the protections provided
by secure boot.
Adding a patch to fix this by retain the secure_boot flag in original
kernel.
secure_boot flag in boot_params is set in EFI stub, but kexec bypasses the
stub. Fixing this issue by copying secure_boot flag across kexec reboot.
Signed-off-by: Dave Young <dyoung@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
cc: kexec@lists.infradead.org
---
arch/x86/kernel/kexec-bzimage64.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 5ebcd02cbca7..d2f4e706a428 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -180,6 +180,7 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
if (efi_enabled(EFI_OLD_MEMMAP))
return 0;
+ params->secure_boot = boot_params.secure_boot;
ei->efi_loader_signature = current_ei->efi_loader_signature;
ei->efi_systab = current_ei->efi_systab;
ei->efi_systab_hi = current_ei->efi_systab_hi;
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 08/29] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
From: Matthew Garrett @ 2019-07-31 22:15 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Jiri Bohac,
David Howells, Matthew Garrett, Dave Young, kexec
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Jiri Bohac <jbohac@suse.cz>
This is a preparatory patch for kexec_file_load() lockdown. A locked down
kernel needs to prevent unsigned kernel images from being loaded with
kexec_file_load(). Currently, the only way to force the signature
verification is compiling with KEXEC_VERIFY_SIG. This prevents loading
usigned images even when the kernel is not locked down at runtime.
This patch splits KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG
turns on the signature verification but allows unsigned images to be
loaded. KEXEC_SIG_FORCE disallows images without a valid signature.
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Jiri Bohac <jbohac@suse.cz>
Reviewed-by: Dave Young <dyoung@redhat.com>
cc: kexec@lists.infradead.org
---
arch/x86/Kconfig | 20 +++++++++----
crypto/asymmetric_keys/verify_pefile.c | 4 ++-
include/linux/kexec.h | 4 +--
kernel/kexec_file.c | 41 ++++++++++++++++++++++----
4 files changed, 55 insertions(+), 14 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 05e78acb187c..fd2cd4f861cc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2032,20 +2032,30 @@ config KEXEC_FILE
config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE
-config KEXEC_VERIFY_SIG
+config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
---help---
- This option makes kernel signature verification mandatory for
- the kexec_file_load() syscall.
- In addition to that option, you need to enable signature
+ This option makes the kexec_file_load() syscall check for a valid
+ signature of the kernel image. The image can still be loaded without
+ a valid signature unless you also enable KEXEC_SIG_FORCE, though if
+ there's a signature that we can check, then it must be valid.
+
+ In addition to this option, you need to enable signature
verification for the corresponding kernel image type being
loaded in order for this to work.
+config KEXEC_SIG_FORCE
+ bool "Require a valid signature in kexec_file_load() syscall"
+ depends on KEXEC_SIG
+ ---help---
+ This option makes kernel signature verification mandatory for
+ the kexec_file_load() syscall.
+
config KEXEC_BZIMAGE_VERIFY_SIG
bool "Enable bzImage signature verification support"
- depends on KEXEC_VERIFY_SIG
+ depends on KEXEC_SIG
depends on SIGNED_PE_FILE_VERIFICATION
select SYSTEM_TRUSTED_KEYRING
---help---
diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index 3b303fe2f061..cc9dbcecaaca 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -96,7 +96,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
if (!ddir->certs.virtual_address || !ddir->certs.size) {
pr_debug("Unsigned PE binary\n");
- return -EKEYREJECTED;
+ return -ENODATA;
}
chkaddr(ctx->header_size, ddir->certs.virtual_address,
@@ -403,6 +403,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen,
* (*) 0 if at least one signature chain intersects with the keys in the trust
* keyring, or:
*
+ * (*) -ENODATA if there is no signature present.
+ *
* (*) -ENOPKG if a suitable crypto module couldn't be found for a check on a
* chain.
*
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 305f6a5ca4fe..998f77c3a0e1 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -125,7 +125,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char *kernel_buf,
unsigned long cmdline_len);
typedef int (kexec_cleanup_t)(void *loader_data);
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
typedef int (kexec_verify_sig_t)(const char *kernel_buf,
unsigned long kernel_len);
#endif
@@ -134,7 +134,7 @@ struct kexec_file_ops {
kexec_probe_t *probe;
kexec_load_t *load;
kexec_cleanup_t *cleanup;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
kexec_verify_sig_t *verify_sig;
#endif
};
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b8cc032d5620..875482c34154 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -88,7 +88,7 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
return kexec_image_post_load_cleanup_default(image);
}
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
unsigned long buf_len)
{
@@ -186,7 +186,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
const char __user *cmdline_ptr,
unsigned long cmdline_len, unsigned flags)
{
- int ret = 0;
+ const char *reason;
+ int ret;
void *ldata;
loff_t size;
@@ -202,14 +203,42 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
if (ret)
goto out;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
image->kernel_buf_len);
- if (ret) {
- pr_debug("kernel signature verification failed.\n");
+ switch (ret) {
+ case 0:
+ break;
+
+ /* Certain verification errors are non-fatal if we're not
+ * checking errors, provided we aren't mandating that there
+ * must be a valid signature.
+ */
+ case -ENODATA:
+ reason = "kexec of unsigned image";
+ goto decide;
+ case -ENOPKG:
+ reason = "kexec of image with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "kexec of image with unavailable key";
+ decide:
+ if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
+ pr_notice("%s rejected\n", reason);
+ goto out;
+ }
+
+ ret = 0;
+ break;
+
+ /* All other errors are fatal, including nomem, unparseable
+ * signatures and signature check failures - even if signatures
+ * aren't required.
+ */
+ default:
+ pr_notice("kernel signature verification failed (%d).\n", ret);
goto out;
}
- pr_debug("kernel signature verification successful.\n");
#endif
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 10/29] hibernate: Disable when the kernel is locked down
From: Matthew Garrett @ 2019-07-31 22:15 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Josh Boyer,
David Howells, Matthew Garrett, Kees Cook, rjw, pavel, linux-pm
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Josh Boyer <jwboyer@fedoraproject.org>
There is currently no way to verify the resume image when returning
from hibernate. This might compromise the signed modules trust model,
so until we can work with signed hibernate images we disable it when the
kernel is locked down.
Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: rjw@rjwysocki.net
Cc: pavel@ucw.cz
cc: linux-pm@vger.kernel.org
---
include/linux/security.h | 1 +
kernel/power/hibernate.c | 3 ++-
security/lockdown/lockdown.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 69c5de539e9a..304a155a5628 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -106,6 +106,7 @@ enum lockdown_reason {
LOCKDOWN_MODULE_SIGNATURE,
LOCKDOWN_DEV_MEM,
LOCKDOWN_KEXEC,
+ LOCKDOWN_HIBERNATION,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index cd7434e6000d..3c0a5a8170b0 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -30,6 +30,7 @@
#include <linux/ctype.h>
#include <linux/genhd.h>
#include <linux/ktime.h>
+#include <linux/security.h>
#include <trace/events/power.h>
#include "power.h"
@@ -68,7 +69,7 @@ static const struct platform_hibernation_ops *hibernation_ops;
bool hibernation_available(void)
{
- return (nohibernate == 0);
+ return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
}
/**
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 6f302c156bc8..a0996f75629f 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -21,6 +21,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
[LOCKDOWN_KEXEC] = "kexec of unsigned images",
+ [LOCKDOWN_HIBERNATION] = "hibernation",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 11/29] PCI: Lock down BAR access when the kernel is locked down
From: Matthew Garrett @ 2019-07-31 22:15 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Matthew Garrett,
David Howells, Matthew Garrett, Bjorn Helgaas, Kees Cook,
linux-pci
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Matthew Garrett <mjg59@srcf.ucam.org>
Any hardware that can potentially generate DMA has to be locked down in
order to avoid it being possible for an attacker to modify kernel code,
allowing them to circumvent disabled module loading or module signing.
Default to paranoid - in future we can potentially relax this for
sufficiently IOMMU-isolated devices.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
cc: linux-pci@vger.kernel.org
---
drivers/pci/pci-sysfs.c | 16 ++++++++++++++++
drivers/pci/proc.c | 14 ++++++++++++--
drivers/pci/syscall.c | 4 +++-
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
5 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 965c72104150..396c1a90c0e1 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -906,6 +906,11 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
unsigned int size = count;
loff_t init_off = off;
u8 *data = (u8 *) buf;
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
if (off > dev->cfg_size)
return 0;
@@ -1167,6 +1172,11 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
int bar = (unsigned long)attr->private;
enum pci_mmap_state mmap_type;
struct resource *res = &pdev->resource[bar];
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
return -EINVAL;
@@ -1243,6 +1253,12 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
struct bin_attribute *attr, char *buf,
loff_t off, size_t count)
{
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
+
return pci_resource_io(filp, kobj, attr, buf, off, count, true);
}
diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index fe7fe678965b..5495537c60c2 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -13,6 +13,7 @@
#include <linux/seq_file.h>
#include <linux/capability.h>
#include <linux/uaccess.h>
+#include <linux/security.h>
#include <asm/byteorder.h>
#include "pci.h"
@@ -115,7 +116,11 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
struct pci_dev *dev = PDE_DATA(ino);
int pos = *ppos;
int size = dev->cfg_size;
- int cnt;
+ int cnt, ret;
+
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
if (pos >= size)
return 0;
@@ -196,6 +201,10 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
#endif /* HAVE_PCI_MMAP */
int ret = 0;
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
+
switch (cmd) {
case PCIIOC_CONTROLLER:
ret = pci_domain_nr(dev->bus);
@@ -238,7 +247,8 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma)
struct pci_filp_private *fpriv = file->private_data;
int i, ret, write_combine = 0, res_bit = IORESOURCE_MEM;
- if (!capable(CAP_SYS_RAWIO))
+ if (!capable(CAP_SYS_RAWIO) ||
+ security_locked_down(LOCKDOWN_PCI_ACCESS))
return -EPERM;
if (fpriv->mmap_state == pci_mmap_io) {
diff --git a/drivers/pci/syscall.c b/drivers/pci/syscall.c
index d96626c614f5..31e39558d49d 100644
--- a/drivers/pci/syscall.c
+++ b/drivers/pci/syscall.c
@@ -7,6 +7,7 @@
#include <linux/errno.h>
#include <linux/pci.h>
+#include <linux/security.h>
#include <linux/syscalls.h>
#include <linux/uaccess.h>
#include "pci.h"
@@ -90,7 +91,8 @@ SYSCALL_DEFINE5(pciconfig_write, unsigned long, bus, unsigned long, dfn,
u32 dword;
int err = 0;
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) ||
+ security_locked_down(LOCKDOWN_PCI_ACCESS))
return -EPERM;
dev = pci_get_domain_bus_and_slot(0, bus, dfn);
diff --git a/include/linux/security.h b/include/linux/security.h
index 304a155a5628..8adbd62b7669 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -107,6 +107,7 @@ enum lockdown_reason {
LOCKDOWN_DEV_MEM,
LOCKDOWN_KEXEC,
LOCKDOWN_HIBERNATION,
+ LOCKDOWN_PCI_ACCESS,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index a0996f75629f..655fe388e615 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -22,6 +22,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
[LOCKDOWN_KEXEC] = "kexec of unsigned images",
[LOCKDOWN_HIBERNATION] = "hibernation",
+ [LOCKDOWN_PCI_ACCESS] = "direct PCI access",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 13/29] x86/msr: Restrict MSR access when the kernel is locked down
From: Matthew Garrett @ 2019-07-31 22:16 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Matthew Garrett,
Matthew Garrett, David Howells, Kees Cook, Thomas Gleixner, x86
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Matthew Garrett <mjg59@srcf.ucam.org>
Writing to MSRs should not be allowed if the kernel is locked down, since
it could lead to execution of arbitrary code in kernel mode. Based on a
patch by Kees Cook.
Signed-off-by: Matthew Garrett <mjg59@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
cc: x86@kernel.org
---
arch/x86/kernel/msr.c | 8 ++++++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 10 insertions(+)
diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index 3db2252b958d..1547be359d7f 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -34,6 +34,7 @@
#include <linux/notifier.h>
#include <linux/uaccess.h>
#include <linux/gfp.h>
+#include <linux/security.h>
#include <asm/cpufeature.h>
#include <asm/msr.h>
@@ -79,6 +80,10 @@ static ssize_t msr_write(struct file *file, const char __user *buf,
int err = 0;
ssize_t bytes = 0;
+ err = security_locked_down(LOCKDOWN_MSR);
+ if (err)
+ return err;
+
if (count % 8)
return -EINVAL; /* Invalid chunk size */
@@ -130,6 +135,9 @@ static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
err = -EFAULT;
break;
}
+ err = security_locked_down(LOCKDOWN_MSR);
+ if (err)
+ break;
err = wrmsr_safe_regs_on_cpu(cpu, regs);
if (err)
break;
diff --git a/include/linux/security.h b/include/linux/security.h
index 79250b2ffb8f..155ff026eca4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -109,6 +109,7 @@ enum lockdown_reason {
LOCKDOWN_HIBERNATION,
LOCKDOWN_PCI_ACCESS,
LOCKDOWN_IOPORT,
+ LOCKDOWN_MSR,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 316f7cf4e996..d99c0bee739d 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -24,6 +24,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_HIBERNATION] = "hibernation",
[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
[LOCKDOWN_IOPORT] = "raw io port access",
+ [LOCKDOWN_MSR] = "raw MSR access",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
From: Matthew Garrett @ 2019-07-31 22:16 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Josh Boyer,
David Howells, Matthew Garrett, Kees Cook, Dave Young, linux-acpi
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Josh Boyer <jwboyer@redhat.com>
This option allows userspace to pass the RSDP address to the kernel, which
makes it possible for a user to modify the workings of hardware. Reject
the option when the kernel is locked down. This requires some reworking
of the existing RSDP command line logic, since the early boot code also
makes use of a command-line passed RSDP when locating the SRAT table
before the lockdown code has been initialised. This is achieved by
separating the command line RSDP path in the early boot code from the
generic RSDP path, and then copying the command line RSDP into boot
params in the kernel proper if lockdown is not enabled. If lockdown is
enabled and an RSDP is provided on the command line, this will only be
used when parsing SRAT (which shouldn't permit kernel code execution)
and will be ignored in the rest of the kernel.
(Modified by Matthew Garrett in order to handle the early boot RSDP
environment)
Signed-off-by: Josh Boyer <jwboyer@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
cc: Dave Young <dyoung@redhat.com>
cc: linux-acpi@vger.kernel.org
---
arch/x86/boot/compressed/acpi.c | 19 +++++++++++++------
arch/x86/include/asm/acpi.h | 9 +++++++++
arch/x86/include/asm/x86_init.h | 2 ++
arch/x86/kernel/acpi/boot.c | 5 +++++
arch/x86/kernel/x86_init.c | 1 +
drivers/acpi/osl.c | 14 +++++++++++++-
include/linux/acpi.h | 6 ++++++
7 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 15255f388a85..149795c369f2 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -26,7 +26,7 @@ struct mem_vector immovable_mem[MAX_NUMNODES*2];
*/
#define MAX_ADDR_LEN 19
-static acpi_physical_address get_acpi_rsdp(void)
+static acpi_physical_address get_cmdline_acpi_rsdp(void)
{
acpi_physical_address addr = 0;
@@ -278,10 +278,7 @@ acpi_physical_address get_rsdp_addr(void)
{
acpi_physical_address pa;
- pa = get_acpi_rsdp();
-
- if (!pa)
- pa = boot_params->acpi_rsdp_addr;
+ pa = boot_params->acpi_rsdp_addr;
/*
* Try to get EFI data from setup_data. This can happen when we're a
@@ -311,7 +308,17 @@ static unsigned long get_acpi_srat_table(void)
char arg[10];
u8 *entry;
- rsdp = (struct acpi_table_rsdp *)(long)boot_params->acpi_rsdp_addr;
+ /*
+ * Check whether we were given an RSDP on the command line. We don't
+ * stash this in boot params because the kernel itself may have
+ * different ideas about whether to trust a command-line parameter.
+ */
+ rsdp = (struct acpi_table_rsdp *)get_cmdline_acpi_rsdp();
+
+ if (!rsdp)
+ rsdp = (struct acpi_table_rsdp *)(long)
+ boot_params->acpi_rsdp_addr;
+
if (!rsdp)
return 0;
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index aac686e1e005..bc9693c9107e 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -117,6 +117,12 @@ static inline bool acpi_has_cpu_in_madt(void)
return !!acpi_lapic;
}
+#define ACPI_HAVE_ARCH_SET_ROOT_POINTER
+static inline void acpi_arch_set_root_pointer(u64 addr)
+{
+ x86_init.acpi.set_root_pointer(addr);
+}
+
#define ACPI_HAVE_ARCH_GET_ROOT_POINTER
static inline u64 acpi_arch_get_root_pointer(void)
{
@@ -125,6 +131,7 @@ static inline u64 acpi_arch_get_root_pointer(void)
void acpi_generic_reduced_hw_init(void);
+void x86_default_set_root_pointer(u64 addr);
u64 x86_default_get_root_pointer(void);
#else /* !CONFIG_ACPI */
@@ -138,6 +145,8 @@ static inline void disable_acpi(void) { }
static inline void acpi_generic_reduced_hw_init(void) { }
+static inline void x86_default_set_root_pointer(u64 addr) { }
+
static inline u64 x86_default_get_root_pointer(void)
{
return 0;
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index ac0934189017..19435858df5f 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -134,10 +134,12 @@ struct x86_hyper_init {
/**
* struct x86_init_acpi - x86 ACPI init functions
+ * @set_root_poitner: set RSDP address
* @get_root_pointer: get RSDP address
* @reduced_hw_early_init: hardware reduced platform early init
*/
struct x86_init_acpi {
+ void (*set_root_pointer)(u64 addr);
u64 (*get_root_pointer)(void);
void (*reduced_hw_early_init)(void);
};
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 17b33ef604f3..04205ce127a1 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1760,6 +1760,11 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
e820__update_table_print();
}
+void x86_default_set_root_pointer(u64 addr)
+{
+ boot_params.acpi_rsdp_addr = addr;
+}
+
u64 x86_default_get_root_pointer(void)
{
return boot_params.acpi_rsdp_addr;
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index 1bef687faf22..18a799c8fa28 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -95,6 +95,7 @@ struct x86_init_ops x86_init __initdata = {
},
.acpi = {
+ .set_root_pointer = x86_default_set_root_pointer,
.get_root_pointer = x86_default_get_root_pointer,
.reduced_hw_early_init = acpi_generic_reduced_hw_init,
},
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 9c0edf2fc0dd..d43df3a3fa8d 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -26,6 +26,7 @@
#include <linux/list.h>
#include <linux/jiffies.h>
#include <linux/semaphore.h>
+#include <linux/security.h>
#include <asm/io.h>
#include <linux/uaccess.h>
@@ -180,8 +181,19 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
acpi_physical_address pa;
#ifdef CONFIG_KEXEC
- if (acpi_rsdp)
+ /*
+ * We may have been provided with an RSDP on the command line,
+ * but if a malicious user has done so they may be pointing us
+ * at modified ACPI tables that could alter kernel behaviour -
+ * so, we check the lockdown status before making use of
+ * it. If we trust it then also stash it in an architecture
+ * specific location (if appropriate) so it can be carried
+ * over further kexec()s.
+ */
+ if (acpi_rsdp && !security_locked_down(LOCKDOWN_ACPI_TABLES)) {
+ acpi_arch_set_root_pointer(acpi_rsdp);
return acpi_rsdp;
+ }
#endif
pa = acpi_arch_get_root_pointer();
if (pa)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index e40e1e27ed8e..6b35f2f4cab3 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -643,6 +643,12 @@ bool acpi_gtdt_c3stop(int type);
int acpi_arch_timer_mem_init(struct arch_timer_mem *timer_mem, int *timer_count);
#endif
+#ifndef ACPI_HAVE_ARCH_SET_ROOT_POINTER
+static inline void acpi_arch_set_root_pointer(u64 addr)
+{
+}
+#endif
+
#ifndef ACPI_HAVE_ARCH_GET_ROOT_POINTER
static inline u64 acpi_arch_get_root_pointer(void)
{
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 16/29] acpi: Disable ACPI table override if the kernel is locked down
From: Matthew Garrett @ 2019-07-31 22:16 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Linn Crosetto,
David Howells, Matthew Garrett, Kees Cook, linux-acpi
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: Linn Crosetto <lcrosetto@gmail.com>
From the kernel documentation (initrd_table_override.txt):
If the ACPI_INITRD_TABLE_OVERRIDE compile option is true, it is possible
to override nearly any ACPI table provided by the BIOS with an
instrumented, modified one.
When lockdown is enabled, the kernel should disallow any unauthenticated
changes to kernel space. ACPI tables contain code invoked by the kernel,
so do not allow ACPI tables to be overridden if the kernel is locked down.
Signed-off-by: Linn Crosetto <lcrosetto@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
cc: linux-acpi@vger.kernel.org
---
drivers/acpi/tables.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index b32327759380..180ac4329763 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -20,6 +20,7 @@
#include <linux/memblock.h>
#include <linux/earlycpio.h>
#include <linux/initrd.h>
+#include <linux/security.h>
#include "internal.h"
#ifdef CONFIG_ACPI_CUSTOM_DSDT
@@ -578,6 +579,11 @@ void __init acpi_table_upgrade(void)
if (table_nr == 0)
return;
+ if (security_locked_down(LOCKDOWN_ACPI_TABLES)) {
+ pr_notice("kernel is locked down, ignoring table override\n");
+ return;
+ }
+
acpi_tables_addr =
memblock_find_in_range(0, ACPI_TABLE_UPGRADE_MAX_PHYS,
all_tables_size, PAGE_SIZE);
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 17/29] Prohibit PCMCIA CIS storage when the kernel is locked down
From: Matthew Garrett @ 2019-07-31 22:16 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, David Howells,
Dominik Brodowski, Matthew Garrett, Kees Cook
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: David Howells <dhowells@redhat.com>
Prohibit replacement of the PCMCIA Card Information Structure when the
kernel is locked down.
Suggested-by: Dominik Brodowski <linux@dominikbrodowski.net>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
drivers/pcmcia/cistpl.c | 5 +++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index abd029945cc8..629359fe3513 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -21,6 +21,7 @@
#include <linux/pci.h>
#include <linux/ioport.h>
#include <linux/io.h>
+#include <linux/security.h>
#include <asm/byteorder.h>
#include <asm/unaligned.h>
@@ -1575,6 +1576,10 @@ static ssize_t pccard_store_cis(struct file *filp, struct kobject *kobj,
struct pcmcia_socket *s;
int error;
+ error = security_locked_down(LOCKDOWN_PCMCIA_CIS);
+ if (error)
+ return error;
+
s = to_socket(container_of(kobj, struct device, kobj));
if (off)
diff --git a/include/linux/security.h b/include/linux/security.h
index 1c32522b3c5a..3773ad09b831 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -111,6 +111,7 @@ enum lockdown_reason {
LOCKDOWN_IOPORT,
LOCKDOWN_MSR,
LOCKDOWN_ACPI_TABLES,
+ LOCKDOWN_PCMCIA_CIS,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index ecb51b1a5c03..22482e1b9a77 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -26,6 +26,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_IOPORT] = "raw io port access",
[LOCKDOWN_MSR] = "raw MSR access",
[LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
+ [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH V37 18/29] Lock down TIOCSSERIAL
From: Matthew Garrett @ 2019-07-31 22:16 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, David Howells,
Greg Kroah-Hartman, Matthew Garrett, Kees Cook, Jiri Slaby,
linux-serial
In-Reply-To: <20190731221617.234725-1-matthewgarrett@google.com>
From: David Howells <dhowells@redhat.com>
Lock down TIOCSSERIAL as that can be used to change the ioport and irq
settings on a serial port. This only appears to be an issue for the serial
drivers that use the core serial code. All other drivers seem to either
ignore attempts to change port/irq or give an error.
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
cc: Jiri Slaby <jslaby@suse.com>
Cc: linux-serial@vger.kernel.org
---
drivers/tty/serial/serial_core.c | 5 +++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 4223cb496764..6e713be1d4e9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -22,6 +22,7 @@
#include <linux/serial_core.h>
#include <linux/delay.h>
#include <linux/mutex.h>
+#include <linux/security.h>
#include <linux/irq.h>
#include <linux/uaccess.h>
@@ -862,6 +863,10 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
goto check_and_exit;
}
+ retval = security_locked_down(LOCKDOWN_TIOCSSERIAL);
+ if (retval && (change_irq || change_port))
+ goto exit;
+
/*
* Ask the low level driver to verify the settings.
*/
diff --git a/include/linux/security.h b/include/linux/security.h
index 3773ad09b831..8f7048395114 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -112,6 +112,7 @@ enum lockdown_reason {
LOCKDOWN_MSR,
LOCKDOWN_ACPI_TABLES,
LOCKDOWN_PCMCIA_CIS,
+ LOCKDOWN_TIOCSSERIAL,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 22482e1b9a77..00a3a6438dd2 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -27,6 +27,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_MSR] = "raw MSR access",
[LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
+ [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
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