🛡️ Sentinel: [CRITICAL] Fix SSH key creation TOCTOU vulnerability

.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.

build.sh

@@ -77,6 +77,7 @@ get_shell_scripts() {
         "$ROOT_DIR/scripts"
         "$ROOT_DIR/scripts/backup"
         "$ROOT_DIR/cron"
+        "$ROOT_DIR/tests"
         "$ROOT_DIR"
     )
 

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 <<EOF > "$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 <<EOF > "$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"

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