Check for unnecessary privilege escalation (#1743)

Resolves tiny-pilot/tinypilot-pro#1214
<s>Blocked by https://github.com/tiny-pilot/tinypilot/pull/1744</s>
<s>Blocked by https://github.com/tiny-pilot/tinypilot/pull/1745</s>

This PR adds a dev script that checks for possible cases of privilege
escalation in tinypilot-writable scripts (i.e., `scripts/`).

The script only does a superficial check that root privileges were at
least considered by matching on:
> This script doesn't require root privileges.

Example output of `dev-scripts/check-privilege-guard`:
```
$ ./dev-scripts/check-privilege-guard 
These files are missing a guard against privilege escalation:
scripts/is-ssh-enabled
scripts/streaming-mode
scripts/update-service
scripts/upgrade
Please add the following check (or similar) to the above scripts:
if [[ "${EUID}" == 0 ]]; then
  >&2 echo "This script doesn't require root privileges."
  >&2 echo 'Please re-run as tinypilot:'
  >&2 echo "  runuser tinypilot --command '$0 $*'"
  exit 1
fi
```

Notes

1. <s>These tinypilot-writable scripts legitimately require root
privileges:
    * `scripts/install-bundle`
    * `script/upgrade`
   
So they do risk being used for privilege escalation, but they are/should
never be executed by privileged scripts on the device.
    
    I've also added a superficial check for this too.</s>
2. This PR also fixes the privilege escalation issues that
`dev-scripts/check-privilege-guard` as picked up. As a reminder, the fix
is a runtime error asking for reduced permissions which is something
we'll only encounter when we physically test the device. So as a result,
this PR also tries to avoid those runtime errors by running these
identified scripts as `tinypilot` where needed:
    ```
    runuser tinypilot --command '/opt/tinypilot/scripts/some-script'
    ```

<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1743"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

.circleci/config.yml

@@ -22,6 +22,14 @@ jobs:
  - run:
  name: Run static analysis on bash scripts
  command: ./dev-scripts/check-bash
+ check_privilege_guard:
+ docker:
+ - image: cimg/base:2020.01
+ steps:
+ - checkout
+ - run:
+ name: Check for unnecessary privilege escalation
+ command: ./dev-scripts/check-privilege-guard
  lint_sql:
  docker:
  - image: cimg/python:3.9.17
@@ -272,6 +280,7 @@ workflows:
  jobs:
  - check_whitespace
  - check_bash
+ - check_privilege_guard
  - lint_sql
  - check_style
  - decode_edid

debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs

@@ -109,7 +109,7 @@ print_info "Checking if filesystem is read-only..."
 print_info "Checking if SSH is enabled..."
 {
  SSH_STATUS="disabled"
- if /opt/tinypilot/scripts/is-ssh-enabled ; then
+ if runuser tinypilot --command '/opt/tinypilot/scripts/is-ssh-enabled' ; then
  SSH_STATUS="enabled"
  fi
  readonly SSH_STATUS
@@ -166,7 +166,7 @@ print_info "Checking for voltage issues..."
 print_info "Checking TinyPilot streaming mode..."
 {
  echo "Streaming mode"
- echo "Selected mode: $(/opt/tinypilot/scripts/streaming-mode)"
+ echo "Selected mode: $(runuser tinypilot --command '/opt/tinypilot/scripts/streaming-mode')"
  printf "Current mode: "
  # H264 mode is considered active when the last Janus video log line contains
  # "Memsink opened; reading frames".

dev-scripts/check-privilege-guard

@@ -0,0 +1,34 @@
+#!/bin/bash
+#
+# Check that the TinyPilot scripts contain a guard against privilege escalation.
+#
+# This script enforces a pattern of ensuring that scripts which are writable by
+# the `tinypilot` user don't get executed with unnecessary root privileges.
+
+# Exit on first failing command.
+set -e
+
+# Exit on unset variable.
+set -u
+
+# Find TinyPilot scripts that don't guard against privilege escalation.
+MATCHES="$(grep \
+ --files-without-match \
+ --fixed-strings \
+ --regexp "This script doesn't require root privileges." \
+ scripts/*; true)"
+readonly MATCHES
+if [[ -n "${MATCHES}" ]]; then
+ >&2 echo 'These files are missing a guard against privilege escalation:'
+ >&2 echo "${MATCHES}"
+ >&2 echo 'Please add the following check (or similar) to the above scripts:'
+ >&2 cat <<'EOF'
+if [[ "${EUID}" == 0 ]]; then
+ >&2 echo "This script doesn't require root privileges."
+ >&2 echo 'Please re-run as tinypilot:'
+ >&2 echo " runuser tinypilot --command '$0 $*'"
+ exit 1
+fi
+EOF
+ exit 1
+fi

scripts/is-ssh-enabled

@@ -8,6 +8,13 @@ set -u
 # Exit on first error.
 set -e
 
+if [[ "${EUID}" == 0 ]]; then
+ >&2 echo "This script doesn't require root privileges."
+ >&2 echo 'Please re-run as tinypilot:'
+ >&2 echo " runuser tinypilot --command '$0 $*'"
+ exit 1
+fi
+
 print_help() {
  cat << EOF
 Usage: ${0##*/} [-h]

scripts/streaming-mode

@@ -8,6 +8,13 @@ set -e
 # Exit on unset variable.
 set -u
 
+if [[ "${EUID}" == 0 ]]; then
+ >&2 echo "This script doesn't require root privileges."
+ >&2 echo 'Please re-run as tinypilot:'
+ >&2 echo " runuser tinypilot --command '$0 $*'"
+ exit 1
+fi
+
 SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
 readonly SCRIPT_DIR
 cd "${SCRIPT_DIR}/.."

scripts/update-service

@@ -10,7 +10,9 @@
 
 import argparse
 import logging
+import os
 import subprocess
+import sys
 
 # We’re importing the log package first because it needs to overwrite the
 # app-wide logger class before any other module loads it.
@@ -61,6 +63,15 @@ def main(_):
 
 
 if __name__ == '__main__':
+ # Ensure that the script doesn't have unnecessary privileges.
+ if os.geteuid() == 0:
+ print("This script doesn't require root privileges.", file=sys.stderr)
+ print('Please re-run as tinypilot:', file=sys.stderr)
+ print(' runuser tinypilot --command',
+ f"'{' '.join(sys.argv)}'",
+ file=sys.stderr)
+ sys.exit(1)
+
  parser = argparse.ArgumentParser(
  prog='TinyPilot Update Service',
  formatter_class=argparse.ArgumentDefaultsHelpFormatter)