* [PATCH 0/4] ltp: add tests for some recently-fixed keyrings bugs
@ 2017-10-10 17:51 ` Eric Biggers
0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: keyrings
From: Eric Biggers <ebiggers@google.com>
Add tests to LTP for a few more longstanding bugs which were recently
fixed in the kernel keyrings API.
Eric Biggers (4):
lapi/keyctl.h: add a few missing definitions
syscalls/keyctl06: new test for keyring_read() buffer overrun
syscalls/keyctl07: new test for oops when reading negative key
syscalls/add_key03: new test for forging user keyrings
include/lapi/keyctl.h | 20 +++++
runtest/syscalls | 3 +
testcases/kernel/syscalls/.gitignore | 3 +
testcases/kernel/syscalls/add_key/add_key03.c | 122 ++++++++++++++++++++++++++
testcases/kernel/syscalls/keyctl/keyctl06.c | 68 ++++++++++++++
testcases/kernel/syscalls/keyctl/keyctl07.c | 89 +++++++++++++++++++
6 files changed, 305 insertions(+)
create mode 100644 testcases/kernel/syscalls/add_key/add_key03.c
create mode 100644 testcases/kernel/syscalls/keyctl/keyctl06.c
create mode 100644 testcases/kernel/syscalls/keyctl/keyctl07.c
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH 0/4] ltp: add tests for some recently-fixed keyrings bugs
@ 2017-10-10 17:51 ` Eric Biggers
0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: ltp
From: Eric Biggers <ebiggers@google.com>
Add tests to LTP for a few more longstanding bugs which were recently
fixed in the kernel keyrings API.
Eric Biggers (4):
lapi/keyctl.h: add a few missing definitions
syscalls/keyctl06: new test for keyring_read() buffer overrun
syscalls/keyctl07: new test for oops when reading negative key
syscalls/add_key03: new test for forging user keyrings
include/lapi/keyctl.h | 20 +++++
runtest/syscalls | 3 +
testcases/kernel/syscalls/.gitignore | 3 +
testcases/kernel/syscalls/add_key/add_key03.c | 122 ++++++++++++++++++++++++++
testcases/kernel/syscalls/keyctl/keyctl06.c | 68 ++++++++++++++
testcases/kernel/syscalls/keyctl/keyctl07.c | 89 +++++++++++++++++++
6 files changed, 305 insertions(+)
create mode 100644 testcases/kernel/syscalls/add_key/add_key03.c
create mode 100644 testcases/kernel/syscalls/keyctl/keyctl06.c
create mode 100644 testcases/kernel/syscalls/keyctl/keyctl07.c
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] lapi/keyctl.h: add a few missing definitions
2017-10-10 17:51 ` [LTP] " Eric Biggers
@ 2017-10-10 17:51 ` Eric Biggers
-1 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: keyrings
From: Eric Biggers <ebiggers@google.com>
KEYCTL_REVOKE, KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING, and
KEY_SPEC_USER_SESSION_KEYRING are used by existing tests.
KEY_SPEC_USER_KEYRING will be used by an upcoming test.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
include/lapi/keyctl.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/include/lapi/keyctl.h b/include/lapi/keyctl.h
index 3e7ce4708..73f4fdc90 100644
--- a/include/lapi/keyctl.h
+++ b/include/lapi/keyctl.h
@@ -81,6 +81,10 @@ static inline long keyctl(int cmd, ...)
# define KEYCTL_UPDATE 2
#endif
+#ifndef KEYCTL_REVOKE
+# define KEYCTL_REVOKE 3
+#endif
+
#ifndef KEYCTL_SETPERM
# define KEYCTL_SETPERM 5
#endif
@@ -89,14 +93,30 @@ static inline long keyctl(int cmd, ...)
# define KEYCTL_UNLINK 9
#endif
+#ifndef KEYCTL_READ
+# define KEYCTL_READ 11
+#endif
+
#ifndef KEY_SPEC_THREAD_KEYRING
# define KEY_SPEC_THREAD_KEYRING -1
#endif
+#ifndef KEY_SPEC_PROCESS_KEYRING
+# define KEY_SPEC_PROCESS_KEYRING -2
+#endif
+
#ifndef KEY_SPEC_SESSION_KEYRING
# define KEY_SPEC_SESSION_KEYRING -3
#endif
+#ifndef KEY_SPEC_USER_KEYRING
+# define KEY_SPEC_USER_KEYRING -4
+#endif
+
+#ifndef KEY_SPEC_USER_SESSION_KEYRING
+# define KEY_SPEC_USER_SESSION_KEYRING -5
+#endif
+
#ifndef KEY_REQKEY_DEFL_THREAD_KEYRING
# define KEY_REQKEY_DEFL_THREAD_KEYRING 1
#endif
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH 1/4] lapi/keyctl.h: add a few missing definitions
@ 2017-10-10 17:51 ` Eric Biggers
0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: ltp
From: Eric Biggers <ebiggers@google.com>
KEYCTL_REVOKE, KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING, and
KEY_SPEC_USER_SESSION_KEYRING are used by existing tests.
KEY_SPEC_USER_KEYRING will be used by an upcoming test.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
include/lapi/keyctl.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/include/lapi/keyctl.h b/include/lapi/keyctl.h
index 3e7ce4708..73f4fdc90 100644
--- a/include/lapi/keyctl.h
+++ b/include/lapi/keyctl.h
@@ -81,6 +81,10 @@ static inline long keyctl(int cmd, ...)
# define KEYCTL_UPDATE 2
#endif
+#ifndef KEYCTL_REVOKE
+# define KEYCTL_REVOKE 3
+#endif
+
#ifndef KEYCTL_SETPERM
# define KEYCTL_SETPERM 5
#endif
@@ -89,14 +93,30 @@ static inline long keyctl(int cmd, ...)
# define KEYCTL_UNLINK 9
#endif
+#ifndef KEYCTL_READ
+# define KEYCTL_READ 11
+#endif
+
#ifndef KEY_SPEC_THREAD_KEYRING
# define KEY_SPEC_THREAD_KEYRING -1
#endif
+#ifndef KEY_SPEC_PROCESS_KEYRING
+# define KEY_SPEC_PROCESS_KEYRING -2
+#endif
+
#ifndef KEY_SPEC_SESSION_KEYRING
# define KEY_SPEC_SESSION_KEYRING -3
#endif
+#ifndef KEY_SPEC_USER_KEYRING
+# define KEY_SPEC_USER_KEYRING -4
+#endif
+
+#ifndef KEY_SPEC_USER_SESSION_KEYRING
+# define KEY_SPEC_USER_SESSION_KEYRING -5
+#endif
+
#ifndef KEY_REQKEY_DEFL_THREAD_KEYRING
# define KEY_REQKEY_DEFL_THREAD_KEYRING 1
#endif
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] syscalls/keyctl06: new test for keyring_read() buffer overrun
2017-10-10 17:51 ` [LTP] " Eric Biggers
@ 2017-10-10 17:51 ` Eric Biggers
-1 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: keyrings
From: Eric Biggers <ebiggers@google.com>
Add a test for a bug which caused the kernel to write past the end of
the provided buffer when using KEYCTL_READ to read from a keyring.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
runtest/syscalls | 1 +
testcases/kernel/syscalls/.gitignore | 1 +
testcases/kernel/syscalls/keyctl/keyctl06.c | 68 +++++++++++++++++++++++++++++
3 files changed, 70 insertions(+)
create mode 100644 testcases/kernel/syscalls/keyctl/keyctl06.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 2362a231d..67a7362ee 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -500,6 +500,7 @@ keyctl02 keyctl02
keyctl03 keyctl03
keyctl04 keyctl04
keyctl05 keyctl05
+keyctl06 keyctl06
kcmp01 kcmp01
kcmp02 kcmp02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 930f3f999..1e573d9a4 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -462,6 +462,7 @@
/keyctl/keyctl03
/keyctl/keyctl04
/keyctl/keyctl05
+/keyctl/keyctl06
/kcmp/kcmp01
/kcmp/kcmp02
/kcmp/kcmp03
diff --git a/testcases/kernel/syscalls/keyctl/keyctl06.c b/testcases/kernel/syscalls/keyctl/keyctl06.c
new file mode 100644
index 000000000..88734313d
--- /dev/null
+++ b/testcases/kernel/syscalls/keyctl/keyctl06.c
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2017 Google, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Regression test for commit e645016abc80 ("KEYS: fix writing past end of
+ * user-supplied buffer in keyring_read()").
+ */
+
+#include <errno.h>
+
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static key_serial_t add_test_key(const char *description)
+{
+ TEST(add_key("user", description, "payload", 7,
+ KEY_SPEC_PROCESS_KEYRING));
+ if (TEST_RETURN < 0)
+ tst_brk(TBROK | TTERRNO, "Failed to add test key");
+ return TEST_RETURN;
+}
+
+static void do_test(void)
+{
+ key_serial_t key_ids[2];
+ key_serial_t key_id_1 = add_test_key("key1");
+ key_serial_t key_id_2 = add_test_key("key2");
+
+ memset(key_ids, 0, sizeof(key_ids));
+ TEST(keyctl(KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING,
+ (char *)key_ids, sizeof(key_serial_t)));
+ if (TEST_RETURN < 0)
+ tst_brk(TBROK | TTERRNO, "KEYCTL_READ failed");
+
+ if (key_ids[1] != 0)
+ tst_brk(TFAIL, "KEYCTL_READ overran the buffer");
+
+ if (key_ids[0] = 0)
+ tst_brk(TBROK, "KEYCTL_READ didn't read anything");
+
+ if (key_ids[0] != key_id_1 && key_ids[0] != key_id_2)
+ tst_brk(TBROK, "KEYCTL_READ didn't return correct key ID");
+
+ if (TEST_RETURN != sizeof(key_serial_t)) {
+ tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
+ TEST_RETURN, sizeof(key_serial_t));
+ }
+
+ tst_res(TPASS, "KEYCTL_READ didn't overrun the buffer");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+};
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH 2/4] syscalls/keyctl06: new test for keyring_read() buffer overrun
@ 2017-10-10 17:51 ` Eric Biggers
0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: ltp
From: Eric Biggers <ebiggers@google.com>
Add a test for a bug which caused the kernel to write past the end of
the provided buffer when using KEYCTL_READ to read from a keyring.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
runtest/syscalls | 1 +
testcases/kernel/syscalls/.gitignore | 1 +
testcases/kernel/syscalls/keyctl/keyctl06.c | 68 +++++++++++++++++++++++++++++
3 files changed, 70 insertions(+)
create mode 100644 testcases/kernel/syscalls/keyctl/keyctl06.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 2362a231d..67a7362ee 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -500,6 +500,7 @@ keyctl02 keyctl02
keyctl03 keyctl03
keyctl04 keyctl04
keyctl05 keyctl05
+keyctl06 keyctl06
kcmp01 kcmp01
kcmp02 kcmp02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 930f3f999..1e573d9a4 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -462,6 +462,7 @@
/keyctl/keyctl03
/keyctl/keyctl04
/keyctl/keyctl05
+/keyctl/keyctl06
/kcmp/kcmp01
/kcmp/kcmp02
/kcmp/kcmp03
diff --git a/testcases/kernel/syscalls/keyctl/keyctl06.c b/testcases/kernel/syscalls/keyctl/keyctl06.c
new file mode 100644
index 000000000..88734313d
--- /dev/null
+++ b/testcases/kernel/syscalls/keyctl/keyctl06.c
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2017 Google, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Regression test for commit e645016abc80 ("KEYS: fix writing past end of
+ * user-supplied buffer in keyring_read()").
+ */
+
+#include <errno.h>
+
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static key_serial_t add_test_key(const char *description)
+{
+ TEST(add_key("user", description, "payload", 7,
+ KEY_SPEC_PROCESS_KEYRING));
+ if (TEST_RETURN < 0)
+ tst_brk(TBROK | TTERRNO, "Failed to add test key");
+ return TEST_RETURN;
+}
+
+static void do_test(void)
+{
+ key_serial_t key_ids[2];
+ key_serial_t key_id_1 = add_test_key("key1");
+ key_serial_t key_id_2 = add_test_key("key2");
+
+ memset(key_ids, 0, sizeof(key_ids));
+ TEST(keyctl(KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING,
+ (char *)key_ids, sizeof(key_serial_t)));
+ if (TEST_RETURN < 0)
+ tst_brk(TBROK | TTERRNO, "KEYCTL_READ failed");
+
+ if (key_ids[1] != 0)
+ tst_brk(TFAIL, "KEYCTL_READ overran the buffer");
+
+ if (key_ids[0] == 0)
+ tst_brk(TBROK, "KEYCTL_READ didn't read anything");
+
+ if (key_ids[0] != key_id_1 && key_ids[0] != key_id_2)
+ tst_brk(TBROK, "KEYCTL_READ didn't return correct key ID");
+
+ if (TEST_RETURN != sizeof(key_serial_t)) {
+ tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
+ TEST_RETURN, sizeof(key_serial_t));
+ }
+
+ tst_res(TPASS, "KEYCTL_READ didn't overrun the buffer");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+};
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] syscalls/keyctl07: new test for oops when reading negative key
2017-10-10 17:51 ` [LTP] " Eric Biggers
@ 2017-10-10 17:51 ` Eric Biggers
-1 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: keyrings
From: Eric Biggers <ebiggers@google.com>
Add a test for a bug which caused the kernel to dereference a bogus
pointer when using KEYCTL_READ to read from a negative key.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
runtest/syscalls | 1 +
testcases/kernel/syscalls/.gitignore | 1 +
testcases/kernel/syscalls/keyctl/keyctl07.c | 89 +++++++++++++++++++++++++++++
3 files changed, 91 insertions(+)
create mode 100644 testcases/kernel/syscalls/keyctl/keyctl07.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 67a7362ee..649dbfa6c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -501,6 +501,7 @@ keyctl03 keyctl03
keyctl04 keyctl04
keyctl05 keyctl05
keyctl06 keyctl06
+keyctl07 keyctl07
kcmp01 kcmp01
kcmp02 kcmp02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 1e573d9a4..53ad7ca2b 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -463,6 +463,7 @@
/keyctl/keyctl04
/keyctl/keyctl05
/keyctl/keyctl06
+/keyctl/keyctl07
/kcmp/kcmp01
/kcmp/kcmp02
/kcmp/kcmp03
diff --git a/testcases/kernel/syscalls/keyctl/keyctl07.c b/testcases/kernel/syscalls/keyctl/keyctl07.c
new file mode 100644
index 000000000..c41d7040f
--- /dev/null
+++ b/testcases/kernel/syscalls/keyctl/keyctl07.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2017 Google, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Regression test for commit 37863c43b2c6 ("KEYS: prevent KEYCTL_READ on
+ * negative key").
+ */
+
+#include <errno.h>
+
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static void do_test(void)
+{
+ key_serial_t key_id;
+ char buffer[128];
+
+ /*
+ * Create a negatively instantiated key of the "user" key type. This
+ * key type is chosen because it has a ->read() method (which makes the
+ * bug reachable) and is available whenever CONFIG_KEYS is enabled.
+ *
+ * request_key() will result in the creation of a negative key provided
+ * that /sbin/request-key isn't configured to positively instantiate the
+ * key, based on the provided type, description, and callout_info. If
+ * /sbin/request-key doesn't exist, errno will be ENOENT; while if it
+ * does exist and we specify some random unprefixed description, errno
+ * should be ENOKEY (since /sbin/request-key should not be configured to
+ * instantiate random user keys). In either case a negative key should
+ * be created and we can continue on with the test. Negative keys last
+ * for 60 seconds so there should be plenty of time for the test.
+ */
+ TEST(request_key("user", "description", "callout_info",
+ KEY_SPEC_PROCESS_KEYRING));
+ if (TEST_RETURN != -1)
+ tst_brk(TBROK, "request_key() unexpectedly succeeded");
+
+ if (TEST_ERRNO != ENOKEY && TEST_ERRNO != ENOENT) {
+ tst_brk(TBROK | TTERRNO,
+ "request_key() failed with unexpected error");
+ }
+
+ /* Get the ID of the negative key by reading the keyring */
+ TEST(keyctl(KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING,
+ &key_id, sizeof(key_id)));
+ if (TEST_RETURN < 0)
+ tst_brk(TBROK | TTERRNO, "KEYCTL_READ unexpectedly failed");
+ if (TEST_RETURN != sizeof(key_id)) {
+ tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
+ TEST_RETURN, sizeof(key_id));
+ }
+
+ /*
+ * Now try to read the negative key. Unpatched kernels will oops trying
+ * to read from memory address 0x00000000ffffff92.
+ */
+ tst_res(TINFO, "trying to read from the negative key...");
+ TEST(keyctl(KEYCTL_READ, key_id, buffer, sizeof(buffer)));
+ if (TEST_RETURN != -1) {
+ tst_res(TFAIL,
+ "KEYCTL_READ on negative key unexpectedly succeeded");
+ return;
+ }
+ if (TEST_ERRNO != ENOKEY) {
+ tst_res(TFAIL | TTERRNO,
+ "KEYCTL_READ on negative key failed with unexpected error");
+ return;
+ }
+ tst_res(TPASS, "KEYCTL_READ on negative key expectedly failed with ENOKEY");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+};
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH 3/4] syscalls/keyctl07: new test for oops when reading negative key
@ 2017-10-10 17:51 ` Eric Biggers
0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: ltp
From: Eric Biggers <ebiggers@google.com>
Add a test for a bug which caused the kernel to dereference a bogus
pointer when using KEYCTL_READ to read from a negative key.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
runtest/syscalls | 1 +
testcases/kernel/syscalls/.gitignore | 1 +
testcases/kernel/syscalls/keyctl/keyctl07.c | 89 +++++++++++++++++++++++++++++
3 files changed, 91 insertions(+)
create mode 100644 testcases/kernel/syscalls/keyctl/keyctl07.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 67a7362ee..649dbfa6c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -501,6 +501,7 @@ keyctl03 keyctl03
keyctl04 keyctl04
keyctl05 keyctl05
keyctl06 keyctl06
+keyctl07 keyctl07
kcmp01 kcmp01
kcmp02 kcmp02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 1e573d9a4..53ad7ca2b 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -463,6 +463,7 @@
/keyctl/keyctl04
/keyctl/keyctl05
/keyctl/keyctl06
+/keyctl/keyctl07
/kcmp/kcmp01
/kcmp/kcmp02
/kcmp/kcmp03
diff --git a/testcases/kernel/syscalls/keyctl/keyctl07.c b/testcases/kernel/syscalls/keyctl/keyctl07.c
new file mode 100644
index 000000000..c41d7040f
--- /dev/null
+++ b/testcases/kernel/syscalls/keyctl/keyctl07.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2017 Google, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Regression test for commit 37863c43b2c6 ("KEYS: prevent KEYCTL_READ on
+ * negative key").
+ */
+
+#include <errno.h>
+
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static void do_test(void)
+{
+ key_serial_t key_id;
+ char buffer[128];
+
+ /*
+ * Create a negatively instantiated key of the "user" key type. This
+ * key type is chosen because it has a ->read() method (which makes the
+ * bug reachable) and is available whenever CONFIG_KEYS is enabled.
+ *
+ * request_key() will result in the creation of a negative key provided
+ * that /sbin/request-key isn't configured to positively instantiate the
+ * key, based on the provided type, description, and callout_info. If
+ * /sbin/request-key doesn't exist, errno will be ENOENT; while if it
+ * does exist and we specify some random unprefixed description, errno
+ * should be ENOKEY (since /sbin/request-key should not be configured to
+ * instantiate random user keys). In either case a negative key should
+ * be created and we can continue on with the test. Negative keys last
+ * for 60 seconds so there should be plenty of time for the test.
+ */
+ TEST(request_key("user", "description", "callout_info",
+ KEY_SPEC_PROCESS_KEYRING));
+ if (TEST_RETURN != -1)
+ tst_brk(TBROK, "request_key() unexpectedly succeeded");
+
+ if (TEST_ERRNO != ENOKEY && TEST_ERRNO != ENOENT) {
+ tst_brk(TBROK | TTERRNO,
+ "request_key() failed with unexpected error");
+ }
+
+ /* Get the ID of the negative key by reading the keyring */
+ TEST(keyctl(KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING,
+ &key_id, sizeof(key_id)));
+ if (TEST_RETURN < 0)
+ tst_brk(TBROK | TTERRNO, "KEYCTL_READ unexpectedly failed");
+ if (TEST_RETURN != sizeof(key_id)) {
+ tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
+ TEST_RETURN, sizeof(key_id));
+ }
+
+ /*
+ * Now try to read the negative key. Unpatched kernels will oops trying
+ * to read from memory address 0x00000000ffffff92.
+ */
+ tst_res(TINFO, "trying to read from the negative key...");
+ TEST(keyctl(KEYCTL_READ, key_id, buffer, sizeof(buffer)));
+ if (TEST_RETURN != -1) {
+ tst_res(TFAIL,
+ "KEYCTL_READ on negative key unexpectedly succeeded");
+ return;
+ }
+ if (TEST_ERRNO != ENOKEY) {
+ tst_res(TFAIL | TTERRNO,
+ "KEYCTL_READ on negative key failed with unexpected error");
+ return;
+ }
+ tst_res(TPASS, "KEYCTL_READ on negative key expectedly failed with ENOKEY");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+};
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] syscalls/add_key03: new test for forging user keyrings
2017-10-10 17:51 ` [LTP] " Eric Biggers
@ 2017-10-10 17:51 ` Eric Biggers
-1 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: keyrings
From: Eric Biggers <ebiggers@google.com>
Add a test for a bug which allowed a user to create the user and user
session keyrings for another user.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
runtest/syscalls | 1 +
testcases/kernel/syscalls/.gitignore | 1 +
testcases/kernel/syscalls/add_key/add_key03.c | 122 ++++++++++++++++++++++++++
3 files changed, 124 insertions(+)
create mode 100644 testcases/kernel/syscalls/add_key/add_key03.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 649dbfa6c..0d2254393 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -13,6 +13,7 @@ acct01 acct01
add_key01 add_key01
add_key02 add_key02
+add_key03 add_key03
adjtimex01 adjtimex01
adjtimex02 adjtimex02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 53ad7ca2b..7dea3b2c7 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -8,6 +8,7 @@
/acct/acct01
/add_key/add_key01
/add_key/add_key02
+/add_key/add_key03
/adjtimex/adjtimex01
/adjtimex/adjtimex02
/alarm/alarm01
diff --git a/testcases/kernel/syscalls/add_key/add_key03.c b/testcases/kernel/syscalls/add_key/add_key03.c
new file mode 100644
index 000000000..8c68f8635
--- /dev/null
+++ b/testcases/kernel/syscalls/add_key/add_key03.c
@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2017 Google, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Regression test for commit 237bbd29f7a0 ("KEYS: prevent creating a different
+ * user's keyrings"). The bug allowed any random user to create a keyring named
+ * "_uid.$UID" (or "_uid_ses.$UID"), and it would become the user keyring (or
+ * user session keyring) for user $UID, provided that it hadn't already been
+ * created.
+ *
+ * This test must be run as root so that it has permission to switch to another
+ * user ID and check whether the keyrings are wrong. However, the underlying
+ * bug is actually reachable/exploitable by a non-root user.
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static key_serial_t create_keyring(const char *description)
+{
+ TEST(add_key("keyring", description, NULL, 0,
+ KEY_SPEC_PROCESS_KEYRING));
+ if (TEST_RETURN < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "unable to create keyring '%s'", description);
+ }
+ return TEST_RETURN;
+}
+
+static key_serial_t get_keyring_id(key_serial_t special_id)
+{
+ TEST(keyctl(KEYCTL_GET_KEYRING_ID, special_id, 1));
+ if (TEST_RETURN < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "unable to get ID of keyring %d", special_id);
+ }
+ return TEST_RETURN;
+}
+
+static void unlink_keyring(key_serial_t id)
+{
+ TEST(keyctl(KEYCTL_UNLINK, id, KEY_SPEC_PROCESS_KEYRING));
+ if (TEST_RETURN < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "unable to unlink the keyring we created");
+ }
+}
+
+static void do_test(void)
+{
+ int i;
+
+ /*
+ * Try with multiple user IDs before reporting success. By chance, some
+ * users may already have an existing user keyring; the bug will not be
+ * reproducible for them.
+ */
+ for (i = 0; i < 10; i++) {
+ char description[32];
+ uid_t uid;
+ key_serial_t fake_user_keyring;
+ key_serial_t fake_user_session_keyring;
+
+ uid = rand();
+ if (uid = 0)
+ continue;
+
+ sprintf(description, "_uid.%u", uid);
+ fake_user_keyring = create_keyring(description);
+ sprintf(description, "_uid_ses.%u", uid);
+ fake_user_session_keyring = create_keyring(description);
+
+ TEST(setreuid(uid, 0));
+ if (TEST_RETURN < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "unable to set real uid to %u", uid);
+ }
+
+ if (fake_user_keyring = get_keyring_id(KEY_SPEC_USER_KEYRING)) {
+ tst_brk(TFAIL,
+ "created user keyring for another user");
+ }
+
+ if (fake_user_session_keyring =
+ get_keyring_id(KEY_SPEC_USER_SESSION_KEYRING)) {
+ tst_brk(TFAIL,
+ "created user session keyring for another user");
+ }
+
+ TEST(setreuid(0, 0));
+ if (TEST_RETURN < 0)
+ tst_brk(TBROK | TTERRNO, "unable to reset real uid");
+ uid++;
+
+ unlink_keyring(fake_user_keyring);
+ unlink_keyring(fake_user_session_keyring);
+ }
+ tst_res(TPASS, "expectedly could not create another user's keyrings");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+ .needs_root = 1,
+};
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH 4/4] syscalls/add_key03: new test for forging user keyrings
@ 2017-10-10 17:51 ` Eric Biggers
0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2017-10-10 17:51 UTC (permalink / raw)
To: ltp
From: Eric Biggers <ebiggers@google.com>
Add a test for a bug which allowed a user to create the user and user
session keyrings for another user.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
runtest/syscalls | 1 +
testcases/kernel/syscalls/.gitignore | 1 +
testcases/kernel/syscalls/add_key/add_key03.c | 122 ++++++++++++++++++++++++++
3 files changed, 124 insertions(+)
create mode 100644 testcases/kernel/syscalls/add_key/add_key03.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 649dbfa6c..0d2254393 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -13,6 +13,7 @@ acct01 acct01
add_key01 add_key01
add_key02 add_key02
+add_key03 add_key03
adjtimex01 adjtimex01
adjtimex02 adjtimex02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 53ad7ca2b..7dea3b2c7 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -8,6 +8,7 @@
/acct/acct01
/add_key/add_key01
/add_key/add_key02
+/add_key/add_key03
/adjtimex/adjtimex01
/adjtimex/adjtimex02
/alarm/alarm01
diff --git a/testcases/kernel/syscalls/add_key/add_key03.c b/testcases/kernel/syscalls/add_key/add_key03.c
new file mode 100644
index 000000000..8c68f8635
--- /dev/null
+++ b/testcases/kernel/syscalls/add_key/add_key03.c
@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2017 Google, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Regression test for commit 237bbd29f7a0 ("KEYS: prevent creating a different
+ * user's keyrings"). The bug allowed any random user to create a keyring named
+ * "_uid.$UID" (or "_uid_ses.$UID"), and it would become the user keyring (or
+ * user session keyring) for user $UID, provided that it hadn't already been
+ * created.
+ *
+ * This test must be run as root so that it has permission to switch to another
+ * user ID and check whether the keyrings are wrong. However, the underlying
+ * bug is actually reachable/exploitable by a non-root user.
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static key_serial_t create_keyring(const char *description)
+{
+ TEST(add_key("keyring", description, NULL, 0,
+ KEY_SPEC_PROCESS_KEYRING));
+ if (TEST_RETURN < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "unable to create keyring '%s'", description);
+ }
+ return TEST_RETURN;
+}
+
+static key_serial_t get_keyring_id(key_serial_t special_id)
+{
+ TEST(keyctl(KEYCTL_GET_KEYRING_ID, special_id, 1));
+ if (TEST_RETURN < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "unable to get ID of keyring %d", special_id);
+ }
+ return TEST_RETURN;
+}
+
+static void unlink_keyring(key_serial_t id)
+{
+ TEST(keyctl(KEYCTL_UNLINK, id, KEY_SPEC_PROCESS_KEYRING));
+ if (TEST_RETURN < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "unable to unlink the keyring we created");
+ }
+}
+
+static void do_test(void)
+{
+ int i;
+
+ /*
+ * Try with multiple user IDs before reporting success. By chance, some
+ * users may already have an existing user keyring; the bug will not be
+ * reproducible for them.
+ */
+ for (i = 0; i < 10; i++) {
+ char description[32];
+ uid_t uid;
+ key_serial_t fake_user_keyring;
+ key_serial_t fake_user_session_keyring;
+
+ uid = rand();
+ if (uid == 0)
+ continue;
+
+ sprintf(description, "_uid.%u", uid);
+ fake_user_keyring = create_keyring(description);
+ sprintf(description, "_uid_ses.%u", uid);
+ fake_user_session_keyring = create_keyring(description);
+
+ TEST(setreuid(uid, 0));
+ if (TEST_RETURN < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "unable to set real uid to %u", uid);
+ }
+
+ if (fake_user_keyring == get_keyring_id(KEY_SPEC_USER_KEYRING)) {
+ tst_brk(TFAIL,
+ "created user keyring for another user");
+ }
+
+ if (fake_user_session_keyring ==
+ get_keyring_id(KEY_SPEC_USER_SESSION_KEYRING)) {
+ tst_brk(TFAIL,
+ "created user session keyring for another user");
+ }
+
+ TEST(setreuid(0, 0));
+ if (TEST_RETURN < 0)
+ tst_brk(TBROK | TTERRNO, "unable to reset real uid");
+ uid++;
+
+ unlink_keyring(fake_user_keyring);
+ unlink_keyring(fake_user_session_keyring);
+ }
+ tst_res(TPASS, "expectedly could not create another user's keyrings");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+ .needs_root = 1,
+};
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [LTP] [PATCH 4/4] syscalls/add_key03: new test for forging user keyrings
2017-10-10 17:51 ` [LTP] " Eric Biggers
@ 2017-10-11 13:47 ` Cyril Hrubis
-1 siblings, 0 replies; 14+ messages in thread
From: Cyril Hrubis @ 2017-10-11 13:47 UTC (permalink / raw)
To: keyrings
Hi!
> +static key_serial_t create_keyring(const char *description)
> +{
> + TEST(add_key("keyring", description, NULL, 0,
> + KEY_SPEC_PROCESS_KEYRING));
> + if (TEST_RETURN < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "unable to create keyring '%s'", description);
> + }
> + return TEST_RETURN;
> +}
> +
> +static key_serial_t get_keyring_id(key_serial_t special_id)
> +{
> + TEST(keyctl(KEYCTL_GET_KEYRING_ID, special_id, 1));
> + if (TEST_RETURN < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "unable to get ID of keyring %d", special_id);
> + }
> + return TEST_RETURN;
> +}
> +
> +static void unlink_keyring(key_serial_t id)
> +{
> + TEST(keyctl(KEYCTL_UNLINK, id, KEY_SPEC_PROCESS_KEYRING));
> + if (TEST_RETURN < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "unable to unlink the keyring we created");
> + }
> +}
> +
> +static void do_test(void)
> +{
> + int i;
> +
> + /*
> + * Try with multiple user IDs before reporting success. By chance, some
> + * users may already have an existing user keyring; the bug will not be
> + * reproducible for them.
> + */
> + for (i = 0; i < 10; i++) {
> + char description[32];
> + uid_t uid;
> + key_serial_t fake_user_keyring;
> + key_serial_t fake_user_session_keyring;
> +
> + uid = rand();
> + if (uid = 0)
> + continue;
We have testcases that look for unused uid with this loop:
for (i = 1; i < 1000; i++) {
if (!getpwuid(i))
return i;
}
What about using this instead of doing 10 random tries?
> + sprintf(description, "_uid.%u", uid);
> + fake_user_keyring = create_keyring(description);
> + sprintf(description, "_uid_ses.%u", uid);
> + fake_user_session_keyring = create_keyring(description);
> +
> + TEST(setreuid(uid, 0));
> + if (TEST_RETURN < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "unable to set real uid to %u", uid);
> + }
I guess that we should add SAFE_SETREUID() to the tst_safe_macros.h
library. We do have SAFE_SETRESUID() though, so we may as well use
SAFE_SETRESUID(uid, -1, -1) here.
> + if (fake_user_keyring = get_keyring_id(KEY_SPEC_USER_KEYRING)) {
> + tst_brk(TFAIL,
> + "created user keyring for another user");
> + }
> +
> + if (fake_user_session_keyring =
> + get_keyring_id(KEY_SPEC_USER_SESSION_KEYRING)) {
> + tst_brk(TFAIL,
> + "created user session keyring for another user");
> + }
> +
> + TEST(setreuid(0, 0));
> + if (TEST_RETURN < 0)
> + tst_brk(TBROK | TTERRNO, "unable to reset real uid");
> + uid++;
> +
> + unlink_keyring(fake_user_keyring);
> + unlink_keyring(fake_user_session_keyring);
> + }
> + tst_res(TPASS, "expectedly could not create another user's keyrings");
> +}
> +
> +static struct tst_test test = {
> + .test_all = do_test,
> + .needs_root = 1,
> +};
> --
> 2.14.2.920.gcf0c67979c-goog
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH 4/4] syscalls/add_key03: new test for forging user keyrings
@ 2017-10-11 13:47 ` Cyril Hrubis
0 siblings, 0 replies; 14+ messages in thread
From: Cyril Hrubis @ 2017-10-11 13:47 UTC (permalink / raw)
To: ltp
Hi!
> +static key_serial_t create_keyring(const char *description)
> +{
> + TEST(add_key("keyring", description, NULL, 0,
> + KEY_SPEC_PROCESS_KEYRING));
> + if (TEST_RETURN < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "unable to create keyring '%s'", description);
> + }
> + return TEST_RETURN;
> +}
> +
> +static key_serial_t get_keyring_id(key_serial_t special_id)
> +{
> + TEST(keyctl(KEYCTL_GET_KEYRING_ID, special_id, 1));
> + if (TEST_RETURN < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "unable to get ID of keyring %d", special_id);
> + }
> + return TEST_RETURN;
> +}
> +
> +static void unlink_keyring(key_serial_t id)
> +{
> + TEST(keyctl(KEYCTL_UNLINK, id, KEY_SPEC_PROCESS_KEYRING));
> + if (TEST_RETURN < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "unable to unlink the keyring we created");
> + }
> +}
> +
> +static void do_test(void)
> +{
> + int i;
> +
> + /*
> + * Try with multiple user IDs before reporting success. By chance, some
> + * users may already have an existing user keyring; the bug will not be
> + * reproducible for them.
> + */
> + for (i = 0; i < 10; i++) {
> + char description[32];
> + uid_t uid;
> + key_serial_t fake_user_keyring;
> + key_serial_t fake_user_session_keyring;
> +
> + uid = rand();
> + if (uid == 0)
> + continue;
We have testcases that look for unused uid with this loop:
for (i = 1; i < 1000; i++) {
if (!getpwuid(i))
return i;
}
What about using this instead of doing 10 random tries?
> + sprintf(description, "_uid.%u", uid);
> + fake_user_keyring = create_keyring(description);
> + sprintf(description, "_uid_ses.%u", uid);
> + fake_user_session_keyring = create_keyring(description);
> +
> + TEST(setreuid(uid, 0));
> + if (TEST_RETURN < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "unable to set real uid to %u", uid);
> + }
I guess that we should add SAFE_SETREUID() to the tst_safe_macros.h
library. We do have SAFE_SETRESUID() though, so we may as well use
SAFE_SETRESUID(uid, -1, -1) here.
> + if (fake_user_keyring == get_keyring_id(KEY_SPEC_USER_KEYRING)) {
> + tst_brk(TFAIL,
> + "created user keyring for another user");
> + }
> +
> + if (fake_user_session_keyring ==
> + get_keyring_id(KEY_SPEC_USER_SESSION_KEYRING)) {
> + tst_brk(TFAIL,
> + "created user session keyring for another user");
> + }
> +
> + TEST(setreuid(0, 0));
> + if (TEST_RETURN < 0)
> + tst_brk(TBROK | TTERRNO, "unable to reset real uid");
> + uid++;
> +
> + unlink_keyring(fake_user_keyring);
> + unlink_keyring(fake_user_session_keyring);
> + }
> + tst_res(TPASS, "expectedly could not create another user's keyrings");
> +}
> +
> +static struct tst_test test = {
> + .test_all = do_test,
> + .needs_root = 1,
> +};
> --
> 2.14.2.920.gcf0c67979c-goog
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [LTP] [PATCH 4/4] syscalls/add_key03: new test for forging user keyrings
2017-10-11 13:47 ` Cyril Hrubis
@ 2017-10-11 13:53 ` Cyril Hrubis
-1 siblings, 0 replies; 14+ messages in thread
From: Cyril Hrubis @ 2017-10-11 13:53 UTC (permalink / raw)
To: keyrings
Hi!
> > +static void do_test(void)
> > +{
> > + int i;
> > +
> > + /*
> > + * Try with multiple user IDs before reporting success. By chance, some
> > + * users may already have an existing user keyring; the bug will not be
> > + * reproducible for them.
> > + */
> > + for (i = 0; i < 10; i++) {
> > + char description[32];
> > + uid_t uid;
> > + key_serial_t fake_user_keyring;
> > + key_serial_t fake_user_session_keyring;
> > +
> > + uid = rand();
> > + if (uid = 0)
> > + continue;
>
> We have testcases that look for unused uid with this loop:
>
> for (i = 1; i < 1000; i++) {
> if (!getpwuid(i))
> return i;
> }
>
> What about using this instead of doing 10 random tries?
Or even better try to get the keyring as an user and proceed with the
test if there is none?
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH 4/4] syscalls/add_key03: new test for forging user keyrings
@ 2017-10-11 13:53 ` Cyril Hrubis
0 siblings, 0 replies; 14+ messages in thread
From: Cyril Hrubis @ 2017-10-11 13:53 UTC (permalink / raw)
To: ltp
Hi!
> > +static void do_test(void)
> > +{
> > + int i;
> > +
> > + /*
> > + * Try with multiple user IDs before reporting success. By chance, some
> > + * users may already have an existing user keyring; the bug will not be
> > + * reproducible for them.
> > + */
> > + for (i = 0; i < 10; i++) {
> > + char description[32];
> > + uid_t uid;
> > + key_serial_t fake_user_keyring;
> > + key_serial_t fake_user_session_keyring;
> > +
> > + uid = rand();
> > + if (uid == 0)
> > + continue;
>
> We have testcases that look for unused uid with this loop:
>
> for (i = 1; i < 1000; i++) {
> if (!getpwuid(i))
> return i;
> }
>
> What about using this instead of doing 10 random tries?
Or even better try to get the keyring as an user and proceed with the
test if there is none?
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-10-11 13:53 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-11 13:47 [LTP] [PATCH 4/4] syscalls/add_key03: new test for forging user keyrings Cyril Hrubis
2017-10-11 13:47 ` Cyril Hrubis
2017-10-11 13:53 ` Cyril Hrubis
2017-10-11 13:53 ` Cyril Hrubis
-- strict thread matches above, loose matches on Subject: below --
2017-10-10 17:51 Eric Biggers
2017-10-10 17:51 ` [LTP] " Eric Biggers
2017-10-10 17:51 [PATCH 3/4] syscalls/keyctl07: new test for oops when reading negative key Eric Biggers
2017-10-10 17:51 ` [LTP] " Eric Biggers
2017-10-10 17:51 [PATCH 2/4] syscalls/keyctl06: new test for keyring_read() buffer overrun Eric Biggers
2017-10-10 17:51 ` [LTP] " Eric Biggers
2017-10-10 17:51 [PATCH 1/4] lapi/keyctl.h: add a few missing definitions Eric Biggers
2017-10-10 17:51 ` [LTP] " Eric Biggers
2017-10-10 17:51 [PATCH 0/4] ltp: add tests for some recently-fixed keyrings bugs Eric Biggers
2017-10-10 17:51 ` [LTP] " Eric Biggers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.