* Re: [PATCH 1/1] IMA event log trimming
From: Gregory Lumen @ 2025-12-08 17:21 UTC (permalink / raw)
To: Roberto Sassu
Cc: James Bottomley, steven chen, linux-integrity, zohar,
roberto.sassu, dmitry.kasatkin, eric.snowberg, paul, jmorris,
serge, linux-security-module, anirudhve, nramas, sushring
In-Reply-To: <1ca00e3238e804db9280abf8655364c2662754ca.camel@huaweicloud.com>
[-- Attachment #1: Type: text/plain, Size: 1701 bytes --]
> Rather than designing the interface absent use cases, could we give use
> cases first so we know it fits?
I would also like to request that we include operational considerations in
the interface design; specifically, which additional signals can or should
be made available to UM to assist in diagnosing log validation failures.
This seems called for as any form of log trimming will introduce a new
potential cause for validation failures (unexpected trimming).
With the proposed changes, the only signal available to system operators
is the validation failure itself, with no signal that could be used to
determine if the failure was the result of an unexpected trim or a loss of
synchronization between the log and the PCRs (either through an unexpected
PCR extend, or tampering with the measurement list). Any of these may
indicate malicious activity, but they may also result from system
configuration issues that operators would need to diagnose and resolve.
Tracking and exposing either the total number of trimmed measurements or
the most recent trimmed-to PCR values by the kernel would allow system
operators to determine whether a failure was caused by unexpected trimming
or integrity issues. (Storing the PCR values also enables validation of
the retained measurements even after an unexpected trim, though I’m unsure
how often that signal would prove useful.)
Neither approach appears to add any additional attack surface beyond
raising the likelihood of incorrectly or insecurely implemented UM
attestation agents. Though that risk (and the additional kernel
complexity) should be weighed against the value of the additional
diagnostic signals.
-Gregory Lumen
^ permalink raw reply
* [PATCH] KEYS: trusted: Fix overwrite of keyhandle parameter
From: Jarkko Sakkinen @ 2025-12-08 14:54 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, stable, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM, open list
tpm2_key_decode() overrides the explicit keyhandle parameter, which can
lead to problems, if the loaded parent handle does not match the handle
stored to the key file. This can easily happen as handle by definition
is an ambiguous attribute.
Cc: stable@vger.kernel.org # v5.13+
Fixes: f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
security/keys/trusted-keys/trusted_tpm2.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index fb76c4ea496f..950684e54c71 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -121,7 +121,9 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
return -ENOMEM;
*buf = blob;
- options->keyhandle = ctx.parent;
+
+ if (!options->keyhandle)
+ options->keyhandle = ctx.parent;
memcpy(blob, ctx.priv, ctx.priv_len);
blob += ctx.priv_len;
--
2.39.5
^ permalink raw reply related
* Re: [PATCH 1/1] IMA event log trimming
From: Roberto Sassu @ 2025-12-08 9:40 UTC (permalink / raw)
To: James Bottomley, steven chen, linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-security-module, anirudhve, gregorylumen,
nramas, sushring
In-Reply-To: <34d739c2cf15baf78dff5acb7ae3ddd7ad47f219.camel@HansenPartnership.com>
On Fri, 2025-12-05 at 13:30 -0500, James Bottomley wrote:
> On Fri, 2025-12-05 at 10:30 +0100, Roberto Sassu wrote:
> > On Tue, 2025-12-02 at 15:28 -0800, steven chen wrote:
> > > This patch is for trimming N entries of the IMA event logs as well
> > > as cleaning the hash table.
> > >
> > > It provides a userspace interface ima_trim_log that can be used to
> > > input number N to let kernel to trim N entries of IMA event logs.
> > > When read this interface, it returns number of entries trimmed last
> > > tim.
> >
> > High-level comments:
> > - It does not offer the possibility to keep the hash table
> > - There is no coordination between taking a snapshot and the readers
> > of
> > the measurements list (I think it is necessary, since reading is
> > based on *pos, which contains the entries read until a given point;
> > if there is a truncate in the middle of the read, *pos would still
> > refer to the non-truncated list and the next read will skip some
> > measurement entries)
>
> Rather than designing the interface absent use cases, could we give use
> cases first so we know it fits? I really only have one: the keylime
> agent, but I believe it's pattern, which would be get TPM quote of
> logging PCRs, certify quote and then trim the log up to the point the
> quote was obtained will be a common pattern (at least I don't think
> anyone would trim the log without quoting it). If you're trimming
> based on a quote, you know the end hash of all the PCRs and you want to
> trim up to that. Since the log is fixed, you can work out what the
> index offset is, but it does seem a bit suboptimal to have to compute
> that, which is why I think trimming by end PCR hash is useful. The
> main point in all of this is, I think, that you don't really get to
> decide which point in the log the TPM will quote: it always quotes it's
> current PCR values, so no-one who cares about log quotes can select the
> number of entries first, they all have to see what PCR values the quote
> returns.
>
> > - While trimming per se is ok, I like more the idea of staging
> > changes and letting the user delete the staged measurements list
> > later
>
> I'm not averse to this, but keylime won't care ... it gets the quote
> and the log and it will verify the log before it will get the agent to
> issue a trim. Is there a use case that would actually need this
> behaviour?
>
> I also think having the base PCRs stored in the kernel after the trim
> (in a way that's easily consumed, like a non measured log entry), thus
> allowing verification from the log up to any current quote is useful.
> The reason I think it's useful is the attestation token one: I can see
> the keylime verifier giving an I've verified the log up to here and all
> the entries look good token that contains the base PCRs and if there
> are quite a few of these floating around (especially if the log isn't
> always trimmed) I could see the kernel base PCR storage being used to
> ask which is the relevant attestation token I should use to verify the
> rest of the log (or even used to detect nefarious trimming designed to
> hide records).
I have the impression that none the functionality you cited has to be
implemented in the kernel, because the only component one can trust to
verify the integrity of the IMA measurements list is the TPM. Whether
either the kernel or user space retain the measurements is irrelevant.
I believe that the only role of the kernel is to get rid of the
measurements entries as fast as possible (the kernel would act more
like a buffer).
This was actually the intent of my original proposal in
https://github.com/linux-integrity/linux/issues/1 . The idea of staging
(was snapshotting, but Mimi thinks the term is not accurate) is simply
to detach the entire IMA measurement list as fast as possible. Further
read and delete after staging is done without interfering with new
measurements (sure, the detaching of the hash table is not yet as
efficient as I hoped).
Anything that you said about verifying the measurements up to a certain
TPM quote can be implemented in user space without a decrease in
security. I don't see the need to synchronize the trim with the
verification in the kernel.
Roberto
^ permalink raw reply
* Re: [PATCH v2] net: ipv6: fix spelling typos in comments
From: ShiHao @ 2025-12-08 5:39 UTC (permalink / raw)
To: Simon Horman
Cc: kuba, davem, dsahern, edumazet, herbert, linux-kernel,
linux-security-module
In-Reply-To: <aTQCXJ7MUvnJpG6B@horms.kernel.org>
On Sat, Dec 06, 2025 at 10:15:56AM +0000, Simon Horman wrote:
> Unfortunately this patches do not apply cleanly on net-next,
> and thus can't be processed by our CI.
>
> When applying manually I see:
>
> Patch failed at 0001 net: ipv6: fix spelling typos in comments
> error: corrupt patch at line 58
>
> Also, net-next is currently closed.
>
> ## Form letter - net-next-closed
>
> The merge window for v6.19 has begun and therefore net-next has closed
> for new drivers, features, code refactoring and optimizations. We are
> currently accepting bug fixes only.
>
> Please repost when net-next reopens.
>
> Due to a combination of the merge-window, travel commitments of the
> maintainers, and the holiday season, net-next will re-open after
> 2nd January.
>
> RFC patches sent for review only are welcome at any time.
>
> See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
Hi Simon
Thanks for letting me know about this i am still new to net-dev
so i did not know about this . I will make sure to take care of
this next time. Again everyone thanks for your time to this matter.
shihao
^ permalink raw reply
* Re: [PATCH v7 0/3] Optimize tpm2_read_public() calls
From: Jarkko Sakkinen @ 2025-12-08 5:15 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251208050620.339408-1-jarkko@kernel.org>
On Mon, Dec 08, 2025 at 07:06:16AM +0200, Jarkko Sakkinen wrote:
> The main goal is fairly straight-forwrd here.
>
> The aim of these patches is optimize the number of tpm2_read_public() calls
> to the bare minimum.
>
> ## About dropping 'parentName' attribute for ASN.1 keys from the patch set
>
> I wrote this section as a remainder as I have facts fresh in my mind so
> that I can return them as soon as there is working group for the ASN.1
> specification. We really need to have this in the spec.
>
> I dropped [1] given that [2] is landing shortly to IETF draft process,
> according to James Bottomley [3]. We will return to [1] as soon as draft
> process is open for comments. Still, that attribute is super important,
> and here is why.
>
> This will cause a overhead as tpm2_unseal_trusted needs to do an
> unnecessary (from pure technical perspective) TPM2_ReadPublic command to
> acquire TPM name of the parent. This is obviously known at the time of
> creation of a key but the information is not stored anywhere by the
> key format.
>
> It also aligns badly with TCG specifications as Table 6 of architecture
> spec explicitly defines a reference (or name) for transient keys,
> persistent keys and NV indexes to be TPM_ALG_ID concatenated together
> with the hash of TPMT_PUBLIC. I.e. the file format is using exactly
> the opposite what should be use as reference for keys than what it
> should use.
>
> Other benefits are of course auto-discovery of parent for a key file,
> which is nasty to do without the name pre-stored.
Right and TPMT_HA is calculated everytime when a trusted key is
created, and right after that the already calculated data is simply
thrown into dumpster. And we are talking about spec compliant way
to refer other keys here, and only 66 bytes of extra payload.
I don't get it. And neither do I get how anyone would want to fix
this issue with TPM2_CreatePrimary interception.
BR, Jarkko
^ permalink raw reply
* [PATCH v7 3/3] tpm2-sessions: Remove AUTH_MAX_NAMES
From: Jarkko Sakkinen @ 2025-12-08 5:06 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251208050620.339408-1-jarkko@kernel.org>
In all of the call sites only one session is ever append. Thus, reduce
AUTH_MAX_NAMES, which leads into removing constant completely.
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v7:
- A new patch.
---
drivers/char/tpm/tpm2-sessions.c | 31 +++++++++++--------------------
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 0816a91134fc..62a200ae72d7 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -72,9 +72,6 @@
#include <crypto/sha2.h>
#include <crypto/utils.h>
-/* maximum number of names the TPM must remember for authorization */
-#define AUTH_MAX_NAMES 3
-
#define AES_KEY_BYTES AES_KEYSIZE_128
#define AES_KEY_BITS (AES_KEY_BYTES*8)
@@ -136,8 +133,8 @@ struct tpm2_auth {
* handle, but they are part of the session by name, which
* we must compute and remember
*/
- u8 name[AUTH_MAX_NAMES][TPM2_MAX_NAME_SIZE];
- u16 name_size_tbl[AUTH_MAX_NAMES];
+ u8 name[TPM2_MAX_NAME_SIZE];
+ u16 name_size;
};
#ifdef CONFIG_TCG_TPM2_HMAC
@@ -254,11 +251,14 @@ EXPORT_SYMBOL_GPL(tpm2_read_public);
int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
u32 handle, u8 *name, u16 name_size)
{
-#ifdef CONFIG_TCG_TPM2_HMAC
struct tpm2_auth *auth;
- int slot;
int ret;
-#endif
+
+ if (tpm_buf_length(buf) != TPM_HEADER_SIZE) {
+ dev_err(&chip->dev, "too many handles\n");
+ ret = -EIO;
+ goto err;
+ }
if (!tpm2_chip_auth(chip)) {
tpm_buf_append_handle(buf, handle);
@@ -266,12 +266,6 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
}
#ifdef CONFIG_TCG_TPM2_HMAC
- slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
- if (slot >= AUTH_MAX_NAMES) {
- dev_err(&chip->dev, "too many handles\n");
- ret = -EIO;
- goto err;
- }
auth = chip->auth;
if (auth->session != tpm_buf_length(buf)) {
dev_err(&chip->dev, "session state malformed");
@@ -280,16 +274,14 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
}
tpm_buf_append_u32(buf, handle);
auth->session += 4;
- memcpy(auth->name[slot], name, name_size);
- auth->name_size_tbl[slot] = name_size;
+ memcpy(auth->name, name, name_size);
+ auth->name_size = name_size;
#endif
return 0;
-#ifdef CONFIG_TCG_TPM2_HMAC
err:
tpm2_end_auth_session(chip);
return ret;
-#endif
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -658,8 +650,7 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
/* ordinal is already BE */
sha256_update(&sctx, (u8 *)&head->ordinal, sizeof(head->ordinal));
/* add the handle names */
- for (i = 0; i < handles; i++)
- sha256_update(&sctx, auth->name[i], auth->name_size_tbl[i]);
+ sha256_update(&sctx, auth->name, auth->name_size);
if (offset_s != tpm_buf_length(buf))
sha256_update(&sctx, &buf->data[offset_s],
tpm_buf_length(buf) - offset_s);
--
2.39.5
^ permalink raw reply related
* [PATCH v7 2/3] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
From: Jarkko Sakkinen @ 2025-12-08 5:06 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251208050620.339408-1-jarkko@kernel.org>
tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and calls to
tpm_buf_append_name() cause the exact same TPM2_ReadPublic command to be
sent to the chip, causing unnecessary traffic.
1. Export tpm2_read_public in order to make it callable from
'trusted_tpm2'.
2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted() in order to
halve the name resolutions required:
2a. Move tpm2_read_public() calls into trusted_tpm2.
2b. Pass TPM name to tpm_buf_append_name().
2c. Rework tpm_buf_append_name() to use the pre-resolved name.
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v7:
- No changes of any of the versions so far.
---
drivers/char/tpm/tpm2-cmd.c | 3 +-
drivers/char/tpm/tpm2-sessions.c | 95 +++++------------
include/linux/tpm.h | 10 +-
security/keys/trusted-keys/trusted_tpm2.c | 124 ++++++++++++++--------
4 files changed, 118 insertions(+), 114 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 2682f5ec09cb..5b04e74b6377 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -199,7 +199,8 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
if (!disable_pcr_integrity) {
- rc = tpm_buf_append_name(chip, buf, pcr_idx, NULL);
+ rc = tpm_buf_append_name(chip, buf, pcr_idx, (u8 *)&pcr_idx,
+ sizeof(u32));
if (rc)
return rc;
tpm_buf_append_hmac_session(chip, buf, 0, NULL, 0);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index a0c88fb1804c..0816a91134fc 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -136,8 +136,8 @@ struct tpm2_auth {
* handle, but they are part of the session by name, which
* we must compute and remember
*/
- u32 name_h[AUTH_MAX_NAMES];
u8 name[AUTH_MAX_NAMES][TPM2_MAX_NAME_SIZE];
+ u16 name_size_tbl[AUTH_MAX_NAMES];
};
#ifdef CONFIG_TCG_TPM2_HMAC
@@ -163,7 +163,17 @@ static int name_size(const u8 *name)
}
}
-static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
+/**
+ * tpm2_read_public: Resolve TPM name for a handle
+ * @chip: TPM chip to use.
+ * @handle: TPM handle.
+ * @name: A buffer for returning the name blob. Must have a
+ * capacity of 'SHA512_DIGET_SIZE + 2' bytes at minimum
+ *
+ * Returns size of TPM handle name of success.
+ * Returns tpm_transmit_cmd error codes when TPM2_ReadPublic fails.
+ */
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
{
u32 mso = tpm2_handle_mso(handle);
off_t offset = TPM_HEADER_SIZE;
@@ -212,14 +222,16 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
memcpy(name, &buf->data[offset], rc);
return name_size_alg;
}
+EXPORT_SYMBOL_GPL(tpm2_read_public);
#endif /* CONFIG_TCG_TPM2_HMAC */
/**
- * tpm_buf_append_name() - add a handle area to the buffer
- * @chip: the TPM chip structure
- * @buf: The buffer to be appended
- * @handle: The handle to be appended
- * @name: The name of the handle (may be NULL)
+ * tpm_buf_append_name() - Append a handle and store TPM name
+ * @chip: TPM chip to use.
+ * @buf: TPM buffer containing the TPM command in-transit.
+ * @handle: TPM handle to be appended.
+ * @name: TPM name of the handle
+ * @name_size: Size of the TPM name.
*
* In order to compute session HMACs, we need to know the names of the
* objects pointed to by the handles. For most objects, this is simply
@@ -236,15 +248,14 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
* will be caused by an incorrect programming model and indicated by a
* kernel message.
*
- * Ends the authorization session on failure.
+ * Returns zero on success.
+ * Returns -EIO when the authorization area state is malformed.
*/
int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name)
+ u32 handle, u8 *name, u16 name_size)
{
#ifdef CONFIG_TCG_TPM2_HMAC
- enum tpm2_mso_type mso = tpm2_handle_mso(handle);
struct tpm2_auth *auth;
- u16 name_size_alg;
int slot;
int ret;
#endif
@@ -269,36 +280,15 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
}
tpm_buf_append_u32(buf, handle);
auth->session += 4;
-
- if (mso == TPM2_MSO_PERSISTENT ||
- mso == TPM2_MSO_VOLATILE ||
- mso == TPM2_MSO_NVRAM) {
- if (!name) {
- ret = tpm2_read_public(chip, handle, auth->name[slot]);
- if (ret < 0)
- goto err;
-
- name_size_alg = ret;
- }
- } else {
- if (name) {
- dev_err(&chip->dev, "handle 0x%08x does not use a name\n",
- handle);
- ret = -EIO;
- goto err;
- }
- }
-
- auth->name_h[slot] = handle;
- if (name)
- memcpy(auth->name[slot], name, name_size_alg);
+ memcpy(auth->name[slot], name, name_size);
+ auth->name_size_tbl[slot] = name_size;
#endif
return 0;
#ifdef CONFIG_TCG_TPM2_HMAC
err:
tpm2_end_auth_session(chip);
- return tpm_ret_to_err(ret);
+ return ret;
#endif
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -606,22 +596,8 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
attrs = chip->cc_attrs_tbl[i];
handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+ offset_s += handles * sizeof(u32);
- /*
- * just check the names, it's easy to make mistakes. This
- * would happen if someone added a handle via
- * tpm_buf_append_u32() instead of tpm_buf_append_name()
- */
- for (i = 0; i < handles; i++) {
- u32 handle = tpm_buf_read_u32(buf, &offset_s);
-
- if (auth->name_h[i] != handle) {
- dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
- ret = -EIO;
- goto err;
- }
- }
- /* point offset_s to the start of the sessions */
val = tpm_buf_read_u32(buf, &offset_s);
/* point offset_p to the start of the parameters */
offset_p = offset_s + val;
@@ -682,23 +658,8 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
/* ordinal is already BE */
sha256_update(&sctx, (u8 *)&head->ordinal, sizeof(head->ordinal));
/* add the handle names */
- for (i = 0; i < handles; i++) {
- enum tpm2_mso_type mso = tpm2_handle_mso(auth->name_h[i]);
-
- if (mso == TPM2_MSO_PERSISTENT ||
- mso == TPM2_MSO_VOLATILE ||
- mso == TPM2_MSO_NVRAM) {
- ret = name_size(auth->name[i]);
- if (ret < 0)
- goto err;
-
- sha256_update(&sctx, auth->name[i], ret);
- } else {
- __be32 h = cpu_to_be32(auth->name_h[i]);
-
- sha256_update(&sctx, (u8 *)&h, 4);
- }
- }
+ for (i = 0; i < handles; i++)
+ sha256_update(&sctx, auth->name[i], auth->name_size_tbl[i]);
if (offset_s != tpm_buf_length(buf))
sha256_update(&sctx, &buf->data[offset_s],
tpm_buf_length(buf) - offset_s);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index b4b5bd586501..db716841973f 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -539,7 +539,7 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
}
int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name);
+ u32 handle, u8 *name, u16 name_size);
void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
@@ -553,6 +553,7 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
int rc);
void tpm2_end_auth_session(struct tpm_chip *chip);
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name);
#else
#include <linux/unaligned.h>
@@ -576,6 +577,13 @@ static inline int tpm_buf_check_hmac_response(struct tpm_chip *chip,
{
return rc;
}
+
+static inline int tpm2_read_public(struct tpm_chip *chip, u32 handle,
+ void *name)
+{
+ memcpy(name, &handle, sizeof(u32));
+ return sizeof(u32);
+}
#endif /* CONFIG_TCG_TPM2_HMAC */
#endif
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index fb76c4ea496f..b05808c39d9d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -233,7 +233,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
+ u8 parent_name[TPM2_MAX_NAME_SIZE];
off_t offset = TPM_HEADER_SIZE;
+ u16 parent_name_size;
int blob_len = 0;
int hash;
u32 flags;
@@ -250,6 +252,12 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (rc)
return rc;
+ rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+ if (rc < 0)
+ goto out_put;
+
+ parent_name_size = rc;
+
rc = tpm2_start_auth_session(chip);
if (rc)
goto out_put;
@@ -264,7 +272,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
tpm_buf_init(buf, TPM_BUFSIZE);
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
- rc = tpm_buf_append_name(chip, buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
goto out_put;
@@ -356,48 +365,38 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
}
/**
- * tpm2_load_cmd() - execute a TPM2_Load command
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: returned blob handle
+ * tpm2_load_cmd() - Execute TPM2_Load
+ * @chip: TPM chip to use.
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
+ * @parent_name: A cryptographic name, i.e. a TPMT_HA blob, of the
+ * parent key.
+ * @blob: The decoded payload for the key.
+ * @blob_handle: On success, will contain handle to the loaded keyedhash
+ * blob.
*
- * Return: 0 on success.
- * -E2BIG on wrong payload size.
- * -EPERM on tpm error status.
- * < 0 error from tpm_send.
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load fails.
*/
static int tpm2_load_cmd(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options,
+ u8 *parent_name,
+ u16 parent_name_size,
+ const u8 *blob,
u32 *blob_handle)
{
u8 *blob_ref __free(kfree) = NULL;
unsigned int private_len;
unsigned int public_len;
unsigned int blob_len;
- u8 *blob, *pub;
+ const u8 *pub;
int rc;
u32 attrs;
- rc = tpm2_key_decode(payload, options, &blob);
- if (rc) {
- /* old form */
- blob = payload->blob;
- payload->old_format = 1;
- } else {
- /* Bind for cleanup: */
- blob_ref = blob;
- }
-
- /* new format carries keyhandle but old format doesn't */
- if (!options->keyhandle)
- return -EINVAL;
-
/* must be big enough for at least the two be16 size counts */
if (payload->blob_len < 4)
- return -EINVAL;
+ return -E2BIG;
private_len = get_unaligned_be16(blob);
@@ -436,7 +435,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
tpm_buf_init(buf, TPM_BUFSIZE);
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
- rc = tpm_buf_append_name(chip, buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
return rc;
@@ -464,20 +464,23 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
}
/**
- * tpm2_unseal_cmd() - execute a TPM2_Unload command
+ * tpm2_unseal_cmd() - Execute TPM2_Unload
*
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: blob handle
+ * @chip: TPM chip to use
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
+ * @parent_name: A cryptographic name, i.e. a TPMT_HA blob, of the
+ * parent key.
+ * @blob_handle: Handle to the loaded keyedhash blob.
*
- * Return: 0 on success
- * -EPERM on tpm error status
- * < 0 error from tpm_send
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load fails.
*/
static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options,
+ u8 *parent_name,
+ u16 parent_name_size,
u32 blob_handle)
{
struct tpm_header *head;
@@ -497,7 +500,8 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm_buf_init(buf, TPM_BUFSIZE);
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
- rc = tpm_buf_append_name(chip, buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
return rc;
@@ -567,30 +571,60 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
}
/**
- * tpm2_unseal_trusted() - unseal the payload of a trusted key
+ * tpm2_unseal_trusted() - Unseal a trusted key
+ * @chip: TPM chip to use.
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
*
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- *
- * Return: Same as with tpm_send.
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Return -EINVAL when parent's key handle has not been set.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load or TPM2_Unseal
+ * fails.
*/
int tpm2_unseal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
+ u8 *blob_ref __free(kfree) = NULL;
+ u8 parent_name[TPM2_MAX_NAME_SIZE];
+ u16 parent_name_size;
u32 blob_handle;
+ u8 *blob;
int rc;
+ /*
+ * Try to decode the provided blob as an ASN.1 blob. Assume that the
+ * blob is in the legacy format if decoding does not end successfully.
+ */
+ rc = tpm2_key_decode(payload, options, &blob);
+ if (rc) {
+ blob = payload->blob;
+ payload->old_format = 1;
+ } else {
+ blob_ref = blob;
+ }
+
+ if (!options->keyhandle)
+ return -EINVAL;
+
rc = tpm_try_get_ops(chip);
if (rc)
return rc;
- rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
+ rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+ if (rc < 0)
+ goto out;
+
+ parent_name_size = rc;
+
+ rc = tpm2_load_cmd(chip, payload, options, parent_name,
+ parent_name_size, blob, &blob_handle);
if (rc)
goto out;
- rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+ rc = tpm2_unseal_cmd(chip, payload, options, parent_name,
+ parent_name_size, blob_handle);
+
tpm2_flush_context(chip, blob_handle);
out:
--
2.39.5
^ permalink raw reply related
* [PATCH v7 1/3] tpm2-sessions: Define TPM2_NAME_MAX_SIZE
From: Jarkko Sakkinen @ 2025-12-08 5:06 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251208050620.339408-1-jarkko@kernel.org>
This is somewhat cosmetic change but it does serve a purpose on tracking
the value set for the maximum length of TPM names, and to clearly states
what components it is composed of. It also anchors the value so that when
buffers are declared for this particular purpose, the same value is used
for the capacity.
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v7:
- A new patch.
---
drivers/char/tpm/tpm2-sessions.c | 2 +-
include/linux/tpm.h | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 79f27a46bd7f..a0c88fb1804c 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -137,7 +137,7 @@ struct tpm2_auth {
* we must compute and remember
*/
u32 name_h[AUTH_MAX_NAMES];
- u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE];
+ u8 name[AUTH_MAX_NAMES][TPM2_MAX_NAME_SIZE];
};
#ifdef CONFIG_TCG_TPM2_HMAC
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 42e2a091f43d..b4b5bd586501 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -28,7 +28,16 @@
#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
#define TPM_BUFSIZE 4096
+/*
+ * SHA-512 is, as of today, the largest digest in the TCG algorithm repository.
+ */
#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
+
+/*
+ * A TPM name digest i.e., TPMT_HA, is a concatenation of TPM_ALG_ID of the
+ * name algorithm and hash of TPMT_PUBLIC.
+ */
+#define TPM2_MAX_NAME_SIZE (TPM2_MAX_DIGEST_SIZE + 2)
#define TPM2_MAX_PCR_BANKS 8
struct tpm_chip;
--
2.39.5
^ permalink raw reply related
* [PATCH v7 0/3] Optimize tpm2_read_public() calls
From: Jarkko Sakkinen @ 2025-12-08 5:06 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Jarkko Sakkinen, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list
The main goal is fairly straight-forwrd here.
The aim of these patches is optimize the number of tpm2_read_public() calls
to the bare minimum.
## About dropping 'parentName' attribute for ASN.1 keys from the patch set
I wrote this section as a remainder as I have facts fresh in my mind so
that I can return them as soon as there is working group for the ASN.1
specification. We really need to have this in the spec.
I dropped [1] given that [2] is landing shortly to IETF draft process,
according to James Bottomley [3]. We will return to [1] as soon as draft
process is open for comments. Still, that attribute is super important,
and here is why.
This will cause a overhead as tpm2_unseal_trusted needs to do an
unnecessary (from pure technical perspective) TPM2_ReadPublic command to
acquire TPM name of the parent. This is obviously known at the time of
creation of a key but the information is not stored anywhere by the
key format.
It also aligns badly with TCG specifications as Table 6 of architecture
spec explicitly defines a reference (or name) for transient keys,
persistent keys and NV indexes to be TPM_ALG_ID concatenated together
with the hash of TPMT_PUBLIC. I.e. the file format is using exactly
the opposite what should be use as reference for keys than what it
should use.
Other benefits are of course auto-discovery of parent for a key file,
which is nasty to do without the name pre-stored.
[1] https://lore.kernel.org/linux-integrity/20251207173210.93765-3-jarkko@kernel.org/
[2] https://www.hansenpartnership.com/draft-bottomley-tpm2-keys.txt
[3] https://lore.kernel.org/linux-integrity/89d90617ba9b7a5eff1d5fad6bb9773033d3c18c.camel@HansenPartnership.com/
Jarkko Sakkinen (3):
tpm2-sessions: Define TPM2_NAME_MAX_SIZE
KEYS: trusted: Re-orchestrate tpm2_read_public() calls
tpm2-sessions: Remove AUTH_MAX_NAMES
drivers/char/tpm/tpm2-cmd.c | 3 +-
drivers/char/tpm/tpm2-sessions.c | 116 ++++++--------------
include/linux/tpm.h | 19 +++-
security/keys/trusted-keys/trusted_tpm2.c | 124 ++++++++++++++--------
4 files changed, 133 insertions(+), 129 deletions(-)
--
2.39.5
^ permalink raw reply
* まるなげ福祉事業
From: 福祉フランチャイズ本部 @ 2025-12-08 1:50 UTC (permalink / raw)
To: linux-security-module
新規事業をご検討中の経営者様へ
いつもお世話になっております。
「社会貢献性の高い事業で、確実な収益を上げたい」
そう考え、成長著しい福祉市場にご関心をお持ちのことと存じます。
しかし、同時にこうも考えていませんか？
「市場は魅力的だが、専門的な法規制や複雑な運営、人材採用は荷が重すぎる…」
「もし失敗したら、多額の投資が無駄になってしまうのでは…」
ご安心ください。その不安こそ、私たちが解決したい最大の課題でした。
この障壁を根本から取り除くのが、私たちtocotocoだけの
【運営本部代行プラン】です。
煩雑な運営業務は全て本部がプロフェッショナルとして代行します。
オーナー様には、「投資家」として市場の確実な成長という
最大の果実のみを受け取っていただきます。
―――――――――――
廃業率0.055％ 業態を選べる
障がい福祉フランチャイズ
<ご視聴予約はこちら>
https://fc-tocotoco.work/25/
〇オンラインで開催中
12月9日（火）10:00〜11:00
12月17日（水）15:00〜16:00
ご都合の良い日程をお選びいただけます。
―――――――――――
■ 失敗の不安を解消。本部代行プランの3大メリット
１．専門知識、一切不要
複雑な行政への請求や法改正対応、専門スタッフの採用・管理まで、全て本部が代行します。
異業種出身であることを気にする必要は一切ありません。
２．早期の投資回収を実現
煩雑な運営業務から解放され、オーナー様は事業拡大、
そして投資利回り50%以上、6ヶ月〜での投資回収実績を持つ、
確実な収益構造の構築に専念できます。
３．廃業率0.055％の安定性を最大限享受
業界トップクラスの実績とノウハウを持つ本部が現場を担うため、
「運営不安による失敗」というリスクが限りなくゼロに近づきます。
■ 市場の確実性を数字で証明
・市場規模：4兆円超え
・需要：障がい者数 毎月4万人増加
・安定性：廃業率0.055％（行政による総量規制で事業が守られています）
この巨大かつ安定した市場で、本部が代行することで、
より確実性の高い事業運営が実現します。
【運営代行プラン】は説明会でのみ詳細を公開
投資したいが運営までは荷が重い、という経営者様のために開発されたこの特別プランと、
5つの高収益業態（訪問看護、グループホームなど）の具体的な収益モデルは、
下記の説明会でのみ公開しております。
新規事業のリスクを最小化し、確実な収益源を確保したい方は、
ぜひこの機会にご参加ください。
▼ 詳細はこちら
https://fc-tocotoco.work/25/
―――
tocotoco株式会社 セミナー事務局
東京都千代田区九段南2丁目3−25
03-5256-7578
‥‥‥‥
本メールのご不要な方には大変ご迷惑をおかけいたしました。
メール停止ご希望の方は、お手数ですが下記URLにて、
お手続きをお願いいたします。
https://fc-tocotoco.work/mail/
―――
^ permalink raw reply
* Re: [PATCH v6 2/2] KEYS: trusted: Store parent's name to the encoded keys
From: Jarkko Sakkinen @ 2025-12-07 21:06 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <aTXelmqQ_y7zX-KZ@kernel.org>
On Sun, Dec 07, 2025 at 10:07:55PM +0200, Jarkko Sakkinen wrote:
> On Sun, Dec 07, 2025 at 07:57:13PM +0200, Jarkko Sakkinen wrote:
> > On Sun, Dec 07, 2025 at 07:32:10PM +0200, Jarkko Sakkinen wrote:
> > > Extend TPMKey ASN.1 speciication [1] with an optional 'parentName'
> > > attribute containing TPM name of the parent key (in other words, TPMT_HA
> > > blob).
> > >
> > > The life-cycle for trusted keys will now proceed as follows:
> > >
> > > 1. Encode parent's name to the 'paretName' during tpm2_key_encode().
> > > 2. During tpm2_unseal_trusted, read parent's name from 'parentName'. When
> > > the attribute is not available, fallback on doing tpm2_read_public().
> > >
> > > In other words, in the common (i.e., not loading a legacy key blob),
> > > tpm2_read_public() will now only happen at the time when a key is first
> > > created.
> > >
> > > In addition, move tpm2_read_public() to 'tpm2-cmd.c' and make its body
> > > unconditional so that the binary format of the saved keys is not dependent
> > > on kernel configuration.
> > >
> > > [1] https://www.hansenpartnership.com/draft-bottomley-tpm2-keys.txt
> > >
> > > Cc: tpm2@lists.linux.dev
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >
> > As an alternative workaround I think the following could be possibly
> > done (I need to trial it first though):
> >
> > 1. Maintain a cache where a name gets added at the time of
> > tpm2_seal_trusted(). It is from TPMT_HA to TPMT_HA mapping,
> > mapping TPMT_HA of the key to the TPMT of the parent.
> > 2. At thet time tpm2_unseal_trusted() retrieve name of thet
> > parent from the cache.
> >
> > Capturing TPM2_CreatePrimary would be essentially duct taping the
> > spec but I guess this could be more generally applicable. It neither
> > addresses persistent keys nor secondary parent keys, which we *have
> > to support*, as the kernel interface does.
>
> I think it is better to focus on merging the first patch and continue
> this in the working groups mailing list first before implementing
> complex quirks to address flaws of the file format.
Storing the name of the parent to the key file has also some other
advantages such as enabling parent auto discovery, which is especially
useful with transient keys. Parent handle is not useful data as by
definition it has an ambiguous meaning.
It is neither "insecure" as child key is tied to one single parent but
at the same parent's handle is completely useless and irrelevant data.
There's nothing "niche" IMHO on storing parent's name at least, and
it is very limiting not to have it.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v6 2/2] KEYS: trusted: Store parent's name to the encoded keys
From: Jarkko Sakkinen @ 2025-12-07 20:07 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <aTW_-fv8B6q6TAMx@kernel.org>
On Sun, Dec 07, 2025 at 07:57:13PM +0200, Jarkko Sakkinen wrote:
> On Sun, Dec 07, 2025 at 07:32:10PM +0200, Jarkko Sakkinen wrote:
> > Extend TPMKey ASN.1 speciication [1] with an optional 'parentName'
> > attribute containing TPM name of the parent key (in other words, TPMT_HA
> > blob).
> >
> > The life-cycle for trusted keys will now proceed as follows:
> >
> > 1. Encode parent's name to the 'paretName' during tpm2_key_encode().
> > 2. During tpm2_unseal_trusted, read parent's name from 'parentName'. When
> > the attribute is not available, fallback on doing tpm2_read_public().
> >
> > In other words, in the common (i.e., not loading a legacy key blob),
> > tpm2_read_public() will now only happen at the time when a key is first
> > created.
> >
> > In addition, move tpm2_read_public() to 'tpm2-cmd.c' and make its body
> > unconditional so that the binary format of the saved keys is not dependent
> > on kernel configuration.
> >
> > [1] https://www.hansenpartnership.com/draft-bottomley-tpm2-keys.txt
> >
> > Cc: tpm2@lists.linux.dev
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> As an alternative workaround I think the following could be possibly
> done (I need to trial it first though):
>
> 1. Maintain a cache where a name gets added at the time of
> tpm2_seal_trusted(). It is from TPMT_HA to TPMT_HA mapping,
> mapping TPMT_HA of the key to the TPMT of the parent.
> 2. At thet time tpm2_unseal_trusted() retrieve name of thet
> parent from the cache.
>
> Capturing TPM2_CreatePrimary would be essentially duct taping the
> spec but I guess this could be more generally applicable. It neither
> addresses persistent keys nor secondary parent keys, which we *have
> to support*, as the kernel interface does.
I think it is better to focus on merging the first patch and continue
this in the working groups mailing list first before implementing
complex quirks to address flaws of the file format.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v6 2/2] KEYS: trusted: Store parent's name to the encoded keys
From: Jarkko Sakkinen @ 2025-12-07 17:57 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251207173210.93765-3-jarkko@kernel.org>
On Sun, Dec 07, 2025 at 07:32:10PM +0200, Jarkko Sakkinen wrote:
> Extend TPMKey ASN.1 speciication [1] with an optional 'parentName'
> attribute containing TPM name of the parent key (in other words, TPMT_HA
> blob).
>
> The life-cycle for trusted keys will now proceed as follows:
>
> 1. Encode parent's name to the 'paretName' during tpm2_key_encode().
> 2. During tpm2_unseal_trusted, read parent's name from 'parentName'. When
> the attribute is not available, fallback on doing tpm2_read_public().
>
> In other words, in the common (i.e., not loading a legacy key blob),
> tpm2_read_public() will now only happen at the time when a key is first
> created.
>
> In addition, move tpm2_read_public() to 'tpm2-cmd.c' and make its body
> unconditional so that the binary format of the saved keys is not dependent
> on kernel configuration.
>
> [1] https://www.hansenpartnership.com/draft-bottomley-tpm2-keys.txt
>
> Cc: tpm2@lists.linux.dev
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
As an alternative workaround I think the following could be possibly
done (I need to trial it first though):
1. Maintain a cache where a name gets added at the time of
tpm2_seal_trusted(). It is from TPMT_HA to TPMT_HA mapping,
mapping TPMT_HA of the key to the TPMT of the parent.
2. At thet time tpm2_unseal_trusted() retrieve name of thet
parent from the cache.
Capturing TPM2_CreatePrimary would be essentially duct taping the
spec but I guess this could be more generally applicable. It neither
addresses persistent keys nor secondary parent keys, which we *have
to support*, as the kernel interface does.
BR, Jarkko
^ permalink raw reply
* [PATCH v6 2/2] KEYS: trusted: Store parent's name to the encoded keys
From: Jarkko Sakkinen @ 2025-12-07 17:32 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251207173210.93765-1-jarkko@kernel.org>
Extend TPMKey ASN.1 speciication [1] with an optional 'parentName'
attribute containing TPM name of the parent key (in other words, TPMT_HA
blob).
The life-cycle for trusted keys will now proceed as follows:
1. Encode parent's name to the 'paretName' during tpm2_key_encode().
2. During tpm2_unseal_trusted, read parent's name from 'parentName'. When
the attribute is not available, fallback on doing tpm2_read_public().
In other words, in the common (i.e., not loading a legacy key blob),
tpm2_read_public() will now only happen at the time when a key is first
created.
In addition, move tpm2_read_public() to 'tpm2-cmd.c' and make its body
unconditional so that the binary format of the saved keys is not dependent
on kernel configuration.
[1] https://www.hansenpartnership.com/draft-bottomley-tpm2-keys.txt
Cc: tpm2@lists.linux.dev
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v6:
- Addressed the issues reported by James.
Updated tag index to '7', as '6' is reserved by "creation data proposal"
to which I could not find a reference but presumably exists:
https://lore.kernel.org/linux-integrity/89d90617ba9b7a5eff1d5fad6bb9773033d3c18c.camel@HansenPartnership.com/#t
Once I become aware of the mailing list of IETF WG for TPM2 ASN.1 key
spec, so could not cc to patch over there buth at least the conflict has
been addressed. I can add the list ass CC one I know th address.
v5:
- Rebased to the mainline.
v4:
- No changes.
v3:
- Fixed three low-hanging fruit issues I found myself.
v2:
- A new patch.
---
drivers/char/tpm/tpm2-cmd.c | 83 ++++++++++++++++++++
drivers/char/tpm/tpm2-sessions.c | 85 ---------------------
include/linux/tpm.h | 9 +--
security/keys/trusted-keys/tpm2key.asn1 | 17 ++++-
security/keys/trusted-keys/trusted_tpm2.c | 93 ++++++++++++++++-------
5 files changed, 166 insertions(+), 121 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 5b04e74b6377..83a70680cfe9 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -757,3 +757,86 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
return -1;
}
+
+/*
+ * Name Size based on TPM algorithm (assumes no hash bigger than 255)
+ */
+static int name_size(const u8 *name)
+{
+ u16 hash_alg = get_unaligned_be16(name);
+
+ switch (hash_alg) {
+ case TPM_ALG_SHA1:
+ return SHA1_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA256:
+ return SHA256_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA384:
+ return SHA384_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA512:
+ return SHA512_DIGEST_SIZE + 2;
+ default:
+ pr_warn("tpm: unsupported name algorithm: 0x%04x\n", hash_alg);
+ return -EINVAL;
+ }
+}
+
+/**
+ * tpm2_read_public: Resolve TPM name for a handle
+ * @chip: TPM chip to use.
+ * @handle: TPM handle.
+ * @name: A buffer for returning the name blob. Must have a
+ * capacity of 'SHA512_DIGET_SIZE + 2' bytes at minimum
+ *
+ * Returns size of TPM handle name of success.
+ * Returns tpm_transmit_cmd error codes when TPM2_ReadPublic fails.
+ */
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
+{
+ u32 mso = tpm2_handle_mso(handle);
+ off_t offset = TPM_HEADER_SIZE;
+ int rc, name_size_alg;
+
+ if (mso != TPM2_MSO_PERSISTENT && mso != TPM2_MSO_VOLATILE &&
+ mso != TPM2_MSO_NVRAM) {
+ memcpy(name, &handle, sizeof(u32));
+ return sizeof(u32);
+ }
+
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
+ tpm_buf_append_u32(buf, handle);
+
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_ReadPublic");
+ if (rc)
+ return tpm_ret_to_err(rc);
+
+ /* Skip TPMT_PUBLIC: */
+ offset += tpm_buf_read_u16(buf, &offset);
+
+ /*
+ * Ensure space for the length field of TPM2B_NAME and hashAlg field of
+ * TPMT_HA (the extra four bytes).
+ */
+ if (offset + 4 > tpm_buf_length(buf))
+ return -EIO;
+
+ rc = tpm_buf_read_u16(buf, &offset);
+ name_size_alg = name_size(&buf->data[offset]);
+
+ if (name_size_alg < 0)
+ return name_size_alg;
+
+ if (rc != name_size_alg)
+ return -EIO;
+
+ if (offset + rc > tpm_buf_length(buf))
+ return -EIO;
+
+ memcpy(name, &buf->data[offset], rc);
+ return name_size_alg;
+}
+EXPORT_SYMBOL_GPL(tpm2_read_public);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 6b7112a7c30c..daf78c2e905f 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -140,91 +140,6 @@ struct tpm2_auth {
u16 name_size_tbl[AUTH_MAX_NAMES];
};
-#ifdef CONFIG_TCG_TPM2_HMAC
-/*
- * Name Size based on TPM algorithm (assumes no hash bigger than 255)
- */
-static int name_size(const u8 *name)
-{
- u16 hash_alg = get_unaligned_be16(name);
-
- switch (hash_alg) {
- case TPM_ALG_SHA1:
- return SHA1_DIGEST_SIZE + 2;
- case TPM_ALG_SHA256:
- return SHA256_DIGEST_SIZE + 2;
- case TPM_ALG_SHA384:
- return SHA384_DIGEST_SIZE + 2;
- case TPM_ALG_SHA512:
- return SHA512_DIGEST_SIZE + 2;
- default:
- pr_warn("tpm: unsupported name algorithm: 0x%04x\n", hash_alg);
- return -EINVAL;
- }
-}
-
-/**
- * tpm2_read_public: Resolve TPM name for a handle
- * @chip: TPM chip to use.
- * @handle: TPM handle.
- * @name: A buffer for returning the name blob. Must have a
- * capacity of 'SHA512_DIGET_SIZE + 2' bytes at minimum
- *
- * Returns size of TPM handle name of success.
- * Returns tpm_transmit_cmd error codes when TPM2_ReadPublic fails.
- */
-int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
-{
- u32 mso = tpm2_handle_mso(handle);
- off_t offset = TPM_HEADER_SIZE;
- int rc, name_size_alg;
-
- if (mso != TPM2_MSO_PERSISTENT && mso != TPM2_MSO_VOLATILE &&
- mso != TPM2_MSO_NVRAM) {
- memcpy(name, &handle, sizeof(u32));
- return sizeof(u32);
- }
-
- struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- tpm_buf_init(buf, TPM_BUFSIZE);
- tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
- tpm_buf_append_u32(buf, handle);
-
- rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_ReadPublic");
- if (rc)
- return tpm_ret_to_err(rc);
-
- /* Skip TPMT_PUBLIC: */
- offset += tpm_buf_read_u16(buf, &offset);
-
- /*
- * Ensure space for the length field of TPM2B_NAME and hashAlg field of
- * TPMT_HA (the extra four bytes).
- */
- if (offset + 4 > tpm_buf_length(buf))
- return -EIO;
-
- rc = tpm_buf_read_u16(buf, &offset);
- name_size_alg = name_size(&buf->data[offset]);
-
- if (name_size_alg < 0)
- return name_size_alg;
-
- if (rc != name_size_alg)
- return -EIO;
-
- if (offset + rc > tpm_buf_length(buf))
- return -EIO;
-
- memcpy(name, &buf->data[offset], rc);
- return name_size_alg;
-}
-EXPORT_SYMBOL_GPL(tpm2_read_public);
-#endif /* CONFIG_TCG_TPM2_HMAC */
-
/**
* tpm_buf_append_name() - Append a handle and store TPM name
* @chip: TPM chip to use.
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 3f1d8c27394b..40d1ede005b0 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -478,6 +478,7 @@ extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
extern struct tpm_chip *tpm_default_chip(void);
void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
int tpm2_find_hash_alg(unsigned int crypto_id);
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name);
static inline void tpm_buf_append_empty_auth(struct tpm_buf *buf, u32 handle)
{
@@ -544,7 +545,6 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
int rc);
void tpm2_end_auth_session(struct tpm_chip *chip);
-int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name);
#else
#include <linux/unaligned.h>
@@ -568,13 +568,6 @@ static inline int tpm_buf_check_hmac_response(struct tpm_chip *chip,
{
return rc;
}
-
-static inline int tpm2_read_public(struct tpm_chip *chip, u32 handle,
- void *name)
-{
- memcpy(name, &handle, sizeof(u32));
- return sizeof(u32);
-}
#endif /* CONFIG_TCG_TPM2_HMAC */
#endif
diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/security/keys/trusted-keys/tpm2key.asn1
index f57f869ad600..c48ea5a14219 100644
--- a/security/keys/trusted-keys/tpm2key.asn1
+++ b/security/keys/trusted-keys/tpm2key.asn1
@@ -1,11 +1,26 @@
---
--- ASN.1 for TPM 2.0 keys
---
+TPMPolicy ::= SEQUENCE {
+ commandCode [0] EXPLICIT INTEGER,
+ commandPolicy [1] EXPLICIT OCTET STRING
+}
+
+TPMAuthPolicy ::= SEQUENCE {
+ name [0] EXPLICIT UTF8String OPTIONAL,
+ policy [1] EXPLICIT SEQUENCE OF TPMPolicy
+}
TPMKey ::= SEQUENCE {
type OBJECT IDENTIFIER ({tpm2_key_type}),
emptyAuth [0] EXPLICIT BOOLEAN OPTIONAL,
+ policy [1] EXPLICIT SEQUENCE OF TPMPolicy OPTIONAL,
+ secret [2] EXPLICIT OCTET STRING OPTIONAL,
+ authPolicy [3] EXPLICIT SEQUENCE OF TPMAuthPolicy OPTIONAL,
+ description [4] EXPLICIT UTF8String OPTIONAL,
+ rsaParent [5] EXPLICIT BOOLEAN OPTIONAL,
+ parentName [7] EXPLICIT OCTET STRING OPTIONAL ({tpm2_key_parent_name}),
parent INTEGER ({tpm2_key_parent}),
pubkey OCTET STRING ({tpm2_key_pub}),
privkey OCTET STRING ({tpm2_key_priv})
- }
+}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 0feb96cab62d..6ceb8cff3830 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -20,16 +20,26 @@
static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 };
+enum tpm_key_tag {
+ TPM_KEY_TAG_EMPTY_AUTH = 0,
+ TPM_KEY_TAG_POLICY = 1,
+ TPM_KEY_TAG_SECRET = 2,
+ TPM_KEY_TAG_AUTH_POLICY = 3,
+ TPM_KEY_TAG_DESCRIPTION = 4,
+ TPM_KEY_TAG_RSA_PARENT = 5,
+ TPM_KEY_TAG_PARENT_NAME = 7,
+};
+
static int tpm2_key_encode(struct trusted_key_payload *payload,
struct trusted_key_options *options,
- u8 *src, u32 len)
+ u8 *src, u32 len, u8 *parent_name,
+ u16 parent_name_size)
{
const int SCRATCH_SIZE = PAGE_SIZE;
- u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
- u8 *work = scratch, *work1;
- u8 *end_work = scratch + SCRATCH_SIZE;
- u8 *priv, *pub;
+ u8 *end_work, *name_end;
u16 priv_len, pub_len;
+ u8 *work, *work1;
+ u8 *priv, *pub;
int ret;
priv_len = get_unaligned_be16(src) + 2;
@@ -40,23 +50,41 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
pub_len = get_unaligned_be16(src) + 2;
pub = src;
+ u8 *scratch __free(kfree) = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
if (!scratch)
return -ENOMEM;
+ work = scratch;
+ end_work = scratch + SCRATCH_SIZE;
+
work = asn1_encode_oid(work, end_work, tpm2key_oid,
asn1_oid_len(tpm2key_oid));
if (options->blobauth_len == 0) {
- unsigned char bool[3], *w = bool;
- /* tag 0 is emptyAuth */
- w = asn1_encode_boolean(w, w + sizeof(bool), true);
- if (WARN(IS_ERR(w), "BUG: Boolean failed to encode")) {
- ret = PTR_ERR(w);
- goto err;
+ u8 *bool_end;
+ u8 bool[3];
+
+ bool_end = asn1_encode_boolean(&bool[0], &bool[sizeof(bool)],
+ true);
+ if (IS_ERR(bool_end)) {
+ pr_err("BUG: Boolean failed to encode\n");
+ return PTR_ERR(bool_end);
}
- work = asn1_encode_tag(work, end_work, 0, bool, w - bool);
+
+ work = asn1_encode_tag(work, end_work, TPM_KEY_TAG_EMPTY_AUTH,
+ bool, bool_end - bool);
}
+ u8 *name_encoded __free(kfree) = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
+ if (!name_encoded)
+ return -ENOMEM;
+
+ name_end = asn1_encode_octet_string(&name_encoded[0],
+ &name_encoded[SCRATCH_SIZE],
+ parent_name, parent_name_size);
+ work = asn1_encode_tag(work, end_work, TPM_KEY_TAG_PARENT_NAME,
+ name_encoded, name_end - name_encoded);
+
/*
* Assume both octet strings will encode to a 2 byte definite length
*
@@ -65,8 +93,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
*/
if (WARN(work - scratch + pub_len + priv_len + 14 > SCRATCH_SIZE,
"BUG: scratch buffer is too small")) {
- ret = -EINVAL;
- goto err;
+ return -EINVAL;
}
work = asn1_encode_integer(work, end_work, options->keyhandle);
@@ -79,15 +106,10 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
if (IS_ERR(work1)) {
ret = PTR_ERR(work1);
pr_err("BUG: ASN.1 encoder failed with %d\n", ret);
- goto err;
+ return ret;
}
- kfree(scratch);
return work1 - payload->blob;
-
-err:
- kfree(scratch);
- return ret;
}
struct tpm2_key_context {
@@ -96,11 +118,13 @@ struct tpm2_key_context {
u32 pub_len;
const u8 *priv;
u32 priv_len;
+ const u8 *name;
+ u32 name_len;
};
static int tpm2_key_decode(struct trusted_key_payload *payload,
struct trusted_key_options *options,
- u8 **buf)
+ u8 **buf, u8 *parent_name, u16 *parent_name_size)
{
int ret;
struct tpm2_key_context ctx;
@@ -127,6 +151,8 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
blob += ctx.priv_len;
memcpy(blob, ctx.pub, ctx.pub_len);
+ memcpy(parent_name, ctx.name, ctx.name_len);
+ *parent_name_size = ctx.name_len;
return 0;
}
@@ -190,6 +216,16 @@ int tpm2_key_priv(void *context, size_t hdrlen,
return 0;
}
+int tpm2_key_parent_name(void *context, size_t hdrlen, unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_context *ctx = context;
+
+ ctx->name = value;
+ ctx->name_len = vlen;
+
+ return 0;
+}
/**
* tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
*
@@ -351,7 +387,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
}
- blob_len = tpm2_key_encode(payload, options, &buf->data[offset], blob_len);
+ blob_len = tpm2_key_encode(payload, options, &buf->data[offset],
+ blob_len, parent_name, parent_name_size);
if (blob_len < 0)
rc = blob_len;
@@ -596,7 +633,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
* Try to decode the provided blob as an ASN.1 blob. Assume that the
* blob is in the legacy format if decoding does not end successfully.
*/
- rc = tpm2_key_decode(payload, options, &blob);
+ rc = tpm2_key_decode(payload, options, &blob, &parent_name[0], &parent_name_size);
if (rc) {
blob = payload->blob;
payload->old_format = 1;
@@ -611,11 +648,13 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
if (rc)
return rc;
- rc = tpm2_read_public(chip, options->keyhandle, parent_name);
- if (rc < 0)
- goto out;
+ if (!parent_name_size) {
+ rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+ if (rc < 0)
+ goto out;
- parent_name_size = rc;
+ parent_name_size = rc;
+ }
rc = tpm2_load_cmd(chip, payload, options, parent_name,
parent_name_size, blob, &blob_handle);
--
2.39.5
^ permalink raw reply related
* [PATCH v6 1/2] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
From: Jarkko Sakkinen @ 2025-12-07 17:32 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251207173210.93765-1-jarkko@kernel.org>
tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and calls to
tpm_buf_append_name() cause the exact same TPM2_ReadPublic command to be
sent to the chip, causing unnecessary traffic.
1. Export tpm2_read_public in order to make it callable from
'trusted_tpm2'.
2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted() in order to
halve the name resolutions required:
2a. Move tpm2_read_public() calls into trusted_tpm2.
2b. Pass TPM name to tpm_buf_append_name().
2c. Rework tpm_buf_append_name() to use the pre-resolved name.
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v6:
- No changes.
v5:
- Rebased to the mainline.
v4:
- No changes
v3:
- No changes.
v2:
- No changes.
---
drivers/char/tpm/tpm2-cmd.c | 3 +-
drivers/char/tpm/tpm2-sessions.c | 95 +++++------------
include/linux/tpm.h | 10 +-
security/keys/trusted-keys/trusted_tpm2.c | 124 ++++++++++++++--------
4 files changed, 118 insertions(+), 114 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 2682f5ec09cb..5b04e74b6377 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -199,7 +199,8 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
if (!disable_pcr_integrity) {
- rc = tpm_buf_append_name(chip, buf, pcr_idx, NULL);
+ rc = tpm_buf_append_name(chip, buf, pcr_idx, (u8 *)&pcr_idx,
+ sizeof(u32));
if (rc)
return rc;
tpm_buf_append_hmac_session(chip, buf, 0, NULL, 0);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 79f27a46bd7f..6b7112a7c30c 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -136,8 +136,8 @@ struct tpm2_auth {
* handle, but they are part of the session by name, which
* we must compute and remember
*/
- u32 name_h[AUTH_MAX_NAMES];
u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE];
+ u16 name_size_tbl[AUTH_MAX_NAMES];
};
#ifdef CONFIG_TCG_TPM2_HMAC
@@ -163,7 +163,17 @@ static int name_size(const u8 *name)
}
}
-static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
+/**
+ * tpm2_read_public: Resolve TPM name for a handle
+ * @chip: TPM chip to use.
+ * @handle: TPM handle.
+ * @name: A buffer for returning the name blob. Must have a
+ * capacity of 'SHA512_DIGET_SIZE + 2' bytes at minimum
+ *
+ * Returns size of TPM handle name of success.
+ * Returns tpm_transmit_cmd error codes when TPM2_ReadPublic fails.
+ */
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
{
u32 mso = tpm2_handle_mso(handle);
off_t offset = TPM_HEADER_SIZE;
@@ -212,14 +222,16 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
memcpy(name, &buf->data[offset], rc);
return name_size_alg;
}
+EXPORT_SYMBOL_GPL(tpm2_read_public);
#endif /* CONFIG_TCG_TPM2_HMAC */
/**
- * tpm_buf_append_name() - add a handle area to the buffer
- * @chip: the TPM chip structure
- * @buf: The buffer to be appended
- * @handle: The handle to be appended
- * @name: The name of the handle (may be NULL)
+ * tpm_buf_append_name() - Append a handle and store TPM name
+ * @chip: TPM chip to use.
+ * @buf: TPM buffer containing the TPM command in-transit.
+ * @handle: TPM handle to be appended.
+ * @name: TPM name of the handle
+ * @name_size: Size of the TPM name.
*
* In order to compute session HMACs, we need to know the names of the
* objects pointed to by the handles. For most objects, this is simply
@@ -236,15 +248,14 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
* will be caused by an incorrect programming model and indicated by a
* kernel message.
*
- * Ends the authorization session on failure.
+ * Returns zero on success.
+ * Returns -EIO when the authorization area state is malformed.
*/
int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name)
+ u32 handle, u8 *name, u16 name_size)
{
#ifdef CONFIG_TCG_TPM2_HMAC
- enum tpm2_mso_type mso = tpm2_handle_mso(handle);
struct tpm2_auth *auth;
- u16 name_size_alg;
int slot;
int ret;
#endif
@@ -269,36 +280,15 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
}
tpm_buf_append_u32(buf, handle);
auth->session += 4;
-
- if (mso == TPM2_MSO_PERSISTENT ||
- mso == TPM2_MSO_VOLATILE ||
- mso == TPM2_MSO_NVRAM) {
- if (!name) {
- ret = tpm2_read_public(chip, handle, auth->name[slot]);
- if (ret < 0)
- goto err;
-
- name_size_alg = ret;
- }
- } else {
- if (name) {
- dev_err(&chip->dev, "handle 0x%08x does not use a name\n",
- handle);
- ret = -EIO;
- goto err;
- }
- }
-
- auth->name_h[slot] = handle;
- if (name)
- memcpy(auth->name[slot], name, name_size_alg);
+ memcpy(auth->name[slot], name, name_size);
+ auth->name_size_tbl[slot] = name_size;
#endif
return 0;
#ifdef CONFIG_TCG_TPM2_HMAC
err:
tpm2_end_auth_session(chip);
- return tpm_ret_to_err(ret);
+ return ret;
#endif
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -606,22 +596,8 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
attrs = chip->cc_attrs_tbl[i];
handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+ offset_s += handles * sizeof(u32);
- /*
- * just check the names, it's easy to make mistakes. This
- * would happen if someone added a handle via
- * tpm_buf_append_u32() instead of tpm_buf_append_name()
- */
- for (i = 0; i < handles; i++) {
- u32 handle = tpm_buf_read_u32(buf, &offset_s);
-
- if (auth->name_h[i] != handle) {
- dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
- ret = -EIO;
- goto err;
- }
- }
- /* point offset_s to the start of the sessions */
val = tpm_buf_read_u32(buf, &offset_s);
/* point offset_p to the start of the parameters */
offset_p = offset_s + val;
@@ -682,23 +658,8 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
/* ordinal is already BE */
sha256_update(&sctx, (u8 *)&head->ordinal, sizeof(head->ordinal));
/* add the handle names */
- for (i = 0; i < handles; i++) {
- enum tpm2_mso_type mso = tpm2_handle_mso(auth->name_h[i]);
-
- if (mso == TPM2_MSO_PERSISTENT ||
- mso == TPM2_MSO_VOLATILE ||
- mso == TPM2_MSO_NVRAM) {
- ret = name_size(auth->name[i]);
- if (ret < 0)
- goto err;
-
- sha256_update(&sctx, auth->name[i], ret);
- } else {
- __be32 h = cpu_to_be32(auth->name_h[i]);
-
- sha256_update(&sctx, (u8 *)&h, 4);
- }
- }
+ for (i = 0; i < handles; i++)
+ sha256_update(&sctx, auth->name[i], auth->name_size_tbl[i]);
if (offset_s != tpm_buf_length(buf))
sha256_update(&sctx, &buf->data[offset_s],
tpm_buf_length(buf) - offset_s);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 42e2a091f43d..3f1d8c27394b 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -530,7 +530,7 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
}
int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name);
+ u32 handle, u8 *name, u16 name_size);
void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
@@ -544,6 +544,7 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
int rc);
void tpm2_end_auth_session(struct tpm_chip *chip);
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name);
#else
#include <linux/unaligned.h>
@@ -567,6 +568,13 @@ static inline int tpm_buf_check_hmac_response(struct tpm_chip *chip,
{
return rc;
}
+
+static inline int tpm2_read_public(struct tpm_chip *chip, u32 handle,
+ void *name)
+{
+ memcpy(name, &handle, sizeof(u32));
+ return sizeof(u32);
+}
#endif /* CONFIG_TCG_TPM2_HMAC */
#endif
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index fb76c4ea496f..0feb96cab62d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -233,7 +233,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
+ u8 parent_name[2 + SHA512_DIGEST_SIZE];
off_t offset = TPM_HEADER_SIZE;
+ u16 parent_name_size;
int blob_len = 0;
int hash;
u32 flags;
@@ -250,6 +252,12 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (rc)
return rc;
+ rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+ if (rc < 0)
+ goto out_put;
+
+ parent_name_size = rc;
+
rc = tpm2_start_auth_session(chip);
if (rc)
goto out_put;
@@ -264,7 +272,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
tpm_buf_init(buf, TPM_BUFSIZE);
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
- rc = tpm_buf_append_name(chip, buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
goto out_put;
@@ -356,48 +365,38 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
}
/**
- * tpm2_load_cmd() - execute a TPM2_Load command
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: returned blob handle
+ * tpm2_load_cmd() - Execute TPM2_Load
+ * @chip: TPM chip to use.
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
+ * @parent_name: A cryptographic name, i.e. a TPMT_HA blob, of the
+ * parent key.
+ * @blob: The decoded payload for the key.
+ * @blob_handle: On success, will contain handle to the loaded keyedhash
+ * blob.
*
- * Return: 0 on success.
- * -E2BIG on wrong payload size.
- * -EPERM on tpm error status.
- * < 0 error from tpm_send.
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load fails.
*/
static int tpm2_load_cmd(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options,
+ u8 *parent_name,
+ u16 parent_name_size,
+ const u8 *blob,
u32 *blob_handle)
{
u8 *blob_ref __free(kfree) = NULL;
unsigned int private_len;
unsigned int public_len;
unsigned int blob_len;
- u8 *blob, *pub;
+ const u8 *pub;
int rc;
u32 attrs;
- rc = tpm2_key_decode(payload, options, &blob);
- if (rc) {
- /* old form */
- blob = payload->blob;
- payload->old_format = 1;
- } else {
- /* Bind for cleanup: */
- blob_ref = blob;
- }
-
- /* new format carries keyhandle but old format doesn't */
- if (!options->keyhandle)
- return -EINVAL;
-
/* must be big enough for at least the two be16 size counts */
if (payload->blob_len < 4)
- return -EINVAL;
+ return -E2BIG;
private_len = get_unaligned_be16(blob);
@@ -436,7 +435,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
tpm_buf_init(buf, TPM_BUFSIZE);
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
- rc = tpm_buf_append_name(chip, buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
return rc;
@@ -464,20 +464,23 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
}
/**
- * tpm2_unseal_cmd() - execute a TPM2_Unload command
+ * tpm2_unseal_cmd() - Execute TPM2_Unload
*
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: blob handle
+ * @chip: TPM chip to use
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
+ * @parent_name: A cryptographic name, i.e. a TPMT_HA blob, of the
+ * parent key.
+ * @blob_handle: Handle to the loaded keyedhash blob.
*
- * Return: 0 on success
- * -EPERM on tpm error status
- * < 0 error from tpm_send
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load fails.
*/
static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options,
+ u8 *parent_name,
+ u16 parent_name_size,
u32 blob_handle)
{
struct tpm_header *head;
@@ -497,7 +500,8 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm_buf_init(buf, TPM_BUFSIZE);
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
- rc = tpm_buf_append_name(chip, buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
return rc;
@@ -567,30 +571,60 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
}
/**
- * tpm2_unseal_trusted() - unseal the payload of a trusted key
+ * tpm2_unseal_trusted() - Unseal a trusted key
+ * @chip: TPM chip to use.
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
*
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- *
- * Return: Same as with tpm_send.
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Return -EINVAL when parent's key handle has not been set.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load or TPM2_Unseal
+ * fails.
*/
int tpm2_unseal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
+ u8 *blob_ref __free(kfree) = NULL;
+ u8 parent_name[2 + SHA512_DIGEST_SIZE];
+ u16 parent_name_size;
u32 blob_handle;
+ u8 *blob;
int rc;
+ /*
+ * Try to decode the provided blob as an ASN.1 blob. Assume that the
+ * blob is in the legacy format if decoding does not end successfully.
+ */
+ rc = tpm2_key_decode(payload, options, &blob);
+ if (rc) {
+ blob = payload->blob;
+ payload->old_format = 1;
+ } else {
+ blob_ref = blob;
+ }
+
+ if (!options->keyhandle)
+ return -EINVAL;
+
rc = tpm_try_get_ops(chip);
if (rc)
return rc;
- rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
+ rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+ if (rc < 0)
+ goto out;
+
+ parent_name_size = rc;
+
+ rc = tpm2_load_cmd(chip, payload, options, parent_name,
+ parent_name_size, blob, &blob_handle);
if (rc)
goto out;
- rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+ rc = tpm2_unseal_cmd(chip, payload, options, parent_name,
+ parent_name_size, blob_handle);
+
tpm2_flush_context(chip, blob_handle);
out:
--
2.39.5
^ permalink raw reply related
* [PATCH v6 0/2] Optimize tpm2_read_public() calls
From: Jarkko Sakkinen @ 2025-12-07 17:32 UTC (permalink / raw)
To: linux-integrity
Cc: tpm2, Jarkko Sakkinen, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list
The main goal is fairly straight-forwrd here.
The aim of these patches is optimize the number of tpm2_read_public()
calls to the bare minimum. The last patch is a bit out of scope but
I included because it builds on top of this work.
Jarkko Sakkinen (2):
KEYS: trusted: Re-orchestrate tpm2_read_public() calls
KEYS: trusted: Store parent's name to the encoded keys
drivers/char/tpm/tpm2-cmd.c | 86 ++++++++-
drivers/char/tpm/tpm2-sessions.c | 156 ++--------------
include/linux/tpm.h | 3 +-
security/keys/trusted-keys/tpm2key.asn1 | 17 +-
security/keys/trusted-keys/trusted_tpm2.c | 207 +++++++++++++++-------
5 files changed, 259 insertions(+), 210 deletions(-)
--
2.39.5
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
From: Jarkko Sakkinen @ 2025-12-07 9:28 UTC (permalink / raw)
To: James Bottomley
Cc: linux-integrity, Peter Huewe, Jason Gunthorpe, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <aTVGiubK5EKitM9u@kernel.org>
On Sun, Dec 07, 2025 at 11:19:10AM +0200, Jarkko Sakkinen wrote:
> On Sun, Dec 07, 2025 at 03:33:17PM +0900, James Bottomley wrote:
> > On Fri, 2025-12-05 at 02:49 +0200, Jarkko Sakkinen wrote:
> > > On Fri, Dec 05, 2025 at 01:20:30AM +0200, Jarkko Sakkinen wrote:
> > > > On Fri, Dec 05, 2025 at 12:31:27AM +0200, Jarkko Sakkinen wrote:
> > > > > tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and
> > > > > calls to
> > > > > tpm_buf_append_name() cause the exact same TPM2_ReadPublic
> > > > > command to be
> > > > > sent to the chip, causing unnecessary traffic.
> > > > >
> > > > > 1. Export tpm2_read_public in order to make it callable from
> > > > > 'trusted_tpm2'.
> > > > > 2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted()
> > > > > in order to
> > > > > halve the name resolutions required:
> > > > > 2a. Move tpm2_read_public() calls into trusted_tpm2.
> > > > > 2b. Pass TPM name to tpm_buf_append_name().
> > > > > 2c. Rework tpm_buf_append_name() to use the pre-resolved name.
> > > > >
> > > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > >
> > > > If ASN.1 blob would contain also name of the parent then zero
> > > > tpm2_read_public() calls would be required i.e., the main
> > > > bottleneck here inherits from the limitations of the file format
> > > > itself.
> > >
> > > Along the lines of attached patch.
> >
> > Well firstly [6] is already being taken by the creation data proposal,
> > so this would introduce an incompatibility between the kernel and the
> > spec, but secondly, if you want something like this in the spec before
> > it goes to the IETF you really need to propose it now.
>
> What is the mailing list for the working group, or is this still
> unclear as of today?
>
> >
> > The problem with this particular addition is that it would be Linux
> > Kernel specific. All the current TSSs already do a cached read public
> > under the covers when they add the session wrappings so the user facing
> > API they expose has nowhere to insert (or easily extract) a name field
> > and thus TSS based implementations would have no incentive to either
> > output or consume this field. That's not to say the standard can't
> > have additions for crazy or niche use cases (that's what the rsaParent
> > flag is: a one off to support a niche SUSE use case) but it would be
> > hard to persuade user implementations to do this so the kernel would
> > have to interoperate with the case where it didn't exist anyway.
>
> It is just matter of conditionally calling tpm2_read_public(). Not
> a big deal.
>
> >
> > The standard use today is with permanent handles for parents, where the
> > parent is created on the fly, so the name is actually returned from
> > TPM2_CreatePrimary for this use case. It should be a simple matter to
> > bring the kernel implementation up to doing this as well.
>
> Intercepting TPM2_CreatePrimary does not provide full coverage, as
> it does not address persistent keys, which kernel interface does
> support.
It also weaker solution in the sense that it lacks support for secondary
keys as parents.
Name digest is universal as it scales primary keys, secondary keys both
persistent and transient.
I'm not really sure why we should use TSS implementations as reference
for anything in this space. We always want to support "superset' because
it actually creates innovation and distruption to the pre-existing
science.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
From: Jarkko Sakkinen @ 2025-12-07 9:19 UTC (permalink / raw)
To: James Bottomley
Cc: linux-integrity, Peter Huewe, Jason Gunthorpe, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <89d90617ba9b7a5eff1d5fad6bb9773033d3c18c.camel@HansenPartnership.com>
On Sun, Dec 07, 2025 at 03:33:17PM +0900, James Bottomley wrote:
> On Fri, 2025-12-05 at 02:49 +0200, Jarkko Sakkinen wrote:
> > On Fri, Dec 05, 2025 at 01:20:30AM +0200, Jarkko Sakkinen wrote:
> > > On Fri, Dec 05, 2025 at 12:31:27AM +0200, Jarkko Sakkinen wrote:
> > > > tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and
> > > > calls to
> > > > tpm_buf_append_name() cause the exact same TPM2_ReadPublic
> > > > command to be
> > > > sent to the chip, causing unnecessary traffic.
> > > >
> > > > 1. Export tpm2_read_public in order to make it callable from
> > > > 'trusted_tpm2'.
> > > > 2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted()
> > > > in order to
> > > > halve the name resolutions required:
> > > > 2a. Move tpm2_read_public() calls into trusted_tpm2.
> > > > 2b. Pass TPM name to tpm_buf_append_name().
> > > > 2c. Rework tpm_buf_append_name() to use the pre-resolved name.
> > > >
> > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > >
> > > If ASN.1 blob would contain also name of the parent then zero
> > > tpm2_read_public() calls would be required i.e., the main
> > > bottleneck here inherits from the limitations of the file format
> > > itself.
> >
> > Along the lines of attached patch.
>
> Well firstly [6] is already being taken by the creation data proposal,
> so this would introduce an incompatibility between the kernel and the
> spec, but secondly, if you want something like this in the spec before
> it goes to the IETF you really need to propose it now.
What is the mailing list for the working group, or is this still
unclear as of today?
>
> The problem with this particular addition is that it would be Linux
> Kernel specific. All the current TSSs already do a cached read public
> under the covers when they add the session wrappings so the user facing
> API they expose has nowhere to insert (or easily extract) a name field
> and thus TSS based implementations would have no incentive to either
> output or consume this field. That's not to say the standard can't
> have additions for crazy or niche use cases (that's what the rsaParent
> flag is: a one off to support a niche SUSE use case) but it would be
> hard to persuade user implementations to do this so the kernel would
> have to interoperate with the case where it didn't exist anyway.
It is just matter of conditionally calling tpm2_read_public(). Not
a big deal.
>
> The standard use today is with permanent handles for parents, where the
> parent is created on the fly, so the name is actually returned from
> TPM2_CreatePrimary for this use case. It should be a simple matter to
> bring the kernel implementation up to doing this as well.
Intercepting TPM2_CreatePrimary does not provide full coverage, as
it does not address persistent keys, which kernel interface does
support.
>
> Regards,
>
> James
>
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
From: James Bottomley @ 2025-12-07 6:33 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: Peter Huewe, Jason Gunthorpe, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <aTIsFJobEyISTO_y@kernel.org>
On Fri, 2025-12-05 at 02:49 +0200, Jarkko Sakkinen wrote:
> On Fri, Dec 05, 2025 at 01:20:30AM +0200, Jarkko Sakkinen wrote:
> > On Fri, Dec 05, 2025 at 12:31:27AM +0200, Jarkko Sakkinen wrote:
> > > tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and
> > > calls to
> > > tpm_buf_append_name() cause the exact same TPM2_ReadPublic
> > > command to be
> > > sent to the chip, causing unnecessary traffic.
> > >
> > > 1. Export tpm2_read_public in order to make it callable from
> > > 'trusted_tpm2'.
> > > 2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted()
> > > in order to
> > > halve the name resolutions required:
> > > 2a. Move tpm2_read_public() calls into trusted_tpm2.
> > > 2b. Pass TPM name to tpm_buf_append_name().
> > > 2c. Rework tpm_buf_append_name() to use the pre-resolved name.
> > >
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >
> > If ASN.1 blob would contain also name of the parent then zero
> > tpm2_read_public() calls would be required i.e., the main
> > bottleneck here inherits from the limitations of the file format
> > itself.
>
> Along the lines of attached patch.
Well firstly [6] is already being taken by the creation data proposal,
so this would introduce an incompatibility between the kernel and the
spec, but secondly, if you want something like this in the spec before
it goes to the IETF you really need to propose it now.
The problem with this particular addition is that it would be Linux
Kernel specific. All the current TSSs already do a cached read public
under the covers when they add the session wrappings so the user facing
API they expose has nowhere to insert (or easily extract) a name field
and thus TSS based implementations would have no incentive to either
output or consume this field. That's not to say the standard can't
have additions for crazy or niche use cases (that's what the rsaParent
flag is: a one off to support a niche SUSE use case) but it would be
hard to persuade user implementations to do this so the kernel would
have to interoperate with the case where it didn't exist anyway.
The standard use today is with permanent handles for parents, where the
parent is created on the fly, so the name is actually returned from
TPM2_CreatePrimary for this use case. It should be a simple matter to
bring the kernel implementation up to doing this as well.
Regards,
James
^ permalink raw reply
* [PATCH v4 5/5] landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-07 1:51 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251207015132.800576-1-utilityemal77@gmail.com>
Add a unit test for rule_flag collection, ensuring that access masks
are properly propagated with the flags.
No changes in v4.
changes v2..v3:
* Removing erroneously misplaced code and placed in the proper
patch.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
security/landlock/ruleset.c | 89 +++++++++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 9152a939d79a..8064139fde8f 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -22,6 +22,7 @@
#include <linux/spinlock.h>
#include <linux/workqueue.h>
#include <uapi/linux/landlock.h>
+#include <kunit/test.h>
#include "access.h"
#include "audit.h"
@@ -770,3 +771,91 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
}
return handled_accesses;
}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+/**
+ * test_unmask_layers_no_inherit - Test landlock_unmask_layers() with no_inherit
+ * @test: The KUnit test context.
+ */
+static void test_unmask_layers_no_inherit(struct kunit *const test)
+{
+ struct landlock_rule *rule;
+ layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS];
+ struct collected_rule_flags rule_flags;
+ const access_mask_t access_request = BIT_ULL(0) | BIT_ULL(1);
+ const layer_mask_t layers_initialized = BIT_ULL(0) | BIT_ULL(1);
+ size_t i;
+
+ rule = kzalloc(struct_size(rule, layers, 2), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, rule);
+
+ rule->num_layers = 2;
+
+ /* Layer 1: allows access 0, no_inherit */
+ rule->layers[0].level = 1;
+ rule->layers[0].access = BIT_ULL(0);
+ rule->layers[0].flags.no_inherit = 1;
+
+ /* Layer 2: allows access 1 */
+ rule->layers[1].level = 2;
+ rule->layers[1].access = BIT_ULL(1);
+
+ /* Case 1: No rule_flags provided (should behave normally) */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), NULL);
+
+ /* Access 0 should be unmasked by layer 1 */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ /* Case 2: rule_flags provided, no existing no_inherit_masks */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+ memset(&rule_flags, 0, sizeof(rule_flags));
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), &rule_flags);
+
+ /* Access 0 should be unmasked by layer 1 */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ /* rule_flags should collect no_inherit from layer 1 */
+ KUNIT_EXPECT_EQ(test, rule_flags.no_inherit_masks, (layer_mask_t)BIT_ULL(0));
+
+ /* Case 3: rule_flags provided, layer 1 is masked by no_inherit_masks */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+ memset(&rule_flags, 0, sizeof(rule_flags));
+ rule_flags.no_inherit_masks = BIT_ULL(0); /* Mask layer 1 */
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), &rule_flags);
+
+ /* Access 0 should NOT be unmasked by layer 1 because it is skipped */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized);
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ kfree(rule);
+}
+
+static struct kunit_case ruleset_test_cases[] = {
+ KUNIT_CASE(test_unmask_layers_no_inherit),
+ {}
+};
+
+static struct kunit_suite ruleset_test_suite = {
+ .name = "landlock_ruleset",
+ .test_cases = ruleset_test_cases,
+};
+
+kunit_test_suite(ruleset_test_suite);
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
--
2.51.0
^ permalink raw reply related
* [PATCH v4 4/5] selftests/landlock: Implement selftests for LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-07 1:51 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251207015132.800576-1-utilityemal77@gmail.com>
Implements 15 selftests for the flag, covering allowed and disallowed
operations on parent
and child directories when this flag is set, as well as multi-layer
configurations
and flag inheritance / audit logging. Also tests a bind mount
configuration.
v3..v4 changes:
* Added 4 new tests for bind mount handling, increasing selftests
from 11 -> 15.
v2..v3 changes:
* Also covers flag inheritance, audit logging and
LANDLOCK_ADD_RULE_QUIET suppression.
* Increases number of selftests from 5 -> 11.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
tools/testing/selftests/landlock/fs_test.c | 710 +++++++++++++++++++++
1 file changed, 710 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 44e131957fba..079742278969 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -1484,6 +1484,111 @@ TEST_F_FORK(layout1, inherit_superset)
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
}
+TEST_F_FORK(layout1, inherit_no_inherit_flag)
+{
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Parent directory still grants write access to its direct children. */
+ EXPECT_EQ(0, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d1, O_WRONLY));
+
+ /* dir_s1d2 gets only its explicit read-only access rights. */
+ EXPECT_EQ(0, test_open(dir_s1d2, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+
+ /* Descendants of dir_s1d2 inherit the reduced access mask. */
+ EXPECT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_nested_levels)
+{
+ int ruleset_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Level 1: s1d1 (RW + REFER + REMOVE + NO_INHERIT) */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ dir_s1d1, LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ /* Level 2: s1d2 (RO + NO_INHERIT) */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ /* Level 3: s1d3 (RW + REFER + REMOVE + NO_INHERIT) */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ dir_s1d3, LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Level 3: s1d3
+ * - RW allowed (unlink file)
+ * - REFER allowed (rename file)
+ * - REMOVE_DIR denied (parent s1d2 is part of direct parent tree)
+ */
+ ASSERT_EQ(0, unlink(file1_s1d3));
+ ASSERT_EQ(0, rename(file2_s1d3, file1_s1d3));
+ ASSERT_EQ(0, rename(file1_s1d3, file2_s1d3));
+ ASSERT_EQ(-1, rmdir(dir_s1d3));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Level 2: s1d2
+ * - RW denied (unlink file), layer is RO
+ * - REFER denied (rename file)
+ * - REMOVE_DIR of s1d2 not allowed (parent s1d1 is part of direct parent tree)
+ */
+ ASSERT_EQ(-1, unlink(file1_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(file2_s1d2, file1_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rmdir(dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Level 1: s1d1
+ * - RW allowed
+ * - Rename allowed (except for direct parent tree s1d2)
+ * - REMOVE_DIR denied (parent tmp is denied)
+ */
+ ASSERT_EQ(0, unlink(file1_s1d1));
+ ASSERT_EQ(0, rename(file2_s1d1, file1_s1d1));
+ ASSERT_EQ(0, rename(file1_s1d1, file2_s1d1));
+ ASSERT_EQ(-1, rmdir(dir_s1d1));
+ ASSERT_EQ(EACCES, errno);
+}
+
TEST_F_FORK(layout0, max_layers)
{
int i, err;
@@ -4408,6 +4513,246 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
ASSERT_EQ(0, close(cli_fd));
}
+TEST_F_FORK(layout1, inherit_no_inherit_topology_dir)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ },
+ {},
+ };
+ int ruleset_fd;
+
+ ruleset_fd = create_ruleset(_metadata,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Adds a no-inherit rule on a leaf directory. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Topology modifications of the rule path and its parents are denied.
+ */
+
+ /* Target directory s1d3 */
+ ASSERT_EQ(-1, rmdir(dir_s1d3));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d3, dir_s2d3));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Parent directory s1d2 */
+ ASSERT_EQ(-1, rmdir(dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Grandparent directory s1d1 */
+ ASSERT_EQ(-1, rmdir(dir_s1d1));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d1, dir_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Sibling operations are allowed.
+ */
+ /* Sibling of s1d3 */
+ ASSERT_EQ(0, unlink(file1_s1d2));
+ /* Sibling of s1d2 */
+ ASSERT_EQ(0, unlink(file1_s1d1));
+
+ /*
+ * Content of the no-inherit directory is restricted by the rule (RO).
+ */
+ ASSERT_EQ(-1, unlink(file1_s1d3));
+ ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F_FORK(layout1, no_inherit_allow_inner_removal)
+{
+ int ruleset_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Content of the no-inherit directory is mutable (RW).
+ * This checks that the no-inherit flag does not seal the content.
+ */
+ ASSERT_EQ(0, unlink(file1_s1d2));
+
+ /*
+ * Topology modifications of the rule path are denied.
+ */
+ ASSERT_EQ(-1, rmdir(dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d2));
+ ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_topology_unrelated)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW,
+ },
+ {},
+ };
+ static const char unrelated_dir[] = TMP_DIR "/s2d1/unrelated";
+ static const char unrelated_file[] = TMP_DIR "/s2d1/unrelated/f1";
+ int ruleset_fd;
+
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Adds a no-inherit rule on a leaf directory unrelated to s2. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Ensure we can still create and delete files outside the sealed branch. */
+ ASSERT_EQ(0, mkdir(unrelated_dir, 0700));
+ ASSERT_EQ(0, mknod(unrelated_file, S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(unrelated_file));
+ ASSERT_EQ(0, rmdir(unrelated_dir));
+
+ /* Existing siblings in s2 remain modifiable. */
+ ASSERT_EQ(0, unlink(file1_s2d1));
+ ASSERT_EQ(0, mknod(file1_s2d1, S_IFREG | 0700, 0));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_descendant_rw)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RO,
+ },
+ {},
+ };
+ const __u64 handled_access = ACCESS_RW | LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE;
+ static const char child_file[] =
+ TMP_DIR "/s1d1/s1d2/s1d3/rw_descendant";
+ int ruleset_fd;
+
+ ruleset_fd = create_ruleset(_metadata, handled_access, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ dir_s1d3, 0);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ ASSERT_EQ(0, mknod(child_file, S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(child_file));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_topology_file)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW,
+ },
+ {},
+ };
+ int ruleset_fd;
+ struct landlock_path_beneath_attr path_beneath = {
+ .allowed_access = ACCESS_RO,
+ };
+
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ path_beneath.parent_fd = open(file1_s1d2, O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, path_beneath.parent_fd);
+ ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &path_beneath,
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ ASSERT_EQ(EINVAL, errno);
+ ASSERT_EQ(0, close(path_beneath.parent_fd));
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_layered)
+{
+ const struct rule layer1_and_2[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ },
+ {},
+ };
+ int ruleset_fd;
+ static const char unrelated_dir[] = TMP_DIR "/s2d1/unrelated";
+ static const char unrelated_file[] = TMP_DIR "/s2d1/unrelated/f1";
+
+ /* Layer 1: RW on TMP_DIR */
+ ruleset_fd = create_ruleset(_metadata,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ layer1_and_2);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Layer 2: Add no-inherit RO rule on s1d2 */
+ ruleset_fd = create_ruleset(_metadata,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ layer1_and_2);
+ ASSERT_LE(0, ruleset_fd);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Operations in unrelated areas should still work */
+ ASSERT_EQ(0, mkdir(unrelated_dir, 0700));
+ ASSERT_EQ(0, mknod(unrelated_file, S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(unrelated_file));
+ ASSERT_EQ(0, rmdir(unrelated_dir));
+
+ /* Creating in s1d1 should be allowed (parent still has RW) */
+ ASSERT_EQ(0, mknod(TMP_DIR "/s1d1/newfile", S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(TMP_DIR "/s1d1/newfile"));
+
+ /* Content of s1d2 should be read-only */
+ ASSERT_EQ(-1, unlink(file1_s1d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Topology changes to s1d2 should be denied */
+ ASSERT_EQ(-1, rename(dir_s1d2, TMP_DIR "/s2d1/renamed"));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Renaming s1d1 should also be denied (it's an ancestor) */
+ ASSERT_EQ(-1, rename(dir_s1d1, TMP_DIR "/s2d1/renamed"));
+ ASSERT_EQ(EACCES, errno);
+}
+
/* clang-format off */
FIXTURE(ioctl) {};
@@ -5747,6 +6092,277 @@ TEST_F_FORK(layout4_disconnected_leafs, read_rename_exchange)
test_renameat(s1d42_bind_fd, "f4", s1d42_bind_fd, "f5"));
}
+/*
+ * Test that LANDLOCK_ADD_RULE_NO_INHERIT on a directory accessed via a mount
+ * point protects the parent hierarchy within the mount from topology changes.
+ *
+ * Layout (after bind mount s1d2 -> s2d2):
+ * tmp
+ * ├── s1d1
+ * │ └── s1d2 [source of bind mount]
+ * │ ├── s1d31
+ * │ │ └── s1d41
+ * │ │ ├── f1
+ * │ │ └── f2
+ * │ └── s1d32
+ * │ └── s1d42
+ * │ ├── f3
+ * │ └── f4
+ * └── s2d1
+ * └── s2d2 [bind mount destination from s1d2]
+ * ├── s1d31 <- parent of protected dir, should be immovable
+ * │ └── s1d41 <- protected with NO_INHERIT
+ * │ ├── f1
+ * │ └── f2
+ * └── s1d32
+ * └── s1d42
+ * ├── f3
+ * └── f4
+ *
+ * When s1d41 (accessed via the mount at s2d2) is protected with NO_INHERIT,
+ * its parent directories within the mount (s1d31) should be immovable.
+ */
+TEST_F_FORK(layout4_disconnected_leafs, no_inherit_mount_parent_rename)
+{
+ int ruleset_fd, s1d41_bind_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Allow full access to TMP_DIR. */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ TMP_DIR, 0);
+
+ /*
+ * Access s1d41 through the bind mount at s2d2 and protect it with
+ * NO_INHERIT. This should seal the parent hierarchy through the mount.
+ */
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = s1d41_bind_fd,
+ .allowed_access = ACCESS_RO,
+ },
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ EXPECT_EQ(0, close(s1d41_bind_fd));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * s1d31 is the parent of s1d41 within the mount. Renaming it should
+ * be denied because it is part of the protected parent hierarchy.
+ * Test via the mount path.
+ */
+ ASSERT_EQ(-1, rename(TMP_DIR "/s2d1/s2d2/s1d31",
+ TMP_DIR "/s2d1/s2d2/s1d31_renamed"));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * s1d32 is a sibling directory (not in the protected parent chain),
+ * so renaming it should be allowed.
+ */
+ ASSERT_EQ(0, rename(TMP_DIR "/s2d1/s2d2/s1d32",
+ TMP_DIR "/s2d1/s2d2/s1d32_renamed"));
+ ASSERT_EQ(0, rename(TMP_DIR "/s2d1/s2d2/s1d32_renamed",
+ TMP_DIR "/s2d1/s2d2/s1d32"));
+
+ /*
+ * Renaming directories not in the protected parent hierarchy should
+ * still be allowed.
+ */
+ ASSERT_EQ(0, rename(TMP_DIR "/s3d1", TMP_DIR "/s3d1_renamed"));
+ ASSERT_EQ(0, rename(TMP_DIR "/s3d1_renamed", TMP_DIR "/s3d1"));
+}
+
+TEST_F_FORK(layout4_disconnected_leafs, no_inherit_mount_parent_rmdir)
+{
+ int ruleset_fd, s1d41_bind_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Allow full access to TMP_DIR. */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ TMP_DIR, 0);
+
+ /*
+ * Access s1d41 through the bind mount at s2d2 and protect it with
+ * NO_INHERIT. This should seal the parent hierarchy through the mount.
+ */
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = s1d41_bind_fd,
+ .allowed_access = ACCESS_RO,
+ },
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ EXPECT_EQ(0, close(s1d41_bind_fd));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * s1d31 is the parent of s1d41 within the mount. Removing it should
+ * be denied because it is part of the protected parent hierarchy.
+ */
+ ASSERT_EQ(-1, rmdir(TMP_DIR "/s2d1/s2d2/s1d31"));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Removing an unrelated directory should still be allowed (if empty).
+ */
+ ASSERT_EQ(0, rmdir(TMP_DIR "/s3d1"));
+ ASSERT_EQ(0, mkdir(TMP_DIR "/s3d1", 0755));
+}
+
+TEST_F_FORK(layout4_disconnected_leafs, no_inherit_mount_parent_link)
+{
+ int ruleset_fd, s1d41_bind_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Allow full access to TMP_DIR. */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ TMP_DIR, 0);
+
+ /*
+ * Access s1d41 through the bind mount at s2d2 and protect it with
+ * NO_INHERIT. This should seal the parent hierarchy through the mount.
+ */
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = s1d41_bind_fd,
+ .allowed_access = ACCESS_RO,
+ },
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ EXPECT_EQ(0, close(s1d41_bind_fd));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Creating a hard link within the protected NO_INHERIT directory should
+ * be denied because NO_INHERIT grants only ACCESS_RO (no MAKE_REG).
+ */
+ ASSERT_EQ(-1, linkat(AT_FDCWD, TMP_DIR "/s2d1/s2d2/s1d31/s1d41/f1",
+ AT_FDCWD, TMP_DIR "/s2d1/s2d2/s1d31/s1d41/f1_link",
+ 0));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Creating links within directories outside the protected chain
+ * (using the mount source path to avoid EXDEV) should still be allowed.
+ */
+ ASSERT_EQ(0, linkat(AT_FDCWD, TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3",
+ AT_FDCWD, TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3_link",
+ 0));
+ ASSERT_EQ(0, unlink(TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3_link"));
+}
+
+/*
+ * Test that NO_INHERIT protection extends to the mount source hierarchy.
+ * If a directory is protected via a mount path, its parents within the
+ * mount source should also be protected from topology changes.
+ */
+TEST_F_FORK(layout4_disconnected_leafs, no_inherit_source_parent_rename)
+{
+ int ruleset_fd, s1d41_bind_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Allow full access to TMP_DIR. */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ TMP_DIR, 0);
+
+ /*
+ * Access s1d41 through the bind mount at s2d2 and protect it with
+ * NO_INHERIT. The source mount path parents should also be protected.
+ */
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = s1d41_bind_fd,
+ .allowed_access = ACCESS_RO,
+ },
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ EXPECT_EQ(0, close(s1d41_bind_fd));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * The mount source is s1d1/s1d2. The protected directory s1d41 is at
+ * s1d1/s1d2/s1d31/s1d41. The parent s1d31 within the mount source
+ * should be protected from topology changes.
+ */
+ ASSERT_EQ(-1, rename(TMP_DIR "/s1d1/s1d2/s1d31",
+ TMP_DIR "/s1d1/s1d2/s1d31_renamed"));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * s1d32 is a sibling, not in the protected parent chain. It should
+ * be renamable.
+ */
+ ASSERT_EQ(0, rename(TMP_DIR "/s1d1/s1d2/s1d32",
+ TMP_DIR "/s1d1/s1d2/s1d32_renamed"));
+ ASSERT_EQ(0, rename(TMP_DIR "/s1d1/s1d2/s1d32_renamed",
+ TMP_DIR "/s1d1/s1d2/s1d32"));
+}
+
/*
* layout5_disconnected_branch before rename:
*
@@ -7231,6 +7847,100 @@ TEST_F(audit_layout1, write_file)
EXPECT_EQ(1, records.domain);
}
+TEST_F(audit_layout1, no_inherit_parent_is_logged)
+{
+ struct audit_records records;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Base read-only rule at s1d1. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1, 0);
+ /* Descendant s1d1/s1d2/s1d3 forbids inheritance but should still log. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+
+ EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+ EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.write_file", file1_s1d2));
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+ EXPECT_EQ(1, records.domain);
+
+ EXPECT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(audit_layout1, no_inherit_blocks_quiet_flag_inheritence)
+{
+ struct audit_records records;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ .quiet_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Base read-only rule at tmp/s1d1 with quiet flag. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1,
+ LANDLOCK_ADD_RULE_QUIET);
+ /* Descendant tmp/s1d1/s1d2/s1d3 forbids inheritance of quiet flag and should still log. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+
+ EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+ EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.write_file", file1_s1d3));
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+ EXPECT_EQ(1, records.domain);
+
+ EXPECT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(audit_layout1, no_inherit_quiet_parent)
+{
+ struct audit_records records;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ .quiet_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Base read-only rule at tmp/s1d1 with quiet flag. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1,
+ LANDLOCK_ADD_RULE_QUIET);
+ /* Access to dir_s1d1 shouldn't log */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+
+ EXPECT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
+ EXPECT_NE(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.write_file", file1_s1d1));
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+ EXPECT_EQ(0, records.domain);
+
+ EXPECT_EQ(0, close(ruleset_fd));
+}
+
TEST_F(audit_layout1, read_file)
{
struct audit_records records;
--
2.51.0
^ permalink raw reply related
* [PATCH v4 3/5] samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to landlock-sandboxer
From: Justin Suess @ 2025-12-07 1:51 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251207015132.800576-1-utilityemal77@gmail.com>
Adds support to landlock-sandboxer with environment variable
LL_FS_NO_INHERIT, which can be tagged on any filesystem object to
suppress access right inheritance.
v3..v4 changes:
* Modified LL_FS_R(O/W)_NO_INHERIT variables to a single variable
to allow access rule combination. (credit to Tingmao Wang)
v2..v3 changes:
* Minor formatting fixes
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
samples/landlock/sandboxer.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 07dc0013ff19..852ffa413c75 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -60,6 +60,7 @@ static inline int landlock_restrict_self(const int ruleset_fd,
#define ENV_FS_RW_NAME "LL_FS_RW"
#define ENV_FS_QUIET_NAME "LL_FS_QUIET"
#define ENV_FS_QUIET_ACCESS_NAME "LL_FS_QUIET_ACCESS"
+#define ENV_FS_NO_INHERIT_NAME "LL_FS_NO_INHERIT"
#define ENV_TCP_BIND_NAME "LL_TCP_BIND"
#define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
#define ENV_NET_QUIET_NAME "LL_NET_QUIET"
@@ -383,6 +384,7 @@ static const char help[] =
"but to test audit we can set " ENV_FORCE_LOG_NAME "=1\n"
ENV_FS_QUIET_NAME " and " ENV_NET_QUIET_NAME ", both optional, can then be used "
"to make access to some denied paths or network ports not trigger audit logging.\n"
+ ENV_FS_NO_INHERIT_NAME " can be used to suppress access right propagation (ABI >= 8).\n"
ENV_FS_QUIET_ACCESS_NAME " and " ENV_NET_QUIET_ACCESS_NAME " can be used to specify "
"which accesses should be quieted (defaults to all):\n"
"* " ENV_FS_QUIET_ACCESS_NAME ": file system accesses to quiet\n"
@@ -430,6 +432,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
};
bool quiet_supported = true;
+ bool no_inherit_supported = true;
int supported_restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
int set_restrict_flags = 0;
@@ -517,8 +520,9 @@ int main(const int argc, char *const argv[], char *const *const envp)
LANDLOCK_ABI_LAST, abi);
__attribute__((fallthrough));
case 7:
- /* Don't add quiet flags for ABI < 8 later on. */
+ /* Don't add quiet/no_inherit flags for ABI < 8 later on. */
quiet_supported = false;
+ no_inherit_supported = false;
__attribute__((fallthrough));
case LANDLOCK_ABI_LAST:
@@ -605,6 +609,13 @@ int main(const int argc, char *const argv[], char *const *const envp)
goto err_close_ruleset;
}
+ /* Don't require this env to be present. */
+ if (no_inherit_supported && getenv(ENV_FS_NO_INHERIT_NAME)) {
+ if (populate_ruleset_fs(ENV_FS_NO_INHERIT_NAME, ruleset_fd, 0,
+ LANDLOCK_ADD_RULE_NO_INHERIT))
+ goto err_close_ruleset;
+ }
+
if (populate_ruleset_net(ENV_TCP_BIND_NAME, ruleset_fd,
LANDLOCK_ACCESS_NET_BIND_TCP, 0)) {
goto err_close_ruleset;
--
2.51.0
^ permalink raw reply related
* [PATCH v4 2/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
From: Justin Suess @ 2025-12-07 1:51 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251207015132.800576-1-utilityemal77@gmail.com>
Implements the syscall side flag handling and kernel api headers for the
LANDLOCK_ADD_RULE_NO_INHERIT flag.
v3..v4 changes:
* Changed documentation to reflect protections now apply to VFS root
instead of the mountpoint.
v2..v3 changes:
* Extended documentation for flag inheritance suppression on
LANDLOCK_ADD_RULE_NO_INHERIT.
* Extended the flag validation rules in the syscall.
* Added mention of no inherit in empty rules in add_rule_path_beneath
as per Tingmao Wang's suggestion.
* Added check for useless no-inherit flag in networking rules.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
include/uapi/linux/landlock.h | 29 +++++++++++++++++++++++++++++
security/landlock/syscalls.c | 16 ++++++++++++----
2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index d4f47d20361a..6ab3e7bd1c81 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -127,10 +127,39 @@ struct landlock_ruleset_attr {
* allowed_access in the passed in rule_attr. When this flag is
* present, the caller is also allowed to pass in an empty
* allowed_access.
+ * %LANDLOCK_ADD_RULE_NO_INHERIT
+ * When set on a rule being added to a ruleset, this flag disables the
+ * inheritance of access rights and flags from parent objects.
+ *
+ * This flag currently applies only to filesystem rules. Adding it to
+ * non-filesystem rules will return -EINVAL, unless future extensions
+ * of Landlock define other hierarchical object types.
+ *
+ * By default, Landlock filesystem rules inherit allowed accesses from
+ * ancestor directories: if a parent directory grants certain rights,
+ * those rights also apply to its children. A rule marked with
+ * LANDLOCK_ADD_RULE_NO_INHERIT stops this propagation at the directory
+ * covered by the rule. Descendants of that directory continue to inherit
+ * normally unless they also have rules using this flag.
+ *
+ * If a regular file is marked with this flag, it will not inherit any
+ * access rights from its parent directories; only the accesses explicitly
+ * allowed by the rule will apply to that file.
+ *
+ * This flag also enforces parent-directory restrictions: rename, rmdir,
+ * link, and other operations that would change the directory's immediate
+ * parent subtree are denied up to the VFS root. This prevents
+ * sandboxed processes from manipulating the filesystem hierarchy to evade
+ * restrictions (e.g., via sandbox-restart attacks).
+ *
+ * In addition, this flag blocks the inheritance of rule-layer flags
+ * (such as the quiet flag) from parent directories to the object covered
+ * by this rule.
*/
/* clang-format off */
#define LANDLOCK_ADD_RULE_QUIET (1U << 0)
+#define LANDLOCK_ADD_RULE_NO_INHERIT (1U << 1)
/* clang-format on */
/**
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 5cf1183bb596..0c815e241c75 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -352,7 +352,7 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
/*
* Informs about useless rule: empty allowed_access (i.e. deny rules)
* are ignored in path walks. However, the rule is not useless if it
- * is there to hold a quiet flag
+ * is there to hold a quiet or no inherit flag.
*/
if (!flags && !path_beneath_attr.allowed_access)
return -ENOMSG;
@@ -407,6 +407,10 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.net)
return -EINVAL;
+ /* No inherit is always useless for this scope */
+ if (flags & LANDLOCK_ADD_RULE_NO_INHERIT)
+ return -EINVAL;
+
/* Denies inserting a rule with port greater than 65535. */
if (net_port_attr.port > U16_MAX)
return -EINVAL;
@@ -424,7 +428,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
* @rule_type: Identify the structure type pointed to by @rule_attr:
* %LANDLOCK_RULE_PATH_BENEATH or %LANDLOCK_RULE_NET_PORT.
* @rule_attr: Pointer to a rule (matching the @rule_type).
- * @flags: Must be 0 or %LANDLOCK_ADD_RULE_QUIET.
+ * @flags: Must be 0 or %LANDLOCK_ADD_RULE_QUIET and/or %LANDLOCK_ADD_RULE_NO_INHERIT.
*
* This system call enables to define a new rule and add it to an existing
* ruleset.
@@ -462,8 +466,12 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
if (!is_initialized())
return -EOPNOTSUPP;
-
- if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
+ /* Checks flag existence */
+ if (flags && flags & ~(LANDLOCK_ADD_RULE_QUIET | LANDLOCK_ADD_RULE_NO_INHERIT))
+ return -EINVAL;
+ /* No inherit may only apply on path_beneath rules. */
+ if ((flags & LANDLOCK_ADD_RULE_NO_INHERIT) &&
+ rule_type != LANDLOCK_RULE_PATH_BENEATH)
return -EINVAL;
/* Gets and checks the ruleset. */
--
2.51.0
^ permalink raw reply related
* [PATCH v4 1/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-07 1:51 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251207015132.800576-1-utilityemal77@gmail.com>
Implements a flag to prevent access grant inheritance within the filesystem
hierarchy for landlock rules.
If a landlock rule on an inode has this flag, any access grants on parent
inodes will be ignored. Moreover, operations that involve altering the
ancestors of the subject with LANDLOCK_ADD_RULE_NO_INHERIT will be
denied up to the VFS root (new in v4).
For example, if /a/b/c/ = read only + LANDLOCK_ADD_RULE_NO_INHERIT and
/ = read write, writes to files in /a/b/c will be denied. Moreover,
moving /a to /bad, removing /a/b/c, or creating links to /a will be
prohibited.
Parent flag inheritance is automatically suppressed by the permission
harvesting logic, which will finish processing early if all relevant
layers are tagged with NO_INHERIT.
And if / has LANDLOCK_ADD_RULE_QUIET, /a/b/c will still audit (handled)
accesses. This is because LANDLOCK_ADD_RULE_NO_INHERIT also
suppresses flag inheritance from parent objects.
The parent directory restrictions mitigate sandbox-restart attacks. For
example, if a sandboxed program is able to move a
LANDLOCK_ADD_RULE_NO_INHERIT restricted directory, upon sandbox restart,
the policy applied naively on the same filenames would be invalid.
Preventing these operations mitigates these attacks.
v3..v4 changes:
* Rebased on v6 of Tingmao Wang's "quiet flag" series.
* Removed unnecessary mask_no_inherit_descendant_layers and related
code at Tingmao Wang's suggestion, simplifying patch.
* Updated to use new disconnected directory handling.
* Improved WARN_ON_ONCE usage. (Thanks Tingmao Wang!)
* Removed redundant loop for single-layer rulesets (again thanks Tingmao
Wang!)
* Protections now apply up to the VFS root, not just the mountpoint.
* Indentation fixes.
* Removed redundant flag marker blocked_flag_masks.
v2..v3 changes:
* Parent directory topology protections now work by lazily
inserting blank rules on parent inodes if they do not
exist. This replaces the previous xarray implementation
with simplified logic.
* Added an optimization to skip further processing if all layers
collected have no inherit.
* Added support to block flag inheritance.
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
security/landlock/fs.c | 389 +++++++++++++++++++++++++++++++++++-
security/landlock/ruleset.c | 19 +-
security/landlock/ruleset.h | 29 ++-
3 files changed, 433 insertions(+), 4 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 0b589263ea42..7b0b77859778 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -317,6 +317,207 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
LANDLOCK_ACCESS_FS_IOCTL_DEV)
/* clang-format on */
+static const struct landlock_rule *find_rule(const struct landlock_ruleset *const domain,
+ const struct dentry *const dentry);
+
+/**
+ * landlock_domain_layers_mask - Build a mask covering all layers of a domain
+ * @domain: The ruleset (domain) to inspect.
+ *
+ * Return a layer mask with a 1 bit for each existing layer of @domain.
+ * If @domain has no layers 0 is returned. If the number of layers is
+ * greater than or equal to the number of bits in layer_mask_t, all bits
+ * are set.
+ */
+static layer_mask_t landlock_domain_layers_mask(const struct landlock_ruleset
+ *const domain)
+{
+ if (!domain || !domain->num_layers)
+ return 0;
+
+ if (domain->num_layers >= sizeof(layer_mask_t) * BITS_PER_BYTE)
+ return (layer_mask_t)~0ULL;
+
+ return GENMASK_ULL(domain->num_layers - 1, 0);
+}
+
+/**
+ * rule_blocks_all_layers_no_inherit - check whether a rule disables inheritance
+ * @domain_layers_mask: Mask describing the domain's active layers.
+ * @rule: Rule to inspect.
+ *
+ * Return true if every layer present in @rule has its no_inherit flag set
+ * and the set of layers covered by the rule equals @domain_layers_mask.
+ * This indicates that the rule prevents inheritance on all layers of the
+ * domain and thus further walking for inheritance checks can stop.
+ */
+static bool rule_blocks_all_layers_no_inherit(const layer_mask_t domain_layers_mask,
+ const struct landlock_rule *const rule)
+{
+ layer_mask_t rule_layers = 0;
+ u32 layer_index;
+
+ if (!domain_layers_mask || !rule)
+ return false;
+
+ for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
+ const struct landlock_layer *const layer =
+ &rule->layers[layer_index];
+ const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
+
+ if (!layer->flags.no_inherit)
+ return false;
+
+ rule_layers |= layer_bit;
+ }
+
+ return rule_layers && rule_layers == domain_layers_mask;
+}
+
+/**
+ * ensure_rule_for_dentry - ensure a ruleset contains a rule entry for dentry,
+ * inserting a blank rule if needed.
+ * @ruleset: Ruleset to modify/inspect. Caller must hold @ruleset->lock.
+ * @dentry: Dentry to ensure a rule exists for.
+ *
+ * If no rule is currently associated with @dentry, insert an empty rule
+ * (with zero access) tied to the backing inode. Returns a pointer to the
+ * rule associated with @dentry on success, NULL when @dentry is negative, or
+ * an ERR_PTR()-encoded error if the rule cannot be created.
+ *
+ * This is useful for LANDLOCK_ADD_RULE_NO_INHERIT processing, where a rule
+ * may need to be created for an ancestor dentry that does not yet have one
+ * to properly track no_inherit flags.
+ *
+ * The flags are set to zero if a rule is newly created, and the caller
+ * is responsible for setting them appropriately.
+ *
+ * The returned rule pointer's lifetime is tied to @ruleset.
+ */
+static const struct landlock_rule *
+ensure_rule_for_dentry(struct landlock_ruleset *const ruleset,
+ struct dentry *const dentry)
+{
+ struct landlock_id id = {
+ .type = LANDLOCK_KEY_INODE,
+ };
+ const struct landlock_rule *rule;
+ int err;
+
+ if (WARN_ON_ONCE(!ruleset || !dentry || d_is_negative(dentry)))
+ return NULL;
+
+ lockdep_assert_held(&ruleset->lock);
+
+ rule = find_rule(ruleset, dentry);
+ if (rule)
+ return rule;
+
+ id.key.object = get_inode_object(d_backing_inode(dentry));
+ if (IS_ERR(id.key.object))
+ return ERR_CAST(id.key.object);
+
+ err = landlock_insert_rule(ruleset, id, 0, 0);
+ landlock_put_object(id.key.object);
+ if (err)
+ return ERR_PTR(err);
+
+ rule = find_rule(ruleset, dentry);
+ if (WARN_ON_ONCE(!rule))
+ return ERR_PTR(-ENOENT);
+ return rule;
+}
+
+/**
+ * mark_no_inherit_ancestors - mark ancestors as having no_inherit descendants
+ * @ruleset: Ruleset to modify. Caller must hold @ruleset->lock.
+ * @path: Path representing the descendant that carries no_inherit bits.
+ * @descendant_layers: Mask of layers from the descendant that should be
+ * advertised to ancestors via has_no_inherit_descendant.
+ *
+ * Walks upward from @dentry and ensures that any ancestor rule contains the
+ * has_no_inherit_descendant marker for the specified @descendant_layers so
+ * parent lookups can quickly detect descendant no_inherit influence.
+ *
+ * Returns 0 on success or a negative errno if ancestor bookkeeping fails.
+ */
+static int mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
+ const struct path *const path,
+ layer_mask_t descendant_layers)
+{
+ struct dentry *cursor;
+ struct path walk_path;
+ int err = 0;
+
+ if (WARN_ON_ONCE(!ruleset || !path || !path->dentry || !path->mnt ||
+ !descendant_layers))
+ return -EINVAL;
+
+ lockdep_assert_held(&ruleset->lock);
+
+ walk_path.mnt = path->mnt;
+ walk_path.dentry = path->dentry;
+ path_get(&walk_path);
+
+ cursor = dget(walk_path.dentry);
+ while (cursor) {
+ struct dentry *parent;
+ const struct landlock_rule *rule;
+
+ /* Follow mounts all the way up to the root. */
+ if (IS_ROOT(cursor)) {
+ dput(cursor);
+ if (!follow_up(&walk_path)) {
+ cursor = NULL;
+ continue;
+ }
+ cursor = dget(walk_path.dentry);
+ }
+
+ parent = dget_parent(cursor);
+ dput(cursor);
+ if (!parent)
+ break;
+
+ if (WARN_ON_ONCE(d_is_negative(parent))) {
+ dput(parent);
+ break;
+ }
+ /*
+ * Ensures a rule exists for the parent dentry,
+ * inserting a blank one if needed.
+ */
+ rule = ensure_rule_for_dentry(ruleset, parent);
+ if (IS_ERR(rule)) {
+ err = PTR_ERR(rule);
+ dput(parent);
+ cursor = NULL;
+ break;
+ }
+ if (rule) {
+ struct landlock_rule *mutable_rule =
+ (struct landlock_rule *)rule;
+ /*
+ * Unmerged rulesets should only have one layer.
+ */
+ if (WARN_ON_ONCE(mutable_rule->num_layers != 1)) {
+ dput(parent);
+ err = -EINVAL;
+ cursor = NULL;
+ break;
+ }
+
+ if (descendant_layers & BIT_ULL(0))
+ mutable_rule->layers[0]
+ .flags.has_no_inherit_descendant = true;
+ }
+
+ cursor = parent;
+ }
+ path_put(&walk_path);
+ return err;
+}
+
/*
* @path: Should have been checked by get_path_from_fd().
*/
@@ -344,13 +545,40 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
return PTR_ERR(id.key.object);
mutex_lock(&ruleset->lock);
err = landlock_insert_rule(ruleset, id, access_rights, flags);
+ if (!err && (flags & LANDLOCK_ADD_RULE_NO_INHERIT)) {
+ const struct landlock_rule *rule;
+ layer_mask_t descendant_layers = 0;
+
+ rule = find_rule(ruleset, path->dentry);
+ /*
+ * This was already checked at function entry.
+ */
+ if (WARN_ON_ONCE(!rule || rule->num_layers != 1))
+ goto out_unlock;
+
+ if (rule->layers[0].flags.no_inherit ||
+ rule->layers[0].flags.has_no_inherit_descendant)
+ descendant_layers = BIT_ULL(0);
+
+ if (descendant_layers) {
+ err = mark_no_inherit_ancestors(ruleset, path,
+ descendant_layers);
+ if (err)
+ goto out_unlock;
+ }
+ }
mutex_unlock(&ruleset->lock);
+out:
/*
* No need to check for an error because landlock_insert_rule()
* increments the refcount for the new object if needed.
*/
landlock_put_object(id.key.object);
return err;
+
+out_unlock:
+ mutex_unlock(&ruleset->lock);
+ goto out;
}
/* Access-control management */
@@ -764,6 +992,8 @@ static bool is_access_to_paths_allowed(
struct landlock_request *const log_request_parent2,
struct dentry *const dentry_child2)
{
+ const layer_mask_t domain_layers_mask =
+ landlock_domain_layers_mask(domain);
bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
child1_is_directory = true, child2_is_directory = true;
struct path walker_path;
@@ -906,6 +1136,10 @@ static bool is_access_to_paths_allowed(
ARRAY_SIZE(*layer_masks_parent2),
rule_flags_parent2);
+ if (rule &&
+ rule_blocks_all_layers_no_inherit(domain_layers_mask, rule))
+ break;
+
/* Stops when a rule from each layer grants access. */
if (allowed_parent1 && allowed_parent2)
break;
@@ -1064,7 +1298,9 @@ static bool collect_domain_accesses(
layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS],
struct collected_rule_flags *const rule_flags)
{
- unsigned long access_dom;
+ access_mask_t access_dom;
+ const layer_mask_t domain_layers_mask =
+ landlock_domain_layers_mask(domain);
bool ret = false;
if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
@@ -1080,9 +1316,11 @@ static bool collect_domain_accesses(
while (true) {
struct dentry *parent_dentry;
+ const struct landlock_rule *rule = find_rule(domain, dir);
+
/* Gets all layers allowing all domain accesses. */
if (landlock_unmask_layers(
- find_rule(domain, dir), access_dom, layer_masks_dom,
+ rule, access_dom, layer_masks_dom,
ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
/*
* Stops when all handled accesses are allowed by at
@@ -1092,6 +1330,10 @@ static bool collect_domain_accesses(
break;
}
+ if (rule &&
+ rule_blocks_all_layers_no_inherit(domain_layers_mask, rule))
+ break;
+
/*
* Stops at the mount point or the filesystem root for a disconnected
* directory.
@@ -1107,6 +1349,120 @@ static bool collect_domain_accesses(
return ret;
}
+/**
+ * collect_topology_sealed_layers - collect layers sealed against topology changes
+ * @domain: Ruleset to consult.
+ * @dentry: Starting dentry for the upward walk.
+ * @override_layers: Optional out parameter filled with layers that are
+ * present on ancestors but considered overrides (not
+ * sealing the topology for descendants).
+ *
+ * Walk upwards from @dentry and return a mask of layers where either the
+ * visited dentry contains a no_inherit rule or ancestors were previously
+ * marked as having a descendant with no_inherit. @override_layers, if not
+ * NULL, is filled with layers that would normally be overridden by more
+ * specific descendant rules.
+ *
+ * Returns a layer mask where set bits indicate layers that are "sealed"
+ * (topology changes like rename/rmdir are denied) for the subtree rooted at
+ * @dentry.
+ *
+ * Useful for LANDLOCK_ADD_RULE_NO_INHERIT parent directory enforcement to ensure
+ * that topology changes do not violate the no_inherit constraints.
+ */
+static layer_mask_t
+collect_topology_sealed_layers(const struct landlock_ruleset *const domain,
+ struct dentry *dentry,
+ layer_mask_t *const override_layers)
+{
+ struct dentry *cursor, *parent;
+ bool include_descendants = true;
+ layer_mask_t sealed_layers = 0;
+
+ if (override_layers)
+ *override_layers = 0;
+
+ if (WARN_ON_ONCE(!domain || !dentry || d_is_negative(dentry)))
+ return 0;
+
+ cursor = dget(dentry);
+ while (cursor) {
+ const struct landlock_rule *rule;
+ u32 layer_index;
+
+ rule = find_rule(domain, cursor);
+ if (rule) {
+ for (layer_index = 0; layer_index < rule->num_layers;
+ layer_index++) {
+ const struct landlock_layer *layer =
+ &rule->layers[layer_index];
+ layer_mask_t layer_bit =
+ BIT_ULL(layer->level - 1);
+
+ if (include_descendants &&
+ (layer->flags.no_inherit ||
+ layer->flags.has_no_inherit_descendant)) {
+ sealed_layers |= layer_bit;
+ } else if (override_layers) {
+ *override_layers |= layer_bit;
+ }
+ }
+ }
+
+ if (sealed_layers || IS_ROOT(cursor))
+ break;
+
+ parent = dget_parent(cursor);
+ dput(cursor);
+ if (!parent)
+ return sealed_layers;
+
+ cursor = parent;
+ include_descendants = false;
+ }
+ dput(cursor);
+ return sealed_layers;
+}
+
+/**
+ * deny_no_inherit_topology_change - deny topology changes on sealed layers
+ * @subject: Subject performing the operation (contains the domain).
+ * @dentry: Dentry that is the target of the topology modification.
+ *
+ * Checks whether any domain layers are sealed against topology changes at
+ * @dentry (via collect_topology_sealed_layers). If so, emit an audit record
+ * and return -EACCES. Otherwise return 0.
+ */
+static int deny_no_inherit_topology_change(const struct landlock_cred_security
+ *subject,
+ struct dentry *dentry)
+{
+ layer_mask_t sealed_layers;
+ layer_mask_t override_layers;
+ unsigned long layer_index;
+
+ if (WARN_ON_ONCE(!subject || !dentry || d_is_negative(dentry)))
+ return 0;
+ sealed_layers = collect_topology_sealed_layers(subject->domain,
+ dentry, &override_layers);
+ sealed_layers &= ~override_layers;
+
+ if (!sealed_layers)
+ return 0;
+
+ layer_index = __ffs((unsigned long)sealed_layers);
+ landlock_log_denial(subject, &(struct landlock_request) {
+ .type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
+ .audit = {
+ .type = LSM_AUDIT_DATA_DENTRY,
+ .u.dentry = dentry,
+ },
+ .layer_plus_one = layer_index + 1,
+ });
+
+ return -EACCES;
+}
+
/**
* current_check_refer_path - Check if a rename or link action is allowed
*
@@ -1191,6 +1547,16 @@ static int current_check_refer_path(struct dentry *const old_dentry,
access_request_parent2 =
get_mode_access(d_backing_inode(old_dentry)->i_mode);
if (removable) {
+ int err;
+
+ err = deny_no_inherit_topology_change(subject, old_dentry);
+ if (err)
+ return err;
+ if (exchange) {
+ err = deny_no_inherit_topology_change(subject, new_dentry);
+ if (err)
+ return err;
+ }
access_request_parent1 |= maybe_remove(old_dentry);
access_request_parent2 |= maybe_remove(new_dentry);
}
@@ -1583,12 +1949,31 @@ static int hook_path_symlink(const struct path *const dir,
static int hook_path_unlink(const struct path *const dir,
struct dentry *const dentry)
{
+ const struct landlock_cred_security *const subject =
+ landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+ int err;
+
+ if (subject) {
+ err = deny_no_inherit_topology_change(subject, dentry);
+ if (err)
+ return err;
+ }
return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FILE);
}
static int hook_path_rmdir(const struct path *const dir,
struct dentry *const dentry)
{
+ const struct landlock_cred_security *const subject =
+ landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+ int err;
+
+ if (subject) {
+ err = deny_no_inherit_topology_change(subject, dentry);
+ if (err)
+ return err;
+ }
+
return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
}
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 750a444e1983..9152a939d79a 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -255,8 +255,13 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
return -EINVAL;
if (WARN_ON_ONCE(this->layers[0].level != 0))
return -EINVAL;
+ /* Merge the flags into the rules */
this->layers[0].access |= (*layers)[0].access;
this->layers[0].flags.quiet |= (*layers)[0].flags.quiet;
+ this->layers[0].flags.no_inherit |=
+ (*layers)[0].flags.no_inherit;
+ this->layers[0].flags.has_no_inherit_descendant |=
+ (*layers)[0].flags.has_no_inherit_descendant;
return 0;
}
@@ -315,7 +320,10 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
.level = 0,
.flags = {
.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
- },
+ .no_inherit = !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
+ .has_no_inherit_descendant =
+ !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
+ }
} };
build_check_layer();
@@ -662,9 +670,18 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
unsigned long access_bit;
bool is_empty;
+ /* Skip layers that already have no inherit flags. */
+ if (rule_flags &&
+ (rule_flags->no_inherit_masks & layer_bit))
+ continue;
+
/* Collect rule flags for each layer. */
if (rule_flags && layer->flags.quiet)
rule_flags->quiet_masks |= layer_bit;
+ if (rule_flags && layer->flags.no_inherit)
+ rule_flags->no_inherit_masks |= layer_bit;
+ if (rule_flags && layer->flags.has_no_inherit_descendant)
+ rule_flags->no_inherit_desc_masks |= layer_bit;
/*
* Records in @layer_masks which layer grants access to each requested
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index eb60db646422..81df6c56a152 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -40,6 +40,21 @@ struct landlock_layer {
* down the file hierarchy.
*/
bool quiet:1;
+ /**
+ * @no_inherit: Prevents this rule from being inherited by
+ * descendant directories in the filesystem layer. Only used
+ * for filesystem rules.
+ */
+ bool no_inherit:1;
+ /**
+ * @has_no_inherit_descendant: Marker to indicate that this layer
+ * has at least one descendant directory with a rule having the
+ * no_inherit flag. Only used for filesystem rules.
+ * This "flag" is not set by the user, but by Landlock on
+ * parent directories of rules when the child rule has
+ * a rule with the no_inherit flag.
+ */
+ bool has_no_inherit_descendant:1;
} flags;
/**
* @access: Bitfield of allowed actions on the kernel object. They are
@@ -49,13 +64,25 @@ struct landlock_layer {
};
/**
- * struct collected_rule_flags - Hold accumulated flags for each layer.
+ * struct collected_rule_flags - Hold accumulated flags and their markers for each layer.
*/
struct collected_rule_flags {
/**
* @quiet_masks: Layers for which the quiet flag is effective.
*/
layer_mask_t quiet_masks;
+ /**
+ * @no_inherit_masks: Layers for which the no_inherit flag is effective.
+ */
+ layer_mask_t no_inherit_masks;
+ /**
+ * @no_inherit_desc_masks: Layers for which the
+ * has_no_inherit_descendant tag is effective.
+ * This is not a flag itself, but a marker set on ancestors
+ * of rules with the no_inherit flag to deny topology changes
+ * in the direct parent path.
+ */
+ layer_mask_t no_inherit_desc_masks;
};
/**
--
2.51.0
^ permalink raw reply related
* [PATCH v4 0/5] Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-07 1:51 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
Hi,
This is version 4 of the LANDLOCK_ADD_RULE_NO_INHERIT series, which
implements a new flag to suppress inheritance of access rights and
flags from parent objects.
This series is rebased on v6 of Tingmao Wang's "quiet flag" series.
The new flag enables policies where a parent directory needs broader
access than its children. For example, a sandbox may permit read-write
access to /home/user but still prohibit writes to ~/.bashrc or
~/.ssh, even though they are nested beneath the parent. Today this is
not possible because access rights always propagate from parent to
child within a layer.
When a rule is added with LANDLOCK_ADD_RULE_NO_INHERIT:
* access rights on parent inodes are ignored for that inode and its
descendants; and
* operations that change the filesystem ancestors of such objects
(via rename, rmdir, link) are denied up to the VFS root
(new in v4); and
* parent flags do not propagate below a NO_INHERIT rule.
These parent-directory restrictions help mitigate sandbox-restart
attacks: a sandboxed process could otherwise move a protected
directory before exit, causing the next sandbox instance to apply its
policy to the wrong path.
Changes since v3:
1. Trimmed core implementation in fs.c by removing redundant functions.
2. Fixed placement/inclusion of prototypes.
3. Added 4 new selftests for bind mount cases.
4. Protections now apply up to the VFS root instead of the mountpoint
root.
Changes since v2:
1. Add six new selftests for the new flag.
2. Add an optimization to stop permission harvesting when all
relevant layers are tagged with NO_INHERIT.
3. Suppress inheritance of parent flags.
4. Rebase onto v5 of the quiet-flag series.
5. Remove the xarray structure used for flag tracking in favor of
blank rule insertion, simplifying the implementation.
6. Fix edge cases involving flag inheritance across multiple
NO_INHERIT layers.
7. Add documenting comments to new functions.
Links:
v1:
https://lore.kernel.org/linux-security-module/20251105180019.1432367-1-utilityemal77@gmail.com/T/#t
v2:
https://lore.kernel.org/linux-security-module/20251120222346.1157004-1-utilityemal77@gmail.com/T/#t
v3:
https://lore.kernel.org/linux-security-module/20251126122039.3832162-1-utilityemal77@gmail.com/
quiet-flag v6:
https://lore.kernel.org/linux-security-module/cover.1765040503.git.m@maowtm.org/
Example usage:
# LL_FS_RO="/a/b/c" LL_FS_RW="/" LL_FS_NO_INHERIT="/a/b/c"
landlock-sandboxer sh
# touch /a/b/c/fi # denied; / RW does not inherit
# rmdir /a/b/c # denied by ancestor protections
# mv /a /bad # denied
# mkdir /a/good; touch /a/good/fi # allowed; unrelated path
Again, if preferred, I'm happy to split the selftests into multiple
commits.
This version simplifies a lot of the code in the core implementation.
Special thanks to Tingmao Wang for your valuable feedback.
Thank you for your time and review.
Regards,
Justin Suess
Justin Suess (5):
landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to
landlock-sandboxer
selftests/landlock: Implement selftests for
LANDLOCK_ADD_RULE_NO_INHERIT
landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT
include/uapi/linux/landlock.h | 29 +
samples/landlock/sandboxer.c | 13 +-
security/landlock/fs.c | 389 ++++++++++-
security/landlock/ruleset.c | 108 +++-
security/landlock/ruleset.h | 29 +-
security/landlock/syscalls.c | 16 +-
tools/testing/selftests/landlock/fs_test.c | 710 +++++++++++++++++++++
7 files changed, 1285 insertions(+), 9 deletions(-)
base-commit: 92f98eb2cc08c6e2d093d4682f1cd1204728e97e
--
2.51.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox