0

I have written a small python program (screen.py) that uses the keyboard lib. The essence of the program is to take screenshots by pressing ctrl + 1 and send them to my github repository by pressing ctrl + 2. Using the keyboard library requires that the program must be launched via sudo.

screen.py:

import keyboard
import pyautogui
import os

screenshot_num = 0

PICS_DIR = os.path.join(os.getcwd(), "pics")
SEND_TO_GIT_SCRIPT = os.path.join(os.getcwd(), "send.sh")
os.makedirs(PICS_DIR, exist_ok=True)


def make_screenshot():
    global screenshot_num
    myScreenshot = pyautogui.screenshot()
    myScreenshot.save(os.path.join(PICS_DIR, str(screenshot_num) + '.png'))
    screenshot_num += 1


def send_to_git():
    os.system(SEND_TO_GIT_SCRIPT)


keyboard.add_hotkey('Ctrl + 1', make_screenshot)
keyboard.add_hotkey('Ctrl + 2', send_to_git)

keyboard.wait('Alt + q')

The problem is that when the program execution reaches the call to send.sh (code below), I get the following error:

ERROR:

» sudo python3 screen.py
New Pics
 3 files changed, 0 insertions(+), 0 deletions(-)
 rewrite screen/pics/0.png (97%)
 rewrite screen/pics/1.png (97%)
 rewrite screen/pics/2.png (97%)
git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

send.sh:

git add ./pics/*
git commit -m 'New Pics'
git push

I connect to github via ssh, the key was created via sudo ssh-keygen. I connect to github via ssh. I have added keys generated both with and without sudo.

  • Tangentially, like the `os.system` documentation already tells you, much better to use `subprocess` here. The advice in [Actual meaning of `shell=True` in `subprocess`](https://stackoverflow.com/questions/3172470/actual-meaning-of-shell-true-in-subprocess) applies in spades to `os.system`, too. – tripleee May 19 '22 at 18:44
  • 1
    The git commands are configured for your default user, thus the root user does not know which settings to use for your push and crashes. Try to avoid running as root, –  May 19 '22 at 18:45
  • Is there a way to run a program without superuser rights inside a python program that has them? – HeartMarshall May 19 '22 at 18:46
  • I understand that git commands should not be executed using sudo. But this inevitably happens, because I have to run the main script through sudo: `sudo python3 screen.py` – HeartMarshall May 19 '22 at 18:55
  • It's generally unwise to run *any* Python program as the super-user because of the difficulty of writing secure Python code (this applies to some extent to *all* programming languages but Python programs are particularly prone to problems). Still, once you *are* running as root, you can switch users *back* to the *original* user to run additional commands: the super-user has all permissions so it's trivial to switch *back to you* to run the `git push`. – torek May 19 '22 at 21:24
  • 1
    In particular, note that `sudo` has `-u ` so that you can simply `sudo -u ...`. The original user name is in `$SUDO_USER` (in the environment). – torek May 19 '22 at 21:27
  • Meanwhile, secure programs that temporarily use superuser privileges should do whatever they need to do *with* those permissions, then *permanently give up* those permissions. In compiled languages like C, without a lot of dynamic loading, this gets you closer to reasonably secure. There are still a lot of security issues due to, e.g., dynamic linking the `libc` library! – torek May 19 '22 at 21:29

1 Answers1

0

You need to run the git commands as your regular user.

Unfortunately, Python only allows you to drop the privileges of the current process; you can then no longer switch back if you need to run some privileged code still.

Best practice and generally just good security hygiene would be to drop root privileges for everything which doesn't specifically require them. I tried to refactor your code to run as little as possible as root, but that turned out to be hard in this case. One would have to carve out parts into subprocesses, which doesn't really make much sense here.

Here's a quick and dirty refactoring which runs the shell subprocess as the invoking user, and fixes the ownership of any created files to avoid ending up with root-owned files created by a regular user, possibly in their home directory or elsewhere where they should be in control of the files.

import os
import subprocess

import keyboard
import pyautogui


screenshot_num = 0

# no need for os.getcwd() here
# see https://stackoverflow.com/questions/45591428
PICS_DIR = "pics"
SEND_TO_GIT_SCRIPT = os.path.join(".", "send.sh")


def fix_owner(filename: str) -> None:
    """
    Ensure that the file is owned by the unprivileged invoking user
    """
    os.chown(filename, int(os.environ["SUDO_UID"]), int(os.environ["SUDO_GID"]))

def unprivileged_subprocess(command: str) -> CompletedProcess:
    """
    Run the shell commands as the unprivileged invoking user
    """
    return subprocess.run(
        ["su", "-", os.environ["SUDO_USER"], "-c", command],
        check=True)


os.makedirs(PICS_DIR, exist_ok=True)
fix_owner(PICS_DIR)

def make_screenshot():
    global screenshot_num
    filename = os.path.join(PICS_DIR, str(screenshot_num) + '.png')
    myScreenshot = pyautogui.screenshot(filename)
    fix_owner(filename)
    screenshot_num += 1

def send_to_git():
    unprivileged_subprocess(SEND_TO_GIT_SCRIPT)


keyboard.add_hotkey('Ctrl + 1', make_screenshot)
keyboard.add_hotkey('Ctrl + 2', send_to_git)

keyboard.wait('Alt + q')

This will crash with an unhelpful error message if the expected environment variables etc are not populated; perhaps add an explicit check to exit with a user-friendly instruction before attempting to do anything really if the script is not being run with sudo.

Perhaps a better general design would be to run the privileged code as a persistent subprocess (subprocess.Popen) and drop the privileges of the main script immediately after starting that up. Unfortunately, I don't know enough about keyboard or PyAutoGUI to attempt to implement that in this context. In vague terms, have something open the keyboard device as root, then communicate the results to the caller somehow? Or perhaps it would be enough to just open the device in privileged mode and then drop privileges for all the subsequent processing.

tripleee
  • 175,061
  • 34
  • 275
  • 318