From 6231e7d75fafc451d4fdd97acd568a03fb5dd490 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 May 2019 12:58:28 +0100 Subject: test: Add basic test coverage for DBUS_COOKIE_SHA1 We don't actually complete successful authentication, because that would require us to generate a cookie and compute the correct SHA1, which is difficult to do in a deterministic authentication script. However, we do assert that dbus#269 (CVE-2019-12749) has been fixed. Signed-off-by: Simon McVittie --- test/Makefile.am | 2 + test/data/auth/cookie-sha1-username.auth-script | 12 ++++ test/data/auth/cookie-sha1.auth-script | 11 ++++ test/internals/dbus-auth-script.c | 73 ++++++++++++++++++++++++- test/test-utils.c | 71 ++++++++++++++++++++++++ test/test-utils.h | 8 +++ 6 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 test/data/auth/cookie-sha1-username.auth-script create mode 100644 test/data/auth/cookie-sha1.auth-script diff --git a/test/Makefile.am b/test/Makefile.am index 7547ea30..8f14ae18 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -630,6 +630,8 @@ static_data = \ data/auth/anonymous-server-successful.auth-script \ data/auth/cancel.auth-script \ data/auth/client-out-of-mechanisms.auth-script \ + data/auth/cookie-sha1-username.auth-script \ + data/auth/cookie-sha1.auth-script \ data/auth/external-auto.auth-script \ data/auth/external-failed.auth-script \ data/auth/external-root.auth-script \ diff --git a/test/data/auth/cookie-sha1-username.auth-script b/test/data/auth/cookie-sha1-username.auth-script new file mode 100644 index 00000000..45668294 --- /dev/null +++ b/test/data/auth/cookie-sha1-username.auth-script @@ -0,0 +1,12 @@ +UNIX_ONLY +SERVER +SEND 'AUTH DBUS_COOKIE_SHA1 WRONG_USERNAME_HEX' +EXPECT_COMMAND REJECTED +EXPECT_STATE WAITING_FOR_INPUT +EXPECT_HAVE_NO_CREDENTIALS +SEND 'AUTH DBUS_COOKIE_SHA1 USERNAME_HEX' +EXPECT_COMMAND DATA +EXPECT_STATE WAITING_FOR_INPUT +EXPECT_HAVE_NO_CREDENTIALS +# We don't actually complete DBUS_COOKIE_SHA1 authentication, because +# it's non-trivial. diff --git a/test/data/auth/cookie-sha1.auth-script b/test/data/auth/cookie-sha1.auth-script new file mode 100644 index 00000000..f0dd33d7 --- /dev/null +++ b/test/data/auth/cookie-sha1.auth-script @@ -0,0 +1,11 @@ +SERVER +SEND 'AUTH DBUS_COOKIE_SHA1 WRONG_USERID_HEX' +EXPECT_COMMAND REJECTED +EXPECT_STATE WAITING_FOR_INPUT +EXPECT_HAVE_NO_CREDENTIALS +SEND 'AUTH DBUS_COOKIE_SHA1 USERID_HEX' +EXPECT_COMMAND DATA +EXPECT_STATE WAITING_FOR_INPUT +EXPECT_HAVE_NO_CREDENTIALS +# We don't actually complete DBUS_COOKIE_SHA1 authentication, because +# it's non-trivial. diff --git a/test/internals/dbus-auth-script.c b/test/internals/dbus-auth-script.c index 85103010..61b8f674 100644 --- a/test/internals/dbus-auth-script.c +++ b/test/internals/dbus-auth-script.c @@ -41,6 +41,8 @@ # include "dbus/dbus-userdb.h" #endif +#include "test/test-utils.h" + /** * @defgroup DBusAuthScript code for running unit test scripts for DBusAuth * @ingroup DBusInternals @@ -526,8 +528,36 @@ _dbus_auth_script_run (const DBusString *filename) { int where; - if (_dbus_string_find (&to_send, 0, - "USERID_HEX", &where)) + if (_dbus_string_find (&to_send, 0, "WRONG_USERID_HEX", &where)) + { + /* This must be checked for before USERID_HEX, because + * that's a substring. */ + DBusString uid = _DBUS_STRING_INIT_INVALID; + + if (!_dbus_string_init (&uid) || + !_dbus_test_append_different_uid (&uid)) + { + _dbus_warn ("no memory for uid"); + _dbus_string_free (&to_send); + _dbus_string_free (&uid); + goto out; + } + + _dbus_string_delete (&to_send, where, + (int) strlen ("WRONG_USERID_HEX")); + + if (!_dbus_string_hex_encode (&uid, 0, &to_send, where)) + { + _dbus_warn ("no memory to subst WRONG_USERID_HEX"); + _dbus_string_free (&to_send); + _dbus_string_free (&uid); + goto out; + } + + _dbus_string_free (&uid); + } + else if (_dbus_string_find (&to_send, 0, + "USERID_HEX", &where)) { DBusString username; @@ -559,6 +589,45 @@ _dbus_auth_script_run (const DBusString *filename) _dbus_string_free (&username); } + else if (_dbus_string_find (&to_send, 0, + "WRONG_USERNAME_HEX", &where)) + { + /* This must be checked for before USERNAME_HEX, because + * that's a substring. */ +#ifdef DBUS_UNIX + DBusString username = _DBUS_STRING_INIT_INVALID; + + if (!_dbus_string_init (&username) || + !_dbus_test_append_different_username (&username)) + { + _dbus_warn ("no memory for username"); + _dbus_string_free (&to_send); + _dbus_string_free (&username); + goto out; + } + + _dbus_string_delete (&to_send, where, + (int) strlen ("WRONG_USERNAME_HEX")); + + if (!_dbus_string_hex_encode (&username, 0, + &to_send, where)) + { + _dbus_warn ("no memory to subst WRONG_USERNAME_HEX"); + _dbus_string_free (&to_send); + _dbus_string_free (&username); + goto out; + } + + _dbus_string_free (&username); +#else + /* No authentication mechanism uses the login name on + * Windows, so there's no point in it appearing in an + * auth script that is not UNIX_ONLY. */ + _dbus_warn ("WRONG_USERNAME_HEX cannot be used on Windows"); + _dbus_string_free (&to_send); + goto out; +#endif + } else if (_dbus_string_find (&to_send, 0, "USERNAME_HEX", &where)) { diff --git a/test/test-utils.c b/test/test-utils.c index 51471db7..60948a71 100644 --- a/test/test-utils.c +++ b/test/test-utils.c @@ -35,8 +35,15 @@ #include #endif +#include + #ifdef DBUS_UNIX +# include +# include + # include +#else +# include #endif #include "dbus/dbus-message-internal.h" @@ -455,6 +462,70 @@ test_pending_call_store_reply (DBusPendingCall *pc, } #ifdef DBUS_ENABLE_EMBEDDED_TESTS + +#ifdef DBUS_UNIX + +/* + * Set uid to a machine-readable authentication identity (numeric Unix + * uid or ConvertSidToStringSid-style Windows SID) that is likely to exist, + * and differs from the identity of the current process. + * + * @param uid Populated with a machine-readable authentication identity + * on success + * @returns #FALSE if no memory + */ +dbus_bool_t +_dbus_test_append_different_uid (DBusString *uid) +{ + if (geteuid () == 0) + return _dbus_string_append (uid, "65534"); + else + return _dbus_string_append (uid, "0"); +} + +/* + * Set uid to a human-readable authentication identity (login name) + * that is likely to exist, and differs from the identity of the current + * process. This function currently only exists on Unix platforms. + * + * @param uid Populated with a machine-readable authentication identity + * on success + * @returns #FALSE if no memory + */ +dbus_bool_t +_dbus_test_append_different_username (DBusString *username) +{ + if (geteuid () == 0) + return _dbus_string_append (username, "nobody"); + else + return _dbus_string_append (username, "root"); +} + +#else /* !defined(DBUS_UNIX) */ + +#define ANONYMOUS_SID "S-1-5-7" +#define LOCAL_SYSTEM_SID "S-1-5-18" + +dbus_bool_t +_dbus_test_append_different_uid (DBusString *uid) +{ + char *sid = NULL; + dbus_bool_t ret; + + if (!_dbus_getsid (&sid, _dbus_getpid ())) + return FALSE; + + if (strcmp (sid, ANONYMOUS_SID) == 0) + ret = _dbus_string_append (uid, LOCAL_SYSTEM_SID); + else + ret = _dbus_string_append (uid, ANONYMOUS_SID); + + LocalFree (sid); + return ret; +} + +#endif /* !defined(DBUS_UNIX) */ + /* * _dbus_test_main: * @argc: number of command-line arguments diff --git a/test/test-utils.h b/test/test-utils.h index a2afe788..8c3c171f 100644 --- a/test/test-utils.h +++ b/test/test-utils.h @@ -88,4 +88,12 @@ int _dbus_test_main (int argc, void (*test_pre_hook) (void), void (*test_post_hook) (void)); +_DBUS_WARN_UNUSED_RESULT +dbus_bool_t _dbus_test_append_different_uid (DBusString *uid); + +#ifdef DBUS_UNIX +_DBUS_WARN_UNUSED_RESULT +dbus_bool_t _dbus_test_append_different_username (DBusString *username); +#endif + #endif -- cgit v1.2.1