[Guy Harris <guy@xxxxxxxxxxxx>]

On Feb 6, 2019, at 5:06 AM, Peter Wu <peter@xxxxxxxxxxxxx> wrote:

> On Wed, Feb 06, 2019 at 12:46:20PM +0000, João Valverde wrote:
> 
>> I have some doubts about the effectiveness and usefulness of this Lua
>> sandbox. I didn't investigate in depth but it seems enabling/disabling the
>> Lua runtime instead would be better, as dictated by policy (whatever that
>> policy is).
> 
> Setting "enable_lua = false" (formerly "disable_lua = true") already
> prevents further Lua code from being executed. Likewise when
> "run_user_scripts_when_superuser" is false and when started as root.
> 
> I also question the utility of disabling the API, hence these patches:
> 
> wslua: do not load console.lua when run as root
> https://code.wireshark.org/review/31912
> 
> wslua: do not partially disable the Lua API when run as root
> https://code.wireshark.org/review/31913
> 
> The first patch can be safely be backported and should fix the issue
> raised by Kali Linux users. Worst-case, it disables the GUI menu option,
> but it has no effect otherwise.
> 
> The second patch removes the security theatre, but depends on the first
> patch to effectively restrict execution of arbitrary user-supplied code.
> It enables arbitrary execution of user-supplied code by default since
> those who execute "tshark -Xlua_script:foo.lua" as root user (or via
> sudo) will expect it to work.
> 
> Finally, note that "started_with_special_privs()" also returns TRUE even
> if the current user has no more privileges. Even if the Wireshark or
> tshark executables were setuid root, these root privileges have already
> been dropped via "relinquish_special_privs_perm()", long before it ever
> gets to the Lua code.

OK, so Wireshark and TShark are normally run in some form of user session, whether it's a GUI session or not; in those sessions, there's normally credentials (user and groups) for the logged-in user.

If Wireshark or TShark is running with the credentials of the logged-in user, it's not running with elevated (or reduced) privileges.  It's still not necessarily secure - bugs attackable by maliciously crafted packets, malicious plugins, etc. may still cause it to perform actions, with the logged-in user's privileges, that the user wouldn't want done - but that's a separate issue.

In that case, I see no reason to limit where Wireshark looks for plugins, or whether a Lua read-evaluate-print loop (REPL loop), such as Tools > Lua > Evaluate, can be run.

On Kali Linux, "the logged-in user" is, apparently, root.  Therefore, I don't think we should have a hard policy of "if Wireshark is running with an effective user ID of root, we won't allow a Lua REPL", or "if Wireshark is running with a real user ID of root, we won't allow a Lua REPL".

If Wireshark or TShark is *not* running with the credentials of the logged-in user, it's probably running with elevated privileges - although it could conceivably be running with *reduced* privileges, if it's set-UID or set-GID to a user or group with fewer privileges than the logged-in user.  I don't think there's an easy way to determine whether you're running with elevated or reduced privileges, unless the credentials you're running with are root on UN*X or an administrator on Windows, in which case the privileges are (almost?) certainly elevated.

If it's running set-UID or set-GID on UN*X - I'm not sure Windows has an equivalent - there's no guarantee that the user has an inherent right to perform arbitrary actions with the credentials under which Wireshark or TShark is running; set-UID/set-GID is intended to allow a user to perform *those actions that the set-UID/set-GID program permits the user to perform* with those credentials.

As such, if Wireshark or TShark is running set-UID or set-GID:

	it mustn't be allowed to load shared libraries from directories that the user specifies by, for example, setting an environment variable;

	it mustn't be allowed to load shared libraries from any directory into which the user can put files;

	it mustn't be allowed to load shared libraries that the user can modify;

	it mustn't be allowed to preload special shared libraries or replace individual shared libraries that the user specifies by, for example, setting an environment variable;

	it mustn't be allowed to run programs from directories that the user specifies by, for example, setting an environment variable;

	it mustn't be allowed to run programs from any directory into which the user can put files;

	it mustn't be allowed to run programs that the user can modify;

	it mustn't be allowed to load compiled plugins or Lua scripts from directories that the user specifies by, for example, setting an environment variable;

	it mustn't be allowed to load compiled plugins or Lua scripts from any directory into which the user can put files;

	it mustn't be allowed to load compiled plugins or Lua scripts that the user can modify;

etc., etc., etc..  The runtime linker, on most if not all platforms, handles the first four of those; we have to handle the rest.

In addition, no REPL for Lua, or any other language giving sufficient power to access files etc., should be allowed.

However, set-UID/set-GID isn't the only way to run a program with credentials other than that of the logged-in user; there's also su, and sudo/doas, on UN*X, and "run as Administrator" on Windows.

For those, an argument could perhaps be made that, if a user has permissions to do that, they *do* have an inherent right to perform arbitrary actions under the credentials of the user with whose credentials they're running the program, so perhaps the set-UID/set-GID restrictions shouldn't be applied in that case.

The issue of whether to print a warning or pop up a dialog with a warning if Wireshark/TShark is {running as root, running set-UID, running as somebody else} is a separate issue.  *That* warning would be because of the issues that the list of "don't allow this with set-UID/set-GID" prohibitions above doesn't address, e.g. "if a maliciously crafted packet can cause arbitrary code execution, it'll cause arbitrary code execution *as root* if you're running Wireshark/TShark as root" or "if a malicious plugin/Lua script does Bad Things, it'll do Bad Things *as root* if you're running Wireshark/TShark as root".

For set-UID/set-GID, we handle that, to some degree, by giving up set-UID/set-GID privileges fairly early in the startup process of Wireshark and TShark, by calling relinquish_special_privs_perm().  Yes, this means that making Wireshark/TShark set-UID or set-GID doesn't really do as much as somebody who makes it set-UID/set-GID might want, e.g. it doesn't give you permissions to open capture files for which you don't have read permission, but, as far as I'm concerned, that's a not a bug, that's a feature - I can think of few, if any, good reasons to make Wireshark or TShark set-UID or set-GID.  If you need special privileges to *capture* traffic, make *dumpcap* set-UID or set-GID or whatever is necessary; that's privilege separation, giving a smaller simpler program elevated privileges (which it, in turn, gives up once it's opened capture devices) rather than giving a program with "OVER TWO MILLION LINES OF SOURCE CODE" elevated privileges.

So the question is whether we should print/pop up a message if TShark/Wireshark is running as root - and, if we do, whether we should have a compile or configuration option to disable that, so it can be disabled on Kali Linux or other OSes where you don't have much of a choice about whether to run them as root.