diff --git a/bin/y-cluster-sudoers b/bin/y-cluster-sudoers index 72d16965..cd0a6819 100755 --- a/bin/y-cluster-sudoers +++ b/bin/y-cluster-sudoers @@ -2,7 +2,7 @@ [ -z "$DEBUG" ] || set -x set -eo pipefail -YSTACK_HOME="$(cd "$(dirname "$0")/.." && pwd)" +YSTACK_HOME="$(cd "$(dirname "$0")/.." && pwd -P)" YBIN="$YSTACK_HOME/bin" SUDOERS_FILE="/etc/sudoers.d/ystack-cluster" # Root-owned snapshots of the helpers: a NOPASSWD rule for a file its grantee can edit @@ -25,8 +25,12 @@ Usernames: one or more users to grant NOPASSWD rules to. Defaults to the current user (via SUDO_USER or USER). Run --write from a checkout you trust at a commit you have reviewed, as it copies -the helpers that will run as root. It refuses a checkout owned by one of the -grantees, whose content they control, unless --allow-grantee-source is given. +the helpers that will run as root. It refuses when a grantee owns or can write the +helpers, this script or any directory above them, or when one of those is a symlink, +unless --allow-grantee-source is given. It cannot vouch for itself: if a grantee +could write this script, running it as root in any mode runs their code. The show +mode run as yourself lists what a grantee owns; group and ACL write access you check +without executing the checkout, e.g. ls -ld on the path, before running it with sudo. Rerun it after updating ystack to refresh the copies; y-k8s-ingress-hosts warns when they differ. EOF @@ -77,15 +81,35 @@ file_owner() { esac } -# A grantee who owns the checkout could change a helper right before the copy is taken -grantee_owned_sources() { - local h owner u - for h in "${HELPERS[@]}" ""; do - owner=$(file_owner "$YBIN/$h") +# The helpers, this script, and every directory above them +source_paths() { + local p d + for p in "${HELPERS[@]/#/$YBIN/}" "$YBIN/y-cluster-sudoers"; do + d=$p + while :; do + echo "$d" + [ "$d" = / ] && break + d=$(dirname "$d") + done + done | sort -u +} + +# A grantee who owns or can write any source path can change a helper between this check +# and the copy, or swap the checkout itself; a symlink can point anywhere. Write access +# (group, other, ACL) is checked only as root, which --write is. +grantee_controlled_sources() { + local p owner u + while IFS= read -r p; do + if [ -L "$p" ]; then echo "$p is a symlink"; fi + owner=$(file_owner "$p") for u in "${USERS[@]}"; do - if [ "$owner" = "$u" ]; then echo "$YBIN/$h is owned by grantee $u"; fi + if [ "$owner" = "$u" ]; then + echo "$p is owned by grantee $u" + elif [ "$(id -u)" -eq 0 ] && sudo -n -u "$u" test -w "$p"; then + echo "$p is writable by grantee $u" + fi done - done + done < <(source_paths) } sudo_rs_check() { @@ -104,8 +128,8 @@ sudo_rs_check() { if [ "$MODE" = "show" ]; then rules echo "# --write would install: ${HELPERS[*]/#/$YBIN/} -> $PRIVILEGED_DIR/ (root:0 0755)" - GRANTEE_OWNED=$(grantee_owned_sources) - [ -z "$GRANTEE_OWNED" ] || { echo "# --write would refuse, a grantee controls the source:"; echo "$GRANTEE_OWNED" | sed 's/^/# /'; } + GRANTEE_CONTROLLED=$(grantee_controlled_sources) + [ -z "$GRANTEE_CONTROLLED" ] || { echo "# --write would refuse, a grantee controls the source:"; echo "$GRANTEE_CONTROLLED" | sed 's/^/# /'; } sudo_rs_check exit 0 fi @@ -116,10 +140,10 @@ if [ "$(id -u)" -ne 0 ]; then exit 1 fi -GRANTEE_OWNED=$(grantee_owned_sources) -if [ -n "$GRANTEE_OWNED" ] && [ "$ALLOW_GRANTEE_SOURCE" != "true" ]; then +GRANTEE_CONTROLLED=$(grantee_controlled_sources) +if [ -n "$GRANTEE_CONTROLLED" ] && [ "$ALLOW_GRANTEE_SOURCE" != "true" ]; then echo "ERROR: refusing to install root-run helpers from a checkout a grantee controls:" >&2 - echo "$GRANTEE_OWNED" >&2 + echo "$GRANTEE_CONTROLLED" >&2 echo "Run from your own ystack checkout, or pass --allow-grantee-source after reviewing it." >&2 exit 1 fi diff --git a/bin/y-etc-hosts-write b/bin/y-etc-hosts-write index 4ae49068..1f0c8916 100755 --- a/bin/y-etc-hosts-write +++ b/bin/y-etc-hosts-write @@ -19,8 +19,9 @@ the block k8s-ingress-hosts owns: everything outside the lines # generated using k8s-ingress-hosts end # must stay as it is in the current file, there is at most one such block, and each entry in it maps plain hostnames to a loopback or private IPv4 address (127/8, -10/8, 172.16/12, 192.168/16), optionally followed by a # comment. Other edits of -/etc/hosts are for root to make. +10/8, 172.16/12, 192.168/16), optionally followed by a # comment. A name that has +an entry outside the block keeps that address. Input is limited to 1 MiB. Other +edits of /etc/hosts are for root to make. Environment: Y_ETC_HOSTS Target path (default: /etc/hosts) @@ -56,7 +57,13 @@ fi TMP=$(mktemp "$TARGET_DIR/.hosts.XXXXXX") trap 'rm -f "$TMP"' EXIT -cat > "$TMP" +# Root writes this into the target's filesystem, so the input is bounded +MAX_BYTES=1048576 +head -c $((MAX_BYTES + 1)) > "$TMP" +if [ "$(wc -c < "$TMP")" -gt "$MAX_BYTES" ]; then + echo "ERROR: refusing input over $MAX_BYTES bytes" >&2 + exit 2 +fi if [ ! -s "$TMP" ]; then echo "ERROR: refusing to write an empty $TARGET" >&2 @@ -86,12 +93,22 @@ if [ -f "$TARGET" ] && ! cmp -s <(outside_block "$TARGET") <(outside_block "$TMP exit 2 fi +# Two passes over the input: the first records the addresses of names outside the block, +# so a block entry cannot give such a name another address (the block may come first) BAD_ENTRIES=$(awk -v s="$BLOCK_START" -v e="$BLOCK_END" ' function octet(x) { return x ~ /^[0-9]+$/ && x + 0 <= 255 } function allowed(ip, o) { if (split(ip, o, ".") != 4 || !octet(o[1]) || !octet(o[2]) || !octet(o[3]) || !octet(o[4])) return 0 return o[1] == 127 || o[1] == 10 || (o[1] == 172 && o[2] >= 16 && o[2] <= 31) || (o[1] == 192 && o[2] == 168) } + NR == FNR { + if ($0 == s) { inb = 1 } else if ($0 == e) { inb = 0 } + else if (!inb && $0 !~ /^[[:space:]]*(#|$)/) { + for (i = 2; i <= NF && $i !~ /^#/; i++) if (!(tolower($i) in outside)) outside[tolower($i)] = $1 + } + next + } + FNR == 1 {inb=0} $0 == s {inb=1; next} $0 == e {inb=0; next} inb && $0 !~ /^[[:space:]]*(#|$)/ { @@ -101,11 +118,13 @@ BAD_ENTRIES=$(awk -v s="$BLOCK_START" -v e="$BLOCK_END" ' for (i = 2; i <= NF && $i !~ /^#/; i++) { names++ if ($i !~ /^[A-Za-z0-9]([-A-Za-z0-9.]*[A-Za-z0-9])?$/) ok = 0 + # Host lookups ignore case, so DB.INTERNAL is db.internal + if ((tolower($i) in outside) && outside[tolower($i)] != $1) ok = 0 } if (!ok || names == 0) print - }' "$TMP") + }' "$TMP" "$TMP") if [ -n "$BAD_ENTRIES" ]; then - echo "ERROR: refusing k8s-ingress-hosts entries that are not a loopback or private IPv4 address followed by hostnames:" >&2 + echo "ERROR: refusing k8s-ingress-hosts entries that are not a loopback or private IPv4 address followed by hostnames, or that give a name from outside the block another address:" >&2 echo "$BAD_ENTRIES" >&2 exit 2 fi