From b1da592f4566260d8ee7455433b568434cd5aeb3 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 10 Feb 2026 04:54:56 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20SSH=20key=20creation=20TOCTOU=20vulnerability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed a race condition where SSH private keys were briefly world-readable during creation. The `op read ... > file` command created the file with default permissions (often 644) before `chmod 600` was applied. Changes: - Modified `tools/setup-ssh-keys.sh` to use `(umask 077; ...)` subshells for sensitive file creation. - Added regression test `tests/test_ssh_creation.sh` to verify permissions. - Updated `build.sh` to include `tests/` in syntax checks. - Added security journal entry in `.jules/sentinel.md`. Verification: - Ran `tests/test_ssh_creation.sh` which confirmed correct permissions (600/700). - Ran `./build.sh` to verify syntax and linting. Co-authored-by: kidchenko <5432753+kidchenko@users.noreply.github.com> --- .jules/sentinel.md | 4 ++ build.sh | 1 + tests/test_ssh_creation.sh | 85 ++++++++++++++++++++++++++++++++++++++ tools/setup-ssh-keys.sh | 7 +++- 4 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 .jules/sentinel.md create mode 100755 tests/test_ssh_creation.sh diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..c9224ae --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-02-10 - [SSH Key Creation Race Condition] +**Vulnerability:** SSH private keys were briefly world-readable during creation because `umask` was not restricted. The `op read ... > file` command created the file with default permissions (often 644/664), and `chmod 600` was only applied afterward. +**Learning:** Shell redirection creates files before `chmod` runs. This creates a Time-of-Check to Time-of-Use (TOCTOU) window where sensitive data is exposed. +**Prevention:** Always wrap sensitive file creation commands in a subshell with restricted umask: `(umask 077; command > sensitive_file)`. This ensures the file is born secure. diff --git a/build.sh b/build.sh index 5b333c8..3f1dd05 100755 --- a/build.sh +++ b/build.sh @@ -77,6 +77,7 @@ get_shell_scripts() { "$ROOT_DIR/scripts" "$ROOT_DIR/scripts/backup" "$ROOT_DIR/cron" + "$ROOT_DIR/tests" "$ROOT_DIR" ) diff --git a/tests/test_ssh_creation.sh b/tests/test_ssh_creation.sh new file mode 100755 index 0000000..1521571 --- /dev/null +++ b/tests/test_ssh_creation.sh @@ -0,0 +1,85 @@ +#!/bin/bash +set -e + +# Setup test environment +TEST_DIR=$(mktemp -d) +export HOME="$TEST_DIR" +export XDG_CONFIG_HOME="$TEST_DIR/.config" +export XDG_DATA_HOME="$TEST_DIR/.local/share" +mkdir -p "$TEST_DIR/.config/dotfiles" +mkdir -p "$TEST_DIR/.local/bin" + +# Add mock bin to PATH +export PATH="$TEST_DIR/.local/bin:$PATH" + +# Create mock op +cat < "$TEST_DIR/.local/bin/op" +#!/bin/bash +if [[ "\$1" == "account" && "\$2" == "list" ]]; then + exit 0 +elif [[ "\$1" == "item" && "\$2" == "get" ]]; then + exit 0 +elif [[ "\$1" == "read" ]]; then + if [[ "\$2" == *"private_key"* ]]; then + echo "MOCK_PRIVATE_KEY_CONTENT" + else + echo "MOCK_PUBLIC_KEY_CONTENT" + fi +else + exit 0 +fi +EOF +chmod +x "$TEST_DIR/.local/bin/op" + +# Create mock yq (script uses it if available) +cat < "$TEST_DIR/.local/bin/yq" +#!/bin/bash +echo "" # Return empty or default +EOF +chmod +x "$TEST_DIR/.local/bin/yq" + +echo "Running setup-ssh-keys.sh restore in test environment..." + +# Determine absolute path to the tool +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(dirname "$SCRIPT_DIR")" +TOOL_PATH="$REPO_ROOT/tools/setup-ssh-keys.sh" + +# Run the tool +# Pass --vault and --name to avoid yq dependency issues or defaults +bash "$TOOL_PATH" restore --vault "testvault" --name "testkey" + +# Verify permissions +PRIVATE_KEY="$HOME/.ssh/id_ed25519" + +if [[ ! -f "$PRIVATE_KEY" ]]; then + echo "ERROR: Private key not created at $PRIVATE_KEY" + ls -la "$HOME/.ssh" + exit 1 +fi + +PERMS=$(stat -c "%a" "$PRIVATE_KEY" 2>/dev/null || stat -f "%Lp" "$PRIVATE_KEY") + +echo "Permissions of private key: $PERMS" + +if [[ "$PERMS" != "600" ]]; then + echo "ERROR: Permissions are not 600!" + exit 1 +fi + +echo "SUCCESS: Key created with correct permissions." + +# Verify directory permissions +SSH_DIR="$HOME/.ssh" +DIR_PERMS=$(stat -c "%a" "$SSH_DIR" 2>/dev/null || stat -f "%Lp" "$SSH_DIR") +echo "Permissions of .ssh directory: $DIR_PERMS" + +if [[ "$DIR_PERMS" != "700" ]]; then + echo "ERROR: Directory permissions are not 700!" + exit 1 +fi + +echo "SUCCESS: Directory created with correct permissions." + +# Cleanup +rm -rf "$TEST_DIR" diff --git a/tools/setup-ssh-keys.sh b/tools/setup-ssh-keys.sh index bde52fd..2ba26c0 100755 --- a/tools/setup-ssh-keys.sh +++ b/tools/setup-ssh-keys.sh @@ -149,11 +149,14 @@ cmd_restore() { say "Restoring SSH key from 1Password..." # Create SSH directory - mkdir -p "$SSH_DIR" + (umask 077 && mkdir -p "$SSH_DIR") chmod 700 "$SSH_DIR" # Read private key from 1Password and save locally - op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE" + ( + umask 077 + op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE" + ) chmod 600 "$PRIVATE_KEY_FILE" # Read public key from 1Password and save locally