Control: tags -1 + patch
Hi
On Thu, Oct 05, 2017 at 09:06:33AM +0200, Salvatore Bonaccorso wrote:
> Source: linux
> Version: 3.16.7-ckt7-1
> Severity: normal
>
> Hi
>
> In 3.16.7-ckt7-1 we applied a backport of "EYS: request_key() should
> reget expired keys rather than give EKEYEXPIRED", adressing #758870,
> 0b0a84154eff56913e91df29de5c3a03a0029e38.
[...]
> Are we potentially miss any relevant needed commits between v3.16..v3.18?
> (054f6180d8b5602b431b5924976c956e760488b1, "KEYS: Simplify
> KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags"?).
I did some bisecting, and indeed it seems we were missing the second
change as mentioned as well in the merge:
https://git.kernel.org/linus/23c836ce5c1e1e0bb942f58a3cbc2f7fc05a08b5
> The second and third fix a bug in NFS idmapper handling whereby a key
> representing a mapping between an id and a name expires and causing
> EKEYEXPIRED to be seen internally in NFS (which prevents the mapping
> from happening) rather than re-looking up the mapping"
With attached patch on top of the (current) jessie branch in git, the
problem is solved.
Regards,
Salvatore
>From 83a57b39cea1cdd82e42483dc5a60f671c32252c Mon Sep 17 00:00:00 2001
From: Salvatore Bonaccorso
Date: Thu, 5 Oct 2017 17:16:34 +0200
Subject: [PATCH] KEYS: Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags
Closes: #877760
---
debian/changelog | 3 +
...fy-KEYRING_SEARCH_-NO-DO-_STATE_CHECK-fla.patch | 96 ++
...t_key-should-reget-expired-keys-rather-th.patch | 8 +-
debian/patches/series | 1 +
4 files changed, 104 insertions(+), 4 deletions(-)
create mode 100644 debian/patches/bugfix/all/KEYS-Simplify-KEYRING_SEARCH_-NO-DO-_STATE_CHECK-fla.patch
diff --git a/debian/changelog b/debian/changelog
index 55ac046e7..9bb4229ac 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -582,6 +582,9 @@ linux (3.16.48-1) UNRELEASED; urgency=medium
- ext4: preserve i_mode if __ext4_set_acl() fails
- ext4: Don't clear SGID when inheriting ACLs
+ [ Salvatore Bonaccorso ]
+ * KEYS: Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags (Closes: #877760)
+
-- Ben Hutchings Sun, 06 Aug 2017 22:03:56 +0100
linux (3.16.43-2+deb8u5) jessie-security; urgency=medium
diff --git a/debian/patches/bugfix/all/KEYS-Simplify-KEYRING_SEARCH_-NO-DO-_STATE_CHECK-fla.patch b/debian/patches/bugfix/all/KEYS-Simplify-KEYRING_SEARCH_-NO-DO-_STATE_CHECK-fla.patch
new file mode 100644
index 0..44f9caee4
--- /dev/null
+++ b/debian/patches/bugfix/all/KEYS-Simplify-KEYRING_SEARCH_-NO-DO-_STATE_CHECK-fla.patch
@@ -0,0 +1,96 @@
+From: David Howells
+Date: Mon, 1 Dec 2014 22:52:50 +
+Subject: KEYS: Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags
+Origin: https://git.kernel.org/linus/054f6180d8b5602b431b5924976c956e760488b1
+
+Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags to be two variations of the
+same flag. They are effectively mutually exclusive and one or the other
+should be provided, but not both.
+
+Keyring cycle detection and key possession determination are the only things
+that set NO_STATE_CHECK, except that neither flag really does anything there
+because neither purpose makes use of the keyring_search_iterator() function,
+but rather provides their own.
+
+For cycle detection we definitely want to check inside of expired keyrings,
+just so that we don't create a cycle we can't get rid of. Revoked keyrings
+are cleared at revocation time and can't then be reused, so shouldn't be a
+problem either way.
+
+For possession determination, we *might* want to validate each keyring before
+searching it: do you possess a key that's hidden behind an expired or just
+plain inaccessible keyring? Currently, the answer is yes. Note that you
+cannot, however, possess a key behind a revoked keyring because they are
+cleared on revocation.
+
+keyring_search() sets DO_STATE_CHECK, which is correct.
+
+request_key_and_link() currently doesn't specify whether to check the key
+state or not - but it should set DO_STATE_CHECK.
+
+key_get_instantiation_authkey() also currently doesn't specify whether to
+check the key state or not - but it probably should also set DO_STATE_CHECK.
+
+Signed-off-by: David Howells
+Tested-by: Chuck Lever
+[carnil: Backported to 3.16: Adjust context]
+---
+ security/keys/keyring.c | 7 ---
+ security/keys/request_key.c | 1 +
+ security/keys/request_key_auth.c | 1 +
+ 3 files changed, 6 insertions(+), 3 deletions(-)
+
+--- a/security/keys/keyring.c
b/security/keys/keyring.c
+@@ -609,6 +609,10 @@ static bool search_nested_keyrings(struc
+ ctx->index_key.type->name,
+ ctx->index_key.description);
+
++#define STATE_CHECKS (KEYRING_SEARCH_NO_STATE_CHECK | KEYRING_SEARCH_DO_STATE_CHECK)
++ BUG_ON((ctx->flags & STATE_CHECKS) == 0 ||
++ (ctx->flags & STATE_CHECKS)
