diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..1fcc3a1 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-02-12 - Insecure SSH Key Creation (TOCTOU) +**Vulnerability:** `tools/setup-ssh-keys.sh` created sensitive SSH private keys with default permissions (usually 664/644) before restricting them with `chmod 600`. This created a race condition (TOCTOU) where the key was briefly readable by other users. +**Learning:** Standard shell redirection (`> file`) uses the default umask (typically 022 or 002), resulting in world-readable files for a split second. Relying on a subsequent `chmod` is insecure for sensitive data. +**Prevention:** Always wrap sensitive file creation commands in a subshell with `umask 077` (e.g., `(umask 077; cmd > file)`). This ensures the file is created with 600 permissions atomically. diff --git a/tests/reproduce_issue.sh b/tests/reproduce_issue.sh new file mode 100755 index 0000000..69f968a --- /dev/null +++ b/tests/reproduce_issue.sh @@ -0,0 +1,62 @@ +#!/bin/bash +set -e + +# Setup mock environment +TEST_DIR="$PWD/tests/tmp" +rm -rf "$TEST_DIR" +mkdir -p "$TEST_DIR/home" +mkdir -p "$TEST_DIR/bin" + +# Mock op +cat << 'EOF' > "$TEST_DIR/bin/op" +#!/bin/bash +if [[ "$1" == "read" ]]; then + echo "MOCK PRIVATE KEY CONTENT" +elif [[ "$1" == "account" ]]; then + exit 0 +elif [[ "$1" == "item" && "$2" == "get" ]]; then + exit 0 +fi +EOF +chmod +x "$TEST_DIR/bin/op" + +# Mock chmod to do nothing, so we can see the permissions at creation +cat << 'EOF' > "$TEST_DIR/bin/chmod" +#!/bin/bash +# no-op +echo "MOCK CHMOD: $@" +EOF +chmod +x "$TEST_DIR/bin/chmod" + +export PATH="$TEST_DIR/bin:$PATH" +export HOME="$TEST_DIR/home" +export XDG_CONFIG_HOME="$HOME/.config" + +# Create config +mkdir -p "$XDG_CONFIG_HOME/dotfiles" +echo "ssh: { vault: 'test', item_name: 'testkey' }" > "$XDG_CONFIG_HOME/dotfiles/config.yaml" + +echo "Running setup-ssh-keys.sh with mocked chmod..." + +# Run the script +./tools/setup-ssh-keys.sh restore > "$TEST_DIR/output.log" 2>&1 || true + +# Check permissions of the private key +KEY_FILE="$TEST_DIR/home/.ssh/id_ed25519" + +if [[ ! -f "$KEY_FILE" ]]; then + echo "FAIL: Key file not found at $KEY_FILE" + exit 1 +fi + +# Get permissions (Linux stat) +PERMS=$(stat -c "%a" "$KEY_FILE") +echo "File permissions detected: $PERMS" + +if [[ "$PERMS" == "600" ]]; then + echo "SECURE: File created with secure permissions (600)" + exit 0 +else + echo "VULNERABILITY CONFIRMED: File created with insecure permissions ($PERMS)" + exit 1 +fi diff --git a/tools/setup-ssh-keys.sh b/tools/setup-ssh-keys.sh index bde52fd..b737e86 100755 --- a/tools/setup-ssh-keys.sh +++ b/tools/setup-ssh-keys.sh @@ -153,7 +153,10 @@ cmd_restore() { 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