Re: [PATCH 2/9] tpm: Allow PCR 23 to be restricted to kernel-only use
On Wed, Feb 24, 2021 at 10:00:53AM -0800, James Bottomley wrote: > On Sat, 2021-02-20 at 01:32 +0000, Matthew Garrett wrote: > > Under certain circumstances it might be desirable to enable the > > creation of TPM-backed secrets that are only accessible to the > > kernel. In an ideal world this could be achieved by using TPM > > localities, but these don't appear to be available on consumer > > systems. > > I don't understand this ... the localities seem to work fine on all the > systems I have ... is this some embedded thing? I haven't made it work on an HP Z440 or a Lenovo P520. So now I'm wondering whether having chipsets with TXT support (even if it's turned off) confuse this point. Sigh. I'd really prefer to use localities than a PCR, so if it works on client platforms I'd be inclined to say we'll do a self-test and go for that, and workstation vendors can just recommend their customers use UPSes or something. > > An alternative is to simply block userland from modifying one of the > > resettable PCRs, leaving it available to the kernel. If the kernel > > ensures that no userland can access the TPM while it is carrying out > > work, it can reset PCR 23, extend it to an arbitrary value, create or > > load a secret, and then reset the PCR again. Even if userland somehow > > obtains the sealed material, it will be unable to unseal it since PCR > > 23 will never be in the appropriate state. > > This seems a bit arbitrary: You're removing this PCR from user space > accessibility, but PCR 23 is defined as "Application Support" how can > we be sure no application will actually want to use it (and then fail)? Absolutely no way of guaranteeing that, and enabling this option is certainly an ABI break. > Since PCRs are very scarce, why not use a NV index instead. They're > still a bounded resource, but most TPMs have far more of them than they > do PCRs, and the address space is much bigger so picking a nice > arbitrary 24 bit value reduces the chance of collisions. How many write cycles do we expect the NV to survive? But I'll find a client system with a TPM and play with locality support there - maybe we can just avoid this problem anyway.
Re: [PATCH v17 08/10] PM: hibernate: disable when there are active secretmem users
On Mon, Feb 22, 2021 at 12:23:59PM +0200, Mike Rapoport wrote: > On Mon, Feb 22, 2021 at 07:34:52AM +0000, Matthew Garrett wrote: > > On Mon, Feb 08, 2021 at 10:49:18AM +0200, Mike Rapoport wrote: > > > > > It is unsafe to allow saving of secretmem areas to the hibernation > > > snapshot as they would be visible after the resume and this essentially > > > will defeat the purpose of secret memory mappings. > > > > Sorry for being a bit late to this - from the point of view of running > > processes (and even the kernel once resume is complete), hibernation is > > effectively equivalent to suspend to RAM. Why do they need to be handled > > differently here? > > Hibernation leaves a copy of the data on the disk which we want to prevent. Who are you worried about seeing it, and at what points in time?
Re: [PATCH v17 08/10] PM: hibernate: disable when there are active secretmem users
On Mon, Feb 08, 2021 at 10:49:18AM +0200, Mike Rapoport wrote: > It is unsafe to allow saving of secretmem areas to the hibernation > snapshot as they would be visible after the resume and this essentially > will defeat the purpose of secret memory mappings. Sorry for being a bit late to this - from the point of view of running processes (and even the kernel once resume is complete), hibernation is effectively equivalent to suspend to RAM. Why do they need to be handled differently here?
Re: [PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data
On Sat, Feb 20, 2021 at 05:09:07AM +0200, Jarkko Sakkinen wrote: > Something popped into mind: could we make PCR 23 reservation dynamic > instead of a config option. > > E.g. if the user space uses it, then it's dirty and hibernate will > fail. I really dislike the static compilation time firewall on it. We can fail hibernation if userland hasn't flagged things, but the concern is that if you hibernate with PCR 23 blocking enabled and then reboot with the blocking disabled, userland can obtain the blob from the hibernation image, extend PCR 23, modify the image and use the key they've recovered to make it look legitimate, enable PCR 23 blocking again and then resume into their own code.
Re: [PATCH 7/9] pm: hibernate: Optionally use TPM-backed keys to protect image integrity
On Fri, Feb 19, 2021 at 06:20:13PM -0800, Randy Dunlap wrote: > For all of the Kconfig* configuration files throughout the source tree, > the indentation is somewhat different. Lines under a ``config`` definition > are indented with one tab, while help text is indented an additional two > spaces. Whoops, I've no idea how I screwed that up. I'll fix for V2, thanks! > Also, one feature should not be responsible for enabling other "subsystems," > such as KEYS and CRYPTO. They should instead be listed as dependencies. ACK.
Re: [PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob
On Sat, Feb 20, 2021 at 05:05:36AM +0200, Jarkko Sakkinen wrote: > On Sat, Feb 20, 2021 at 01:32:49AM +0000, Matthew Garrett wrote: > > Performing any sort of state validation of a sealed TPM blob requires > > being able to access the individual members in the response. Parse the > > blob sufficiently to be able to stash pointers to each member, along > > with the length. > > > > Signed-off-by: Matthew Garrett > > I'll just say LGTM for now. Did not see anything obviously wrong in > the code change (and does make sense to nitpick minor things just > yet). > > Need to understand the whole use case just a little bit better. I wrote this up with some more detail at https://mjg59.dreamwidth.org/55845.html - it seemed longer than appropriate for a commit message, but if you'd like more detail somewhere I can certainly add it.
[PATCH 9/9] pm: hibernate: seal the encryption key with a PCR policy
The key blob is not secret, and by default the TPM will happily unseal
it regardless of system state. We can protect against that by sealing
the secret with a PCR policy - if the current PCR state doesn't match,
the TPM will refuse to release the secret. For now let's just seal it to
PCR 23. In the long term we may want a more flexible policy around this,
such as including PCR 7.
Signed-off-by: Matthew Garrett
---
include/linux/tpm.h | 4 ++
kernel/power/tpm.c | 161 ++--
2 files changed, 160 insertions(+), 5 deletions(-)
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index f6970986b097..2e0141978c87 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -225,18 +225,22 @@ enum tpm2_command_codes {
TPM2_CC_CONTEXT_LOAD= 0x0161,
TPM2_CC_CONTEXT_SAVE= 0x0162,
TPM2_CC_FLUSH_CONTEXT = 0x0165,
+ TPM2_CC_START_AUTH_SESSION = 0x0176,
TPM2_CC_VERIFY_SIGNATURE= 0x0177,
TPM2_CC_GET_CAPABILITY = 0x017A,
TPM2_CC_GET_RANDOM = 0x017B,
TPM2_CC_PCR_READ= 0x017E,
+ TPM2_CC_POLICY_PCR = 0x017F,
TPM2_CC_PCR_EXTEND = 0x0182,
TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
TPM2_CC_HASH_SEQUENCE_START = 0x0186,
+ TPM2_CC_POLICY_GET_DIGEST = 0x0189,
TPM2_CC_CREATE_LOADED = 0x0191,
TPM2_CC_LAST= 0x0193, /* Spec 1.36 */
};
enum tpm2_permanent_handles {
+ TPM2_RH_NULL= 0x4007,
TPM2_RS_PW = 0x4009,
};
diff --git a/kernel/power/tpm.c b/kernel/power/tpm.c
index 34e6cfb98ce4..5de27c2f08be 100644
--- a/kernel/power/tpm.c
+++ b/kernel/power/tpm.c
@@ -125,6 +125,118 @@ static int swsusp_enc_dec(struct trusted_key_payload
*payload, char *buf,
return ret;
}
+static int tpm_setup_policy(struct tpm_chip *chip, int *session_handle)
+{
+ struct tpm_header *head;
+ struct tpm_buf buf;
+ char nonce[32] = {0x00};
+ int rc;
+
+ rc = tpm_buf_init(, TPM2_ST_NO_SESSIONS,
+ TPM2_CC_START_AUTH_SESSION);
+ if (rc)
+ return rc;
+
+ /* Decrypt key */
+ tpm_buf_append_u32(, TPM2_RH_NULL);
+
+ /* Auth entity */
+ tpm_buf_append_u32(, TPM2_RH_NULL);
+
+ /* Nonce - blank is fine here */
+ tpm_buf_append_u16(, sizeof(nonce));
+ tpm_buf_append(, nonce, sizeof(nonce));
+
+ /* Encrypted secret - empty */
+ tpm_buf_append_u16(, 0);
+
+ /* Policy type - session */
+ tpm_buf_append_u8(, 0x01);
+
+ /* Encryption type - NULL */
+ tpm_buf_append_u16(, TPM_ALG_NULL);
+
+ /* Hash type - SHA256 */
+ tpm_buf_append_u16(, TPM_ALG_SHA256);
+
+ rc = tpm_send(chip, buf.data, tpm_buf_length());
+
+ if (rc)
+ goto out;
+
+ head = (struct tpm_header *)buf.data;
+
+ if (be32_to_cpu(head->length) != sizeof(struct tpm_header) +
+ sizeof(int) + sizeof(u16) + sizeof(nonce)) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ *session_handle = be32_to_cpu(*(int *)[10]);
+ memcpy(nonce, [16], sizeof(nonce));
+
+ tpm_buf_destroy();
+
+ rc = tpm_buf_init(, TPM2_ST_NO_SESSIONS, TPM2_CC_POLICY_PCR);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(, *session_handle);
+
+ /* PCR digest - read from the PCR, we'll verify creation data later */
+ tpm_buf_append_u16(, 0);
+
+ /* One PCR */
+ tpm_buf_append_u32(, 1);
+
+ /* SHA256 banks */
+ tpm_buf_append_u16(, TPM_ALG_SHA256);
+
+ /* Select PCR 23 */
+ tpm_buf_append_u32(, 0x0380);
+
+ rc = tpm_send(chip, buf.data, tpm_buf_length());
+
+ if (rc)
+ goto out;
+
+out:
+ tpm_buf_destroy();
+ return rc;
+}
+
+static int tpm_policy_get_digest(struct tpm_chip *chip, int handle,
+char *digest)
+{
+ struct tpm_header *head;
+ struct tpm_buf buf;
+ int rc;
+
+ rc = tpm_buf_init(, TPM2_ST_NO_SESSIONS, TPM2_CC_POLICY_GET_DIGEST);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(, handle);
+
+ rc = tpm_send(chip, buf.data, tpm_buf_length());
+
+ if (rc)
+ goto out;
+
+ head = (struct tpm_header *)buf.data;
+ if (be32_to_cpu(head->length) != sizeof(struct tpm_header) +
+ sizeof(u16) + SHA256_DIGEST_SIZE) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ memcpy(digest, [12], SHA256_DIGEST_SIZE);
+out:
+ tpm_buf_destroy();
+
+ return rc;
+}
+
static int tpm_certify_creationdata(struct tpm_chip *chip,
struct trusted_key_payload *payload)
{
@@ -182,11 +294,14 @@ int swsusp_encrypt_digest(struct swsusp_header *
[PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data
When TPMs generate keys, they can also generate some information
describing the state of the PCRs at creation time. This data can then
later be certified by the TPM, allowing verification of the PCR values.
This allows us to determine the state of the system at the time a key
was generated. Add an additional argument to the trusted key creation
options, allowing the user to provide the set of PCRs that should have
their values incorporated into the creation data.
Signed-off-by: Matthew Garrett
---
.../security/keys/trusted-encrypted.rst | 4 +++
include/keys/trusted-type.h | 1 +
security/keys/trusted-keys/trusted_tpm1.c | 9 +++
security/keys/trusted-keys/trusted_tpm2.c | 25 +--
4 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/Documentation/security/keys/trusted-encrypted.rst
b/Documentation/security/keys/trusted-encrypted.rst
index 1da879a68640..27bc43463ec8 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -72,6 +72,10 @@ Usage::
policyhandle= handle to an authorization policy session that defines the
same policy and with the same hash algorithm as was used
to
seal the key.
+ creationpcrs= hex integer representing the set of PCR values to be
+ included in the PCR creation data. The bit corresponding
+to each PCR should be 1 to be included, 0 to be ignored.
+TPM2 only.
"keyctl print" returns an ascii hex copy of the sealed key, which is in
standard
TPM_STORED_DATA format. The key length for new keys are always in bytes.
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 154d8a1769c3..875e05f33b84 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -47,6 +47,7 @@ struct trusted_key_options {
uint32_t policydigest_len;
unsigned char policydigest[MAX_DIGEST_SIZE];
uint32_t policyhandle;
+ uint32_t creation_pcrs;
};
extern struct key_type key_type_trusted;
diff --git a/security/keys/trusted-keys/trusted_tpm1.c
b/security/keys/trusted-keys/trusted_tpm1.c
index 74d82093cbaa..3d371ab3441f 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -709,6 +709,7 @@ enum {
Opt_hash,
Opt_policydigest,
Opt_policyhandle,
+ Opt_creationpcrs,
};
static const match_table_t key_tokens = {
@@ -724,6 +725,7 @@ static const match_table_t key_tokens = {
{Opt_hash, "hash=%s"},
{Opt_policydigest, "policydigest=%s"},
{Opt_policyhandle, "policyhandle=%s"},
+ {Opt_creationpcrs, "creationpcrs=%s"},
{Opt_err, NULL}
};
@@ -834,6 +836,13 @@ static int getoptions(char *c, struct trusted_key_payload
*pay,
return -EINVAL;
opt->policyhandle = handle;
break;
+ case Opt_creationpcrs:
+ if (!tpm2)
+ return -EINVAL;
+ res = kstrtoint(args[0].from, 16, >creation_pcrs);
+ if (res < 0)
+ return -EINVAL;
+ break;
default:
return -EINVAL;
}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c
b/security/keys/trusted-keys/trusted_tpm2.c
index a3673fffd834..282f956ad610 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -124,7 +124,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
unsigned int offset;
struct tpm_buf buf;
u32 hash;
- int i;
+ int i, j;
int rc;
for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
@@ -181,7 +181,28 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
tpm_buf_append_u16(, 0);
/* creation PCR */
- tpm_buf_append_u32(, 0);
+ if (options->creation_pcrs) {
+ /* One bank */
+ tpm_buf_append_u32(, 1);
+ /* Which bank to use */
+ tpm_buf_append_u16(, hash);
+ /* Length of the PCR bitmask */
+ tpm_buf_append_u8(, 3);
+ /* PCR bitmask */
+ for (i = 0; i < 3; i++) {
+ char tmp = 0;
+
+ for (j = 0; j < 8; j++) {
+ char bit = (i * 8) + j;
+
+ if (options->creation_pcrs & (1 << bit))
+ tmp |= (1 << j);
+ }
+ tpm_buf_append_u8(, tmp);
+ }
+ } else {
+ tpm_buf_append_u32(, 0);
+ }
if (buf.flags & TPM_BUF_OVERFLOW) {
rc = -E2BIG;
--
2.30.0.617.g56c4b15f3c-goog
[PATCH 8/9] pm: hibernate: Verify the digest encryption key
We want to ensure that the key used to encrypt the digest was created by
the kernel during hibernation. To do this we request that the TPM include
information about the value of PCR 23 at the time of key creation in the
sealed blob. On resume, we can ask the TPM to certify that the creation
data is accurate and then make sure that the PCR information in the blob
corresponds to the expected value. Since only the kernel can touch PCR
23, if an attacker generates a key themselves the value of PCR 23 will
have been different, allowing us to reject the key and boot normally
instead of resuming.
Signed-off-by: Matthew Garrett
---
include/linux/tpm.h | 1 +
kernel/power/tpm.c | 150 +++-
2 files changed, 148 insertions(+), 3 deletions(-)
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index e2075e2242a0..f6970986b097 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -216,6 +216,7 @@ enum tpm2_command_codes {
TPM2_CC_SELF_TEST = 0x0143,
TPM2_CC_STARTUP = 0x0144,
TPM2_CC_SHUTDOWN= 0x0145,
+ TPM2_CC_CERTIFYCREATION = 0x014A,
TPM2_CC_NV_READ = 0x014E,
TPM2_CC_CREATE = 0x0153,
TPM2_CC_LOAD= 0x0157,
diff --git a/kernel/power/tpm.c b/kernel/power/tpm.c
index 953dcbdc56d8..34e6cfb98ce4 100644
--- a/kernel/power/tpm.c
+++ b/kernel/power/tpm.c
@@ -14,6 +14,12 @@ static struct tpm_digest digest = { .alg_id = TPM_ALG_SHA256,
0xf1, 0x22, 0x38, 0x6c, 0x33, 0xb1, 0x14, 0xb7, 0xec, 0x05,
0x5f, 0x49}};
+/* sha256(sha256(empty_pcr | digest)) */
+static char expected_digest[] = {0x2f, 0x96, 0xf2, 0x1b, 0x70, 0xa9, 0xe8,
+ 0x42, 0x25, 0x8e, 0x66, 0x07, 0xbe, 0xbc, 0xe3, 0x1f, 0x2c, 0x84, 0x4a,
+ 0x3f, 0x85, 0x17, 0x31, 0x47, 0x9a, 0xa5, 0x53, 0xbb, 0x23, 0x0c, 0x32,
+ 0xf3};
+
struct skcipher_def {
struct scatterlist sg;
struct crypto_skcipher *tfm;
@@ -21,6 +27,39 @@ struct skcipher_def {
struct crypto_wait wait;
};
+static int sha256_data(char *buf, int size, char *output)
+{
+ struct crypto_shash *tfm;
+ struct shash_desc *desc;
+ int ret;
+
+ tfm = crypto_alloc_shash("sha256", 0, 0);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ desc = kmalloc(sizeof(struct shash_desc) +
+ crypto_shash_descsize(tfm), GFP_KERNEL);
+ if (!desc) {
+ crypto_free_shash(tfm);
+ return -ENOMEM;
+ }
+
+ desc->tfm = tfm;
+ ret = crypto_shash_init(desc);
+ if (ret != 0) {
+ crypto_free_shash(tfm);
+ kfree(desc);
+ return ret;
+ }
+
+ crypto_shash_update(desc, buf, size);
+ crypto_shash_final(desc, output);
+ crypto_free_shash(desc->tfm);
+ kfree(desc);
+
+ return 0;
+}
+
static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
int enc)
{
@@ -86,6 +125,58 @@ static int swsusp_enc_dec(struct trusted_key_payload
*payload, char *buf,
return ret;
}
+static int tpm_certify_creationdata(struct tpm_chip *chip,
+ struct trusted_key_payload *payload)
+{
+ struct tpm_header *head;
+ struct tpm_buf buf;
+ int rc;
+
+ rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CERTIFYCREATION);
+ if (rc)
+ return rc;
+
+ /* Use TPM_RH_NULL for signHandle */
+ tpm_buf_append_u32(, 0x4007);
+
+ /* Object handle */
+ tpm_buf_append_u32(, payload->blob_handle);
+
+ /* Auth */
+ tpm_buf_append_u32(, 9);
+ tpm_buf_append_u32(, TPM2_RS_PW);
+ tpm_buf_append_u16(, 0);
+ tpm_buf_append_u8(, 0);
+ tpm_buf_append_u16(, 0);
+
+ /* Qualifying data */
+ tpm_buf_append_u16(, 0);
+
+ /* Creation data hash */
+ tpm_buf_append_u16(, payload->creation_hash_len);
+ tpm_buf_append(, payload->creation_hash,
+ payload->creation_hash_len);
+
+ /* signature scheme */
+ tpm_buf_append_u16(, TPM_ALG_NULL);
+
+ /* creation ticket */
+ tpm_buf_append(, payload->tk, payload->tk_len);
+
+ rc = tpm_send(chip, buf.data, tpm_buf_length());
+ if (rc)
+ goto out;
+
+ head = (struct tpm_header *)buf.data;
+
+ if (head->return_code != 0)
+ rc = -EINVAL;
+out:
+ tpm_buf_destroy();
+
+ return rc;
+}
+
int swsusp_encrypt_digest(struct swsusp_header *header)
{
const struct cred *cred = current_cred();
@@ -95,7 +186,7 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
struct key *key;
int ret, i;
- char *keyinfo = "new\t32\tkeyhandle=0x8101";
+ char *keyinfo =
"ne
[PATCH 7/9] pm: hibernate: Optionally use TPM-backed keys to protect image integrity
A plain hash protects the hibernation image against accidental
modification, but in the face of an active attack the hash can simply be
updated to match the new image. Generate a random AES key and seal this
with the TPM, and use it to encrypt the hash. On resume, the key can be
unsealed and used to decrypt the hash. By setting PCR 23 to a specific
value we can verify that the key used was generated by the kernel during
hibernation and prevent an attacker providing their own key.
Signed-off-by: Matthew Garrett
---
kernel/power/Kconfig | 15 ++
kernel/power/Makefile| 1 +
kernel/power/hibernate.c | 11 +-
kernel/power/swap.c | 99 +++--
kernel/power/swap.h | 38 +
kernel/power/tpm.c | 294 +++
kernel/power/tpm.h | 37 +
7 files changed, 417 insertions(+), 78 deletions(-)
create mode 100644 kernel/power/swap.h
create mode 100644 kernel/power/tpm.c
create mode 100644 kernel/power/tpm.h
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index a7320f07689d..0279cc10f319 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -92,6 +92,21 @@ config HIBERNATION_SNAPSHOT_DEV
If in doubt, say Y.
+config SECURE_HIBERNATION
+ bool "Implement secure hibernation support"
+ depends on HIBERNATION && TCG_TPM
+ select KEYS
+ select TRUSTED_KEYS
+ select CRYPTO
+ select CRYPTO_SHA256
+ select CRYPTO_AES
+ select TCG_TPM_RESTRICT_PCR
+ help
+ Use a TPM-backed key to securely determine whether a hibernation
+image was written out by the kernel and has not been tampered with.
+This requires a TCG-compliant TPM2 device, which is present on most
+modern hardware.
+
config PM_STD_PARTITION
string "Default resume partition"
depends on HIBERNATION
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 5899260a8bef..2edfef897607 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SUSPEND) += suspend.o
obj-$(CONFIG_PM_TEST_SUSPEND) += suspend_test.o
obj-$(CONFIG_HIBERNATION) += hibernate.o snapshot.o swap.o
obj-$(CONFIG_HIBERNATION_SNAPSHOT_DEV) += user.o
+obj-$(CONFIG_SECURE_HIBERNATION) += tpm.o
obj-$(CONFIG_PM_AUTOSLEEP) += autosleep.o
obj-$(CONFIG_PM_WAKELOCKS) += wakelock.o
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index da0b41914177..608bfbee38f5 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -34,6 +34,7 @@
#include
#include "power.h"
+#include "tpm.h"
static int nocompress;
@@ -81,7 +82,11 @@ void hibernate_release(void)
bool hibernation_available(void)
{
- return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
+ if (security_locked_down(LOCKDOWN_HIBERNATION) &&
+ !secure_hibernation_available())
+ return false;
+
+ return nohibernate == 0;
}
/**
@@ -752,7 +757,9 @@ int hibernate(void)
flags |= SF_NOCOMPRESS_MODE;
else
flags |= SF_CRC32_MODE;
-
+#ifdef CONFIG_SECURE_HIBERNATION
+ flags |= SF_VERIFY_IMAGE;
+#endif
pm_pr_dbg("Writing hibernation image.\n");
error = swsusp_write(flags);
swsusp_free();
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index a13241a20567..eaa585731314 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -32,9 +32,10 @@
#include
#include
#include
-#include
#include "power.h"
+#include "swap.h"
+#include "tpm.h"
#define HIBERNATE_SIG "S1SUSPEND"
@@ -89,34 +90,6 @@ struct swap_map_page_list {
struct swap_map_page_list *next;
};
-/**
- * The swap_map_handle structure is used for handling swap in
- * a file-alike way
- */
-
-struct swap_map_handle {
- struct swap_map_page *cur;
- struct swap_map_page_list *maps;
- struct shash_desc *desc;
- sector_t cur_swap;
- sector_t first_sector;
- unsigned int k;
- unsigned long reqd_free_pages;
- u32 crc32;
- u8 digest[SHA256_DIGEST_SIZE];
-};
-
-struct swsusp_header {
- char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
- sizeof(u32) - SHA256_DIGEST_SIZE];
- u32 crc32;
- u8 digest[SHA256_DIGEST_SIZE];
- sector_t image;
- unsigned int flags; /* Flags to pass to the "boot" kernel */
- charorig_sig[10];
- charsig[10];
-} __packed;
-
static struct swsusp_header *swsusp_header;
/**
@@ -337,6 +310,9 @@ static int mark_swapfiles(struct swap_map_handle *handle,
unsigned int flags)
swsusp_header->crc32 = handle->crc32;
memcpy(swsusp_heade
[PATCH 6/9] pm: hibernate: Optionally store and verify a hash of the image
Calculate and store a cryptographically secure hash of the hibernation
image if SF_VERIFY_IMAGE is set in the hibernation flags. This allows
detection of a corrupt image, but has the disadvantage that it requires
the blocks be read in in linear order.
Signed-off-by: Matthew Garrett
---
kernel/power/power.h | 1 +
kernel/power/swap.c | 131 +++
2 files changed, 110 insertions(+), 22 deletions(-)
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 778bf431ec02..b8e00b9dcee8 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -168,6 +168,7 @@ extern int swsusp_swap_in_use(void);
#define SF_PLATFORM_MODE 1
#define SF_NOCOMPRESS_MODE 2
#define SF_CRC32_MODE 4
+#define SF_VERIFY_IMAGE 8
/* kernel/power/hibernate.c */
extern int swsusp_check(void);
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 72e33054a2e1..a13241a20567 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -31,6 +31,8 @@
#include
#include
#include
+#include
+#include
#include "power.h"
@@ -95,17 +97,20 @@ struct swap_map_page_list {
struct swap_map_handle {
struct swap_map_page *cur;
struct swap_map_page_list *maps;
+ struct shash_desc *desc;
sector_t cur_swap;
sector_t first_sector;
unsigned int k;
unsigned long reqd_free_pages;
u32 crc32;
+ u8 digest[SHA256_DIGEST_SIZE];
};
struct swsusp_header {
char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
- sizeof(u32)];
+ sizeof(u32) - SHA256_DIGEST_SIZE];
u32 crc32;
+ u8 digest[SHA256_DIGEST_SIZE];
sector_t image;
unsigned int flags; /* Flags to pass to the "boot" kernel */
charorig_sig[10];
@@ -305,6 +310,9 @@ static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
* We are relying on the behavior of blk_plug that a thread with
* a plug will flush the plug list before sleeping.
*/
+ if (!hb)
+ return 0;
+
wait_event(hb->wait, atomic_read(>count) == 0);
return blk_status_to_errno(hb->error);
}
@@ -327,6 +335,8 @@ static int mark_swapfiles(struct swap_map_handle *handle,
unsigned int flags)
swsusp_header->flags = flags;
if (flags & SF_CRC32_MODE)
swsusp_header->crc32 = handle->crc32;
+ memcpy(swsusp_header->digest, handle->digest,
+ SHA256_DIGEST_SIZE);
error = hib_submit_io(REQ_OP_WRITE, REQ_SYNC,
swsusp_resume_block, swsusp_header, NULL);
} else {
@@ -417,6 +427,7 @@ static void release_swap_writer(struct swap_map_handle
*handle)
static int get_swap_writer(struct swap_map_handle *handle)
{
int ret;
+ struct crypto_shash *tfm;
ret = swsusp_swap_check();
if (ret) {
@@ -437,7 +448,28 @@ static int get_swap_writer(struct swap_map_handle *handle)
handle->k = 0;
handle->reqd_free_pages = reqd_free_pages();
handle->first_sector = handle->cur_swap;
+
+ tfm = crypto_alloc_shash("sha256", 0, 0);
+ if (IS_ERR(tfm)) {
+ ret = -EINVAL;
+ goto err_rel;
+ }
+ handle->desc = kmalloc(sizeof(struct shash_desc) +
+ crypto_shash_descsize(tfm), GFP_KERNEL);
+ if (!handle->desc) {
+ ret = -ENOMEM;
+ goto err_rel;
+ }
+
+ handle->desc->tfm = tfm;
+
+ ret = crypto_shash_init(handle->desc);
+ if (ret != 0)
+ goto err_free;
+
return 0;
+err_free:
+ kfree(handle->desc);
err_rel:
release_swap_writer(handle);
err_close:
@@ -446,7 +478,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
}
static int swap_write_page(struct swap_map_handle *handle, void *buf,
- struct hib_bio_batch *hb)
+ struct hib_bio_batch *hb, bool hash)
{
int error = 0;
sector_t offset;
@@ -454,6 +486,7 @@ static int swap_write_page(struct swap_map_handle *handle,
void *buf,
if (!handle->cur)
return -EINVAL;
offset = alloc_swapdev_block(root_swap);
+ crypto_shash_update(handle->desc, buf, PAGE_SIZE);
error = write_page(buf, offset, hb);
if (error)
return error;
@@ -496,6 +529,7 @@ static int flush_swap_writer(struct swap_map_handle *handle)
static int swap_writer_finish(struct swap_map_handle *handle,
unsigned int flags, int error)
{
+ crypto_shash_final(handle->desc, handle->digest);
if (!error) {
pr_info("S");
error = mark_swapfiles(handle, flags);
@@ -560,7 +59
[PATCH 4/9] security: keys: trusted: Store the handle of a loaded key
Certain in-kernel operations using a trusted key (such as creation
certification) require knowledge of the handle it's loaded at. Keep
a copy of that in the payload.
Signed-off-by: Matthew Garrett
---
include/keys/trusted-type.h | 1 +
security/keys/trusted-keys/trusted_tpm2.c | 6 --
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 020e01a99ea4..154d8a1769c3 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -21,6 +21,7 @@
struct trusted_key_payload {
struct rcu_head rcu;
+ unsigned int blob_handle;
unsigned int key_len;
unsigned int blob_len;
unsigned int creation_len;
diff --git a/security/keys/trusted-keys/trusted_tpm2.c
b/security/keys/trusted-keys/trusted_tpm2.c
index 6357a51a24e9..a3673fffd834 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -272,11 +272,13 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
}
rc = tpm_send(chip, buf.data, tpm_buf_length());
- if (!rc)
+ if (!rc) {
*blob_handle = be32_to_cpup(
(__be32 *) [TPM_HEADER_SIZE]);
- else
+ payload->blob_handle = *blob_handle;
+ } else {
goto out;
+ }
rc = tpm2_unpack_blob(payload);
out:
--
2.30.0.617.g56c4b15f3c-goog
[PATCH 0/9] Enable hibernation when Lockdown is enabled
Lockdown in integrity mode aims to ensure that arbitrary code cannot end up in ring 0 without owner approval. The combination of module signing and secure boot gets most of the way there, but various other features are disabled by lockdown in order to avoid more convoluted approaches that would enable unwanted code to end up in kernel space. One of these is hibernation, since at present the kernel performs no verification of the code it's resuming. If hibernation were permitted, an attacker with root (but not kernel) privileges could disable swap, write a valid hibernation image containing code of their own choosing to the swap partition, and then reboot. On reboot, the kernel would happily resume the provided code. This patchset aims to provide a secure implementation of hibernation. It is based on the presumption that simply storing a key in-kernel is insufficient, since if Lockdown is merely in integrity (rather than confidentiality) mode we assume that root is able to read kernel memory and so would be able to obtain these secrets. It also aims to be unspoofable - an attacker should not be able to write out a hibernation image using cryptographic material under their control. TPMs can be used to generate key material that is encrypted with a key that does not leave the TPM. This means that we can generate an AES key, encrypt the image hash with it, encrypt it with a TPM-backed key, and store the encrypted key in the hibernation image. On resume, we pass the key back to the TPM, receive the unencrypted AES key, and use that to validate the image. However, this is insufficient. Nothing prevents anyone else with access to the TPM asking it to decrypt the key. We need to be able to distinguish between accesses that come from the kernel directly and accesses that come from userland. TPMs have several Platform Configuration Registers (PCRs) which are used for different purposes. PCRs are initialised to a known value, and cannot be modified directly by the platform. Instead, the platform can provide a hash of some data to the TPM. The TPM combines the existing PCR value with the new hash, and stores the hash of this combination in the PCR. Most PCRs can only be extended, which means that the only way to achieve a specific value for a TPM is to perform the same series of writes. When secrets are encrypted by the TPM, they can be accompanied by a policy that describes the state the TPM must be in in order for it to decrypt them. If the TPM is not in this state, it will refuse to decrypt the material even if it is otherwise capable of doing so. This allows keys to be tied to specific system state - if the system is not in that state, the TPM will not grant access. PCR 23 is special in that it can be reset on demand. This patchset re-purposes PCR 23 and blocks userland's ability to extend or reset it. The kernel is then free to impose policy by resetting PCR 23 to a known starting point, extending it with a known value, encrypting key material with a policy that ties it to PCR 23, and then resetting it. Even if userland has access to the encrypted blob, it cannot decrypt it since it has no way to force PCR 23 to be in the same state. So. During hibernation we freeze userland. We then reset PCR 23 and extend it to a known value. We generate a key, use it and then encrypt it with the TPM. When we encrypt it, we define a policy which states that the TPM must have the same PCR 23 value as it presently does. We also store the current PCR 23 value in the key metadata. On resume, we again freeze userland, reset PCR 23 and extend it to the same value. We decrypt the key, and verify from the metadata that it was created when PCR 23 had the expected value. If so, we use it to decrypt the hash used to verify the hibernation image and ensure that the image matches it. If everything looks good, we resume. If not, we return to userland. Either way, we reset PCR 23 before any userland code runs again. This all works on my machine, but it's imperfect - there's a meaningful performance hit on resume forced by reading all the blocks in-order, and it probably makes more sense to do that after reads are complete instead but I wanted to get the other components of this out for review first. It's also not ideal from a security perspective until there's more ecosystem integration - we need a kernel to be able to assert to a signed bootloader that it implements this, since otherwise an attacker can simply downgrade to a kernel that doesn't implement this policy and gain access to PCR 23 themselves. There's ongoing work in the UEFI bootloader space that would make this possible.
[PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob
Performing any sort of state validation of a sealed TPM blob requires
being able to access the individual members in the response. Parse the
blob sufficiently to be able to stash pointers to each member, along
with the length.
Signed-off-by: Matthew Garrett
---
include/keys/trusted-type.h | 8 +++
security/keys/trusted-keys/trusted_tpm2.c | 67 ++-
2 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a61d8f..020e01a99ea4 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -16,14 +16,22 @@
#define MAX_BLOB_SIZE 512
#define MAX_PCRINFO_SIZE 64
#define MAX_DIGEST_SIZE64
+#define MAX_CREATION_DATA 412
+#define MAX_TK 76
struct trusted_key_payload {
struct rcu_head rcu;
unsigned int key_len;
unsigned int blob_len;
+ unsigned int creation_len;
+ unsigned int creation_hash_len;
+ unsigned int tk_len;
unsigned char migratable;
unsigned char key[MAX_KEY_SIZE + 1];
unsigned char blob[MAX_BLOB_SIZE];
+ unsigned char *creation;
+ unsigned char *creation_hash;
+ unsigned char *tk;
};
struct trusted_key_options {
diff --git a/security/keys/trusted-keys/trusted_tpm2.c
b/security/keys/trusted-keys/trusted_tpm2.c
index 08ec7f48f01d..6357a51a24e9 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -50,6 +50,63 @@ static void tpm2_buf_append_auth(struct tpm_buf *buf, u32
session_handle,
tpm_buf_append(buf, hmac, hmac_len);
}
+static int tpm2_unpack_blob(struct trusted_key_payload *payload)
+{
+ int tmp, offset;
+
+ /* Find the length of the private data */
+ tmp = be16_to_cpup((__be16 *) >blob[0]);
+ offset = tmp + 2;
+ if (offset > payload->blob_len)
+ return -EFAULT;
+
+ /* Find the length of the public data */
+ tmp = be16_to_cpup((__be16 *) >blob[offset]);
+ offset += tmp + 2;
+ if (offset > payload->blob_len)
+ return -EFAULT;
+
+ /* Find the length of the creation data and store it */
+ tmp = be16_to_cpup((__be16 *) >blob[offset]);
+ if (tmp > MAX_CREATION_DATA)
+ return -E2BIG;
+
+ if ((offset + tmp + 2) > payload->blob_len)
+ return -EFAULT;
+
+ payload->creation = >blob[offset + 2];
+ payload->creation_len = tmp;
+ offset += tmp + 2;
+
+ /* Find the length of the creation hash and store it */
+ tmp = be16_to_cpup((__be16 *) >blob[offset]);
+ if (tmp > MAX_DIGEST_SIZE)
+ return -E2BIG;
+
+ if ((offset + tmp + 2) > payload->blob_len)
+ return -EFAULT;
+
+ payload->creation_hash = >blob[offset + 2];
+ payload->creation_hash_len = tmp;
+ offset += tmp + 2;
+
+ /*
+* Store the creation ticket. TPMT_TK_CREATION is two bytes of tag,
+* four bytes of handle, and then the digest length and digest data
+*/
+ tmp = be16_to_cpup((__be16 *) >blob[offset + 6]);
+ if (tmp > MAX_TK)
+ return -E2BIG;
+
+ if ((offset + tmp + 8) > payload->blob_len)
+ return -EFAULT;
+
+ payload->tk = >blob[offset];
+ payload->tk_len = tmp + 8;
+
+ return 0;
+}
+
/**
* tpm2_seal_trusted() - seal the payload of a trusted key
*
@@ -64,6 +121,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_options *options)
{
unsigned int blob_len;
+ unsigned int offset;
struct tpm_buf buf;
u32 hash;
int i;
@@ -139,14 +197,16 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
rc = -E2BIG;
goto out;
}
- if (tpm_buf_length() < TPM_HEADER_SIZE + 4 + blob_len) {
+ offset = TPM_HEADER_SIZE + 4;
+ if (tpm_buf_length() < offset + blob_len) {
rc = -EFAULT;
goto out;
}
- memcpy(payload->blob, [TPM_HEADER_SIZE + 4], blob_len);
+ memcpy(payload->blob, [offset], blob_len);
payload->blob_len = blob_len;
+ rc = tpm2_unpack_blob(payload);
out:
tpm_buf_destroy();
@@ -215,7 +275,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
if (!rc)
*blob_handle = be32_to_cpup(
(__be32 *) [TPM_HEADER_SIZE]);
+ else
+ goto out;
+ rc = tpm2_unpack_blob(payload);
out:
tpm_buf_destroy();
--
2.30.0.617.g56c4b15f3c-goog
[PATCH 1/9] tpm: Add support for in-kernel resetting of PCRs
Add an internal command for resetting a PCR.
Signed-off-by: Matthew Garrett
---
drivers/char/tpm/tpm-interface.c | 28 +
drivers/char/tpm/tpm.h | 2 ++
drivers/char/tpm/tpm1-cmd.c | 34 ++
drivers/char/tpm/tpm2-cmd.c | 36
include/linux/tpm.h | 7 +++
5 files changed, 107 insertions(+)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1621ce818705..17b8643ee109 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -342,6 +342,34 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
}
EXPORT_SYMBOL_GPL(tpm_pcr_extend);
+/**
+ * tpm_pcr_reset - reset the specified PCR
+ * @chip: a tpm_chip instance, %NULL for the default chip
+ * @pcr_idx: the PCR to be reset
+ *
+ * Return: same as with tpm_transmit_cmd()
+ */
+int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx)
+{
+ int rc;
+
+ chip = tpm_find_get_ops(chip);
+ if (!chip)
+ return -ENODEV;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ rc = tpm2_pcr_reset(chip, pcr_idx);
+ goto out;
+ }
+
+ rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR");
+
+out:
+ tpm_put_ops(chip);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_pcr_reset);
+
/**
* tpm_send - send a TPM command
* @chip: a tpm_chip instance, %NULL for the default chip
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 947d1db0a5cc..746f7696bdc0 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -176,6 +176,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip);
unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
const char *log_msg);
+int tpm1_pcr_reset(struct tpm_chip *chip, u32 pcr_idx, const char *log_msg);
int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
const char *desc, size_t min_cap_length);
@@ -220,6 +221,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest, u16 *digest_size_ptr);
int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digests);
+int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx);
int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
u32 *value, const char *desc);
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index ca7158fa6e6c..36990e9d2dc1 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -478,6 +478,40 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
const u8 *hash,
return rc;
}
+struct tpm_pcr_selection {
+ u16 size_of_select;
+ u8 pcr_select[3];
+} __packed;
+
+#define TPM_ORD_PCR_RESET 200
+int tpm1_pcr_reset(struct tpm_chip *chip, u32 pcr_idx, const char *log_msg)
+{
+ struct tpm_pcr_selection selection;
+ struct tpm_buf buf;
+ int i, rc;
+ char tmp;
+
+ rc = tpm_buf_init(, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_RESET);
+ if (rc)
+ return rc;
+
+ selection.size_of_select = 3;
+
+ for (i = 0; i < selection.size_of_select; i++) {
+ tmp = 0;
+ if (pcr_idx / 3 == i) {
+ pcr_idx -= i * 8;
+ tmp |= 1 << pcr_idx;
+ }
+ selection.pcr_select[i] = tmp;
+ }
+ tpm_buf_append(, (u8 *), sizeof(selection));
+
+ rc = tpm_transmit_cmd(chip, , sizeof(selection), log_msg);
+ tpm_buf_destroy();
+ return rc;
+}
+
#define TPM_ORD_GET_CAP 101
ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
const char *desc, size_t min_cap_length)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index eff1f12d981a..9609ae8086c6 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -269,6 +269,42 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
return rc;
}
+/**
+ * tpm2_pcr_reset() - reset a PCR
+ *
+ * @chip: TPM chip to use.
+ * @pcr_idx: index of the PCR.
+ *
+ * Return: Same as with tpm_transmit_cmd.
+ */
+int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx)
+{
+ struct tpm_buf buf;
+ struct tpm2_null_auth_area auth_area;
+ int rc;
+
+ rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_PCR_RESET);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(, pcr_idx);
+
+ auth_area.handle = cpu_to_be32(TPM2_RS_PW);
+ auth_area.nonce_size = 0;
+ auth_area.attributes = 0;
+
[PATCH 2/9] tpm: Allow PCR 23 to be restricted to kernel-only use
Under certain circumstances it might be desirable to enable the creation
of TPM-backed secrets that are only accessible to the kernel. In an
ideal world this could be achieved by using TPM localities, but these
don't appear to be available on consumer systems. An alternative is to
simply block userland from modifying one of the resettable PCRs, leaving
it available to the kernel. If the kernel ensures that no userland can
access the TPM while it is carrying out work, it can reset PCR 23,
extend it to an arbitrary value, create or load a secret, and then reset
the PCR again. Even if userland somehow obtains the sealed material, it
will be unable to unseal it since PCR 23 will never be in the
appropriate state.
Signed-off-by: Matthew Garrett
---
drivers/char/tpm/Kconfig | 10 +
drivers/char/tpm/tpm-dev-common.c | 8 +++
drivers/char/tpm/tpm.h| 21 +++
drivers/char/tpm/tpm1-cmd.c | 35 +++
drivers/char/tpm/tpm2-cmd.c | 22 +++
drivers/char/tpm/tpm2-space.c | 2 +-
6 files changed, 97 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index a18c314da211..bba30fb16a2e 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -190,4 +190,14 @@ config TCG_FTPM_TEE
This driver proxies for firmware TPM running in TEE.
source "drivers/char/tpm/st33zp24/Kconfig"
+
+config TCG_TPM_RESTRICT_PCR
+ bool "Restrict userland access to PCR 23"
+ depends on TCG_TPM
+ help
+ If set, block userland from extending or resetting PCR 23. This
+ allows it to be restricted to in-kernel use, preventing userland
+ from being able to make use of data sealed to the TPM by the kernel.
+ This is required for secure hibernation support, but should be left
+ disabled if any userland may require access to PCR23.
endif # TCG_TPM
diff --git a/drivers/char/tpm/tpm-dev-common.c
b/drivers/char/tpm/tpm-dev-common.c
index 1784530b8387..d3db4fd76257 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -193,6 +193,14 @@ ssize_t tpm_common_write(struct file *file, const char
__user *buf,
priv->response_read = false;
*off = 0;
+ if (priv->chip->flags & TPM_CHIP_FLAG_TPM2)
+ ret = tpm2_cmd_restricted(priv->chip, priv->data_buffer, size);
+ else
+ ret = tpm1_cmd_restricted(priv->chip, priv->data_buffer, size);
+
+ if (ret)
+ goto out;
+
/*
* If in nonblocking mode schedule an async job to send
* the command return the size.
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 746f7696bdc0..8eed5016d733 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -232,6 +232,8 @@ void tpm2_shutdown(struct tpm_chip *chip, u16
shutdown_type);
unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
int tpm2_probe(struct tpm_chip *chip);
int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip);
+int tpm_find_and_validate_cc(struct tpm_chip *chip, struct tpm_space *space,
+const void *buf, size_t bufsiz);
int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
int tpm2_init_space(struct tpm_space *space, unsigned int buf_size);
void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
@@ -245,4 +247,23 @@ void tpm_bios_log_setup(struct tpm_chip *chip);
void tpm_bios_log_teardown(struct tpm_chip *chip);
int tpm_dev_common_init(void);
void tpm_dev_common_exit(void);
+
+#ifdef CONFIG_TCG_TPM_RESTRICT_PCR
+#define TPM_RESTRICTED_PCR 23
+
+int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size);
+int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size);
+#else
+static inline int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer,
+ size_t size)
+{
+ return 0;
+}
+
+static inline int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer,
+ size_t size)
+{
+ return 0;
+}
+#endif
#endif
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 36990e9d2dc1..2dab1647d89c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -840,3 +840,38 @@ int tpm1_get_pcr_allocation(struct tpm_chip *chip)
return 0;
}
+
+#ifdef CONFIG_TCG_TPM_RESTRICT_PCR
+int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
+{
+ struct tpm_header *header = (struct tpm_header *)buffer;
+ char len, offset;
+ u32 *pcr;
+ int pos;
+
+ switch (be32_to_cpu(header->ordinal)) {
+ case TPM_ORD_PCR_EXTEND:
+ if (size < (TPM_HEADER_SIZE + sizeof(u32)))
+ return -EINVAL;
+ pcr = (u32 *)[TPM_HEADER_SIZE];
+
Re: [PATCH 1/3] thermal: core: Add indication for userspace usage
On Sun, Nov 29, 2020 at 9:36 PM Kai-Heng Feng wrote: > > We are seeing thermal shutdown on Intel based mobile workstations, the > shutdown happens during the first trip handle in > thermal_zone_device_register(): > kernel: thermal thermal_zone15: critical temperature reached (101 C), > shutting down Is the temperature reported by the thermal zone actually correct here? 101 C seems extremely excessive.
Re: [PATCH] thermal/core: Make 'forced_passive' as obsolete candidate
On Sun, Dec 13, 2020 at 12:39:26AM +0100, Daniel Lezcano wrote: > On 12/12/2020 21:08, Matthew Garrett wrote: > > Anything that provides a trip point that has no active notifications and > > doesn't provide any information that tells the kernel to poll it. > > I'm not able to create a setup as you describe working correctly with > the forced passive trip point. > > The forced passive trip can not be detected as there is no comparison > with the defined temperature in the thermal_zone_device_update() function. The logic seems to be in the step_wise thermal governor. I'm not sure why it would be used in thermal_zone_device_update() - the entire point is that we don't get updates from the device? > If my analysis is correct, this 'feature' is broken since years, more > than 8 years to be exact and nobody complained. I've no problem with it being removed if there are no users, but in that case the justification should be rewritten - ACPI table updates aren't a complete replacement for the functionality offered (and can't be used if the lockdown LSM is being used in any case).
Re: [PATCH] thermal/core: Make 'forced_passive' as obsolete candidate
On Sat, Dec 12, 2020 at 10:11:31AM +0100, Daniel Lezcano wrote: > On 12/12/2020 04:50, Matthew Garrett wrote: > > Yes - what's the reason to do so? > > I'm cleaning up the thermal core code, so questioning every old ABI. > > > The code isn't specific to ACPI, > > so being able to override ACPI tables doesn't seem to justify it. > > I agree, the code is no specific to ACPI. > > What non-ACPI architecture, without device tree or platform data would > need the 'passive' option today ? Anything that provides a trip point that has no active notifications and doesn't provide any information that tells the kernel to poll it.
Re: [PATCH] thermal/core: Make 'forced_passive' as obsolete candidate
On Fri, Dec 11, 2020 at 02:17:55PM +0100, Daniel Lezcano wrote: > On 08/12/2020 16:30, Daniel Lezcano wrote: > > The passive file in sysfs forces the usage of a passive trip point set > > by the userspace when a broken BIOS does not provide the mitigation > > temperature for such thermal zone. The hardware evolved a lot since > > 2008 as a good thermal management is no longer an option. > > > > Linux on the other side also provides now a way to load fixed ACPI > > table via the option ACPI_TABLE_UPGRADE, so additionnal trip point > > could be added there. > > > > Set the option obsolete and plan to remove it, so the corresponding > > code can be removed from the core code and allow more cleanups the > > thermal framework deserves. > > > > Signed-off-by: Daniel Lezcano > > --- > > Is there any concern about this change ? Yes - what's the reason to do so? The code isn't specific to ACPI, so being able to override ACPI tables doesn't seem to justify it.
Re: [PATCH -v2.1] x86/msr: Filter MSR writes
On Tue, Nov 17, 2020 at 1:21 PM Matthew Garrett wrote: > > On Tue, Nov 17, 2020 at 1:00 PM Mathieu Chouquet-Stringer > wrote: > > > I'm late to the party but it seems allowing MSR_IA32_ENERGY_PERF_BIAS > > has the downside of flagging the kernel as tainted without telling you > > why if you use something like x86_energy_perf_policy (from > > tools/power/x86/x86_energy_perf_policy) which itself is used by tuned. > > I initially pushed back against a kernel interface for > MSR_IA32_ENERGY_PERF_BIAS (cc: Len Brown, who tried mightily to > convince me I was wrong here) on the grounds that it was exporting an > implementation detail rather than providing a generic interface, and > that it was something that could be done via userland instead. I > thought we'd end up with more examples of similar functionality and > could tie it into something more reasonable - history has proven me > wrong on that. I think it's probably reasonable to dust off the driver > that Len submitted however many years ago and push that into the > kernel now. But ha ok based on Borislav's response it looks like someone's already done that.
Re: [PATCH -v2.1] x86/msr: Filter MSR writes
On Tue, Nov 17, 2020 at 1:00 PM Mathieu Chouquet-Stringer wrote: > I'm late to the party but it seems allowing MSR_IA32_ENERGY_PERF_BIAS > has the downside of flagging the kernel as tainted without telling you > why if you use something like x86_energy_perf_policy (from > tools/power/x86/x86_energy_perf_policy) which itself is used by tuned. I initially pushed back against a kernel interface for MSR_IA32_ENERGY_PERF_BIAS (cc: Len Brown, who tried mightily to convince me I was wrong here) on the grounds that it was exporting an implementation detail rather than providing a generic interface, and that it was something that could be done via userland instead. I thought we'd end up with more examples of similar functionality and could tie it into something more reasonable - history has proven me wrong on that. I think it's probably reasonable to dust off the driver that Len submitted however many years ago and push that into the kernel now.
Re: [PATCH] platform/x86: dell-privacy: Add support for new privacy driver
On Wed, Nov 11, 2020 at 07:21:07AM +, Yuan, Perry wrote:
> > > +status = acpi_evaluate_object(NULL, ACPI_PRIVACY_EC_ACK, NULL,
> > NULL);
> > > +if (ACPI_FAILURE(status)) {
> > > +dev_err(led_cdev->dev, "Error setting privacy audio EC ack
> > value: %d\n",status);
> > > +return -EIO;
> > > +}
> > > +return 0;
> > > +}
> >
> > What's actually being set here? You don't seem to be passing any arguments.
>
> Yes, it is a EC ack notification without any arguments needed.
I'm confused why it's being exposed as an LED device in that case -
there's an expectation that this is something that actually controls a
real LED, which means responding to state. Are you able to share the
acpidump of a machine with this device?
--
Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH] platform/x86: dell-privacy: Add support for new privacy driver
On Tue, Nov 03, 2020 at 04:55:42AM -0800, Perry Yuan wrote:
> +#define PRIVACY_PlATFORM_NAME "dell-privacy-acpi"
> +#define ACPI_PRIVACY_DEVICE "\\_SB.PC00.LPCB.ECDV"
This looks like the EC rather than a privacy device? If so, you probably
want to collaborate with the EC driver to obtain the handle rather than
depending on the path, unless it's guaranteed that this path will never
change.
> +static int micmute_led_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> +acpi_status status;
> +
> +status = acpi_evaluate_object(NULL, ACPI_PRIVACY_EC_ACK, NULL, NULL);
> +if (ACPI_FAILURE(status)) {
> +dev_err(led_cdev->dev, "Error setting privacy audio EC ack value:
> %d\n",status);
> +return -EIO;
> +}
> +return 0;
> +}
What's actually being set here? You don't seem to be passing any
arguments.
> +static const struct acpi_device_id privacy_acpi_device_ids[] = {
> +{"PNP0C09", 0},
Oooh no please don't do this - you'll trigger autoloading on everything
that exposes a PNP0C09 device.
--
Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
On Thu, Oct 15, 2020 at 2:44 PM Jerry Snitselaar wrote: > > There is a misconfiguration in the bios of the gpio pin used for the > interrupt in the T490s. When interrupts are enabled in the tpm_tis > driver code this results in an interrupt storm. This was initially > reported when we attempted to enable the interrupt code in the tpm_tis > driver, which previously wasn't setting a flag to enable it. Due to > the reports of the interrupt storm that code was reverted and we went back > to polling instead of using interrupts. Now that we know the T490s problem > is a firmware issue, add code to check if the system is a T490s and > disable interrupts if that is the case. This will allow us to enable > interrupts for everyone else. If the user has a fixed bios they can > force the enabling of interrupts with tpm_tis.interrupts=1 on the > kernel command line. I think an implication of this is that systems haven't been well-tested with interrupts enabled. In general when we've found a firmware issue in one place it ends up happening elsewhere as well, so it wouldn't surprise me if there are other machines that will also be unhappy with interrupts enabled. Would it be possible to automatically detect this case (eg, if we get more than a certain number of interrupts in a certain timeframe immediately after enabling the interrupt) and automatically fall back to polling in that case? It would also mean that users with fixed firmware wouldn't need to pass a parameter.
Re: [PATCH] x86/msr: do not warn on writes to OC_MAILBOX
On Tue, Sep 8, 2020 at 1:35 PM Andy Lutomirski wrote: > Undervolting is a bit different. It’s a genuinely useful configuration that > can affect system stability. In general, I think it should be allowed, and > it should have a real driver in tree. Agree that this should be a proper driver rather than permitting arbitrary poking (especially if this isn't an architecturally defined MSR - there's no guarantee that it'll have the same functionality everywhere). > But this has a tricky interaction with lockdown. An interface that allows > root to destabilize a system may well allow root to escalate privileges. But > I think that making lockdown=integrity prevent tuning voltages and such would > be quite obnoxious. Indeed - plundervolt.com is a demonstration of this. Any realistic attack involves being able to drop the voltage enough to interfere with a calculation and then raise it again before everything else falls over, so simply applying some rate limiting seems like it would be sufficient. > Should there perhaps be a separate lockdown bit for stability? If it's a sysfs interface then I think it'd be easy enough for people who care to just add an SELinux or Apparmor rule, tbh.
Re: [PATCH] platform/x86:dell-laptop:Add battery charging thresholds and charging mode switch.
On Tue, Aug 04, 2020 at 05:46:30AM +, Yuan, Perry wrote: > It is not patch issue, the kernel config needs to add "CONFIG_ACPI_BATTERY=y" In that case you probably want to add a dependency to ACPI_BATTERY in the DELL_LAPTOP Kconfig. -- Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH] platform/x86:dell-laptop:Add battery charging thresholds and charging mode switch.
On Tue, Jul 28, 2020 at 11:54:24PM -0700, Perry Yuan wrote: This seems extremely useful, but: > diff --git a/Documentation/ABI/testing/sysfs-class-power > b/Documentation/ABI/testing/sysfs-class-power > index bf3b48f022dc..a8adc3b0ca4b 100644 > --- a/Documentation/ABI/testing/sysfs-class-power > +++ b/Documentation/ABI/testing/sysfs-class-power > @@ -334,6 +334,29 @@ Description: > Access: Read > Valid values: Represented in microvolts > > +What: > /sys/class/power_supply//charge_control_charging_mode > +Date:March 2020 > +Contact: linux...@vger.kernel.org The values here seem very Dell specific, but this is going into a generic sysfs path. Really stuff here should be as vendor independent as possible. If these values don't correspond to a wider industry specification it probably makes sense to make this something Dell specific. -- Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH -v2.1] x86/msr: Filter MSR writes
On Tue, Jul 14, 2020 at 9:04 AM Chris Down wrote: > Either way, again, this isn't really the point. :-) The point is that there > _are_ currently widespread cases involving poking MSRs from userspace, however > sacrilegious or ugly (which I agree with!), and while people should be told > about that, it's excessive to have the potential to take up 80% of kmsg in the > default configuration. It doesn't take thousands of messages to get the > message > across, that's what a custom printk ratelimit is for. Agreed - we should now offer all the necessary interfaces to avoid userspace having to hit MSRs directly for thermal management, but that wasn't always the case, and as a result there's tooling that still behaves this way.
Re: Lost PCIe PME after a914ff2d78ce ("PCI/ASPM: Don't select CONFIG_PCIEASPM by default")
On Sat, May 30, 2020 at 08:33:50AM +0200, Heiner Kallweit wrote:
> It *was* default y. This changed with a914ff2d78ce ("PCI/ASPM: Don't
> select CONFIG_PCIEASPM by default") and that's what triggered the
> problem. If there's no easy solution, then maybe it's best to revert
> the change for now.
Oh, sorry, I was looking at mainline. CONFIG_PCIEASPM should
*definitely* be enabled by default - platforms expect the OS to support
it. If we want to get rid of default y then I think it'd make more sense
to have a CONFIG_DISABLE_PCIEASPM that's under EXPERT, and people who
really want to disable the code can do so.
--
Matthew Garrett | mj...@srcf.ucam.org
Re: Lost PCIe PME after a914ff2d78ce ("PCI/ASPM: Don't select CONFIG_PCIEASPM by default")
On Sat, May 30, 2020 at 12:26:17AM +0200, Heiner Kallweit wrote: > Current situation means that PME is unusable on all systems where > pcie_aspm_support_enabled() returns false, what is basically every > system except EXPERT mode is enabled and CONFIG_PCIEASPM is set. > So we definitely need to do something. CONFIG_PCIEASPM is default y. I don't think there's huge value in adding complexity to deal with it being disabled, given that the kernel is then in a configuration that no vendor is testing against. There are existing runtime mechanisms to disable it at runtime. -- Matthew Garrett | mj...@srcf.ucam.org
Re: Lost PCIe PME after a914ff2d78ce ("PCI/ASPM: Don't select CONFIG_PCIEASPM by default")
On Fri, May 29, 2020 at 03:21:35PM -0500, Bjorn Helgaas wrote: > Yeah, that makes sense. I can't remember the details, but I'm pretty > sure there's a reason why we ask for the whole set of things. Seems > like it solved some problem. I think Matthew Garrett might have been > involved in that. This was https://bugzilla.redhat.com/show_bug.cgi?id=638912 - some firmware misbehaves unless you pass the same set of supported functionality as Windows does. -- Matthew Garrett | mj...@srcf.ucam.org
Re: [GRUB PATCH RFC 12/18] i386/efi: Report UEFI Secure Boot status to the Linux kernel
On Wed, May 6, 2020 at 6:33 AM Daniel Kiper wrote: > > On Tue, May 05, 2020 at 10:29:05AM -0700, Matthew Garrett wrote: > > On Mon, May 4, 2020 at 4:25 PM Daniel Kiper wrote: > > > > > > Otherwise the kernel does not know its state and cannot enable various > > > security features depending on UEFI Secure Boot. > > > > I think this needs more context. If the kernel is loaded via the EFI > > boot stub, the kernel is aware of the UEFI secure boot state. Why > > duplicate this functionality in order to avoid the EFI stub? > > It seems to me that this issue was discussed here [1] and here [2]. > So, if you want me to improve the commit message I am OK with that. Yes, I think just providing an explanation for why it's currently necessary for you to duplicate this is reasonable.
Re: [GRUB PATCH RFC 12/18] i386/efi: Report UEFI Secure Boot status to the Linux kernel
On Mon, May 4, 2020 at 4:25 PM Daniel Kiper wrote: > > Otherwise the kernel does not know its state and cannot enable various > security features depending on UEFI Secure Boot. I think this needs more context. If the kernel is loaded via the EFI boot stub, the kernel is aware of the UEFI secure boot state. Why duplicate this functionality in order to avoid the EFI stub?
Re: [GIT PULL][SECURITY] Kernel lockdown patches for v5.4
On Wed, Sep 25, 2019 at 7:54 AM Jiri Kosina wrote: > Seems like this didn't happen (yet) ... are there any plans to either drop > it for good, or merge it? rc1 isn't out yet, so I'm just waiting to see what happens.
Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
On Fri, Sep 20, 2019 at 12:51:12PM -0700, Linus Torvalds wrote: > So we absolutely _will_ come up with some way 0 ends the wait. Whether > it's _just_ a timeout, or whether it's jitter-entropy or whatever, it > will happen. FWIW, Zircon uses the jitter entropy generator to seed the CRNG and documented their findings in https://fuchsia.dev/fuchsia-src/zircon/jitterentropy/config-basic . -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On Tue, Sep 17, 2019 at 11:38:33PM +0200, Martin Steigerwald wrote: > My understanding of entropy always has been that only a certain amount > of it can be produced in a certain amount of time. If that is wrong… > please by all means, please teach me, how it would be. getrandom() will never "consume entropy" in a way that will block any users of getrandom(). If you don't have enough collected entropy to seed the rng, getrandom() will block. If you do, getrandom() will generate as many numbers as you ask it to, even if no more entropy is ever collected by the system. So it doesn't matter how many clients you have calling getrandom() in the boot process - either there'll be enough entropy available to satisfy all of them, or there'll be too little to satisfy any of them. -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On Tue, Sep 17, 2019 at 06:20:02PM +0100, Matthew Garrett wrote: > AES keys are used for a variety of long-lived purposes (eg, disk > encryption). And as an example of when we'd want to do that during early boot - swap is frequently encrypted with a random key generated on each boot, but it's still important for that key to be strong in order to avoid someone being able to recover the contents of swap. -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On Tue, Sep 17, 2019 at 07:16:41PM +0200, Willy Tarreau wrote: > On Tue, Sep 17, 2019 at 05:34:56PM +0100, Matthew Garrett wrote: > > On Tue, Sep 17, 2019 at 09:27:44AM -0700, Linus Torvalds wrote: > > > > > Does anybody believe that 128 bits of randomness is a good basis for a > > > long-term secure key? > > > > Yes, it's exactly what you'd expect for an AES 128 key, which is still > > considered to be secure. > > AES keys are for symmetrical encryption and thus as such are short-lived. > We're back to what Linus was saying about the fact that our urandom is > already very good for such use cases, it should just not be used to > produce long-lived keys (i.e. asymmetrical). AES keys are used for a variety of long-lived purposes (eg, disk encryption). -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On Tue, Sep 17, 2019 at 09:27:44AM -0700, Linus Torvalds wrote: > Does anybody believe that 128 bits of randomness is a good basis for a > long-term secure key? Yes, it's exactly what you'd expect for an AES 128 key, which is still considered to be secure. -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On 16 September 2019 18:41:36 GMT-07:00, Linus Torvalds wrote: >On Mon, Sep 16, 2019 at 6:24 PM Matthew Garrett >wrote: >> >> Exactly the scenario where you want getrandom() to block, yes. > >It *would* block. Just not forever. It's already not forever - there's enough running in the background of that system that it'll unblock eventually. >And btw, the whole "generate key at boot when nothing else is going >on" is already broken, so presumably nobody actually does it. If nothing ever did this, why was getrandom() designed in a way to protect against this situation? >See why I'm saying "hypothetical"? You're doing it again. > >> >Then you have to ignore the big warning too. >> >> The big warning that's only printed in dmesg? > >Well, the patch actually made getrandom() return en error too, but you >seem more interested in the hypotheticals than in arguing actualities. If you want to be safe, terminate the process. -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On 16 September 2019 18:05:57 GMT-07:00, Linus Torvalds wrote: >On Mon, Sep 16, 2019 at 4:29 PM Ahmed S. Darwish >wrote: >> >> Linus, in all honesty, the other case is _not_ a hypothetical . > >Oh yes it is. > >You're confusing "use" with "breakage". > >The _use_ of getrandom(0) for key generation isn't hypothetical. > >But the _breakage_ from the suggested patch that makes it time out is. > >See the difference? > >The thing is, to break, you have to > > (a) do that key generation at boot time > > (b) do it on an idle machine that doesn't have entropy Exactly the scenario where you want getrandom() to block, yes. >in order to basically reproduce the current boot-time hang situation >with the broken gdm, except with an actual "generate key". > >Then you have to ignore the big warning too. The big warning that's only printed in dmesg? -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On 16 September 2019 16:18:00 GMT-07:00, Linus Torvalds wrote: >On Mon, Sep 16, 2019 at 4:11 PM Matthew Garrett >wrote: >> >> In one case we have "Systems don't boot, but you can downgrade your >> kernel" and in the other case we have "Your cryptographic keys are >weak >> and you have no way of knowing unless you read dmesg", and I think >> causing boot problems is the better outcome here. > >Or: In one case you have a real and present problem. In the other >case, people are talking hypotheticals. (resending because accidental HTML, sorry about that) We've been recommending that people use the default getrandom() behaviour for key generation since it was merged. Github shows users, and it's likely there's cases in internal code as well. -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On 16 September 2019 16:18:00 GMT-07:00, Linus Torvalds wrote: >On Mon, Sep 16, 2019 at 4:11 PM Matthew Garrett >wrote: >> >> In one case we have "Systems don't boot, but you can downgrade your >> kernel" and in the other case we have "Your cryptographic keys are >weak >> and you have no way of knowing unless you read dmesg", and I think >> causing boot problems is the better outcome here. > >Or: In one case you have a real and present problem. In the other >case, people are talking hypotheticals. We've been recommending that people use getrandom() for key generation since it was first added to the kernel. Github suggests there are users in the wild - there's almost certainly more cases where internal code depends on the existing semantics. -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On Mon, Sep 16, 2019 at 10:44:31AM -0700, Linus Torvalds wrote: > - admit that the current situation actually causes problems, and has > _existing_ bugs. > > - throw it out the window, with the timeout and big BIG warning when > the problem cases trigger The semantics many people want for secure key generation is urandom, but with a guarantee that it's seeded. getrandom()'s default behaviour at present provides that, and as a result it's used for a bunch of key generation. Changing the default (even with kernel warnings) seems like it risks people generating keys from an unseeded prng, and that seems like a bad thing? It's definitely unfortunate that getrandom() doesn't have a GRND_URANDOM flag that would make it useful for the "I want some vaguely random numbers but I don't care that much and I don't necessarily have access to /dev/urandom" case, but at the moment we have no way of distinguishing between applications that are making this call because they want the semantics of urandom but need it to be seeded (which is one of the usecases getrandom() was introduced for in the first place) and applications that are making this call because it was convenient and the kernel usually ended up generating enough entropy in the first place. Given the ambiguity, I don't see an easy way to solve for the latter without breaking the former - and that could have some *very* bad outcomes. -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On Tue, Sep 17, 2019 at 04:13:36AM +0500, Alexander E. Patrakov wrote: > 17.09.2019 04:11, Matthew Garrett пишет: > > In one case we have "Systems don't boot, but you can downgrade your > > kernel" > > You can't. There are way too many dedicated server providers where there is > no IPMI or any equivalent, and the only help that the staff can do is to > reinstall, wiping your data. In which case you're presumably running a distro kernel that's had a decent amount of testing before you upgrade to it? -- Matthew Garrett | mj...@srcf.ucam.org
Re: Linux 5.3-rc8
On Mon, Sep 16, 2019 at 04:05:47PM -0700, Linus Torvalds wrote: > On Mon, Sep 16, 2019 at 4:02 PM Matthew Garrett wrote: > > Changing the default (even with kernel warnings) seems like > > it risks people generating keys from an unseeded prng, and that seems > > like a bad thing? > > I agree that it's a horrible thing, but the fact that the default 0 > behavior had that "wait for entropy" is what now causes boot problems > for people. In one case we have "Systems don't boot, but you can downgrade your kernel" and in the other case we have "Your cryptographic keys are weak and you have no way of knowing unless you read dmesg", and I think causing boot problems is the better outcome here. -- Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH V40 03/29] security: Add a static lockdown policy LSM
On Wed, Sep 4, 2019 at 12:51 PM Matthew Garrett wrote: > On Fri, Aug 30, 2019 at 9:28 AM David Howells wrote: > > > +static int lock_kernel_down(const char *where, enum lockdown_reason > > > level) > > > > Is the last parameter the reason or the level? You're mixing the terms. > > Fair. Actually, on re-reading, I think this correct - this is setting the lockdown level, it's just that the lockdown level is an enum lockdown_reason for the sake of convenience.
[PATCH 2/2] kexec: Fix file verification on S390
I accidentally typoed this #ifdef, so verification would always be
disabled.
Signed-off-by: Matthew Garrett
Reported-by: Philipp Rudo
---
arch/s390/kernel/kexec_elf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/kernel/kexec_elf.c b/arch/s390/kernel/kexec_elf.c
index 9b4f37a4edf1..9da6fa30c447 100644
--- a/arch/s390/kernel/kexec_elf.c
+++ b/arch/s390/kernel/kexec_elf.c
@@ -130,7 +130,7 @@ static int s390_elf_probe(const char *buf, unsigned long
len)
const struct kexec_file_ops s390_kexec_elf_ops = {
.probe = s390_elf_probe,
.load = s390_elf_load,
-#ifdef CONFIG_KEXEC__SIG
+#ifdef CONFIG_KEXEC_SIG
.verify_sig = s390_verify_sig,
#endif /* CONFIG_KEXEC_SIG */
};
--
2.23.0.162.g0b9fbb3734-goog
[PATCH 0/2] Minor lockdown fixups
Constify some arrays and fix an #ifdef that I typoed.
[PATCH 1/2] security: constify some arrays in lockdown LSM
No reason for these not to be const.
Signed-off-by: Matthew Garrett
Suggested-by: David Howells
---
security/lockdown/lockdown.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 0068cec77c05..8a10b43daf74 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -16,7 +16,7 @@
static enum lockdown_reason kernel_locked_down;
-static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
+static const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_NONE] = "none",
[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
@@ -40,7 +40,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
-static enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
+static const enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX};
--
2.23.0.162.g0b9fbb3734-goog
Re: [PATCH V40 04/29] lockdown: Enforce module signatures if the kernel is locked down
On Fri, Aug 30, 2019 at 9:31 AM David Howells wrote:
>
> Matthew Garrett wrote:
>
> > enum lockdown_reason {
> > LOCKDOWN_NONE,
> > + LOCKDOWN_MODULE_SIGNATURE,
> > LOCKDOWN_INTEGRITY_MAX,
> > LOCKDOWN_CONFIDENTIALITY_MAX,
> > };
>
> Aren't you mixing disjoint sets?
The goal is to be able to check whether any given lockdown reason is a
matter of integrity or confidentiality in a straightforward way.
> > + [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
>
> Wouldn't it be better to pass this string as a parameter to
> security_locked_down()?
I thought about that, but it's not how any other LSM hooks behave. I
think it's probably easier to revisit that when we see how other LSMs
want to make use of the data.
Re: [PATCH V40 03/29] security: Add a static lockdown policy LSM
On Fri, Aug 30, 2019 at 9:28 AM David Howells wrote:
>
> Matthew Garrett wrote:
>
> > +static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
>
> const char *const maybe?
Seems reasonable.
> > +static enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
> > + LOCKDOWN_INTEGRITY_MAX,
> > +
> > LOCKDOWN_CONFIDENTIALITY_MAX};
> > +
>
> const?
>
> Isn't this also a 1:1 mapping?
Sorry, a 1:1 mapping to what?
> > +static int lock_kernel_down(const char *where, enum lockdown_reason level)
>
> Is the last parameter the reason or the level? You're mixing the terms.
Fair.
Re: [RFC PATCH v3 00/16] Core scheduling v3
Apple have provided a sysctl that allows applications to indicate that
specific threads should make use of core isolation while allowing
the rest of the system to make use of SMT, and browsers (Safari, Firefox
and Chrome, at least) are now making use of this. Trying to do something
similar using cgroups seems a bit awkward. Would something like this be
reasonable? Having spoken to the Chrome team, I believe that the
semantics we want are:
1) A thread to be able to indicate that it should not run on the same
core as anything not in posession of the same cookie
2) Descendents of that thread to (by default) have the same cookie
3) No other thread be able to obtain the same cookie
4) Threads not be able to rejoin the global group (ie, threads can
segregate themselves from their parent and peers, but can never rejoin
that group once segregated)
but don't know if that's what everyone else would want.
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 094bb03b9cc2..5d411246d4d5 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -229,4 +229,5 @@ struct prctl_mm_map {
# define PR_PAC_APDBKEY(1UL << 3)
# define PR_PAC_APGAKEY(1UL << 4)
+#define PR_CORE_ISOLATE55
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 12df0e5434b8..a054cfcca511 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2486,6 +2486,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2,
unsigned long, arg3,
return -EINVAL;
error = PAC_RESET_KEYS(me, arg2);
break;
+ case PR_CORE_ISOLATE:
+#ifdef CONFIG_SCHED_CORE
+ current->core_cookie = (unsigned long)current;
+#else
+ result = -EINVAL;
+#endif
+ break;
default:
error = -EINVAL;
break;
--
Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH] x86: tpm: Remove a busy bit of the NVS area for supporting AMD's fTPM
On Wed, Aug 28, 2019 at 01:36:30AM +0900, Seunghun Han wrote: > I got your point. Is there any problem if some regions which don't > need to be handled in NVS area are saved and restored? If there is a > problem, how about adding code for ignoring the regions in NVS area to > the nvs.c file like Jarkko said? If we add the code, we can save and > restore NVS area without driver's interaction. The only thing that knows which regions should be skipped by the NVS driver is the hardware specific driver, so the TPM driver needs to ask the NVS driver to ignore that region and grant control to the TPM driver. -- Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH] x86: tpm: Remove a busy bit of the NVS area for supporting AMD's fTPM
On Tue, Aug 27, 2019 at 1:23 AM Seunghun Han wrote: > If the regions allocated in the NVS region need to be handled by a > driver, the callback mechanism is good for it. However, this case > doesn't need it because the regions allocated in NVS are just I/O > regions. > > In my opinion, if the driver wants to handle the region in the NVS > while suspending or hibernating, it has to use register_pm_notifier() > function and handle the event. We already had the mechanism that could > ensure that the cases you worried about would be handled, so it seems > to me that removing the busy bit from the NVS region is fine. No. The NVS regions are regions that need to be saved and restored over hibernation, but which aren't otherwise handled by a driver - that's why the NVS code exists. If drivers are allowed to bind to NVS regions without explicit handling, they risk conflicting with that.
Re: [PATCH] x86: tpm: Remove a busy bit of the NVS area for supporting AMD's fTPM
On Mon, Aug 26, 2019 at 1:18 AM Seunghun Han wrote: > To support AMD's fTPM, I removed the busy bit from the ACPI NVS area like > the reserved area so that AMD's fTPM regions could be assigned in it. drivers/acpi/nvs.c saves and restores the contents of NVS regions, and if other drivers use these regions without any awareness of this then things may break. I'm reluctant to say that just unilaterally marking these regions as available is a good thing, but it's clearly what's expected by AMD's implementation. One approach would be to have a callback into the nvs code to indicate that a certain region should be handed off to a driver, which would ensure that we can handle this on a case by case basis?
Re: linux-next: Signed-off-by missing for commit in the security tree
On Tue, Aug 20, 2019 at 3:32 PM Stephen Rothwell wrote:
>
> Hi all,
>
> Commit
>
> 9d1f8be5cf42 ("bpf: Restrict bpf when kernel lockdown is in confidentiality
> mode")
>
> is missing a Signed-off-by from its author.
I'm providing this under category (c) of the DCO.
Re: [PATCH V40 11/29] PCI: Lock down BAR access when the kernel is locked down
On Tue, Aug 20, 2019 at 12:45 PM Bjorn Helgaas wrote: > Since I've acked this and it's 11/29, I've been assuming you intend > to merge the whole series together. But the fact that it's up to V40 > makes me wonder if you're waiting for me to merge this one. Just let > me know either way. James has merged the series.
[PATCH V40 03/29] security: Add a static lockdown policy LSM
While existing LSMs can be extended to handle lockdown policy,
distributions generally want to be able to apply a straightforward
static policy. This patch adds a simple LSM that can be configured to
reject either integrity or all lockdown queries, and can be configured
at runtime (through securityfs), boot time (via a kernel parameter) or
build time (via a kconfig option). Based on initial code by David
Howells.
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: David Howells
---
.../admin-guide/kernel-parameters.txt | 9 +
include/linux/security.h | 3 +
security/Kconfig | 11 +-
security/Makefile | 2 +
security/lockdown/Kconfig | 46 +
security/lockdown/Makefile| 1 +
security/lockdown/lockdown.c | 169 ++
7 files changed, 236 insertions(+), 5 deletions(-)
create mode 100644 security/lockdown/Kconfig
create mode 100644 security/lockdown/Makefile
create mode 100644 security/lockdown/lockdown.c
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 138f6664b2e2..0f28350f1ee6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2244,6 +2244,15 @@
lockd.nlm_udpport=M [NFS] Assign UDP port.
Format:
+ lockdown= [SECURITY]
+ { integrity | confidentiality }
+ Enable the kernel lockdown feature. If set to
+ integrity, kernel features that allow userland to
+ modify the running kernel are disabled. If set to
+ confidentiality, kernel features that allow userland
+ to extract confidential information from the kernel
+ are also disabled.
+
locktorture.nreaders_stress= [KNL]
Set the number of locking read-acquisition kthreads.
Defaults to being automatically set based on the
diff --git a/include/linux/security.h b/include/linux/security.h
index 04cf48fab15d..74787335d9ce 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -97,6 +97,9 @@ enum lsm_event {
* 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.
+ *
+ * If you add to this, remember to extend lockdown_reasons in
+ * security/lockdown/lockdown.c.
*/
enum lockdown_reason {
LOCKDOWN_NONE,
diff --git a/security/Kconfig b/security/Kconfig
index 466cc1f8ffed..7c62d446e209 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -237,6 +237,7 @@ source "security/apparmor/Kconfig"
source "security/loadpin/Kconfig"
source "security/yama/Kconfig"
source "security/safesetid/Kconfig"
+source "security/lockdown/Kconfig"
source "security/integrity/Kconfig"
@@ -276,11 +277,11 @@ endchoice
config LSM
string "Ordered list of enabled LSMs"
- default
"yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if
DEFAULT_SECURITY_SMACK
- default
"yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if
DEFAULT_SECURITY_APPARMOR
- default "yama,loadpin,safesetid,integrity,tomoyo" if
DEFAULT_SECURITY_TOMOYO
- default "yama,loadpin,safesetid,integrity" if DEFAULT_SECURITY_DAC
- default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
+ default
"lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if
DEFAULT_SECURITY_SMACK
+ default
"lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if
DEFAULT_SECURITY_APPARMOR
+ default "lockdown,yama,loadpin,safesetid,integrity,tomoyo" if
DEFAULT_SECURITY_TOMOYO
+ default "lockdown,yama,loadpin,safesetid,integrity" if
DEFAULT_SECURITY_DAC
+ default
"lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
help
A comma-separated list of LSMs, in initialization order.
Any LSMs left off this list will be ignored. This can be
diff --git a/security/Makefile b/security/Makefile
index c598b904938f..be1dd9d2cb2f 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -11,6 +11,7 @@ subdir-$(CONFIG_SECURITY_APPARMOR)+= apparmor
subdir-$(CONFIG_SECURITY_YAMA) += yama
subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin
subdir-$(CONFIG_SECURITY_SAFESETID)+= safesetid
+subdir-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown
# always enable default capabilities
obj-y += commoncap.o
@@ -27,6 +28,7 @@ obj-$(CONFIG_SECURITY_APPARMOR)
[PATCH V40 14/29] ACPI: Limit access to custom_method when the kernel is locked down
From: Matthew Garrett
custom_method effectively allows arbitrary access to system memory, making
it possible for an attacker to circumvent restrictions on module loading.
Disable it if the kernel is locked down.
Signed-off-by: Matthew Garrett
Signed-off-by: David Howells
Reviewed-by: Kees Cook
cc: linux-a...@vger.kernel.org
Signed-off-by: James Morris
---
drivers/acpi/custom_method.c | 6 ++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 8 insertions(+)
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index b2ef4c2ec955..7031307becd7 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -29,6 +30,11 @@ static ssize_t cm_write(struct file *file, const char __user
* user_buf,
struct acpi_table_header table;
acpi_status status;
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
+ if (ret)
+ return ret;
if (!(*ppos)) {
/* parse the table header to get the table length */
diff --git a/include/linux/security.h b/include/linux/security.h
index 010637a79eac..390e39395112 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -110,6 +110,7 @@ enum lockdown_reason {
LOCKDOWN_PCI_ACCESS,
LOCKDOWN_IOPORT,
LOCKDOWN_MSR,
+ LOCKDOWN_ACPI_TABLES,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index b1c1c72440d5..6d44db0ddffa 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -25,6 +25,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
[LOCKDOWN_IOPORT] = "raw io port access",
[LOCKDOWN_MSR] = "raw MSR access",
+ [LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 27/29] tracefs: Restrict tracefs when the kernel is locked down
Tracefs may release more information about the kernel than desirable, so
restrict it when the kernel is locked down in confidentiality mode by
preventing open().
(Fixed by Ben Hutchings to avoid a null dereference in
default_file_open())
Signed-off-by: Matthew Garrett
Reviewed-by: Steven Rostedt (VMware)
Cc: Ben Hutchings
---
fs/tracefs/inode.c | 42 +++-
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index a5bab190a297..761af8ce4015 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#define TRACEFS_DEFAULT_MODE 0700
@@ -27,6 +28,25 @@ static struct vfsmount *tracefs_mount;
static int tracefs_mount_count;
static bool tracefs_registered;
+static int default_open_file(struct inode *inode, struct file *filp)
+{
+ struct dentry *dentry = filp->f_path.dentry;
+ struct file_operations *real_fops;
+ int ret;
+
+ if (!dentry)
+ return -EINVAL;
+
+ ret = security_locked_down(LOCKDOWN_TRACEFS);
+ if (ret)
+ return ret;
+
+ real_fops = dentry->d_fsdata;
+ if (!real_fops->open)
+ return 0;
+ return real_fops->open(inode, filp);
+}
+
static ssize_t default_read_file(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
@@ -221,6 +241,12 @@ static int tracefs_apply_options(struct super_block *sb)
return 0;
}
+static void tracefs_destroy_inode(struct inode *inode)
+{
+ if (S_ISREG(inode->i_mode))
+ kfree(inode->i_fop);
+}
+
static int tracefs_remount(struct super_block *sb, int *flags, char *data)
{
int err;
@@ -257,6 +283,7 @@ static int tracefs_show_options(struct seq_file *m, struct
dentry *root)
static const struct super_operations tracefs_super_operations = {
.statfs = simple_statfs,
.remount_fs = tracefs_remount,
+ .destroy_inode = tracefs_destroy_inode,
.show_options = tracefs_show_options,
};
@@ -387,6 +414,7 @@ struct dentry *tracefs_create_file(const char *name,
umode_t mode,
struct dentry *parent, void *data,
const struct file_operations *fops)
{
+ struct file_operations *proxy_fops;
struct dentry *dentry;
struct inode *inode;
@@ -402,8 +430,20 @@ struct dentry *tracefs_create_file(const char *name,
umode_t mode,
if (unlikely(!inode))
return failed_creating(dentry);
+ proxy_fops = kzalloc(sizeof(struct file_operations), GFP_KERNEL);
+ if (unlikely(!proxy_fops)) {
+ iput(inode);
+ return failed_creating(dentry);
+ }
+
+ if (!fops)
+ fops = _file_operations;
+
+ dentry->d_fsdata = (void *)fops;
+ memcpy(proxy_fops, fops, sizeof(*proxy_fops));
+ proxy_fops->open = default_open_file;
inode->i_mode = mode;
- inode->i_fop = fops ? fops : _file_operations;
+ inode->i_fop = proxy_fops;
inode->i_private = data;
d_instantiate(dentry, inode);
fsnotify_create(dentry->d_parent->d_inode, dentry);
diff --git a/include/linux/security.h b/include/linux/security.h
index 152824b6f456..429f9f03372b 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -121,6 +121,7 @@ enum lockdown_reason {
LOCKDOWN_KPROBES,
LOCKDOWN_BPF_READ,
LOCKDOWN_PERF,
+ LOCKDOWN_TRACEFS,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index edd1fff0147d..84df03b1f5a7 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -36,6 +36,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_KPROBES] = "use of kprobes",
[LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
[LOCKDOWN_PERF] = "unsafe use of perf",
+ [LOCKDOWN_TRACEFS] = "use of tracefs",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 21/29] lockdown: Lock down /proc/kcore
From: David Howells
Disallow access to /proc/kcore when the kernel is locked down to prevent
access to cryptographic data. This is limited to lockdown
confidentiality mode and is still permitted in integrity mode.
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Signed-off-by: James Morris
---
fs/proc/kcore.c | 5 +
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index f5834488b67d..ee2c576cc94e 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
#include "internal.h"
@@ -545,6 +546,10 @@ read_kcore(struct file *file, char __user *buffer, size_t
buflen, loff_t *fpos)
static int open_kcore(struct inode *inode, struct file *filp)
{
+ int ret = security_locked_down(LOCKDOWN_KCORE);
+
+ if (ret)
+ return ret;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
diff --git a/include/linux/security.h b/include/linux/security.h
index d8db7ea4c4bf..669e8de5299d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -116,6 +116,7 @@ enum lockdown_reason {
LOCKDOWN_MODULE_PARAMETERS,
LOCKDOWN_MMIOTRACE,
LOCKDOWN_INTEGRITY_MAX,
+ LOCKDOWN_KCORE,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 2eadbe0667e7..403b30357f75 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -31,6 +31,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
[LOCKDOWN_MMIOTRACE] = "unsafe mmio",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
+ [LOCKDOWN_KCORE] = "/proc/kcore access",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 22/29] lockdown: Lock down tracing and perf kprobes when in confidentiality mode
From: David Howells
Disallow the creation of perf and ftrace kprobes when the kernel is
locked down in confidentiality mode by preventing their registration.
This prevents kprobes from being used to access kernel memory to steal
crypto data, but continues to allow the use of kprobes from signed
modules.
Reported-by: Alexei Starovoitov
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Acked-by: Masami Hiramatsu
Reviewed-by: Kees Cook
Cc: Naveen N. Rao
Cc: Anil S Keshavamurthy
Cc: da...@davemloft.net
Cc: Masami Hiramatsu
Signed-off-by: James Morris
---
include/linux/security.h | 1 +
kernel/trace/trace_kprobe.c | 5 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index 669e8de5299d..0b2529dbf0f4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -117,6 +117,7 @@ enum lockdown_reason {
LOCKDOWN_MMIOTRACE,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_KCORE,
+ LOCKDOWN_KPROBES,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 7d736248a070..fcb28b0702b2 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include "trace_dynevent.h"
#include "trace_kprobe_selftest.h"
@@ -415,6 +416,10 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
{
int i, ret;
+ ret = security_locked_down(LOCKDOWN_KPROBES);
+ if (ret)
+ return ret;
+
if (trace_probe_is_registered(>tp))
return -EINVAL;
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 403b30357f75..27b2cf51e443 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -32,6 +32,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_MMIOTRACE] = "unsafe mmio",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_KCORE] = "/proc/kcore access",
+ [LOCKDOWN_KPROBES] = "use of kprobes",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 29/29] lockdown: Print current->comm in restriction messages
Print the content of current->comm in messages generated by lockdown to
indicate a restriction that was hit. This makes it a bit easier to find
out what caused the message.
The message now patterned something like:
Lockdown: : is restricted; see man kernel_lockdown.7
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Signed-off-by: James Morris
---
fs/proc/kcore.c | 5 +++--
security/lockdown/lockdown.c | 8 ++--
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index ee2c576cc94e..e2ed8e08cc7a 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -548,11 +548,12 @@ static int open_kcore(struct inode *inode, struct file
*filp)
{
int ret = security_locked_down(LOCKDOWN_KCORE);
- if (ret)
- return ret;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
+ if (ret)
+ return ret;
+
filp->private_data = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!filp->private_data)
return -ENOMEM;
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 84df03b1f5a7..0068cec77c05 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -81,10 +81,14 @@ early_param("lockdown", lockdown_param);
*/
static int lockdown_is_locked_down(enum lockdown_reason what)
{
+ if (WARN(what >= LOCKDOWN_CONFIDENTIALITY_MAX,
+"Invalid lockdown reason"))
+ return -EPERM;
+
if (kernel_locked_down >= what) {
if (lockdown_reasons[what])
- pr_notice("Lockdown: %s is restricted; see man
kernel_lockdown.7\n",
- lockdown_reasons[what]);
+ pr_notice("Lockdown: %s: %s is restricted; see man
kernel_lockdown.7\n",
+ current->comm, lockdown_reasons[what]);
return -EPERM;
}
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 20/29] x86/mmiotrace: Lock down the testmmiotrace module
From: David Howells
The testmmiotrace module shouldn't be permitted when the kernel is locked
down as it can be used to arbitrarily read and write MMIO space. This is
a runtime check rather than buildtime in order to allow configurations
where the same kernel may be run in both locked down or permissive modes
depending on local policy.
Suggested-by: Thomas Gleixner
Signed-off-by: David Howells
Acked-by: Steven Rostedt (VMware)
Reviewed-by: Kees Cook
cc: Thomas Gleixner
cc: Steven Rostedt
cc: Ingo Molnar
cc: "H. Peter Anvin"
cc: x...@kernel.org
Signed-off-by: James Morris
---
arch/x86/mm/testmmiotrace.c | 5 +
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/arch/x86/mm/testmmiotrace.c b/arch/x86/mm/testmmiotrace.c
index 0881e1ff1e58..a8bd952e136d 100644
--- a/arch/x86/mm/testmmiotrace.c
+++ b/arch/x86/mm/testmmiotrace.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
static unsigned long mmio_address;
module_param_hw(mmio_address, ulong, iomem, 0);
@@ -115,6 +116,10 @@ static void do_test_bulk_ioremapping(void)
static int __init init(void)
{
unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
+ int ret = security_locked_down(LOCKDOWN_MMIOTRACE);
+
+ if (ret)
+ return ret;
if (mmio_address == 0) {
pr_err("you have to use the module argument mmio_address.\n");
diff --git a/include/linux/security.h b/include/linux/security.h
index 1a3404f9c060..d8db7ea4c4bf 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -114,6 +114,7 @@ enum lockdown_reason {
LOCKDOWN_PCMCIA_CIS,
LOCKDOWN_TIOCSSERIAL,
LOCKDOWN_MODULE_PARAMETERS,
+ LOCKDOWN_MMIOTRACE,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 0fa434294667..2eadbe0667e7 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -29,6 +29,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
[LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
+ [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: David Howells
bpf_read() and bpf_read_str() could potentially be abused to (eg) allow
private keys in kernel memory to be leaked. Disable them if the kernel
has been locked down in confidentiality mode.
Suggested-by: Alexei Starovoitov
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
cc: net...@vger.kernel.org
cc: Chun-Yi Lee
cc: Alexei Starovoitov
Cc: Daniel Borkmann
Signed-off-by: James Morris
---
include/linux/security.h | 1 +
kernel/trace/bpf_trace.c | 10 ++
security/lockdown/lockdown.c | 1 +
3 files changed, 12 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index 0b2529dbf0f4..e604f4c67f03 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -118,6 +118,7 @@ enum lockdown_reason {
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_KCORE,
LOCKDOWN_KPROBES,
+ LOCKDOWN_BPF_READ,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1c9a4745e596..33a954c367f3 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -139,8 +139,13 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const
void *, unsafe_ptr)
{
int ret;
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (ret < 0)
+ goto out;
+
ret = probe_kernel_read(dst, unsafe_ptr, size);
if (unlikely(ret < 0))
+out:
memset(dst, 0, size);
return ret;
@@ -566,6 +571,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
{
int ret;
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (ret < 0)
+ goto out;
+
/*
* The strncpy_from_unsafe() call will likely not fill the entire
* buffer, but that's okay in this circumstance as we're probing
@@ -577,6 +586,7 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
*/
ret = strncpy_from_unsafe(dst, unsafe_ptr, size);
if (unlikely(ret < 0))
+out:
memset(dst, 0, size);
return ret;
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 27b2cf51e443..2397772c56bd 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_KCORE] = "/proc/kcore access",
[LOCKDOWN_KPROBES] = "use of kprobes",
+ [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 09/29] kexec_file: Restrict at runtime if the kernel is locked down
From: Jiri Bohac When KEXEC_SIG is not enabled, kernel should not load images through kexec_file systemcall if the kernel is locked down. [Modified by David Howells to fit with modifications to the previous patch and to return -EPERM if the kernel is locked down for consistency with other lockdowns. Modified by Matthew Garrett to remove the IMA integration, which will be replaced by integrating with the IMA architecture policy patches.] Signed-off-by: Jiri Bohac Signed-off-by: David Howells Signed-off-by: Matthew Garrett cc: ke...@lists.infradead.org --- kernel/kexec_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index 972931201995..43109ef4d6bf 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -208,7 +208,7 @@ kimage_validate_signature(struct kimage *image) return ret; } - return 0; + return security_locked_down(LOCKDOWN_KEXEC); /* All other errors are fatal, including nomem, unparseable * signatures and signature check failures - even if signatures -- 2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 19/29] lockdown: Lock down module params that specify hardware parameters (eg. ioport)
From: David Howells
Provided an annotation for module parameters that specify hardware
parameters (such as io ports, iomem addresses, irqs, dma channels, fixed
dma buffers and other types).
Suggested-by: Alan Cox
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: Jessica Yu
Signed-off-by: James Morris
---
include/linux/security.h | 1 +
kernel/params.c | 21 -
security/lockdown/lockdown.c | 1 +
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index b4a85badb03a..1a3404f9c060 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -113,6 +113,7 @@ enum lockdown_reason {
LOCKDOWN_ACPI_TABLES,
LOCKDOWN_PCMCIA_CIS,
LOCKDOWN_TIOCSSERIAL,
+ LOCKDOWN_MODULE_PARAMETERS,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/params.c b/kernel/params.c
index cf448785d058..8e56f8b12d8f 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_SYSFS
/* Protects all built-in parameters, modules use their own param_lock */
@@ -96,13 +97,19 @@ bool parameq(const char *a, const char *b)
return parameqn(a, b, strlen(a)+1);
}
-static void param_check_unsafe(const struct kernel_param *kp)
+static bool param_check_unsafe(const struct kernel_param *kp)
{
+ if (kp->flags & KERNEL_PARAM_FL_HWPARAM &&
+ security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
+ return false;
+
if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
pr_notice("Setting dangerous option %s - tainting kernel\n",
kp->name);
add_taint(TAINT_USER, LOCKDEP_STILL_OK);
}
+
+ return true;
}
static int parse_one(char *param,
@@ -132,8 +139,10 @@ static int parse_one(char *param,
pr_debug("handling %s with %p\n", param,
params[i].ops->set);
kernel_param_lock(params[i].mod);
- param_check_unsafe([i]);
- err = params[i].ops->set(val, [i]);
+ if (param_check_unsafe([i]))
+ err = params[i].ops->set(val, [i]);
+ else
+ err = -EPERM;
kernel_param_unlock(params[i].mod);
return err;
}
@@ -553,8 +562,10 @@ static ssize_t param_attr_store(struct module_attribute
*mattr,
return -EPERM;
kernel_param_lock(mk->mod);
- param_check_unsafe(attribute->param);
- err = attribute->param->ops->set(buf, attribute->param);
+ if (param_check_unsafe(attribute->param))
+ err = attribute->param->ops->set(buf, attribute->param);
+ else
+ err = -EPERM;
kernel_param_unlock(mk->mod);
if (!err)
return len;
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 771c77f9c04a..0fa434294667 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -28,6 +28,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
+ [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 26/29] debugfs: Restrict debugfs when the kernel is locked down
From: David Howells
Disallow opening of debugfs files that might be used to muck around when
the kernel is locked down as various drivers give raw access to hardware
through debugfs. Given the effort of auditing all 2000 or so files and
manually fixing each one as necessary, I've chosen to apply a heuristic
instead. The following changes are made:
(1) chmod and chown are disallowed on debugfs objects (though the root dir
can be modified by mount and remount, but I'm not worried about that).
(2) When the kernel is locked down, only files with the following criteria
are permitted to be opened:
- The file must have mode 00444
- The file must not have ioctl methods
- The file must not have mmap
(3) When the kernel is locked down, files may only be opened for reading.
Normal device interaction should be done through configfs, sysfs or a
miscdev, not debugfs.
Note that this makes it unnecessary to specifically lock down show_dsts(),
show_devs() and show_call() in the asus-wmi driver.
I would actually prefer to lock down all files by default and have the
the files unlocked by the creator. This is tricky to manage correctly,
though, as there are 19 creation functions and ~1600 call sites (some of
them in loops scanning tables).
Signed-off-by: David Howells
cc: Andy Shevchenko
cc: acpi4asus-u...@lists.sourceforge.net
cc: platform-driver-...@vger.kernel.org
cc: Matthew Garrett
cc: Thomas Gleixner
Cc: Greg KH
Cc: Rafael J. Wysocki
Signed-off-by: Matthew Garrett
Signed-off-by: James Morris
---
fs/debugfs/file.c| 30 ++
fs/debugfs/inode.c | 32 ++--
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
4 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index ddd708b09fa1..5d3e449b5988 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -136,6 +137,25 @@ void debugfs_file_put(struct dentry *dentry)
}
EXPORT_SYMBOL_GPL(debugfs_file_put);
+/*
+ * Only permit access to world-readable files when the kernel is locked down.
+ * We also need to exclude any file that has ways to write or alter it as root
+ * can bypass the permissions check.
+ */
+static bool debugfs_is_locked_down(struct inode *inode,
+ struct file *filp,
+ const struct file_operations *real_fops)
+{
+ if ((inode->i_mode & 0) == 0444 &&
+ !(filp->f_mode & FMODE_WRITE) &&
+ !real_fops->unlocked_ioctl &&
+ !real_fops->compat_ioctl &&
+ !real_fops->mmap)
+ return false;
+
+ return security_locked_down(LOCKDOWN_DEBUGFS);
+}
+
static int open_proxy_open(struct inode *inode, struct file *filp)
{
struct dentry *dentry = F_DENTRY(filp);
@@ -147,6 +167,11 @@ static int open_proxy_open(struct inode *inode, struct
file *filp)
return r == -EIO ? -ENOENT : r;
real_fops = debugfs_real_fops(filp);
+
+ r = debugfs_is_locked_down(inode, filp, real_fops);
+ if (r)
+ goto out;
+
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not clean up after itself at exit? */
@@ -272,6 +297,11 @@ static int full_proxy_open(struct inode *inode, struct
file *filp)
return r == -EIO ? -ENOENT : r;
real_fops = debugfs_real_fops(filp);
+
+ r = debugfs_is_locked_down(inode, filp, real_fops);
+ if (r)
+ goto out;
+
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not cleanup after itself at exit? */
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index acef14ad53db..c8613bcad252 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -32,6 +33,32 @@ static struct vfsmount *debugfs_mount;
static int debugfs_mount_count;
static bool debugfs_registered;
+/*
+ * Don't allow access attributes to be changed whilst the kernel is locked down
+ * so that we can use the file mode as part of a heuristic to determine whether
+ * to lock down individual files.
+ */
+static int debugfs_setattr(struct dentry *dentry, struct iattr *ia)
+{
+ int ret = security_locked_down(LOCKDOWN_DEBUGFS);
+
+ if (ret && (ia->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)))
+ return ret;
+ return simple_setattr(dentry, ia);
+}
+
+static const struct inode_operations debugfs_file_inode_operations = {
+ .setattr= debugfs_setattr,
+};
+static const struct inode_operations debugfs_dir_inode_operations = {
+ .lookup = simple_lo
[PATCH V40 10/29] hibernate: Disable when the kernel is locked down
From: Josh Boyer
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
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: r...@rjwysocki.net
Cc: pa...@ucw.cz
cc: linux...@vger.kernel.org
Signed-off-by: James Morris
---
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 b607a8ac97fe..80ac7fb27aa9 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
#include
#include
+#include
#include
#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 aaf30ad351f9..3462f7edcaac 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.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 24/29] lockdown: Lock down perf when in confidentiality mode
From: David Howells
Disallow the use of certain perf facilities that might allow userspace to
access kernel data.
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: Peter Zijlstra
Cc: Ingo Molnar
Cc: Arnaldo Carvalho de Melo
Signed-off-by: James Morris
---
include/linux/security.h | 1 +
kernel/events/core.c | 7 +++
security/lockdown/lockdown.c | 1 +
3 files changed, 9 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index e604f4c67f03..b94f1e697537 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -119,6 +119,7 @@ enum lockdown_reason {
LOCKDOWN_KCORE,
LOCKDOWN_KPROBES,
LOCKDOWN_BPF_READ,
+ LOCKDOWN_PERF,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f85929ce13be..8732f980a4fc 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10798,6 +10798,13 @@ SYSCALL_DEFINE5(perf_event_open,
perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
return -EACCES;
+ err = security_locked_down(LOCKDOWN_PERF);
+ if (err && (attr.sample_type & PERF_SAMPLE_REGS_INTR))
+ /* REGS_INTR can leak data, lockdown must prevent this */
+ return err;
+
+ err = 0;
+
/*
* In cgroup mode, the pid argument is used to pass the fd
* opened to the cgroup directory in cgroupfs. The cpu argument
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 2397772c56bd..3d7b1039457b 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -34,6 +34,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_KCORE] = "/proc/kcore access",
[LOCKDOWN_KPROBES] = "use of kprobes",
[LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
+ [LOCKDOWN_PERF] = "unsafe use of perf",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 25/29] kexec: Allow kexec_file() with appropriate IMA policy when locked down
Systems in lockdown mode should block the kexec of untrusted kernels.
For x86 and ARM we can ensure that a kernel is trustworthy by validating
a PE signature, but this isn't possible on other architectures. On those
platforms we can use IMA digital signatures instead. Add a function to
determine whether IMA has or will verify signatures for a given event type,
and if so permit kexec_file() even if the kernel is otherwise locked down.
This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set
in order to prevent an attacker from loading additional keys at runtime.
Signed-off-by: Matthew Garrett
Acked-by: Mimi Zohar
Cc: Dmitry Kasatkin
Cc: linux-integr...@vger.kernel.org
Signed-off-by: James Morris
---
include/linux/ima.h | 9 ++
kernel/kexec_file.c | 10 +-
security/integrity/ima/ima.h| 2 ++
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_policy.c | 50 +
5 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 00036d2f57c3..8e2f324fb901 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -129,4 +129,13 @@ static inline int ima_inode_removexattr(struct dentry
*dentry,
return 0;
}
#endif /* CONFIG_IMA_APPRAISE */
+
+#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
+extern bool ima_appraise_signature(enum kernel_read_file_id func);
+#else
+static inline bool ima_appraise_signature(enum kernel_read_file_id func)
+{
+ return false;
+}
+#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
#endif /* _LINUX_IMA_H */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 43109ef4d6bf..7f4a618fc8c1 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -208,7 +208,15 @@ kimage_validate_signature(struct kimage *image)
return ret;
}
- return security_locked_down(LOCKDOWN_KEXEC);
+ /* If IMA is guaranteed to appraise a signature on the kexec
+* image, permit it even if the kernel is otherwise locked
+* down.
+*/
+ if (!ima_appraise_signature(READING_KEXEC_IMAGE) &&
+ security_locked_down(LOCKDOWN_KEXEC))
+ return -EPERM;
+
+ return 0;
/* All other errors are fatal, including nomem, unparseable
* signatures and signature check failures - even if signatures
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ca10917b5f89..874bd77d3b91 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -111,6 +111,8 @@ struct ima_kexec_hdr {
u64 count;
};
+extern const int read_idmap[];
+
#ifdef CONFIG_HAVE_IMA_KEXEC
void ima_load_kexec_buffer(void);
#else
diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index 1cffda4412b7..1747bc7bcb60 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -469,7 +469,7 @@ int ima_read_file(struct file *file, enum
kernel_read_file_id read_id)
return 0;
}
-static const int read_idmap[READING_MAX_ID] = {
+const int read_idmap[READING_MAX_ID] = {
[READING_FIRMWARE] = FIRMWARE_CHECK,
[READING_FIRMWARE_PREALLOC_BUFFER] = FIRMWARE_CHECK,
[READING_MODULE] = MODULE_CHECK,
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index 7b53f2ca58e2..b8773f05f9da 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1339,3 +1339,53 @@ int ima_policy_show(struct seq_file *m, void *v)
return 0;
}
#endif /* CONFIG_IMA_READ_POLICY */
+
+#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
+/*
+ * ima_appraise_signature: whether IMA will appraise a given function using
+ * an IMA digital signature. This is restricted to cases where the kernel
+ * has a set of built-in trusted keys in order to avoid an attacker simply
+ * loading additional keys.
+ */
+bool ima_appraise_signature(enum kernel_read_file_id id)
+{
+ struct ima_rule_entry *entry;
+ bool found = false;
+ enum ima_hooks func;
+
+ if (id >= READING_MAX_ID)
+ return false;
+
+ func = read_idmap[id] ?: FILE_CHECK;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, ima_rules, list) {
+ if (entry->action != APPRAISE)
+ continue;
+
+ /*
+* A generic entry will match, but otherwise require that it
+* match the func we're looking for
+*/
+ if (entry->func && entry->func != func)
+ continue;
+
+ /*
+* We require this to be a digi
[PATCH V40 18/29] lockdown: Lock down TIOCSSERIAL
From: David Howells
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
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
cc: Jiri Slaby
Cc: linux-ser...@vger.kernel.org
Signed-off-by: James Morris
---
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 83f4dd0bfd74..bbad407557b9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
@@ -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 683f0607e6f2..b4a85badb03a 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 db3477585972..771c77f9c04a 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.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 17/29] lockdown: Prohibit PCMCIA CIS storage when the kernel is locked down
From: David Howells
Prohibit replacement of the PCMCIA Card Information Structure when the
kernel is locked down.
Suggested-by: Dominik Brodowski
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Signed-off-by: James Morris
---
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
#include
#include
+#include
#include
#include
@@ -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 390e39395112..683f0607e6f2 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 6d44db0ddffa..db3477585972 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.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 16/29] acpi: Disable ACPI table override if the kernel is locked down
From: Linn Crosetto
>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
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
cc: linux-a...@vger.kernel.org
Signed-off-by: James Morris
---
drivers/acpi/tables.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index de974322a197..b7c29a11c0c1 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include "internal.h"
#ifdef CONFIG_ACPI_CUSTOM_DSDT
@@ -577,6 +578,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.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
From: Josh Boyer
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
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
cc: Dave Young
cc: linux-a...@vger.kernel.org
Signed-off-by: James Morris
---
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 ad84239e595e..e726e9b44bb1 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;
@@ -215,10 +215,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;
if (!pa)
pa = efi_get_rsdp_addr();
@@ -240,7 +237,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 b85a7c54c6a1..d584128435cb 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
[PATCH V40 12/29] x86: Lock down IO port access when the kernel is locked down
From: Matthew Garrett
IO port access would permit users to gain access to PCI configuration
registers, which in turn (on a lot of hardware) give access to MMIO
register space. This would potentially permit root to trigger arbitrary
DMA, so lock it down by default.
This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
KDDISABIO console ioctls.
Signed-off-by: Matthew Garrett
Signed-off-by: David Howells
Reviewed-by: Kees Cook
cc: x...@kernel.org
Signed-off-by: James Morris
---
arch/x86/kernel/ioport.c | 7 +--
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 0fe1c8782208..61a89d3c0382 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -31,7 +32,8 @@ long ksys_ioperm(unsigned long from, unsigned long num, int
turn_on)
if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
return -EINVAL;
- if (turn_on && !capable(CAP_SYS_RAWIO))
+ if (turn_on && (!capable(CAP_SYS_RAWIO) ||
+ security_locked_down(LOCKDOWN_IOPORT)))
return -EPERM;
/*
@@ -126,7 +128,8 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
return -EINVAL;
/* Trying to gain more privileges? */
if (level > old) {
- if (!capable(CAP_SYS_RAWIO))
+ if (!capable(CAP_SYS_RAWIO) ||
+ security_locked_down(LOCKDOWN_IOPORT))
return -EPERM;
}
regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
diff --git a/include/linux/security.h b/include/linux/security.h
index 2b763f0ee352..cd93fa5d3c6d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -108,6 +108,7 @@ enum lockdown_reason {
LOCKDOWN_KEXEC,
LOCKDOWN_HIBERNATION,
LOCKDOWN_PCI_ACCESS,
+ LOCKDOWN_IOPORT,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 410e90eda848..8b7d65dbb086 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -23,6 +23,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_KEXEC] = "kexec of unsigned images",
[LOCKDOWN_HIBERNATION] = "hibernation",
[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
+ [LOCKDOWN_IOPORT] = "raw io port access",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 13/29] x86/msr: Restrict MSR access when the kernel is locked down
From: Matthew Garrett
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
Signed-off-by: David Howells
Acked-by: Kees Cook
Reviewed-by: Thomas Gleixner
cc: x...@kernel.org
Signed-off-by: James Morris
---
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
#include
#include
+#include
#include
#include
@@ -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 cd93fa5d3c6d..010637a79eac 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 8b7d65dbb086..b1c1c72440d5 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.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 11/29] PCI: Lock down BAR access when the kernel is locked down
From: Matthew Garrett
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
Signed-off-by: Matthew Garrett
Acked-by: Bjorn Helgaas
Reviewed-by: Kees Cook
cc: linux-...@vger.kernel.org
Signed-off-by: James Morris
---
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 6d27475e39b2..ec103a7e13fc 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -903,6 +903,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;
@@ -1164,6 +1169,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 = >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;
@@ -1240,6 +1250,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 445b51db75b0..e29b0d5ced62 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#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
#include
+#include
#include
#include
#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 80ac7fb27aa9..2b763f0ee352 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 3462f7edcaac..410e90eda848 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -22,6 +22,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_D
[PATCH V40 01/29] security: Support early LSMs
The lockdown module is intended to allow for kernels to be locked down
early in boot - sufficiently early that we don't have the ability to
kmalloc() yet. Add support for early initialisation of some LSMs, and
then add them to the list of names when we do full initialisation later.
Early LSMs are initialised in link order and cannot be overridden via
boot parameters, and cannot make use of kmalloc() (since the allocator
isn't initialised yet).
(Fixed by Stephen Rothwell to include a stub to fix builds when
!CONFIG_SECURITY)
Signed-off-by: Matthew Garrett
Acked-by: Kees Cook
Acked-by: Casey Schaufler
Cc: Stephen Rothwell
---
include/asm-generic/vmlinux.lds.h | 8 -
include/linux/lsm_hooks.h | 6
include/linux/security.h | 6
init/main.c | 1 +
security/security.c | 50 ++-
5 files changed, 62 insertions(+), 9 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 088987e9a3ea..c1807d14daa3 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -208,8 +208,13 @@
__start_lsm_info = .; \
KEEP(*(.lsm_info.init)) \
__end_lsm_info = .;
+#define EARLY_LSM_TABLE() . = ALIGN(8); \
+ __start_early_lsm_info = .; \
+ KEEP(*(.early_lsm_info.init)) \
+ __end_early_lsm_info = .;
#else
#define LSM_TABLE()
+#define EARLY_LSM_TABLE()
#endif
#define ___OF_TABLE(cfg, name) _OF_TABLE_##cfg(name)
@@ -609,7 +614,8 @@
ACPI_PROBE_TABLE(irqchip) \
ACPI_PROBE_TABLE(timer) \
EARLYCON_TABLE()\
- LSM_TABLE()
+ LSM_TABLE() \
+ EARLY_LSM_TABLE()
#define INIT_TEXT \
*(.init.text .init.text.*) \
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 47f58cfb6a19..b02e8bb6654d 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2104,12 +2104,18 @@ struct lsm_info {
};
extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
+extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
#define DEFINE_LSM(lsm)
\
static struct lsm_info __lsm_##lsm \
__used __section(.lsm_info.init)\
__aligned(sizeof(unsigned long))
+#define DEFINE_EARLY_LSM(lsm) \
+ static struct lsm_info __early_lsm_##lsm\
+ __used __section(.early_lsm_info.init) \
+ __aligned(sizeof(unsigned long))
+
#ifdef CONFIG_SECURITY_SELINUX_DISABLE
/*
* Assuring the safety of deleting a security module is up to
diff --git a/include/linux/security.h b/include/linux/security.h
index 659071c2e57c..c5dd90981c98 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -195,6 +195,7 @@ int unregister_lsm_notifier(struct notifier_block *nb);
/* prototypes */
extern int security_init(void);
+extern int early_security_init(void);
/* Security operations */
int security_binder_set_context_mgr(struct task_struct *mgr);
@@ -423,6 +424,11 @@ static inline int security_init(void)
return 0;
}
+static inline int early_security_init(void)
+{
+ return 0;
+}
+
static inline int security_binder_set_context_mgr(struct task_struct *mgr)
{
return 0;
diff --git a/init/main.c b/init/main.c
index 66a196c5e4c3..598effd29a0a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -569,6 +569,7 @@ asmlinkage __visible void __init start_kernel(void)
boot_cpu_init();
page_address_init();
pr_notice("%s", linux_banner);
+ early_security_init();
setup_arch(_line);
mm_init_cpumask(_mm);
setup_command_line(command_line);
diff --git a/security/security.c b/security/security.c
index f493db0bf62a..ef4a0111c8b4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -33,6 +33,7 @@
/* How many LSMs were built into the kernel? */
#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
+#define EARLY_LSM_COUNT (__end_early_lsm_info - __start_early_lsm_info)
struct security_hook_heads security_hook_heads __lsm_ro_after_init;
static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
@@ -277,6 +278,8 @@ static void __init ordered_lsm_parse(const char *order,
const char *origin)
static void __init lsm_early_cred(struct cred *cred);
s
[PATCH V40 04/29] lockdown: Enforce module signatures if the kernel is locked down
From: David Howells
If the kernel is locked down, require that all modules have valid
signatures that we can verify.
I have adjusted the errors generated:
(1) If there's no signature (ENODATA) or we can't check it (ENOPKG,
ENOKEY), then:
(a) If signatures are enforced then EKEYREJECTED is returned.
(b) If there's no signature or we can't check it, but the kernel is
locked down then EPERM is returned (this is then consistent with
other lockdown cases).
(2) If the signature is unparseable (EBADMSG, EINVAL), the signature fails
the check (EKEYREJECTED) or a system error occurs (eg. ENOMEM), we
return the error we got.
Note that the X.509 code doesn't check for key expiry as the RTC might not
be valid or might not have been transferred to the kernel's clock yet.
[Modified by Matthew Garrett to remove the IMA integration. This will
be replaced with integration with the IMA architecture policy
patchset.]
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: Jessica Yu
Signed-off-by: James Morris
---
include/linux/security.h | 1 +
init/Kconfig | 5 +
kernel/module.c | 37 +---
security/lockdown/Kconfig| 1 +
security/lockdown/lockdown.c | 1 +
5 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 74787335d9ce..9e8abb60a99f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -103,6 +103,7 @@ enum lsm_event {
*/
enum lockdown_reason {
LOCKDOWN_NONE,
+ LOCKDOWN_MODULE_SIGNATURE,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/init/Kconfig b/init/Kconfig
index 0e2344389501..e6069368f278 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1939,6 +1939,11 @@ config MODULE_SIG
kernel build dependency so that the signing tool can use its crypto
library.
+ You should enable this option if you wish to use either
+ CONFIG_SECURITY_LOCKDOWN_LSM or lockdown functionality imposed via
+ another LSM - otherwise unsigned modules will be loadable regardless
+ of the lockdown policy.
+
!!!WARNING!!! If you enable this option, you MUST make sure that the
module DOES NOT get stripped after being signed. This includes the
debuginfo strip done by some packagers (such as rpmbuild) and
diff --git a/kernel/module.c b/kernel/module.c
index 80c7c09584cf..2206c08a5e10 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2753,8 +2753,9 @@ static inline void kmemleak_load_module(const struct
module *mod,
#ifdef CONFIG_MODULE_SIG
static int module_sig_check(struct load_info *info, int flags)
{
- int err = -ENOKEY;
+ int err = -ENODATA;
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+ const char *reason;
const void *mod = info->hdr;
/*
@@ -2769,16 +2770,38 @@ static int module_sig_check(struct load_info *info, int
flags)
err = mod_verify_sig(mod, info);
}
- if (!err) {
+ switch (err) {
+ case 0:
info->sig_ok = true;
return 0;
- }
- /* Not having a signature is only an error if we're strict. */
- if (err == -ENOKEY && !is_module_sig_enforced())
- err = 0;
+ /* We don't permit modules to be loaded into trusted kernels
+* without a valid signature on them, but if we're not
+* enforcing, certain errors are non-fatal.
+*/
+ case -ENODATA:
+ reason = "Loading of unsigned module";
+ goto decide;
+ case -ENOPKG:
+ reason = "Loading of module with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "Loading of module with unavailable key";
+ decide:
+ if (is_module_sig_enforced()) {
+ pr_notice("%s is rejected\n", reason);
+ return -EKEYREJECTED;
+ }
- return err;
+ return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+
+ /* All other errors are fatal, including nomem, unparseable
+* signatures and signature check failures - even if signatures
+* aren't required.
+*/
+ default:
+ return err;
+ }
}
#else /* !CONFIG_MODULE_SIG */
static int module_sig_check(struct load_info *info, int flags)
diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
index 7a1d213227a4..e84ddf484010 100644
--- a/security/lockdown/Kconfig
+++ b/security/lockdown/Kconfig
@@ -1,6 +1,7 @@
config SECURITY_LOCKDOWN_LSM
bool "Basic module for enforcing kernel lockdown"
depen
[PATCH V40 06/29] kexec_load: Disable at runtime if the kernel is locked down
From: Matthew Garrett
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
Signed-off-by: Matthew Garrett
Acked-by: Dave Young
Reviewed-by: Kees Cook
cc: ke...@lists.infradead.org
Signed-off-by: James Morris
---
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 e5dd446ef35b..b607a8ac97fe 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 240ecaa10a1d..aaf30ad351f9 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.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 05/29] lockdown: Restrict /dev/{mem,kmem,port} when the kernel is locked down
From: Matthew Garrett
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
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: x...@kernel.org
Signed-off-by: James Morris
---
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
#include
#include
-
#include
+#include
#ifdef CONFIG_IA64
# include
@@ -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 9e8abb60a99f..e5dd446ef35b 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 d8e42125a5dd..240ecaa10a1d 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.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 02/29] security: Add a "locked down" LSM hook
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
Acked-by: Kees Cook
Acked-by: Casey Schaufler
---
include/linux/lsm_hooks.h | 7 +++
include/linux/security.h | 32
security/security.c | 6 ++
3 files changed, 45 insertions(+)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index b02e8bb6654d..2f4ba9062fb8 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1446,6 +1446,11 @@
* @bpf_prog_free_security:
* Clean up the security information stored inside bpf prog.
*
+ * @locked_down
+ * Determine whether a kernel feature that potentially enables arbitrary
+ * code execution in kernel space should be permitted.
+ *
+ * @what: kernel feature being accessed
*/
union security_list_options {
int (*binder_set_context_mgr)(struct task_struct *mgr);
@@ -1807,6 +1812,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 +2052,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 c5dd90981c98..04cf48fab15d 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_lsm_notifier(enum lsm_event event, void *data)
@@ -1210,6 +1238,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 ef4a0111c8b4..7fc373486d7a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2389,3 +2389,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.23.0.rc1.153.gdeed80330f-goog
[PATCH V40 00/29] Add kernel lockdown functionality
After chatting with James in person, I'm resending the full set with the fixes merged in in order to avoid any bisect issues. There should be no functional changes other than avoiding build failures with some configs, and fixing the oops in tracefs.
Re: [PATCH V38 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
On Wed, Aug 14, 2019 at 10:46 AM Borislav Petkov wrote: > Yeah, ok, I see what you're doing there. AFAICT, you do that in > > setup_arch->acpi_boot_table_init-> ... -> acpi_os_get_root_pointer() Right. > I hope nothing needs it earlier because then we'll have to restructure > again... Passing the RSDP via command line is a pretty grotesque hack - we should just set up boot params in kexec_file, which would leave this as a problem for legacy kexec and nothing else.
Re: [PATCH V38 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
On Wed, Aug 14, 2019 at 12:25 AM Borislav Petkov wrote: > #if defined(CONFIG_RANDOMIZE_BASE) && defined(CONFIG_MEMORY_HOTREMOVE) > > false and thus not available to early code anymore. We explicitly don't want to pay attention to the acpi_rsdp kernel parameter in early boot except for the case of finding the SRAT table, and we only need that if CONFIG_RANDOMIZE_BASE and CONFIG_MEMORY_HOTREMOVE are set. However, we *do* want to tell the actual kernel where the RSDP is if we found it via some other means, so we can't just clear the boot parameters value. The kernel proper will parse the command line again and will then (if lockdown isn't enabled) override the actual value we passed up in boot params. So I think this is ok? (Sorry for not Cc:ing x86, clear oversight on my part)
Re: [PATCH V38 00/29] security: Add support for locking down the kernel
On Fri, Aug 9, 2019 at 11:08 PM James Morris wrote: > Please verify and test, as I had to make a few minor fixups for my v5.2 > base. Thanks James - there's a few small fixups required, would you like those as separate patches or should I just send you a fixed copy of the original patchset?
Re: [Linux 5.2.x] /sys/kernel/debug/tracing/events/power/cpu_idle/id: BUG: kernel NULL pointer dereference, address: 0000000000000000
On Mon, Aug 12, 2019 at 8:29 AM Steven Rostedt wrote: > From what I understand, Matthew's "lockdown" work is to prevent the > system from doing anything to see what is happening in the kernel. This > includes tracefs. This looks like it is working as designed. Oopsing the kernel isn't working as designed. Ben has a patch to fix this.
[PATCH V39] Enforce module signatures if the kernel is locked down
From: David Howells
If the kernel is locked down, require that all modules have valid
signatures that we can verify.
I have adjusted the errors generated:
(1) If there's no signature (ENODATA) or we can't check it (ENOPKG,
ENOKEY), then:
(a) If signatures are enforced then EKEYREJECTED is returned.
(b) If there's no signature or we can't check it, but the kernel is
locked down then EPERM is returned (this is then consistent with
other lockdown cases).
(2) If the signature is unparseable (EBADMSG, EINVAL), the signature fails
the check (EKEYREJECTED) or a system error occurs (eg. ENOMEM), we
return the error we got.
Note that the X.509 code doesn't check for key expiry as the RTC might not
be valid or might not have been transferred to the kernel's clock yet.
[Modified by Matthew Garrett to remove the IMA integration. This will
be replaced with integration with the IMA architecture policy
patchset.]
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: Jessica Yu
---
include/linux/security.h | 1 +
init/Kconfig | 5 +
kernel/module.c | 37 +---
security/lockdown/Kconfig| 1 +
security/lockdown/lockdown.c | 1 +
5 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 54a0532ec12f..8e70063074a1 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -103,6 +103,7 @@ enum lsm_event {
*/
enum lockdown_reason {
LOCKDOWN_NONE,
+ LOCKDOWN_MODULE_SIGNATURE,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/init/Kconfig b/init/Kconfig
index bd7d650d4a99..1f0f53774c3e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2017,6 +2017,11 @@ config MODULE_SIG
kernel build dependency so that the signing tool can use its crypto
library.
+ You should enable this option if you wish to use either
+ CONFIG_SECURITY_LOCKDOWN_LSM or lockdown functionality imposed via
+ another LSM - otherwise unsigned modules will be loadable regardless
+ of the lockdown policy.
+
!!!WARNING!!! If you enable this option, you MUST make sure that the
module DOES NOT get stripped after being signed. This includes the
debuginfo strip done by some packagers (such as rpmbuild) and
diff --git a/kernel/module.c b/kernel/module.c
index cd8df51d..318209889e26 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2771,8 +2771,9 @@ static inline void kmemleak_load_module(const struct
module *mod,
#ifdef CONFIG_MODULE_SIG
static int module_sig_check(struct load_info *info, int flags)
{
- int err = -ENOKEY;
+ int err = -ENODATA;
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+ const char *reason;
const void *mod = info->hdr;
/*
@@ -2787,16 +2788,38 @@ static int module_sig_check(struct load_info *info, int
flags)
err = mod_verify_sig(mod, info);
}
- if (!err) {
+ switch (err) {
+ case 0:
info->sig_ok = true;
return 0;
- }
- /* Not having a signature is only an error if we're strict. */
- if (err == -ENOKEY && !is_module_sig_enforced())
- err = 0;
+ /* We don't permit modules to be loaded into trusted kernels
+* without a valid signature on them, but if we're not
+* enforcing, certain errors are non-fatal.
+*/
+ case -ENODATA:
+ reason = "Loading of unsigned module";
+ goto decide;
+ case -ENOPKG:
+ reason = "Loading of module with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "Loading of module with unavailable key";
+ decide:
+ if (is_module_sig_enforced()) {
+ pr_notice("%s is rejected\n", reason);
+ return -EKEYREJECTED;
+ }
- return err;
+ return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+
+ /* All other errors are fatal, including nomem, unparseable
+* signatures and signature check failures - even if signatures
+* aren't required.
+*/
+ default:
+ return err;
+ }
}
#else /* !CONFIG_MODULE_SIG */
static int module_sig_check(struct load_info *info, int flags)
diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
index 7374ba76d8eb..f9dd683261e9 100644
--- a/security/lockdown/Kconfig
+++ b/security/lockdown/Kconfig
@@ -1,6 +1,7 @@
config SECURITY_LOCKDOWN_LSM
bool "Basic module for enforcing kernel lockdown"
depends on SECURITY
+ s
[PATCH V39] Lock down module params that specify hardware parameters (eg. ioport)
From: David Howells
Provided an annotation for module parameters that specify hardware
parameters (such as io ports, iomem addresses, irqs, dma channels, fixed
dma buffers and other types).
Suggested-by: Alan Cox
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: Jessica Yu
---
include/linux/security.h | 1 +
kernel/params.c | 21 -
security/lockdown/lockdown.c | 1 +
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 8f7048395114..43fa3486522b 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -113,6 +113,7 @@ enum lockdown_reason {
LOCKDOWN_ACPI_TABLES,
LOCKDOWN_PCMCIA_CIS,
LOCKDOWN_TIOCSSERIAL,
+ LOCKDOWN_MODULE_PARAMETERS,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/params.c b/kernel/params.c
index cf448785d058..8e56f8b12d8f 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_SYSFS
/* Protects all built-in parameters, modules use their own param_lock */
@@ -96,13 +97,19 @@ bool parameq(const char *a, const char *b)
return parameqn(a, b, strlen(a)+1);
}
-static void param_check_unsafe(const struct kernel_param *kp)
+static bool param_check_unsafe(const struct kernel_param *kp)
{
+ if (kp->flags & KERNEL_PARAM_FL_HWPARAM &&
+ security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
+ return false;
+
if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
pr_notice("Setting dangerous option %s - tainting kernel\n",
kp->name);
add_taint(TAINT_USER, LOCKDEP_STILL_OK);
}
+
+ return true;
}
static int parse_one(char *param,
@@ -132,8 +139,10 @@ static int parse_one(char *param,
pr_debug("handling %s with %p\n", param,
params[i].ops->set);
kernel_param_lock(params[i].mod);
- param_check_unsafe([i]);
- err = params[i].ops->set(val, [i]);
+ if (param_check_unsafe([i]))
+ err = params[i].ops->set(val, [i]);
+ else
+ err = -EPERM;
kernel_param_unlock(params[i].mod);
return err;
}
@@ -553,8 +562,10 @@ static ssize_t param_attr_store(struct module_attribute
*mattr,
return -EPERM;
kernel_param_lock(mk->mod);
- param_check_unsafe(attribute->param);
- err = attribute->param->ops->set(buf, attribute->param);
+ if (param_check_unsafe(attribute->param))
+ err = attribute->param->ops->set(buf, attribute->param);
+ else
+ err = -EPERM;
kernel_param_unlock(mk->mod);
if (!err)
return len;
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 00a3a6438dd2..5177938cfa0d 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -28,6 +28,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
+ [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
Re: [PATCH V37 04/29] Enforce module signatures if the kernel is locked down
On Thu, Aug 8, 2019 at 3:01 AM Jessica Yu wrote: > If you're confident that a hard dependency is not the right approach, > then perhaps we could add a comment in the Kconfig (You could take a > look at the comment under MODULE_SIG_ALL in init/Kconfig for an > example)? If someone is configuring the kernel on their own then it'd > be nice to let them know, otherwise having a lockdown kernel without > module signatures would defeat the purpose of lockdown no? :-) James, what would your preference be here? Jessica is right that not having CONFIG_MODULE_SIG enabled means lockdown probably doesn't work as expected, but tying it to the lockdown LSM seems inappropriate when another LSM could be providing lockdown policy and run into the same issue. Should this just be mentioned in the CONFIG_MODULE_SIG Kconfig help?
[PATCH V38 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
From: Josh Boyer
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
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
cc: Dave Young
cc: linux-a...@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
[PATCH V38 06/29] kexec_load: Disable at runtime if the kernel is locked down
From: Matthew Garrett
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
Signed-off-by: Matthew Garrett
Acked-by: Dave Young
Reviewed-by: Kees Cook
cc: ke...@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
[PATCH V38 17/29] Prohibit PCMCIA CIS storage when the kernel is locked down
From: David Howells
Prohibit replacement of the PCMCIA Card Information Structure when the
kernel is locked down.
Suggested-by: Dominik Brodowski
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
---
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
#include
#include
+#include
#include
#include
@@ -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
[PATCH V38 10/29] hibernate: Disable when the kernel is locked down
From: Josh Boyer
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
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Kees Cook
Cc: r...@rjwysocki.net
Cc: pa...@ucw.cz
cc: linux...@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
#include
#include
+#include
#include
#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
[PATCH V38 11/29] PCI: Lock down BAR access when the kernel is locked down
From: Matthew Garrett
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
Signed-off-by: Matthew Garrett
Acked-by: Bjorn Helgaas
Reviewed-by: Kees Cook
cc: linux-...@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 = >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
#include
#include
+#include
#include
#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
#include
+#include
#include
#include
#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] = "/de
[PATCH V38 14/29] ACPI: Limit access to custom_method when the kernel is locked down
From: Matthew Garrett
custom_method effectively allows arbitrary access to system memory, making
it possible for an attacker to circumvent restrictions on module loading.
Disable it if the kernel is locked down.
Signed-off-by: Matthew Garrett
Signed-off-by: David Howells
Reviewed-by: Kees Cook
cc: linux-a...@vger.kernel.org
---
drivers/acpi/custom_method.c | 6 ++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 8 insertions(+)
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index b2ef4c2ec955..7031307becd7 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -29,6 +30,11 @@ static ssize_t cm_write(struct file *file, const char __user
* user_buf,
struct acpi_table_header table;
acpi_status status;
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
+ if (ret)
+ return ret;
if (!(*ppos)) {
/* parse the table header to get the table length */
diff --git a/include/linux/security.h b/include/linux/security.h
index 155ff026eca4..1c32522b3c5a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -110,6 +110,7 @@ enum lockdown_reason {
LOCKDOWN_PCI_ACCESS,
LOCKDOWN_IOPORT,
LOCKDOWN_MSR,
+ LOCKDOWN_ACPI_TABLES,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index d99c0bee739d..ecb51b1a5c03 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -25,6 +25,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]
= {
[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
[LOCKDOWN_IOPORT] = "raw io port access",
[LOCKDOWN_MSR] = "raw MSR access",
+ [LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.770.g0f2c4a37fd-goog
[PATCH V38 08/29] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
From: Jiri Bohac
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
Signed-off-by: David Howells
Signed-off-by: Matthew Garrett
Reviewed-by: Jiri Bohac
Reviewed-by: Dave Young
cc: ke...@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_p
