2023-12-30 12:03:23

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 6.6 067/156] keys, dns: Allow key types (eg. DNS) to be reclaimed immediately on expiry

6.6-stable review patch. If anyone has any objections, please let me know.

------------------

From: David Howells <[email protected]>

[ Upstream commit 39299bdd2546688d92ed9db4948f6219ca1b9542 ]

If a key has an expiration time, then when that time passes, the key is
left around for a certain amount of time before being collected (5 mins by
default) so that EKEYEXPIRED can be returned instead of ENOKEY. This is a
problem for DNS keys because we want to redo the DNS lookup immediately at
that point.

Fix this by allowing key types to be marked such that keys of that type
don't have this extra period, but are reclaimed as soon as they expire and
turn this on for dns_resolver-type keys. To make this easier to handle,
key->expiry is changed to be permanent if TIME64_MAX rather than 0.

Furthermore, give such new-style negative DNS results a 1s default expiry
if no other expiry time is set rather than allowing it to stick around
indefinitely. This shouldn't be zero as ls will follow a failing stat call
immediately with a second with AT_SYMLINK_NOFOLLOW added.

Fixes: 1a4240f4764a ("DNS: Separate out CIFS DNS Resolver code")
Signed-off-by: David Howells <[email protected]>
Tested-by: Markus Suvanto <[email protected]>
cc: Wang Lei <[email protected]>
cc: Jeff Layton <[email protected]>
cc: Steve French <[email protected]>
cc: Marc Dionne <[email protected]>
cc: Jarkko Sakkinen <[email protected]>
cc: "David S. Miller" <[email protected]>
cc: Eric Dumazet <[email protected]>
cc: Jakub Kicinski <[email protected]>
cc: Paolo Abeni <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
Signed-off-by: Sasha Levin <[email protected]>
---
include/linux/key-type.h | 1 +
net/dns_resolver/dns_key.c | 10 +++++++++-
security/keys/gc.c | 31 +++++++++++++++++++++----------
security/keys/internal.h | 11 ++++++++++-
security/keys/key.c | 15 +++++----------
security/keys/proc.c | 2 +-
6 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 7d985a1dfe4af..5caf3ce823733 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -73,6 +73,7 @@ struct key_type {

unsigned int flags;
#define KEY_TYPE_NET_DOMAIN 0x00000001 /* Keys of this type have a net namespace domain */
+#define KEY_TYPE_INSTANT_REAP 0x00000002 /* Keys of this type don't have a delay after expiring */

/* vet a description */
int (*vet_description)(const char *description);
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index 01e54b46ae0b9..2a6d363763a2b 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -91,6 +91,7 @@ const struct cred *dns_resolver_cache;
static int
dns_resolver_preparse(struct key_preparsed_payload *prep)
{
+ const struct dns_server_list_v1_header *v1;
const struct dns_payload_header *bin;
struct user_key_payload *upayload;
unsigned long derrno;
@@ -122,6 +123,13 @@ dns_resolver_preparse(struct key_preparsed_payload *prep)
return -EINVAL;
}

+ v1 = (const struct dns_server_list_v1_header *)bin;
+ if ((v1->status != DNS_LOOKUP_GOOD &&
+ v1->status != DNS_LOOKUP_GOOD_WITH_BAD)) {
+ if (prep->expiry == TIME64_MAX)
+ prep->expiry = ktime_get_real_seconds() + 1;
+ }
+
result_len = datalen;
goto store_result;
}
@@ -314,7 +322,7 @@ static long dns_resolver_read(const struct key *key,

struct key_type key_type_dns_resolver = {
.name = "dns_resolver",
- .flags = KEY_TYPE_NET_DOMAIN,
+ .flags = KEY_TYPE_NET_DOMAIN | KEY_TYPE_INSTANT_REAP,
.preparse = dns_resolver_preparse,
.free_preparse = dns_resolver_free_preparse,
.instantiate = generic_key_instantiate,
diff --git a/security/keys/gc.c b/security/keys/gc.c
index 3c90807476eb0..eaddaceda14ea 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -66,6 +66,19 @@ void key_schedule_gc(time64_t gc_at)
}
}

+/*
+ * Set the expiration time on a key.
+ */
+void key_set_expiry(struct key *key, time64_t expiry)
+{
+ key->expiry = expiry;
+ if (expiry != TIME64_MAX) {
+ if (!(key->type->flags & KEY_TYPE_INSTANT_REAP))
+ expiry += key_gc_delay;
+ key_schedule_gc(expiry);
+ }
+}
+
/*
* Schedule a dead links collection run.
*/
@@ -176,7 +189,6 @@ static void key_garbage_collector(struct work_struct *work)
static u8 gc_state; /* Internal persistent state */
#define KEY_GC_REAP_AGAIN 0x01 /* - Need another cycle */
#define KEY_GC_REAPING_LINKS 0x02 /* - We need to reap links */
-#define KEY_GC_SET_TIMER 0x04 /* - We need to restart the timer */
#define KEY_GC_REAPING_DEAD_1 0x10 /* - We need to mark dead keys */
#define KEY_GC_REAPING_DEAD_2 0x20 /* - We need to reap dead key links */
#define KEY_GC_REAPING_DEAD_3 0x40 /* - We need to reap dead keys */
@@ -184,21 +196,17 @@ static void key_garbage_collector(struct work_struct *work)

struct rb_node *cursor;
struct key *key;
- time64_t new_timer, limit;
+ time64_t new_timer, limit, expiry;

kenter("[%lx,%x]", key_gc_flags, gc_state);

limit = ktime_get_real_seconds();
- if (limit > key_gc_delay)
- limit -= key_gc_delay;
- else
- limit = key_gc_delay;

/* Work out what we're going to be doing in this pass */
gc_state &= KEY_GC_REAPING_DEAD_1 | KEY_GC_REAPING_DEAD_2;
gc_state <<= 1;
if (test_and_clear_bit(KEY_GC_KEY_EXPIRED, &key_gc_flags))
- gc_state |= KEY_GC_REAPING_LINKS | KEY_GC_SET_TIMER;
+ gc_state |= KEY_GC_REAPING_LINKS;

if (test_and_clear_bit(KEY_GC_REAP_KEYTYPE, &key_gc_flags))
gc_state |= KEY_GC_REAPING_DEAD_1;
@@ -233,8 +241,11 @@ static void key_garbage_collector(struct work_struct *work)
}
}

- if (gc_state & KEY_GC_SET_TIMER) {
- if (key->expiry > limit && key->expiry < new_timer) {
+ expiry = key->expiry;
+ if (expiry != TIME64_MAX) {
+ if (!(key->type->flags & KEY_TYPE_INSTANT_REAP))
+ expiry += key_gc_delay;
+ if (expiry > limit && expiry < new_timer) {
kdebug("will expire %x in %lld",
key_serial(key), key->expiry - limit);
new_timer = key->expiry;
@@ -276,7 +287,7 @@ static void key_garbage_collector(struct work_struct *work)
*/
kdebug("pass complete");

- if (gc_state & KEY_GC_SET_TIMER && new_timer != (time64_t)TIME64_MAX) {
+ if (new_timer != TIME64_MAX) {
new_timer += key_gc_delay;
key_schedule_gc(new_timer);
}
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 3c1e7122076b9..ec2ec335b6133 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -174,6 +174,7 @@ extern unsigned key_gc_delay;
extern void keyring_gc(struct key *keyring, time64_t limit);
extern void keyring_restriction_gc(struct key *keyring,
struct key_type *dead_type);
+void key_set_expiry(struct key *key, time64_t expiry);
extern void key_schedule_gc(time64_t gc_at);
extern void key_schedule_gc_links(void);
extern void key_gc_keytype(struct key_type *ktype);
@@ -222,10 +223,18 @@ extern struct key *key_get_instantiation_authkey(key_serial_t target_id);
*/
static inline bool key_is_dead(const struct key *key, time64_t limit)
{
+ time64_t expiry = key->expiry;
+
+ if (expiry != TIME64_MAX) {
+ if (!(key->type->flags & KEY_TYPE_INSTANT_REAP))
+ expiry += key_gc_delay;
+ if (expiry <= limit)
+ return true;
+ }
+
return
key->flags & ((1 << KEY_FLAG_DEAD) |
(1 << KEY_FLAG_INVALIDATED)) ||
- (key->expiry > 0 && key->expiry <= limit) ||
key->domain_tag->removed;
}

diff --git a/security/keys/key.c b/security/keys/key.c
index 5c0c7df833f8a..5f103b2713c64 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -294,6 +294,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
key->uid = uid;
key->gid = gid;
key->perm = perm;
+ key->expiry = TIME64_MAX;
key->restrict_link = restrict_link;
key->last_used_at = ktime_get_real_seconds();

@@ -463,10 +464,7 @@ static int __key_instantiate_and_link(struct key *key,
if (authkey)
key_invalidate(authkey);

- if (prep->expiry != TIME64_MAX) {
- key->expiry = prep->expiry;
- key_schedule_gc(prep->expiry + key_gc_delay);
- }
+ key_set_expiry(key, prep->expiry);
}
}

@@ -606,8 +604,7 @@ int key_reject_and_link(struct key *key,
atomic_inc(&key->user->nikeys);
mark_key_instantiated(key, -error);
notify_key(key, NOTIFY_KEY_INSTANTIATED, -error);
- key->expiry = ktime_get_real_seconds() + timeout;
- key_schedule_gc(key->expiry + key_gc_delay);
+ key_set_expiry(key, ktime_get_real_seconds() + timeout);

if (test_and_clear_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags))
awaken = 1;
@@ -722,16 +719,14 @@ struct key_type *key_type_lookup(const char *type)

void key_set_timeout(struct key *key, unsigned timeout)
{
- time64_t expiry = 0;
+ time64_t expiry = TIME64_MAX;

/* make the changes with the locks held to prevent races */
down_write(&key->sem);

if (timeout > 0)
expiry = ktime_get_real_seconds() + timeout;
-
- key->expiry = expiry;
- key_schedule_gc(key->expiry + key_gc_delay);
+ key_set_expiry(key, expiry);

up_write(&key->sem);
}
diff --git a/security/keys/proc.c b/security/keys/proc.c
index d0cde6685627f..4f4e2c1824f18 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -198,7 +198,7 @@ static int proc_keys_show(struct seq_file *m, void *v)

/* come up with a suitable timeout value */
expiry = READ_ONCE(key->expiry);
- if (expiry == 0) {
+ if (expiry == TIME64_MAX) {
memcpy(xbuf, "perm", 5);
} else if (now >= expiry) {
memcpy(xbuf, "expd", 5);
--
2.43.0





2024-01-05 02:14:09

by Jeffrey Altman

[permalink] [raw]
Subject: Re: [PATCH 6.6 067/156] keys, dns: Allow key types (eg. DNS) to be reclaimed immediately on expiry

On 12/30/2023 6:58 AM, Greg Kroah-Hartman wrote:
> 6.6-stable review patch. If anyone has any objections, please let me know.
>
> ------------------
>
> From: David Howells <[email protected]>
>
> [ Upstream commit 39299bdd2546688d92ed9db4948f6219ca1b9542 ]
Greg,

Upstream commit 39299bdd2546688d92ed9db4948f6219ca1b9542 ("keys, dns:
Allow key types (eg. DNS) to be reclaimed immediately on expiry") was
subsequently fixed by

  commit 1997b3cb4217b09e49659b634c94da47f0340409
  Author: Edward Adam Davis <[email protected]>
  Date:   Sun Dec 24 00:02:49 2023 +0000

    keys, dns: Fix missing size check of V1 server-list header

  Fixes: b946001d3bb1 ("keys, dns: Allow key types (eg. DNS) to be
reclaimed immediately on expiry")

If it is not too late, would it be possible to apply 1997b3cb421 to the
branches b946001d3bb1 was cherry-picked to before release?
I believe the complete set of branches are

  linux-6.6.y, linux-6.1.y, linux-5.15.y, linux-5.10.y, linux-5.0.y

Thank you.

Jeffrey Altman


Attachments:
smime.p7s (3.94 kB)
S/MIME Cryptographic Signature

2024-01-05 09:51:57

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 6.6 067/156] keys, dns: Allow key types (eg. DNS) to be reclaimed immediately on expiry

On Thu, Jan 04, 2024 at 09:13:34PM -0500, Jeffrey E Altman wrote:
> On 12/30/2023 6:58 AM, Greg Kroah-Hartman wrote:
> > 6.6-stable review patch. If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: David Howells <[email protected]>
> >
> > [ Upstream commit 39299bdd2546688d92ed9db4948f6219ca1b9542 ]
> Greg,
>
> Upstream commit 39299bdd2546688d92ed9db4948f6219ca1b9542 ("keys, dns: Allow
> key types (eg. DNS) to be reclaimed immediately on expiry") was subsequently
> fixed by
>
> ? commit 1997b3cb4217b09e49659b634c94da47f0340409
> ? Author: Edward Adam Davis <[email protected]>
> ? Date:?? Sun Dec 24 00:02:49 2023 +0000
>
> ??? keys, dns: Fix missing size check of V1 server-list header
>
> ? Fixes: b946001d3bb1 ("keys, dns: Allow key types (eg. DNS) to be reclaimed
> immediately on expiry")
>
> If it is not too late, would it be possible to apply 1997b3cb421 to the
> branches b946001d3bb1 was cherry-picked to before release?
> I believe the complete set of branches are
>
> ? linux-6.6.y, linux-6.1.y, linux-5.15.y, linux-5.10.y, linux-5.0.y

The stable trees were already released with this change in it, so I'll
queue this up for the next round, thanks.

greg k-h

2024-01-05 10:07:06

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 6.6 067/156] keys, dns: Allow key types (eg. DNS) to be reclaimed immediately on expiry

On Fri, Jan 05, 2024 at 10:51:50AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Jan 04, 2024 at 09:13:34PM -0500, Jeffrey E Altman wrote:
> > On 12/30/2023 6:58 AM, Greg Kroah-Hartman wrote:
> > > 6.6-stable review patch. If anyone has any objections, please let me know.
> > >
> > > ------------------
> > >
> > > From: David Howells <[email protected]>
> > >
> > > [ Upstream commit 39299bdd2546688d92ed9db4948f6219ca1b9542 ]
> > Greg,
> >
> > Upstream commit 39299bdd2546688d92ed9db4948f6219ca1b9542 ("keys, dns: Allow
> > key types (eg. DNS) to be reclaimed immediately on expiry") was subsequently
> > fixed by
> >
> > ? commit 1997b3cb4217b09e49659b634c94da47f0340409
> > ? Author: Edward Adam Davis <[email protected]>
> > ? Date:?? Sun Dec 24 00:02:49 2023 +0000
> >
> > ??? keys, dns: Fix missing size check of V1 server-list header
> >
> > ? Fixes: b946001d3bb1 ("keys, dns: Allow key types (eg. DNS) to be reclaimed
> > immediately on expiry")
> >
> > If it is not too late, would it be possible to apply 1997b3cb421 to the
> > branches b946001d3bb1 was cherry-picked to before release?
> > I believe the complete set of branches are
> >
> > ? linux-6.6.y, linux-6.1.y, linux-5.15.y, linux-5.10.y, linux-5.0.y
>
> The stable trees were already released with this change in it, so I'll
> queue this up for the next round, thanks.

Ah, I see what happened, that line:
Fixes: b946001d3bb1 ("keys, dns: Allow key types (eg. DNS) to be reclaimed immediately on expiry")
refers to a commit that is not in Linus's tree, and isn't the sha1 that
you are pointing at here either.

So I'll go add this manually, but this is why our checking scripts
missed this, please be more careful about using the proper SHA1 values
in commits. Using invalid ones is almost worse than not using them at
allm as it gives you the false sense that the markings are correct.

thanks,

greg k-h