Skip to content
This repository was archived by the owner on Nov 1, 2021. It is now read-only.

[RFC] Add wlr-exec-secure-unstable-v1.xml #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[RFC] Add wlr-exec-secure-unstable-v1.xml #27

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions unstable/wlr-exec-secure-unstable-v1.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
<?xml version="1.0" encoding="UTF-8"?>
<protocol name="wlr_exec_secure_unstable_v1">
<copyright>
Copyright © 2018 Drew DeVault

Permission is hereby granted, free of charge, to any person obtaining a
copy of this software and associated documentation files (the "Software"),
to deal in the Software without restriction, including without limitation
the rights to use, copy, modify, merge, publish, distribute, sublicense,
and/or sell copies of the Software, and to permit persons to whom the
Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice (including the next
paragraph) shall be included in all copies or substantial portions of the
Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS IN THE SOFTWARE.
</copyright>

<description summary="a protocol for executing clients with higher permissions">
This protocol allows unprivileged clients to ask the compositor to execute
a program which can access protected Wayland protocols.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the description somewhat confusing (I read it as that the protocol is not about privileges but rather to make sure that the program is spawned in a compositor controlled environment). I'd simply drop the

which can access protected Wayland protocols

part since this is not part of the protocol or add a

 which can access protected Wayland protocols via the <TBD> protocol.

I'd also drop the "with higher permissions" part from the short description since this is (as I read it) not what the protocol does.

The rationale for this protocol is that the execution environment of an
arbitrary client cannot be reliably verified by the compositor. Environment
variables like LD_PRELOAD may alter the runtime behavior of the client to
malicious ends. However, if the compositor creates the client socket and
exec's the client program itself, it can be sure of the client's execution
environment.

If a client is run with the WAYLAND_SOCKET environment variable set, it can
assume that the socket represents an authenticated Wayland connection and
it will be able to use any secure protocols it is afforded. If not, the
client will likely connect to the Unix socket specified by WAYLAND_DISPLAY,
but it will be an unauthenticated connection. If it needs authenticated
protocols to accomplish its basic tasks, it will likely wish to use
wlr_exec_secure_unstable_v1 to ask the compositor to execute itself again.
It's at the client's discretion to exit once the compositor runs the second
process or to wait for it to complete.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's unlikely that is how it's going to pan out.
It'll probably end up being wayland-exec ./my-prog args, where this hypothetical wayland-exec is just a small program that handles all of this. Or otherwise small interactive launcher programs which try to open the real program.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more likely that if my-prog wants to access Wayland protocols it knows require authentication, it will naturally include a means to authenticate. This would become a basic requirement of that software.

</description>

<interface name="zwlr_exec_manager_v1" version="1">
<description summary="interface for requesting execution handles">
Clients which want the compositor to execute another client should create
a zwlr_exec_handle_v1 and use it to configure and run the client.
</description>

<request name="exec_process">
<description summary="creates a wlr_exec_handle_v1 resource">
Creates an exec_handle for the requested command. The compositor can
choose to disallow this request for certain commands, in which case a
protocol error is issued. The command will be executed without a shell.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should probably be explicit that the arguments are passed unmodified to execv, instead of just saying "executed without a shell".
No $VAR or ~ expansion, etc.

</description>
<arg name="handle" type="new_id" interface="zwlr_exec_handle_v1"/>
<arg name="command" type="array" summary="the command to execute"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is argv[0] handled?
Maybe the actual executable you call and the arguments (with argv[0]) should be separate like the execv calls.

</request>

<request name="destroy" type="destructor">
<description summary="destroy the manager">
All objects created by the manager will still remain valid, until their
appropriate destroy request has been called.
</description>
</request>
</interface>

<interface name="zwlr_exec_handle_v1" version="1">
<description summary="an executing or to-be-executed process">
This resource represents a process executed by the compositor, or a
process which will be executed by the compositor shortly.

The client should transfer any appropriate state, then send an exec
request and await the exit event.

Once the compositor receives an exec request, it should fork, close all
file descriptors (except for the transferred ones), then dup2 them to the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close all file descriptors

Including stdin, stdout, and stderr? Should they be set to /dev/null?
Obviously if they're explicitly set with fd, they should be set to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably specify that the behavior of stdin/stdout/stderr is unspecified if the client doesn't transfer them.

appropriate file descriptor number. It should then exec (with PATH lookup,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 to exec'ing with PATH lookup. To apply security rules we need the full executable path anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair.

e.g. via execlp or exevp) the requested command. Then it should send the
pid event, followed by the exit event when the child terminates.
</description>

<request name="environment">
<description summary="set an environment variable in the process">
Sets an environment variable for the new process. The compositor can
choose not to set any variables, including for example LD_PRELOAD.
</description>
<arg name="name" type="string" summary="environment variable name"/>
<arg name="value" type="string" summary="environment variable value"/>
</request>

<request name="fd">
<description summary="transfer a file descriptor to the process">
Transfers a file descriptor to the new process. The compositor will use
dup2 to assign the specified file descriptor number, then close the
danging fd.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dangling fd

</description>
<arg name="fd" type="fd" summary="file descriptor to transfer"/>
<arg name="num" type="uint" summary="first parameter to dup2"/>
</request>

<request name="exec">
<description summary="start the new process">
Asks the compositor to start the new process. Any request other than
destroy after this point is a protocol violation.
</description>
</request>

<event name="pid">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in my comment about "exit", I don't really think this is necessary or that helpful, and there are other ways that this could be found out.

<description summary="the process's pid is available">
The compositor sends this after it has forked, to inform the client of
the new process's pid.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this after fork and before exec of the new process or is this deliberately unspecified?

</description>
<arg name="pid" type="uint"
summary="pid_t of the child process"/>
</event>

<event name="exit">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the idea of this event. It basically asks the compositor to become a service manager, and track extra client state over normal wayland clients. It just seems unnecessary in my opinion.

It also doesn't address harder questions about processes that fork themselves (daemon-like) or otherwise hand their WAYLAND_SOCKET to another process. The compositor doesn't have a way to track this kind of state and shouldn't be expected to.

Also, there is an indefinite amount of time before this is called. How long do we really expect launcher programs to wait around doing nothing? It would just be cleaner if you fire off the client and don't hear about it again.

As far as I can see, there are two potential use cases for this extension:

(General) Application launchers

As stated above, there isn't really a robust way to track the state of the clients (unless you want to implement/depend on full-blown service manager), and application launchers wouldn't care anyway. They would ignore this event, and destroy the object.

Applications gaining privileges for themselves

Perhaps a program wants to provide a proper interactive interface and really do care about the state of the process they create. We don't need the exit event to be able to implement this.
The fd/enviroment arguments allows clients to implement anything special it wants.

You can create a socketpair/pipe in the launcher process, pass it to the child process, and communicate anything you want back via that, including noticing when the socket/pipe closes (e.g. on death).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also doesn't address harder questions about processes that fork themselves (daemon-like) or otherwise hand their WAYLAND_SOCKET to another process. The compositor doesn't have a way to track this kind of state and shouldn't be expected to.

It's not becoming a service manager because I don't expect it to do this, either. This is basically fork/exec/wait the protocol, nothing more and it doesn't need to handle the edge cases. Clients using this and launched by this will be designed to accomodate it, the edge cases are not important.

Perhaps a program wants to provide a proper interactive interface and really do care about the state of the process they create. We don't need the exit event to be able to implement this.

The fd/enviroment arguments allows clients to implement anything special it wants.

I disagree, pid is a natural thing to include in this design and its absence would be glaring. Making applications deal with some secondary exit hatch which sends an exit status down the drain first is dumb. The compositor will already have an event loop and handling SIGCHLD isn't a tall order.

<description summary="the process has exited">
The compositor will send this when the process has terminated, with the
exit status. No further events will be sent after this, and further
requests will be ignored.
</description>
<arg name="status" type="uint" summary="exit status of the process"/>
</event>

<request name="destroy" type="destructor">
<description summary="delete the exec_handle">
Unreferences the exec_handle. If started, the associated process will
not be killed. Sending any additional requests after destroy is a
protocol error.
</description>
</request>
</interface>
</protocol>