diff --git a/.circleci/config.yml b/.circleci/config.yml index aecb8c5d0..e251075a7 100644 --- a/.circleci/config.yml +++ b/.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 diff --git a/debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs b/debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs index 77297caa2..740d8073e 100755 --- a/debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs +++ b/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". diff --git a/dev-scripts/check-privilege-guard b/dev-scripts/check-privilege-guard new file mode 100755 index 000000000..487eb832b --- /dev/null +++ b/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 diff --git a/scripts/is-ssh-enabled b/scripts/is-ssh-enabled index 25ac068e6..958f42062 100755 --- a/scripts/is-ssh-enabled +++ b/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] diff --git a/scripts/streaming-mode b/scripts/streaming-mode index 4eacdddc2..9d10f3acf 100755 --- a/scripts/streaming-mode +++ b/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}/.." diff --git a/scripts/update-service b/scripts/update-service index 2390bb2c4..4b6fd23a5 100755 --- a/scripts/update-service +++ b/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)