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

Trigger Selector #14

Closed
maltfield opened this issue Oct 16, 2020 · 89 comments
Closed

Trigger Selector #14

maltfield opened this issue Oct 16, 2020 · 89 comments
Assignees
Labels
enhancement New feature or request v0.6.0 v0.7.0

Comments

@maltfield
Copy link
Member

This ticket is to track the development of a new feature permitting the user to choose the trigger that's executed.

Currently there is only one hard-coded trigger supported: lockscreen. When this feature is complete, there will be 1-3 triggers supported:

  1. Lockscreen
  2. Soft Shutdown
  3. Hard Shutdown (if implementable on the platform)

To achieve this, the following needs to be added

  1. Create a configuration file in the DATA_DIR to persist settings between runs
  2. In the CLI, implement a --list-triggers option allowing the user to list all available triggers on their platform
  3. In the CLI, implement a -t = --trigger option allowing the user to specify the trigger to be used at runtime. This is to be used in combination with the --arm argument
  4. In the CLI, implement a --set-default-trigger option, which updates the config file's default trigger to be used after arming (affects both the CLI and the GUI).
  5. In the GUI, add a "Settings" screen and cooresponding button to the navigation drawer to access it
  6. In the GUI, add a drop-down menu to select from the available triggers to the "Settings" screen, which updates the config file's default trigger to be used after arming (affects both the CLI and the GUI)

Note: this ticket should not attempt to handle auxiliary triggers (ie: self-destruct triggers or other custom triggers). This will be added later.

@maltfield maltfield self-assigned this Oct 16, 2020
@maltfield maltfield added the enhancement New feature or request label Oct 16, 2020
@stevenj2019
Copy link
Member

the issue here would be a pre requisite to this task.

Will start development now.

@mrx23dot
Copy link

mrx23dot commented Jan 19, 2022

No need to overcomplicate things, do a simple OS call, done

import os

def shutdown():
  if os.name == 'nt':
    # windows 
    cmd = 'shutdown /s /f'
  else:
    # posix
    cmd = 'poweroff' # alias for /sbin/shutdown now

  os.system(cmd)

# from https://stackoverflow.com/questions/34039845/how-to-shutdown-a-computer-using-python

others suggested

For Windows:
shutdown -s -f -t 1

For Linux:
shutdown -h now

or you can copy python code from here https://github.com/0xPoly/Centry

maltfield added a commit that referenced this issue Jul 6, 2022
… the appimage dir's python binary to execute the src/main.py file directly

Also getting started on updating the cli to support multiple triggers

 * #14
@maltfield
Copy link
Member Author

The first part of this feature (limited to the cli only) for implementing a shutdown trigger on all three platforms is being tracked in a branch here:

@maltfield
Copy link
Member Author

maltfield commented Oct 2, 2022

I made some progress on this, and I started implementing the shutdown function for Linux.

        import subprocess
        result = subprocess.run( ['shutdown', '-h', 'now'], capture_output=True, text=True )

Executing this results in

Traceback (most recent call last):
  File "src/main.py", line 142, in <module>
    ret = BusKillCLI()
  File "/home/user/sandbox/buskill-app/src/buskill_cli.py", line 91, in BusKillCLI
    result = subprocess.run( ['shutdown', '-h', 'now'], capture_output=True, text=True )
  File "/tmp/kivy_appdir/opt/python3.7/lib/python3.7/subprocess.py", line 488, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/tmp/kivy_appdir/opt/python3.7/lib/python3.7/subprocess.py", line 800, in __init__
    restore_signals, start_new_session)
  File "/tmp/kivy_appdir/opt/python3.7/lib/python3.7/subprocess.py", line 1551, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'shutdown': 'shutdown'

That's actually because shutdown is in sbin, which isn't in the normal user's $PATH. If we change it to the absolute path...

        import subprocess
        result = subprocess.run( ['/sbin/shutdown', '-h', 'now'], capture_output=True, text=True )

Then it works. Well, at least for me. But this is problematic because:

  1. The user may not have permission to run the shutdown command, and
  2. The shutdown binary may be located somewhere else (eg /usr/sbin/shutdown), depending on the distro

The solution to [1] above by Centry (as linked-to by mrx23dot) is to just run the whole app as root. Well, there will come a time when we definitely need root privileges, but that's a really bad idea. I'm not sure yet how to do this, but probably what we should do is spawn a very minimal child process with root privliges. This child process should:

  1. define very few functions (only those that require superuser permissions to do their job), and
  2. these functions should not accept any user input

I don't know how to do that, but I'll be looking out fo other python programs that do something similar until we need it. @mrx23dot if you're aware of any apps that do this, I'd be happy to hear about them :)

IMHO, the solution [2] is to figure out the path ASAP. Because of the time-sensitive nature of the trigger, we shouldn't wait until the trigger event to try to figure out the path to the binaries we need to shutdown. I'll add this to the init() of the buskill module. Long-term, we should probably have some sort of better way to inform the user if things don't look right. Like, when they hit the "arm" button, it should refuse to switch into "armed" mode if the app has reason to believe that it can't execute the trigger properly.

@maltfield
Copy link
Member Author

I added the logic for a soft shudown on Windows after some breif tests just on a windows VM using the python shell. I could not get the ctypes solution with ExitWindowsEx(0x00000008, 0x00000000) to work, but I did get the command line shutdown command to work with both os.system() and subprocess.

@maltfield
Copy link
Member Author

maltfield commented Oct 3, 2022

I tried my hand at manual tests of the existing Linux shutdown commands in MacOS, but none worked :(

I couldn't find any commands named systemctl nor poweroff in MacOS. I did find /sbin/shutdown, but it wouldn't let me execute it

user@host ~ % /sbin/shutdown -h now
shutdown: NOT super-user
user@host ~ % 

I did find an additional command = hault, but that also wanted root permssion :(

user@host ~ % which halt
/sbin/halt
user@host ~ % /sbin/halt
halt: Operation not permitted
user@host ~ % 

I tried this cute apple script, but that just triggers a popup on the GUI and asks the user to type their password. That's not going to work :'D

user@host ~ % osascript -e 'tell app "System Events" to shut down'
user@host ~ % 

@maltfield
Copy link
Member Author

maltfield commented Oct 3, 2022

meanwhile, this Windows build finished successfully:

I was able to download it on my Windows VM and run it. But there's this damn outstanding upstream issue that prevents the CLI from working on Windows

There's a hack where you can just append | more to the end of the command and it'll work. Here's it working for the new --list-triggers argument

C:\Users\localdomain\Downloads\buskill-win-cli-shutdown-trigger-x86_64\buskill-win-cli-shutdown-trigger-x86_64\buskill-cli-shutdown-trigger-x86_64>buskill.exe --list-triggers | more
buskill version {'VERSION': 'cli-shutdown-trigger', 'GITHUB_REF': 'refs/heads/cli-shutdown-trigger', 'GITHUB_SHA': '1803452a2d8ad235f7d03a1220b3fa530f4214a0', 'SOURCE_DATE_EPOCH': '1664839183'}
DEBUG: EXE_PATH:|C:\Users\localdomain\Downloads\buskill-win-cli-shutdown-trigger-x86_64\buskill-win-cli-shutdown-trigger-x86_64\buskill-cli-shutdown-trigger-x86_64\buskill.exe|
DEBUG: EXE_DIR:|C:\Users\localdomain\Downloads\buskill-win-cli-shutdown-trigger-x86_64\buskill-win-cli-shutdown-trigger-x86_64\buskill-cli-shutdown-trigger-x86_64|
DEBUG: EXE_FILE:|buskill.exe|
DEBUG: APP_DIR:|C:\Users\localdomain\Downloads\buskill-win-cli-shutdown-trigger-x86_64|
DEBUG: APPS_DIR:|C:\Users\localdomain\Downloads|
DEBUG: os.environ['PATH']:|C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Users\localdomain\AppData\Local\Microsoft\WindowsApps;;C:\Users\localdomain\Downloads\buskill-win-cli-shutdown-trigger-x86_64\buskill-win-cli-shutdown-trigger-x86_64\buskill-cli-shutdown-trigger-x86_64;C:\Users\localdomain\Downloads\buskill-win-cli-shutdown-trigger-x86_64|

INFO: using DATA_DIR:|C:\Users\localdomain\Downloads\.buskill|

Supported triggers include:
        lock-screen
        soft-shutdown


C:\Users\localdomain\Downloads\buskill-win-cli-shutdown-trigger-x86_64\buskill-win-cli-shutdown-trigger-x86_64\buskill-cli-shutdown-trigger-x86_64>

But | more only prints after the command finishes running. Meaning, you can use | more to arm BusKill with the soft-shutdown trigger from the CLI, but you the terminal will just stall without any output because the process is still running (listening for usb hotplug events and waiting to trigger)

Well, despite the fact that that I couldn't see what was going on, I gave it a try and started the above build of BusKill with the soft-shutdown trigger with the command

buskill.exe --trigger soft-shutdown --arm | more

I attached a virtual USB drive ([1] Start -> Disk Management (Create and format hard disk partitions) [2] Action -> Attach VHD). And then I ejected that drive.

I was successfully able to shutdown Windows 10 with the soft-shutdown trigger with this build from the CLI

About 1 second after I ejected the disk, the screen locked. Then about 3 seconds later, it took away the input field for the password and said that Windows was shutting down. About 30 seconds later, the machine was off. Awesome!!

@maltfield
Copy link
Member Author

maltfield commented Oct 4, 2022

As for MacOS, I can use sudo shutdown..., but it will prompt the user for their password. I could

  1. Make the user start the BusKill app as root and run the whole app as root (bad security practice)
  2. Make the user update their sudo config to have NOPASSWD sudo permission to execute the shutdown and halt commands for their user (terrible UX)
  3. Tell the user to start the app as root, but then spawn only a small child process with root permissions with only the trigger_softshutdown_mac() function, and drop privileges on the main process

Note: you can test to see if sudo shutdown is going to work by executing the following

sudo shutdown -k now "INFO: Buskill test of soft-shutdown permission. Please ignore."

The above command won't actually shut down the system. It just scares all the users by telling them it will. Anyway, if it exits 0, within 1 second, then we know that the app will be able to shutdown. This could be added to set_trigger('soft-shutdown') and raise an exception if it stalls (prompting for password) or exits non-zero.

Edit: You don't want to use shutdown -k. While it doesn't shutdown the system, it does prevent new logins. No idea what that would do for a normal macbook, but on my MacMini it prevented me from logging-in over ssh. And the machine is headless, so I had to login to MacStadium and trigger a reboot in the WUI. But if you don't have such a WUI, then that's a drive to the DC. I guess

user@buskill:~$ ssh mac


NO LOGINS: System going down at 01:17

INFO: BusKill test of shutdown permissions. Please ignore.

Connection closed by OBFUSCATED
user@buskill:~$

...but I guess this would only be bad for 0.001% of MacOS users 🤔

Actually, I think the best-case would be for the set_trigger() function to test if the shutdown can be called. If it can't, then the BusKill app attempts to automagically restart itself as root (maybe telling the user first and then somehow prompting them for their password for the escalation). Then it kicks-off the minimal child process as root and drops privileges of the main process.

@maltfield
Copy link
Member Author

Oh, I didn't mention it above but I already finished fully testing the linux soft-shutdown logic (before Windows). All that remains is MacOS :/

@maltfield
Copy link
Member Author

@maltfield
Copy link
Member Author

kicks-off the minimal child process as root and drops privileges of the main process.

Here's an example on how this was done in C++ https://stackoverflow.com/questions/58212418/how-to-have-a-non-root-parent-process-spawn-a-root-child-process-under-macos-x

@maltfield
Copy link
Member Author

maltfield commented Oct 4, 2022

I think Apple's native Objective C solution to this has historically been AuthorizationExecuteWithPrivileges(), more recently deprecated in favor of SMJobBless()

@maltfield
Copy link
Member Author

maltfield commented Oct 4, 2022

ok, this seems really silly, but it seems the most common way people popup a GUI dialog asking for auth to get root permissions is using apple script

I tried this over ssh and thought it was a dead-end because I couldn't get it to work

user@host ~ % osascript -e 'do shell script "whoami" with administrator privileges'
0:54: execution error: The administrator username or password was incorrect. (-60007)
user@host ~ % 

...but when I did it in a GUI terminal app over VNC, it actually worked

user@host ~ % osascript -e 'do shell script "whoami" with administrator privileges'
root
user@host ~ % 

As described, it just popped-up a window that said

osascript wants to make changes
Enter your password to allow this.
Username: Michael Altfield
Password: _____
[Cancel] [OK]

After I typed my password correctly into the GUI popup window, the terminal spat-out root

@maltfield
Copy link
Member Author

maltfield commented Oct 4, 2022

I just got the basic framework working for how to launch a new process as root from a non-root process via prompting the user for their password in MacOS.

I created two simple files. One is a script named root_child.py that just prints I am root! if it has root powers. Or it says I am not root :'( if it doesn't have root powers.

maltfield@host spawn_root % cat root_child.py 
#!/usr/bin/env python3
import os

try:
	os.setuid(0)
	print( "I am root!" )
except Exception as e:
	print( "I am not root :'(" )
maltfield@host spawn_root % 

root_child.py is launched from another script called spawn_root.py that just launches the above script in a subprocess as root using the apple script to prompt the user for their password

maltfield@host spawn_root % cat spawn_root.py 
#!/usr/bin/env python3
import os, subprocess

current_uid = os.getuid()
print( "current uid:|" +str(current_uid)+ "|" )

result = subprocess.run( [
 '/usr/bin/osascript', '-e', 'do shell script "./root_child.py" with administrator privileges'
 ], capture_output=True, text=True
 )

print( "returncode:|" +str(result.returncode)+ "|" )
print( "stdout:|" +str(result.stdout)+ "|" )
print( "stderr:|" +str(result.stderr)+ "|" )

maltfield@host spawn_root %

Again, it fails if I execute it in ssh

maltfield@host spawn_root % ./spawn_root.py     
current uid:|502|
returncode:|1|
stdout:||
stderr:|0:63: execution error: The administrator username or password was incorrect. (-60007)
|
maltfield@host spawn_root %  

But it works as desired if I execute it in the "Terminal" app in the GUI

maltfield@host spawn_root % ./spawn_root.py     
current uid:|502|
returncode:|0|
stdout:|I am root!
|
stderr:||
maltfield@host spawn_root %  

TODO: make the root process actually a Thread or multiprocessing.Process that's running asynchronously, daemonized, and can shutdown the system immediately if called by the parent process.

@maltfield
Copy link
Member Author

I'm not quite sure how to utilize this apple script (osascript) solution because:

  1. afaict, I can't use it with a new Thread (since a thread is passed a function, and I don't know htf to wrap a function with a command)
  2. afaict, the same problem exists for multiprocessing
  3. If I launch a new process with subprocess, then how do I communicate with it?

I trust (assume) that creating a new Thread or multiprocessing.Process would only be able to be communicated-with by my parent process (not some random malicious process). This is pretty darn important if someone wants to utilize self-destruct() function. We don't want some random process to be able to call our root-ified process that has the power to wipe everything at a second's notice.

But if I were to use subprocess, then I'd have to hack some strange communication between the processes. But what I really want is just to be able to call a function trigger_softshutdown_mac() and it runs as root.

@maltfield
Copy link
Member Author

I found some open-source code (part of the esky project) that has helper scripts to spawn subprocesses (with communication?) on Linux, Windows, and MacOS:

Here's the one for MacOS:

@maltfield
Copy link
Member Author

maltfield commented Oct 5, 2022

Even the esky project above is using a subprocess and "securely communicating with a sudo subprocess" via a SecureStringPipe, so I spent more time looking into this.

Immediate shutdown

First, I was able to launch a subprocess as root that immediately shuts down. As long as the parent process is executed from the Terminal app in the GUI (as the normal user), then it will prompt the user for their password. Even though the parent has the non-root user's privileges, the subprocess has root permission (uid=0, I confirmed with os.getuid()), and it is able to shutdown immediately.

This, of course, is not so useful. Rather, we want to be able to ask for the password early and then leave that root process in the background. But we need a way to communicate with the root child process to tell it if/when to shutdown. And the child process should only accept input from the parent, not other processes on the system.

Parent (spawn_root.py)

#!/usr/bin/env python3
import subprocess, sys

proc = subprocess.Popen( [
 '/usr/bin/osascript', '-e', 'do shell script "' +sys.executable+ ' root_child.py" with administrator privileges'
 ], stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True
)

Child (root_child.py)

#!/usr/bin/env python3
import os
os.system( "shutdown -h now" )

Non-root child + communication

I was able to get a (parent) process to spawn a child process and to send it commands. The child is very skeptical of anything it receives. First the parent tries to tell it to do something nasty; the child refuses. Then the parent tries to tell it to do something it doesn't understand how to do; the child ignores the command. Finally, the parent tells the child to do a soft-shutdown. The child tries, but it fails because the child is not root.

This is also not especially useful because we need the child to be root.

Parent (spawn_child.py)

#!/usr/bin/env python3
import subprocess, sys

proc = subprocess.Popen(
 [sys.executable, 'child.py'],
 stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True
)

print( "sending malicious command now" )
proc.stdin.write( "Robert'); DROP TABLE Students;\n" )
proc.stdin.flush()
print( proc.stdout.readline() )

print( "sending invalid command now" )
proc.stdin.write( "make-me-a-sandwich\n" )
proc.stdin.flush()
print( proc.stdout.readline() )

print( "sending soft-shutdown command now" )
proc.stdin.write( "soft-shutdown\n" )
proc.stdin.flush()
print( proc.stdout.readline() )

proc.stdin.close()

Child (child.py)

#!/usr/bin/env python3
import os, sys, re

def soft_shutdown():
        try:
                os.system( "shutdown -h now" )
        except Exception as e:
                print( "I am not root :'(" )

if __name__ == "__main__":

        # loop and listen for commands from the parent process
        while True:

                command = sys.stdin.readline().strip()

                # check sanity of recieved command. Be very suspicious
                if not re.match( "^[A-Za-z_-]+$", command ):
                        sys.stdout.write( "ERROR: Bad Command Ignored\n" )
                        sys.stdout.flush()
                        continue

                if command == "soft-shutdown":
                        soft_shutdown()
                        continue
                else:
                        sys.stdout.write( "WARNING: Unknown Command Ignored\n" )
                        sys.stdout.flush()
                        continue

Example Execution

user@host spawn_no_root_communication_working % ./spawn_child.py 
sending malicious command now
ERROR: Bad Command Ignored

sending invalid command now
WARNING: Unknown Command Ignored

sending soft-shutdown command now
shutdown: NOT super-user

Note that the execution hangs at the end. The parent is waiting for the child to exit, but it won't because it's (intentionally) in an infinite loop.

Root Child (Communication Fail)

If I combine the above two, then execution stalls because there appears to be some communication breakdown between the two processes. And it's driving me insane.

Parent (spawn_root.py)

#!/usr/bin/env python3
import subprocess, sys

proc = subprocess.Popen( [
 '/usr/bin/osascript', '-e', 'do shell script "' +sys.executable+ ' root_child.py" with administrator privileges'
 ], stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True
 )

print( "sending malicious command now" )
proc.stdin.write( "Robert'); DROP TABLE Students;\n" )
proc.stdin.flush()
print( proc.stdout.readline() )

print( "sending invalid command now" )
proc.stdin.write( "make-me-a-sandwich\n" )
proc.stdin.flush()
print( proc.stdout.readline() )

print( "sending soft-shutdown command now" )
proc.stdin.write( "soft-shutdown\n" )
proc.stdin.flush()
print( proc.stdout.readline() )

proc.stdin.close()

Child (root_child.py)

#!/usr/bin/env python3
import os, sys, re

def soft_shutdown():
        try:
                os.system( "shutdown -h now" )
        except Exception as e:
                print( "I am not root :'(" )

if __name__ == "__main__":

        # loop and listen for commands from the parent process
        while True:

                command = sys.stdin.readline().strip()

                # check sanity of recieved command. Be very suspicious
                if not re.match( "^[A-Za-z_-]+$", command ):
                        sys.stdout.write( "ERROR: Bad Command Ignored\n" )
                        sys.stdout.flush()
                        continue

                if command == "soft-shutdown":
                        soft_shutdown()
                        continue
                else:
                        sys.stdout.write( "WARNING: Unknown Command Ignored\n" )
                        sys.stdout.flush()
                        continue

Example Execution

user@host spawn_no_root_communication_working % ./spawn_child.py 
sending malicious command now

It just stalls there after the parent attempts to send the message to the child. The parent is blocked while waiting for the response from the child.

@maltfield
Copy link
Member Author

@maltfield
Copy link
Member Author

maltfield commented Oct 6, 2022

I was able to confirm that communication works between the parent & root child process when using sudo.

This works on both Linux and MacOS.

Parent (spawn_root.py)

#!/usr/bin/env python3
import subprocess, sys

from subprocess import PIPE, Popen
from getpass import getpass

proc = subprocess.Popen(
 [ 'sudo', sys.executable, 'root_child.py' ],
 stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True
)

print( "sending soft-shutdown command now" )
proc.stdin.write( "soft-shutdown\n" )
proc.stdin.flush()
print( proc.stdout.readline() )

proc.stdin.close()

Child (root_child.py)

#!/usr/bin/env python3
import os, sys, re, subprocess

if __name__ == "__main__":

	# loop and listen for commands from the parent process
	while True:

		command = sys.stdin.readline().strip()

		# check sanity of recieved command. Be very suspicious
		if not re.match( "^[A-Za-z_-]+$", command ):
			sys.stdout.write( "ERROR: Bad Command Ignored\n" )
			sys.stdout.flush()
			continue

		if command == "soft-shutdown":
			try:
				proc = subprocess.Popen(
#				 [ 'sudo', 'shutdown', '-h', 'now' ],
				 [ 'sudo', 'reboot' ],
				 stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True
				)
				sys.stdout.write( "SUCCESS: I am root!\n" )
				sys.stdout.flush()
			except Exception as e:
				sys.stdout.write( "ERROR: I am not root :'(\n" )
				sys.stdout.flush()
				sys.exit(0)

			continue
		else:
			sys.stdout.write( "WARNING: Unknown Command Ignored\n" )
			sys.stdout.flush()
			continue

Example Execution

maltfield@host simple % ./spawn_root.py  
sending soft-shutdown command now
SUCCESS: I am root!

Traceback (most recent call last):
  File "root_child.py", line 14, in <module>
    sys.stdout.flush()
BrokenPipeError: [Errno 32] Broken pipe
Exception ignored in: <_io.TextIOWrapper name='<stdout>' mode='w' encoding='UTF-8'>
BrokenPipeError: [Errno 32] Broken pipe
maltfield@host simple % Connection to REDACTED closed by remote host.
Connection to REDACTED closed.
user@buskill:~$

Note that I'm using reboot instead of shutdown in this test because last time I tried a shutdown test, I had to call remote hands at MacStadium to press the power button on my Mac Mini. There's a "turn on" button in the WUI, but it didn't work last time 🤦

Anyway, you can see that it prints out that it's root and then immediately the session is killed because the machine rebooted. Success!

However, if I take the exact same script but I replace the root_child.py execution from sudo to apple script (for GUI privilege escalation), then it just gets stuck here

diff

maltfield@host spawn_root_sudo_communication_test % diff simple/spawn_root.py simple_gui/spawn_root.py 
8c8,9
<  [ 'sudo', sys.executable, 'root_child.py' ],
---
> # [ 'sudo', sys.executable, 'root_child.py' ],
>  ['/usr/bin/osascript', '-e', 'do shell script "' +sys.executable+ ' root_child.py" with administrator privileges' ],
maltfield@host spawn_root_sudo_communication_test % 

Example Execution

maltfield@host simple_gui % ./spawn_root.py
sending soft-shutdown command now

maltfield added a commit that referenced this issue Nov 12, 2022
 * made the background of the settings screen grey by using canvas
 * switched from BoxView to GridView to get the list of items in the settings screen to appear as rows (still not 100% functional)
 * gave a temp green/red background to ^ these widgets to aid in getting their placements as-desired

I'm committing this now because I want to change to expierimenting with (a possibly modified version of) Kivy's settings module, rather than re-invent the wheel

 * https://kivy.org/doc/stable/api-kivy.uix.settings.html

Pros of using the settings module:

 1. I don't have to re-invent the wheel; the code is already there and maintained by the kivy team
 2. It has code re-use that automatically parses a config file and displays re-usable kivy elements

Cons of using the settings module

 a. The documentation describes how to persist config options to a JSON file, but I want to use an ini-style format so it's easier to read & edit by-hand by advanced users. However, it appears that the Settings module just uses the kivy config object, which actually just extends the Python ConfigParser. And the default kivy config file (~/.kivy/config.ini) is in-fact an ini file. So this can likely be overcome

 * https://kivy.org/doc/stable/api-kivy.config.html#kivy.config.ConfigParser
 * https://docs.python.org/3/library/configparser.html

b. I don't 100% like the way it looks. It doesn't match what I had expected following the Material Design spec, which may impact UX. But I could always add my own custom classes that extend or modify the api-kivy.uix.settings module to suit our needs..

 * #14
@maltfield
Copy link
Member Author

I spent some time (see last commit above) trying to build a custom Settings screen, but I finally realized I'm just re-inventing the wheel. Kivy has a built-in settings module to handle this that dynamically renders a customizable settings screen from a json file. I didn't want to use this at first because I want to use ini instead of json, but once I realized it's built on-top of Python's ConfigParser, I think I can actually make it an INI (which is actually what kivy's config file format is by default)

@maltfield
Copy link
Member Author

It took a horrible amount of time to work out all the kinks of the Settings screen (and therefore the Config File) in #16, but I think we're finally done with that, which was blocking this.

As part of #16, I already implemented this feature too. The trigger setting and its two possible options ('lock-screen' and soft-shutdown) are defined in the Settings .json file. Then the user's choice is written to the buskill.ini config file, which has now also been merged with the built-in kivy settings (though in distinct sections of the .ini file).

For the most part we're just kivy's built-in Settings modules:

...however, I chose not to us their panels that breaks the settings into sections as it's not consistent with Material Design.

Also, for this particular ticket's trigger Setting, I implemented a custom Setting Type = ComplexOption. The kivy modules include an Option, but I wanted something more complex because I wanted to be able to present more information to the user so they understood what each trigger did and its consequences.

When the user clicks the ComplexSetting, it brings them to a whole new screen (as opposed to opening a modal). This screen has a help button in the ActionBar that tells the user what a trigger is in-general. And the Screen is populated with a list of ComplexOptionItems, which currently includes just lock-screen and soft-shutdown. Because it's on its own screen, we can display both a short name for each trigger and a longer description for each trigger. And an icon for better UX.

We also added some "confirmation" messages to the settings json for the ComplexSetting. If this is set, then it'll display the message in a modal and make the user confirm yes/no before the setting is changed to the given Option. This is important, for example, for some dangerous settings -- such as setting the trigger to soft-shutdown, which may cause data loss (eg if the user is typing an email at the time the BusKill trigger is executed).

I think this ticket is done, but I'll just do some tests of the build on all platforms before I merge this into dev and close this issue.

@maltfield
Copy link
Member Author

I'm doing some tests of the most recent linux build, and I'm sad to say that it doesn't actually shutdown my Debian Linux AppVM

INFO: Detected USB removal event
calling <bound method BusKill.triggerLin of <packages.buskill.BusKill object at 0x7a08c5cd4150>>
DEBUG: BusKill soft-shutdown trigger executing now
INFO: Attempting to execute `poweroff -h`
WARNING: Failed to execute `poweroff -h`!
INFO: Attempting to execute `systemctl poweroff`
ERROR: Failed to execute `systemctl poweroff`! expected str, bytes or os.PathLike object, not NoneType

It detects that the USB drive has been removed. The poweroff command doesn't exist, so it falls-back with systemctl poweroff, which fails. Curiously, if I execute systemctl poweoff from the command-line on this VM, then it executs fine. So it looks like some python implementation error.

@maltfield
Copy link
Member Author

I just tested the latest Windows build, and I confirmed that I was able to arm and lock the screen on the default settings. Then I went to the new Settings Screen, changed the trigger to soft-shutdown, re-armed when prompted, and then the eject successfully triggered an immediate soft shutdown of the Windows 10 HVM.

@maltfield
Copy link
Member Author

maltfield commented Jun 13, 2023

I'm not really able to test a trigger of the app on my remote Mac Mini, but I did open the latest build of this GUI app, change the trigger setting from lock-screen to soft-shutdown and then try to arm it from the CLI.

I found that the GUi was able to update the buskill.ini file, and the CLI used the same buskill.ini file -- but the buskil CLI still defaulted to using the lock-screen trigger, even though the config file was set to soft-shutdown.

Hmm..even if I open the GUI app and press the 'arm' button, it arms with the lock-screen trigger (even though I go to the config file and the GUI Settings Screen and both say the trigger is set to soft-shutdown). If I disarm & re-arm, it keeps arming with lock-screen. BUT if I go to the Settings Screen and then immediately go back (without changing anything or even going into the trigger ComplexSetting sub-screen), it switches to the soft-shutdown trigger.

I confirmed that this bug is present in both Linux and Windows :(

maltfield added a commit that referenced this issue Jun 13, 2023
 * #14 (comment)

This commit fixes a bug where both the CLI and GUI version of this app would startup with the 'lock-screen' trigger, even if the config file was set to use the 'soft-shutdown' trigger.

I added some logic to the buskill class to figure out what the setting was set-to in the config file, rather than just hard-coding it to lock-screen -- since we now have a config file
@maltfield
Copy link
Member Author

maltfield commented Jun 13, 2023

I fixed the buskill module to actually check the config file when first setting its self.trigger, instead of just hard-coding it to lock-screen on __init__()

That also required updating the CLI's ArgParser to not default-to (and call set_trigger() for) lock-screen

I confirmed that this build fixes the issue in Linux, MacOS, and Windows 10

@maltfield
Copy link
Member Author

maltfield commented Jun 13, 2023

regarding the shutdown bug on Linux, I found two distinct issues:

  1. The trigger_softshutdown_lin_poweroff() function was accidentally named trigger_softshutdown_lin_shutdown() -- overriding the function so that shutdown -h now is never actually attempted to be executed
  2. I was setting self.trigger directly in the buskill object's __init__() function, which means the paths to the shutdown binaries were never discovered, as is done when using set_trigger(). Because of this, the path to the systemctl binary was set to None -- hence the exception from subprocess above

maltfield added a commit that referenced this issue Jun 13, 2023
We have to call set_trigger(), else the paths to the binaries never gets set

 * #14 (comment)
@maltfield
Copy link
Member Author

I was able to test that [2] above was fixed by fixing only it first. After testing that fix, I just fixed [1] as well

@maltfield
Copy link
Member Author

found another bug: my call to set_trigger() doesn't fallback. This is only an issue in the CLI

user@disp6142:~/buskill-lin--x86_64$ ./buskill-.AppImage --arm
...
INFO: using DATA_DIR:|/home/user/.local/share/.buskill|
DEBUG: CONF_FILE:|/home/user/.local/share/.buskill/config.ini|

Traceback (most recent call last):
  File "/tmp/.mount_buskilReWwl8/opt/python3.7/lib/python3.7/configparser.py", line 788, in get
    value = d[option]
  File "/tmp/.mount_buskilReWwl8/opt/python3.7/lib/python3.7/collections/__init__.py", line 916, in __getitem__
    return self.__missing__(key)            # support subclasses that define __missing__
  File "/tmp/.mount_buskilReWwl8/opt/python3.7/lib/python3.7/collections/__init__.py", line 908, in __missing__
    raise KeyError(key)
KeyError: 'trigger'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/.mount_buskilReWwl8/opt/src/main.py", line 129, in <module>
    bk = packages.buskill.BusKill()
  File "/tmp/.mount_buskilReWwl8/opt/src/packages/buskill/__init__.py", line 426, in __init__
    self.set_trigger( self.config.get('buskill', 'trigger') )
  File "/tmp/.mount_buskilReWwl8/opt/python3.7/lib/python3.7/configparser.py", line 791, in get
    raise NoOptionError(option, section)
configparser.NoOptionError: No option 'trigger' in section: 'buskill'
user@disp6142:~/buskill-lin--x86_64$ 

maltfield added a commit that referenced this issue Jun 13, 2023
The GUI defaulted to 'lock-screen', but the CLI did not. And it was crashing if 'trigger' wasn't defined in the config file when executed over the CLI. This fixes it

 * #14 (comment)

	INFO: using DATA_DIR:|/home/user/.local/share/.buskill|
	DEBUG: CONF_FILE:|/home/user/.local/share/.buskill/config.ini|

	Traceback (most recent call last):
  	File "/tmp/.mount_buskilReWwl8/opt/python3.7/lib/python3.7/configparser.py", line 788, in get
    	value = d[option]
  	File "/tmp/.mount_buskilReWwl8/opt/python3.7/lib/python3.7/collections/__init__.py", line 916, in __getitem__
    	return self.__missing__(key)            # support subclasses that define __missing__
  	File "/tmp/.mount_buskilReWwl8/opt/python3.7/lib/python3.7/collections/__init__.py", line 908, in __missing__
    	raise KeyError(key)
	KeyError: 'trigger'

	During handling of the above exception, another exception occurred:

	Traceback (most recent call last):
  	File "/tmp/.mount_buskilReWwl8/opt/src/main.py", line 129, in <module>
    	bk = packages.buskill.BusKill()
  	File "/tmp/.mount_buskilReWwl8/opt/src/packages/buskill/__init__.py", line 426, in __init__
    	self.set_trigger( self.config.get('buskill', 'trigger') )
  	File "/tmp/.mount_buskilReWwl8/opt/python3.7/lib/python3.7/configparser.py", line 791, in get
    	raise NoOptionError(option, section)
	configparser.NoOptionError: No option 'trigger' in section: 'buskill'
	user@disp6142:~/buskill-lin--x86_64$
	user@disp6142:~/buskill-lin--x86_64$ ./buskill-.AppImage --arm
@maltfield
Copy link
Member Author

maltfield commented Jun 13, 2023

I just tested the latest build in Linux, it appears to be full functional

And it's fully functional on Windows

I couldn't find any flaws in the latest MacOS build, but I can't test it fully. I noticed that if the trigger is set to soft-shutdown in the config file when the app is launched, then MacOS asks for my password when the app starts (this is to get root permission). But I noticed that if the app starts with lock-screen and then the user changes the Settings to soft-shutdown without restarting the app, and then the click the arm button, then the UI changes to armed but without prompting me for my password. This makes me think that the shutdown will fail because it's not running as root

I confirmed with ps that the child process listening for USB removals doesn't get started as root (uid=0) when the trigger is changed to soft-shutdown in the same runtime.

Working

This is what it looks like when it's working (this happens when the GUI app is started when trigger is set to soft-shutdown in the config file already and the user is prompted for the password).

First we start the app

host:~ root# ps -ef | grep -i buskill
  502 40371     1   0 11:19PM ??         0:01.48 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill
    0 40375 40371   0 11:19PM ??         0:00.11 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/root_child_mac /var/folders/kx/2fp3kfgj4dj7rx9s5mlb52640000gp/T/buskill.log
    0 40381 40338   0 11:19PM ttys000    0:00.00 grep -i buskill
host:~ root# 

Then we arm it

host:~ root# ps -ef | grep -i buskill
  502 40371     1   0 11:19PM ??         0:02.43 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill
    0 40375 40371   0 11:19PM ??         0:00.11 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/root_child_mac /var/folders/kx/2fp3kfgj4dj7rx9s5mlb52640000gp/T/buskill.log
  502 40382 40371   0 11:19PM ??         0:00.35 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill -B -S -E -s -c from multiprocessing.semaphore_tracker import main;main(24)
  502 40383 40371   0 11:19PM ??         0:00.36 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill --multiprocessing-fork tracker_fd=25 pipe_handle=27
    0 40391 40338   0 11:19PM ttys000    0:00.00 grep -i buskill
host:~ root# 

Not working

First we start the app

host:~ root# ps -ef | grep -i buskill
  502 40399     1   0 11:20PM ??         0:01.39 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill
    0 40404 40338   0 11:20PM ttys000    0:00.00 grep -i buskill
host:~ root# 

Then we arm it

host:~ root# ps -ef | grep -i buskill
  502 40399     1   0 11:20PM ??         0:01.39 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill
    0 40404 40338   0 11:20PM ttys000    0:00.00 grep -i buskill
5129:~ root# ps -ef | grep -i buskill
  502 40399     1   0 11:20PM ??         0:02.01 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill
  502 40406 40399   0 11:20PM ??         0:00.34 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill -B -S -E -s -c from multiprocessing.semaphore_tracker import main;main(5)
  502 40407 40399   0 11:20PM ??         0:00.36 /Volumes/buskill-gui-shutdown-trigger/buskill-gui-shutdown-trigger.app/Contents/MacOS/buskill --multiprocessing-fork tracker_fd=21 pipe_handle=23
    0 40415 40338   0 11:20PM ttys000    0:00.00 grep -i buskill
host:~ root# 

@maltfield
Copy link
Member Author

As before, it appears that the issue was that I was directly setting the instance field of bk.trigger when I should have been using set_trigger() -- which is where the logic to spawn a root child process lives. I've changed that in the latest commit

maltfield added a commit that referenced this issue Jun 13, 2023
this commit uses the set_trigger() function of the buskill object after the user changes the trigger in the Settings Screen. If we don't use the function, then the logic for the new trigger isn't followed -- such as finding the relevant `shutdown` binaries and spawning a root child process on MacOS, if needed

 * #14 (comment)
@maltfield
Copy link
Member Author

hmm...when testing on my Mac Mini by executing the main.py script directly for faster iteration, I found that it does prompt me for my password to spawn root child process if it doesn't yet exist (eg if the app was started with the lock-screen trigger set at-launch.

However, I realized that (as I knew before), MacOS can't do this privilege escalation outside the GUI. I was getting errors if I tried it by executing it in an ssh session. Instead, I had to execute it in the Terminal.app that was running inside the GUI (which I access over VNC).

But if that error occurs, it gets dumped to the DEBUG log and the GUI app continues along like nothing happened. The UI says that BusKill is armed, even though it probably won't work.

TODO:

  1. Test again with the GitHub-built pre-release and see if it's attempting to spawn the root child at all (check DEBUG logs). if not, see why that differs from my local runs
  2. Implement better fatal error reporting in the GUI, and don't let the user hit the "Arm" button if set_trigger() doesn't succeed.

@maltfield
Copy link
Member Author

I just tried again with the latest BusKill build for MacOS, and it doesn't even appear to be trying to all set_trigger()?

20:18:29,637 buskill_gui DEBUG DEBUG: Re-arm from 'lock-screen' to 'soft-shutdown' trigger?
20:18:31,660 packages.buskill DEBUG DEBUG: attempting to disarm BusKill
20:18:31,664 packages.buskill INFO INFO: BusKill is disarmed.
20:18:31,668 packages.buskill DEBUG DEBUG: attempting to arm BusKill via <bound method BusKill.armNix of <packages.buskill.BusKill object at 0x111101110>>() with the 'soft-shutdown' trigger

The first lines of set_trigger() should output something like:

03:41:18,855 packages.buskill DEBUG DEBUG: Attempting to set 'trigger' set to 'soft-shutdown'

...but that line is clearly missing, which makes me think that set_trigger() isn't being called at all?

@maltfield
Copy link
Member Author

maltfield commented Jun 14, 2023

I downloaded the latest version of the MacOS build, and it does appear to be fully functional

I mounted the dmg. And then in the GUI Terminal.app, I launched the app, clicked the Arm button, changed the trigger to soft-shutdown in the Settings screen, and returned to the main Screen. Before I even clicked Disarm & Arm now, it prompted me for my password and the DEBUG log showed that it spawned the root child process. Then when I clicked to re-arm, it did so. At this point I could change the trigger setting back-and-forth between lock-screen and soft-shutdown, and it wouldn't re-prompt me for the password because the root child process remains in the background. Awesome :)

The only thing that's a bit weird about this process is that the user may not understand why they're being prompted for their password when clicking the "back" arrow. It might make more sense to move the call of set_trigger() to the specific OptionItem, but that would be much less clean from the code perspective.

I'll leave it as-is for now.

@maltfield
Copy link
Member Author

Alright, that's that. I'm merging this into dev!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v0.6.0 v0.7.0
Projects
None yet
Development

No branches or pull requests

3 participants