🛡️ Sentinel: [CRITICAL] Fix SSH private key creation race condition

.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2024-05-22 - [SSH Private Key Race Condition]
+**Vulnerability:** Found a TOCTOU (Time-of-Check to Time-of-Use) race condition in `tools/setup-ssh-keys.sh`. The script created the private key file using output redirection (`>`) which uses the default umask (typically 022), resulting in the file being world-readable for a brief window before `chmod 600` was executed.
+**Learning:** Shell redirection and standard file creation tools respect the current `umask`. Relying on a subsequent `chmod` to secure sensitive files leaves a window of exposure.
+**Prevention:** Always set `umask 077` (or similar restrictive mask) *before* creating sensitive files in shell scripts to ensure they are born secure. Restore the original umask afterwards if necessary.

tests/verify_ssh_permissions.sh

@@ -0,0 +1,82 @@
+#!/bin/bash
+set -e
+
+# Setup mock environment
+TEST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+REPO_ROOT="$(cd "$TEST_DIR/.." && pwd)"
+MOCK_BIN="$TEST_DIR/bin"
+TEMP_HOME="$TEST_DIR/temp_home"
+
+# Clean up previous run
+rm -rf "$TEMP_HOME"
+rm -rf "$MOCK_BIN"
+mkdir -p "$TEMP_HOME"
+mkdir -p "$MOCK_BIN"
+
+# Mock 'op' command
+cat <<EOF > "$MOCK_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 "PRIVATE KEY CONTENT"
+    elif [[ "\$2" == *"public_key"* ]]; then
+        echo "PUBLIC KEY CONTENT"
+    fi
+fi
+EOF
+chmod +x "$MOCK_BIN/op"
+
+# Mock 'yq' command (optional, but good to have)
+cat <<EOF > "$MOCK_BIN/yq"
+#!/bin/bash
+exit 1 # simulate not installed or failing, script handles it
+EOF
+chmod +x "$MOCK_BIN/yq"
+
+# Add mock bin to PATH
+export PATH="$MOCK_BIN:$PATH"
+export HOME="$TEMP_HOME"
+export XDG_CONFIG_HOME="$TEMP_HOME/.config"
+
+# Run the script
+echo "Running setup-ssh-keys.sh restore..."
+"$REPO_ROOT/tools/setup-ssh-keys.sh" restore
+
+# Verify files exist
+PRIVATE_KEY="$TEMP_HOME/.ssh/id_ed25519"
+PUBLIC_KEY="$TEMP_HOME/.ssh/id_ed25519.pub"
+
+if [[ ! -f "$PRIVATE_KEY" ]]; then
+    echo "ERROR: Private key not found at $PRIVATE_KEY"
+    exit 1
+fi
+
+if [[ ! -f "$PUBLIC_KEY" ]]; then
+    echo "ERROR: Public key not found at $PUBLIC_KEY"
+    exit 1
+fi
+
+# Verify permissions
+# Private key should be 600 (-rw-------)
+PERMS=$(stat -c "%a" "$PRIVATE_KEY" 2>/dev/null || stat -f "%Lp" "$PRIVATE_KEY")
+if [[ "$PERMS" != "600" ]]; then
+    echo "ERROR: Private key permissions are $PERMS, expected 600"
+    exit 1
+fi
+
+# Public key should be 644 (-rw-r--r--)
+PERMS=$(stat -c "%a" "$PUBLIC_KEY" 2>/dev/null || stat -f "%Lp" "$PUBLIC_KEY")
+if [[ "$PERMS" != "644" ]]; then
+    echo "ERROR: Public key permissions are $PERMS, expected 644"
+    exit 1
+fi
+
+echo "SUCCESS: SSH keys restored with correct permissions."
+
+# Cleanup
+rm -rf "$TEMP_HOME"
+rm -rf "$MOCK_BIN"

tools/setup-ssh-keys.sh

@@ -148,14 +148,28 @@ cmd_restore() {
 
     say "Restoring SSH key from 1Password..."
 
-    # Create SSH directory
-    mkdir -p "$SSH_DIR"
+    # Create SSH directory safely
+    if [[ ! -d "$SSH_DIR" ]]; then
+        mkdir -p "$SSH_DIR"
+        chmod 700 "$SSH_DIR"
+    fi
+    # Ensure directory permissions are restrictive
     chmod 700 "$SSH_DIR"
 
+    # Set umask to ensure new files are only readable by owner
+    local old_umask
+    old_umask=$(umask)
+    umask 077
+
     # Read private key from 1Password and save locally
+    # umask 077 ensures file is created with 600 permissions
     op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE"
+    # Redundant chmod just to be sure, but file was created securely
     chmod 600 "$PRIVATE_KEY_FILE"
 
+    # Restore umask for public key (we want 644)
+    umask "$old_umask"
+
     # Read public key from 1Password and save locally
     op read "op://$VAULT/$KEY_NAME/public_key" > "$PUBLIC_KEY_FILE"
     chmod 644 "$PUBLIC_KEY_FILE"