Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: user-configurable command hook on client connect/disconnect #158

Closed
lack opened this issue Sep 28, 2022 · 12 comments
Closed

Comments

@lack
Copy link
Collaborator

lack commented Sep 28, 2022

I would like the ability to configure wayvnc to run a command when a client connects and disconnects.

Rationale

In my case specifically I want to disable my secondary display so all sway desktops move to the single display where wayvnc is connected, and re-enable it when I disconnect.

I could also see other vnc-specific actions could/should be scriptable, like maybe setting DPMS on (#114) or setting the root background to black or sending a slack notification or whatever...

In fact, if we're careful with the calling convention, it could maybe even be used to implement things like asking the current seat for permission to connect (#140)

Considerations

If wayvnc supports multiple simultaneously connected clients, the script should be given the current number of connected clients as an argument or environment variable so it can distinguish between first client connecting vs other clients, and last client disconnecting vs other clients.

Other info that should be available to the script in args or env include:

  • Client IP
  • Client username
  • Client connection type (read-only vs read/write)
  • ...

If we want to be super clever, we could optionally block wayvnc client connect while the script is running, and allow the script to decide (via return code) whether the current client should be allowed to connect. exit 0 => connect whereas exit 1 => reject the connection... That way a script could popup a permission dialog or whatever and let the seated physical user decide whether to allow the new connection etc etc.

Perhaps this implies the utility of 3 distinct script hook points:

  • preconnect (blocks connect, with exit 1 dropping the connection)
  • connected (nonblocking)
  • disconnected (nonblocking, but maybe exit 1 could signal wayvnc itself to shut down?)
@any1
Copy link
Owner

any1 commented Sep 28, 2022

Your main use case seems like something that might be implemented as a builtin feature. Some remote desktop servers support automatic resizing to accommodate the client's window size and I believe that turning off all except 1 displays or even all of them might also be feasible or even necessary when operating in that mode.

Authentication scripts are a good idea. I think that I would prefer embedding a scripting language such as Lua and giving it limited capabilities rather than allowing execution of arbitrary shell commands. A lua script could be pointed to by the config file and it could have hooks for adding authentication handlers, and perhaps even some other useful things.

@lack
Copy link
Collaborator Author

lack commented Sep 29, 2022

Your main use case seems like something that might be implemented as a builtin feature. Some remote desktop servers support automatic resizing to accommodate the client's window size and I believe that turning off all except 1 displays or even all of them might also be feasible or even necessary when operating in that mode.

I guess that depends on how you end up addressing #112; I can see the use case for "don't change anything but show all the displays over VNC" especially in the shared support-via-VNC kind of operation, where you have someone at the physical seat and others VNCd in... but the "scale it for this client and collapse all desktops" makes sense for the "working exclusively from VNC instead of physical seat this time" way of working that I usually use.

It also doesn't address the secondary use-cases I brought up like setting the background black or setting DPMS on.

One alternative implementation option may be to just engineer some mechanism to tell kanshi that vnc has connected and maybe let it take care of the output changes, as it already deals well with physical display changes? VNC isn't a perfect match here since it doesn't necessarily change the display topology...

Authentication scripts are a good idea. I think that I would prefer embedding a scripting language such as Lua and giving it limited capabilities rather than allowing execution of arbitrary shell commands. A lua script could be pointed to by the config file and it could have hooks for adding authentication handlers, and perhaps even some other useful things.

Can you elaborate a bit more on your reasoning here? If you're going to add scripting, why not introduce a fully flexible mechanism at first? Maybe once we learn how such a facility is used we can start mailing some of the more common usages into top-level features or specific domain-specific scripting...

And more selfishly, I always prefer general flexibility... I take sway to be a power-user platform already. The sway config and other related programs (kanshi, swayidle, etc) give this kind of script-driven infinite flexibility that I, as a power-user myself, clearly want, and I want wayvnc to be powerfully flexible in the same way. With a generic mechanism available, I (and others) can implement use cases that neither of us have thought of yet as they come up.

@any1
Copy link
Owner

any1 commented Sep 29, 2022

I guess that depends on how you end up addressing #112; I can see the use case for "don't change anything but show all the displays over VNC" especially in the shared support-via-VNC kind of operation, where you have someone at the physical seat and others VNCd in... but the "scale it for this client and collapse all desktops" makes sense for the "working exclusively from VNC instead of physical seat this time" way of working that I usually use.

These should be two different modes of operations that can be selected using a command line argument. These are common enough uses cases that they shouldn't require customs scripting.

It also doesn't address the secondary use-cases I brought up like setting the background black or setting DPMS on.

It's probably most efficient both with regards to performance and programming effort to turn off all physical displays and create a "virtual" display that can be arbitrarily resized and configured to the client's needs.

One alternative implementation option may be to just engineer some mechanism to tell kanshi that vnc has connected and maybe let it take care of the output changes, as it already deals well with physical display changes? VNC isn't a perfect match here since it doesn't necessarily change the display topology...

Adding some kind of scripting function might be the shortest path towards a solution, but I would prefer a generic solution that works for all supported wayland compositors using wayland protocols. Sway is not the only compositor that people are using wayvnc with. Having special scripts for each of them would be awkward when a more generic solution is possible.

Can you elaborate a bit more on your reasoning here? If you're going to add scripting, why not introduce a fully flexible mechanism at first? Maybe once we learn how such a facility is used we can start mailing some of the more common usages into top-level features or specific domain-specific scripting...

People would complain very loudly if flexibility were removed afterwards.

And more selfishly, I always prefer general flexibility... I take sway to be a power-user platform already. The sway config and other related programs (kanshi, swayidle, etc) give this kind of script-driven infinite flexibility that I, as a power-user myself, clearly want, and I want wayvnc to be powerfully flexible in the same way. With a generic mechanism available, I (and others) can implement use cases that neither of us have thought of yet as they come up.

Wayvnc is a network application and I suspect that custom scripting might cause some unexpected security issues. I can't really point out anything particular. In any case it needs to be though over thoroughly.

@Consolatis
Copy link
Contributor

Consolatis commented Sep 29, 2022

It's probably most efficient both with regards to performance and programming effort to turn off all physical displays and create a "virtual" display that can be arbitrarily resized and configured to the client's needs.

There is also a use-case to create / resize a virtual output without turning the other outputs off: extending the desktop to a tablet or similar remote device.

@any1
Copy link
Owner

any1 commented Sep 29, 2022

There is also a use-case to create / resize a virtual output without turning the other outputs off: extending the desktop to a tablet or similar remote device.

Sure, there could also be a "side-car" mode. It can be achieved currently with sway at least, but is somewhat awkward to set up, I imagine.

I did actually think of a reason now why wayvnc shouldn't call execve(). Setting a seccomp policy to disallow spawning processes is a very effective security measure. Letting users define scripts to be executed from wayvnc precludes such a security measure.

@lack
Copy link
Collaborator Author

lack commented Sep 29, 2022

I guess that depends on how you end up addressing #112; I can see the use case for "don't change anything but show all the displays over VNC" especially in the shared support-via-VNC kind of operation, where you have someone at the physical seat and others VNCd in... but the "scale it for this client and collapse all desktops" makes sense for the "working exclusively from VNC instead of physical seat this time" way of working that I usually use.

These should be two different modes of operations that can be selected using a command line argument. These are common enough uses cases that they shouldn't require customs scripting.

I agree. Maybe 3 modes based on the other comment here! Is itsbest to continue this part of the discussion either in #112 or to create a separate issue?

It also doesn't address the secondary use-cases I brought up like setting the background black or setting DPMS on.

It's probably most efficient both with regards to performance and programming effort to turn off all physical displays and create a "virtual" display that can be arbitrarily resized and configured to the client's needs.

This is a very cool idea too. Again, probably best for a different issue and/or #112.

One alternative implementation option may be to just engineer some mechanism to tell kanshi that vnc has connected and maybe let it take care of the output changes, as it already deals well with physical display changes? VNC isn't a perfect match here since it doesn't necessarily change the display topology...

Adding some kind of scripting function might be the shortest path towards a solution, but I would prefer a generic solution that works for all supported wayland compositors using wayland protocols. Sway is not the only compositor that people are using wayvnc with. Having special scripts for each of them would be awkward when a more generic solution is possible.

This is a good point, and I agree.

Can you elaborate a bit more on your reasoning here? If you're going to add scripting, why not introduce a fully flexible mechanism at first? Maybe once we learn how such a facility is used we can start mailing some of the more common usages into top-level features or specific domain-specific scripting...

People would complain very loudly if flexibility were removed afterwards.

I'm not suggesting we would ever remove the script hooks, but now and then take ideas implemented in scripts and reimplement them in a more native and user-friendly way. Think of a generic scripting interface as the R&D department, and native features as the mature products...

And more selfishly, I always prefer general flexibility... I take sway to be a power-user platform already. The sway config and other related programs (kanshi, swayidle, etc) give this kind of script-driven infinite flexibility that I, as a power-user myself, clearly want, and I want wayvnc to be powerfully flexible in the same way. With a generic mechanism available, I (and others) can implement use cases that neither of us have thought of yet as they come up.

Wayvnc is a network application and I suspect that custom scripting might cause some unexpected security issues. I can't really point out anything particular. In any case it needs to be though over thoroughly.

I don't know a lot about the architecture of wayvnc yet, but I'm sure you're right that this needs to be carefully considered. For example:

  • Does wayvnc ever run as root? Or does it only run as the user whose Wayland session is being shared?
  • If enabling scripting is opt-in via the command line, does this alleviate concerns of people unwittingly enabling scripts they dont know about?
  • If the scripts are local files, does this alleviate concerns about remote users injecting code?
  • If the scripts only run after VNC authentication succeeds, does this alleviate worries about DOS attacks or unauthorized users forcing scripts to run?

@any1
Copy link
Owner

any1 commented Sep 29, 2022

I'm not suggesting we would ever remove the script hooks, but now and then take ideas implemented in scripts and reimplement them in a more native and user-friendly way. Think of a generic scripting interface as the R&D department, and native features as the mature products...

I suppose there could be 3 levels for script privileges: "off", "sandboxed" and "unlimited", the third of which would turn off seccomp and import the embedded scripting language's standard libraries, which would allow them to do pretty much whatever. We could start by implementing the "unlimited" mode and then implement "sandboxed" later.

Does wayvnc ever run as root? Or does it only run as the user whose Wayland session is being shared?

It should not ever run as root.

If enabling scripting is opt-in via the command line, does this alleviate concerns of people unwittingly enabling scripts they dont know about?

I suppose it would, but people often copy and paste things from the internet without much thought, but we can't really protect people from themselves..

If the scripts are local files, does this alleviate concerns about remote users injecting code?

I would never consider an implementation where scripts are anything but local files. :)

If the scripts only run after VNC authentication succeeds, does this alleviate worries about DOS attacks or unauthorized users forcing scripts to run?

Yes, I believe authenticated users can be trusted.

The biggest issue that I can think of with adding script execution is that is weakens the impact of seccomp (which is not currently implemented but should be).

@Consolatis
Copy link
Contributor

Consolatis commented Sep 29, 2022

Sorry for the slight offtopic.

AFAIK there is no wayland protocol to create virtual outputs.
There is however the wlr-output-management protocol to

  • enable / disable outputs
  • set a (custom) mode

So what could be done is

  • let the user create a disabled virtual output by whatever means the compositor provides
  • have a flag for wayvnc (like --headless) which causes it to enable and resize the output already given as argument on the first accepted connection and disable it again when there are no connections left
  • have another flag for wayvnc (like --exclusive) to disable all other outputs on the first accepted connection and enable them again when there are no connections left

These two arguments could then be used together or each on its own.

@any1
Copy link
Owner

any1 commented Sep 29, 2022

AFAIK there is no wayland protocol to create virtual outputs.

I was thinking about making one and I think that it's probably going to be easier to get it accepted than ext-screencopy-v1.

@lack
Copy link
Collaborator Author

lack commented Sep 30, 2022

I just had another thought that may be more security-conscious; what if instead of running these hooks directly, wayvnc published relevant events to a Unix domain socket, dbus service, or other IPC channel?

In order to fit the "confirmation dialog" flow, we would also want the ability for the client to send commands on the socket too (like to force a disconnect, query number of connected users, etc). In many ways this would be like sway's IPC socket protocol.

Then there could be a separate client (or probably, multiple clients) that consumes these events and takes actions:

  • a command line one much like 'swaymsg' for demonstration and scripting
  • a "confirm dialog" one
  • a "run this command on event" one
  • each compositor could implement its own, to be aware of connection events, etc.
  • a waybar plugin
  • etc

@any1
Copy link
Owner

any1 commented Sep 30, 2022

I just had another thought that may be more security-conscious; what if instead of running these hooks directly, wayvnc published relevant events to a Unix domain socket, dbus service, or other IPC channel?

Yes, this would be more security-conscious. A unix socket with json encoded messages will do.

In order to fit the "confirmation dialog" flow, we would also want the ability for the client to send commands on the socket too (like to force a disconnect, query number of connected users, etc). In many ways this would be like sway's IPC socket protocol.

It could be called something like wayvncctl or wvctl.

@lack
Copy link
Collaborator Author

lack commented Nov 13, 2022

I think that with #178 this issue is complete!

At least my original request to have a way to run a script of some kind on client connect/disconnect.

Other events will have to be written for other use cases.

@lack lack closed this as completed Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants