syscalls/add_key03: new test for forging user keyrings
Hi!
Quoted 38 lines
(quoted diff) +38
-0+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.
Review
Quoted 10 lines
(quoted diff) +10
-0+ */ + 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?
Review
Quoted 10 lines
(quoted diff) +10
-0+ 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.
Quoted 32 lines
(quoted diff) +26
-0+ 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, +};Signature · 5 lines
2.14.2.920.gcf0c67979c-goog -- Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Cyril Hrubis [email protected]
Hi!
Quoted 21 lines
Quoted 8 lines
(quoted diff) +8
-0+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.Review
Quoted 10 lines
(quoted diff) +10
-0+ */ + 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:
Quoted 6 lines
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?
Signature · 2 lines
Cyril Hrubis [email protected]