mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
fix: unblock the Ubuntu 26.04 installer (qemu-kvm, sudo-rs) (#665)
setup.sh aborts twice on resolute. qemu-kvm is a virtual package with two providers there, so apt refuses to resolve it; the token was redundant anyway since $QEMU_PACKAGES already installs qemu-system-x86/arm and spinifex execs the qemu-system-* binaries directly. sudo-rs, Ubuntu's default sudo since 25.10, then rejects the sudoers file: it implements no pam_session Defaults setting, and forbids a wildcard inside a command argument. pam_session is now emitted only for classic sudo, and the sysctl grant names a fixed-verb helper instead of a wildcard. The helper is narrower than the rule it replaces: it accepts one interface, a key restricted to rp_filter or accept_local, and a value restricted to 0 or 1, where the old grant permitted any net.ipv4.conf key at any value. Granting sysctl with an unconstrained argument was rejected as root-equivalent. Adds scripts/lint-apt-packages.sh and an Apt Packages workflow so the runtime package list is resolved against each supported suite's real indices in CI.
This commit is contained in:
@@ -0,0 +1,48 @@
|
||||
name: Apt Packages
|
||||
|
||||
# Path-filtered like image-scripts.yml: this resolves setup.sh's runtime package
|
||||
# list against each supported suite's real indices, so a renamed, dropped, or
|
||||
# ambiguously virtual package fails here rather than on a user's machine.
|
||||
on:
|
||||
push:
|
||||
branches:
|
||||
- main
|
||||
- dev
|
||||
paths:
|
||||
- scripts/setup.sh
|
||||
- scripts/lint-apt-packages.sh
|
||||
- .github/workflows/apt-packages.yml
|
||||
pull_request:
|
||||
branches:
|
||||
- main
|
||||
- dev
|
||||
paths:
|
||||
- scripts/setup.sh
|
||||
- scripts/lint-apt-packages.sh
|
||||
- .github/workflows/apt-packages.yml
|
||||
workflow_dispatch:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
apt_packages:
|
||||
name: Resolve apt list (${{ matrix.image }})
|
||||
runs-on: ubuntu-26.04
|
||||
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
# The suites the nightly E2E matrix actually installs on. Ubuntu 22.04 is
|
||||
# accepted by detect_os but has never been exercised and does not resolve.
|
||||
image:
|
||||
- ubuntu:24.04
|
||||
- ubuntu:26.04
|
||||
- debian:13
|
||||
|
||||
steps:
|
||||
- name: Checkout Code
|
||||
uses: actions/checkout@v7
|
||||
|
||||
- name: Resolve runtime package list
|
||||
run: ./scripts/lint-apt-packages.sh ${{ matrix.image }}
|
||||
@@ -250,7 +250,7 @@ install-system:
|
||||
sudo apt-get update && sudo DEBIAN_FRONTEND=noninteractive apt-get install -y \
|
||||
-o Dpkg::Options::="--force-confdef" \
|
||||
-o Dpkg::Options::="--force-confold" \
|
||||
nbdkit nbdkit-plugin-dev pkg-config qemu-system-x86 qemu-system-arm qemu-utils qemu-kvm \
|
||||
nbdkit nbdkit-plugin-dev pkg-config qemu-system-x86 qemu-system-arm qemu-utils \
|
||||
ovmf qemu-efi-aarch64 \
|
||||
libvirt-daemon-system libvirt-clients libvirt-dev make gcc jq curl \
|
||||
iproute2 netcat-openbsd openssh-client wget git unzip sudo xz-utils file \
|
||||
|
||||
@@ -56,7 +56,7 @@ Pre-download the APT packages installed by the production setup. `apt install --
|
||||
sudo apt update
|
||||
sudo apt install --download-only -y \
|
||||
nbdkit nbdkit-plugin-dev pkg-config \
|
||||
qemu-system-x86 qemu-utils qemu-kvm \
|
||||
qemu-system-x86 qemu-utils \
|
||||
ovmf qemu-efi-aarch64 \
|
||||
libvirt-daemon-system libvirt-clients libvirt-dev \
|
||||
ovn-central ovn-host openvswitch-switch \
|
||||
|
||||
Executable
+32
@@ -0,0 +1,32 @@
|
||||
#!/usr/bin/env bash
|
||||
# Resolves setup.sh's runtime apt list against a suite's real indices with
|
||||
# apt-get install -s. Catches a package that has been renamed, dropped, or made
|
||||
# ambiguously virtual before it reaches a user's machine.
|
||||
set -euo pipefail
|
||||
|
||||
IMAGE="${1:?usage: lint-apt-packages.sh <docker image>, e.g. ubuntu:26.04}"
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||
|
||||
# Source setup.sh for the list rather than restating it here, so the lint cannot
|
||||
# drift from what the installer actually asks apt for.
|
||||
INSTALL_SPINIFEX_LIB_ONLY=1
|
||||
export INSTALL_SPINIFEX_LIB_ONLY
|
||||
# shellcheck source=/dev/null
|
||||
. "${SCRIPT_DIR}/setup.sh"
|
||||
|
||||
if [ -z "${APT_RUNTIME_PACKAGES:-}" ]; then
|
||||
echo "setup.sh did not define APT_RUNTIME_PACKAGES" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Both arch-specific qemu packages exist in the amd64 archive, so linting them
|
||||
# together covers the aarch64 install path from an amd64 runner.
|
||||
PACKAGES="qemu-system-x86 qemu-system-arm ${APT_RUNTIME_PACKAGES}"
|
||||
|
||||
echo "Resolving ${IMAGE}..."
|
||||
# shellcheck disable=SC2086
|
||||
docker run --rm "${IMAGE}" bash -c \
|
||||
"apt-get update -qq >/dev/null && apt-get install -s -y -qq ${PACKAGES//$'\n'/ } >/dev/null"
|
||||
|
||||
echo "OK: every package resolves on ${IMAGE}"
|
||||
+93
-12
@@ -24,6 +24,9 @@ set -e
|
||||
INSTALL_SPINIFEX_CHANNEL="${INSTALL_SPINIFEX_CHANNEL:-latest}"
|
||||
INSTALL_BASE_URL="${INSTALL_BASE_URL:-https://install.mulgadc.com}"
|
||||
|
||||
# Referenced by both the sudoers grant and the daemon, so the path is fixed here.
|
||||
ENDPOINT_SYSCTL_HELPER="/usr/local/lib/spinifex/spinifex-set-endpoint-sysctl"
|
||||
|
||||
# --- Colors ---
|
||||
RED='\033[0;31m'
|
||||
GREEN='\033[0;32m'
|
||||
@@ -181,15 +184,76 @@ create_service_users() {
|
||||
}
|
||||
|
||||
# --- Install scoped sudoers rules ---
|
||||
# sudo-rs (Ubuntu's default sudo since 25.10) implements only a subset of
|
||||
# Defaults and rejects an unknown one outright, so pam_session must be omitted
|
||||
# there rather than emitted and ignored.
|
||||
sudo_is_sudo_rs() {
|
||||
sudo --version 2>/dev/null | head -1 | grep -qi 'sudo-rs'
|
||||
}
|
||||
|
||||
# The daemon only ever sets rp_filter and accept_local on an endpoint it just
|
||||
# created. A fixed-verb helper keeps that grant expressible without a wildcard,
|
||||
# which sudo-rs forbids inside command arguments.
|
||||
install_endpoint_sysctl_helper() {
|
||||
$SUDO install -d -m 0755 /usr/local/lib/spinifex
|
||||
$SUDO tee "${ENDPOINT_SYSCTL_HELPER}" > /dev/null << 'HELPER'
|
||||
#!/bin/sh
|
||||
# Sets one of two per-endpoint sysctls for the asymmetric IMDS reply path.
|
||||
# Interface, key and value are all constrained: this runs as root under a
|
||||
# NOPASSWD grant, so it must never pass an arbitrary key through to sysctl.
|
||||
set -eu
|
||||
|
||||
if [ "$#" -ne 3 ]; then
|
||||
echo "usage: $0 <interface> <rp_filter|accept_local> <0|1>" >&2
|
||||
exit 2
|
||||
fi
|
||||
|
||||
iface=$1
|
||||
key=$2
|
||||
value=$3
|
||||
|
||||
# Reject anything that is not a plain interface name, so the key cannot be
|
||||
# steered elsewhere in the sysctl tree with a "." or a "/".
|
||||
case "${iface}" in
|
||||
''|*[!A-Za-z0-9_-]*) echo "invalid interface: ${iface}" >&2; exit 2 ;;
|
||||
esac
|
||||
|
||||
case "${key}" in
|
||||
rp_filter|accept_local) ;;
|
||||
*) echo "key not permitted: ${key}" >&2; exit 2 ;;
|
||||
esac
|
||||
|
||||
case "${value}" in
|
||||
0|1) ;;
|
||||
*) echo "value not permitted: ${value}" >&2; exit 2 ;;
|
||||
esac
|
||||
|
||||
exec sysctl -qw "net.ipv4.conf.${iface}.${key}=${value}"
|
||||
HELPER
|
||||
$SUDO chown root:root "${ENDPOINT_SYSCTL_HELPER}"
|
||||
$SUDO chmod 0755 "${ENDPOINT_SYSCTL_HELPER}"
|
||||
}
|
||||
|
||||
install_sudoers() {
|
||||
stage "installing scoped sudoers rules"
|
||||
$SUDO tee /etc/sudoers.d/spinifex-network > /dev/null << 'SUDOERS'
|
||||
install_endpoint_sysctl_helper
|
||||
|
||||
if sudo_is_sudo_rs; then
|
||||
$SUDO tee /etc/sudoers.d/spinifex-network > /dev/null << 'SUDOERS'
|
||||
# pam_session is omitted: this host runs sudo-rs, which does not implement it.
|
||||
SUDOERS
|
||||
else
|
||||
$SUDO tee /etc/sudoers.d/spinifex-network > /dev/null << 'SUDOERS'
|
||||
# No PAM session for the service users. Every sudo call otherwise logs
|
||||
# "pam_limits: Could not set limit for 'core'": the units set LimitCORE=0 and
|
||||
# their CapabilityBoundingSet omits CAP_SYS_RESOURCE, so pam_limits cannot apply
|
||||
# the limits.conf default and warns. It also means sudo children inherit the
|
||||
# unit's rlimits rather than the system defaults, which is the RG-9 intent.
|
||||
Defaults:spinifex-daemon !pam_session
|
||||
SUDOERS
|
||||
fi
|
||||
|
||||
$SUDO tee -a /etc/sudoers.d/spinifex-network > /dev/null << 'SUDOERS'
|
||||
|
||||
# The OVS/OVN client tools are deliberately absent. They do all their work over
|
||||
# control sockets that setup-ovn.sh group-owns to `spinifex`, so they run as the
|
||||
@@ -208,11 +272,16 @@ Defaults:spinifex-daemon !pam_session
|
||||
|
||||
# Spinifex daemon: tap devices and OpenFlow rules. Its unit holds no
|
||||
# CAP_NET_ADMIN, so unlike vpcd it cannot drop these. ovs-ofctl installs the
|
||||
# per-tap IMDS datapath flows on br-imds; sysctl sets the per-endpoint
|
||||
# rp_filter/accept_local the asymmetric reply path needs.
|
||||
# per-tap IMDS datapath flows on br-imds; the sysctl helper sets the
|
||||
# per-endpoint rp_filter/accept_local the asymmetric reply path needs.
|
||||
spinifex-daemon ALL=(root) NOPASSWD: /sbin/ip, /usr/sbin/ip
|
||||
spinifex-daemon ALL=(root) NOPASSWD: /usr/bin/ovs-ofctl
|
||||
spinifex-daemon ALL=(root) NOPASSWD: /usr/sbin/sysctl -qw net.ipv4.conf.*
|
||||
SUDOERS
|
||||
|
||||
# Separate append so the prose above can stay in a quoted heredoc: it carries
|
||||
# backticks that an expanding one would run as command substitution.
|
||||
$SUDO tee -a /etc/sudoers.d/spinifex-network > /dev/null << SUDOERS
|
||||
spinifex-daemon ALL=(root) NOPASSWD: ${ENDPOINT_SYSCTL_HELPER}
|
||||
SUDOERS
|
||||
$SUDO chmod 0440 /etc/sudoers.d/spinifex-network
|
||||
$SUDO visudo -cf /etc/sudoers.d/spinifex-network || fatal "Invalid sudoers syntax in spinifex-network"
|
||||
@@ -224,6 +293,18 @@ SUDOERS
|
||||
# scripts/iso-builder/build/packages.list in the mulga repo. When you add,
|
||||
# remove, or change a runtime package here, review that file too — drift means
|
||||
# the ISO and `curl | bash` paths install different software.
|
||||
#
|
||||
# Hoisted out of the apt-get call so scripts/lint-apt-packages.sh can resolve the
|
||||
# same list against each supported suite. The arch-specific qemu package is
|
||||
# added by install_apt_deps from detect_arch.
|
||||
APT_RUNTIME_PACKAGES="nbdkit
|
||||
qemu-utils gdisk ovmf qemu-efi-aarch64 less
|
||||
libvirt-daemon-system libvirt-clients
|
||||
pciutils
|
||||
jq curl iproute2 netcat-openbsd wget unzip xz-utils file
|
||||
ovn-central ovn-host openvswitch-switch openvswitch-ipsec strongswan-charon dhcpcd-base
|
||||
chrony"
|
||||
|
||||
install_apt_deps() {
|
||||
stage "installing apt dependencies"
|
||||
if [ "${INSTALL_SPINIFEX_SKIP_APT}" = "1" ]; then
|
||||
@@ -232,14 +313,10 @@ install_apt_deps() {
|
||||
info "Installing system dependencies..."
|
||||
$SUDO apt-get update -qq
|
||||
|
||||
# Unquoted on purpose: both variables are whitespace-separated package lists.
|
||||
# shellcheck disable=SC2086
|
||||
DEBIAN_FRONTEND=noninteractive $SUDO apt-get install -y -qq \
|
||||
nbdkit \
|
||||
$QEMU_PACKAGES qemu-utils gdisk qemu-kvm ovmf qemu-efi-aarch64 less \
|
||||
libvirt-daemon-system libvirt-clients \
|
||||
pciutils \
|
||||
jq curl iproute2 netcat-openbsd wget unzip xz-utils file \
|
||||
ovn-central ovn-host openvswitch-switch openvswitch-ipsec strongswan-charon dhcpcd-base \
|
||||
chrony \
|
||||
$QEMU_PACKAGES $APT_RUNTIME_PACKAGES \
|
||||
> /dev/null
|
||||
|
||||
info "System dependencies installed"
|
||||
@@ -879,4 +956,8 @@ main() {
|
||||
fi
|
||||
}
|
||||
|
||||
main "$@"
|
||||
# Guarded so the package lint can source this file for APT_RUNTIME_PACKAGES
|
||||
# without running the installer.
|
||||
if [ "${INSTALL_SPINIFEX_LIB_ONLY:-0}" != "1" ]; then
|
||||
main "$@"
|
||||
fi
|
||||
|
||||
@@ -59,7 +59,7 @@ func TestInstallIMDSDatapath(t *testing.T) {
|
||||
s := newStubRunner()
|
||||
s.expect("ovs-vsctl", nil, nil)
|
||||
s.expect("ip", nil, nil)
|
||||
s.expect("sysctl", nil, nil)
|
||||
s.expect(utils.EndpointSysctlHelper, nil, nil)
|
||||
s.expect("ovs-ofctl", nil, nil)
|
||||
|
||||
d := imdsDatapathSpec("eni-0abc1234", "02:00:00:00:01:05", "subnet-0fedcba9")
|
||||
@@ -146,7 +146,7 @@ func TestInstallIMDSDatapath_ServingFailureWrapped(t *testing.T) {
|
||||
s := newStubRunner()
|
||||
s.expect("ovs-vsctl", nil, nil)
|
||||
s.expect("ip", nil, nil)
|
||||
s.expect("sysctl", nil, nil)
|
||||
s.expect(utils.EndpointSysctlHelper, nil, nil)
|
||||
s.expect("ovs-ofctl del-flows", nil, nil)
|
||||
// Forward flows (priority=100) are connectivity and must succeed; the demux/egress
|
||||
// flows (priority=200) are serving — fail those to land in the serving stage.
|
||||
|
||||
@@ -204,9 +204,12 @@ func ensureIMDSEndpoint(ctx context.Context, r Runner, d IMDSTapDatapath) error
|
||||
return setEndpointSysctl(ctx, r, d.Endpoint, "accept_local", "1")
|
||||
}
|
||||
|
||||
// setEndpointSysctl goes through the helper rather than sysctl itself: the
|
||||
// daemon's grant has to name a fixed command, since sudo-rs rejects the
|
||||
// wildcard the old `sysctl -qw net.ipv4.conf.*` rule relied on.
|
||||
func setEndpointSysctl(ctx context.Context, r Runner, endpoint, suffix, val string) error {
|
||||
key := "net.ipv4.conf." + endpoint + "." + suffix
|
||||
if _, err := r.Run(ctx, "sysctl", "-qw", key+"="+val); err != nil {
|
||||
if _, err := r.Run(ctx, utils.EndpointSysctlHelper, endpoint, suffix, val); err != nil {
|
||||
return fmt.Errorf("set %s=%s: %w", key, val, err)
|
||||
}
|
||||
return nil
|
||||
|
||||
@@ -5,6 +5,8 @@ import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/utils"
|
||||
)
|
||||
|
||||
func testDatapath() IMDSTapDatapath {
|
||||
@@ -114,7 +116,7 @@ func TestInstallTapDatapath(t *testing.T) {
|
||||
s := newStubRunner()
|
||||
s.expect("ovs-vsctl", nil, nil)
|
||||
s.expect("ip", nil, nil)
|
||||
s.expect("sysctl", nil, nil)
|
||||
s.expect(utils.EndpointSysctlHelper, nil, nil)
|
||||
s.expect("ovs-ofctl", nil, nil)
|
||||
|
||||
d := testDatapath()
|
||||
@@ -130,8 +132,8 @@ func TestInstallTapDatapath(t *testing.T) {
|
||||
"ip link set " + d.Endpoint + " up",
|
||||
"ip addr replace " + imdsMetaAddr + "/32 dev " + d.Endpoint,
|
||||
"ip addr replace " + imdsDNSAddr + "/32 dev " + d.Endpoint,
|
||||
"sysctl -qw net.ipv4.conf." + d.Endpoint + ".rp_filter=0",
|
||||
"sysctl -qw net.ipv4.conf." + d.Endpoint + ".accept_local=1",
|
||||
utils.EndpointSysctlHelper + " " + d.Endpoint + " rp_filter 0",
|
||||
utils.EndpointSysctlHelper + " " + d.Endpoint + " accept_local 1",
|
||||
// Flows are not cleared here: installIMDSDatapath clears the shared cookie
|
||||
// once up front so this install does not wipe the patch's forward flows.
|
||||
// Ingress demux (gateway dst MAC -> endpoint MAC), one per captured addr.
|
||||
@@ -161,7 +163,7 @@ func TestInstallTapDatapathReattachIsIdempotent(t *testing.T) {
|
||||
s := newStubRunner()
|
||||
s.expect("ovs-vsctl", nil, nil)
|
||||
s.expect("ip", nil, nil)
|
||||
s.expect("sysctl", nil, nil)
|
||||
s.expect(utils.EndpointSysctlHelper, nil, nil)
|
||||
s.expect("ovs-ofctl", nil, nil)
|
||||
|
||||
d := testDatapath()
|
||||
|
||||
@@ -33,6 +33,11 @@ var socketClients = map[string]bool{
|
||||
"systemctl": true,
|
||||
}
|
||||
|
||||
// EndpointSysctlHelper is the fixed-verb wrapper setup.sh installs. sudo-rs
|
||||
// (Ubuntu's default sudo since 25.10) forbids a wildcard inside a command
|
||||
// argument, so the daemon's per-endpoint sysctl grant names this instead.
|
||||
const EndpointSysctlHelper = "/usr/local/lib/spinifex/spinifex-set-endpoint-sysctl"
|
||||
|
||||
// Capability bit numbers from linux/capability.h.
|
||||
const (
|
||||
capNetAdmin = 12
|
||||
@@ -48,6 +53,10 @@ func capForTool(name string, args []string) (uint, bool) {
|
||||
return capNetAdmin, true
|
||||
case "arping":
|
||||
return capNetRaw, true
|
||||
case EndpointSysctlHelper:
|
||||
// The helper only ever writes net.ipv4.conf.<iface>.<key>, so a holder of
|
||||
// CAP_NET_ADMIN runs it directly and never reaches the sudoers grant.
|
||||
return capNetAdmin, true
|
||||
case "sysctl":
|
||||
// Only the net.* trees are governed by CAP_NET_ADMIN in the caller's
|
||||
// network namespace; every other key is a root-only write.
|
||||
|
||||
@@ -124,6 +124,35 @@ func TestRG10_VPCDHoldsNoSudoersGrant(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSudoersGrantsCarryNoArgumentWildcard pins the sudo-rs constraint. Ubuntu
|
||||
// has shipped sudo-rs as the default sudo since 25.10, and it rejects a `*`
|
||||
// inside a command argument outright — visudo fails and setup.sh aborts.
|
||||
func TestSudoersGrantsCarryNoArgumentWildcard(t *testing.T) {
|
||||
root := repoRoot(t)
|
||||
setup, err := os.ReadFile(filepath.Join(root, "scripts", "setup.sh"))
|
||||
if err != nil {
|
||||
t.Fatalf("read setup.sh: %v", err)
|
||||
}
|
||||
for i, line := range strings.Split(string(setup), "\n") {
|
||||
if !strings.HasPrefix(line, "spinifex-daemon ALL=") {
|
||||
continue
|
||||
}
|
||||
_, cmds, _ := strings.Cut(line, "NOPASSWD:")
|
||||
for spec := range strings.SplitSeq(cmds, ",") {
|
||||
fields := strings.Fields(spec)
|
||||
// A standalone final `*` is the one form sudo-rs accepts, so drop it
|
||||
// and treat any wildcard left inside an argument as the failure.
|
||||
if n := len(fields); n > 0 && fields[n-1] == "*" {
|
||||
fields = fields[:n-1]
|
||||
}
|
||||
if strings.Contains(strings.Join(fields, " "), "*") {
|
||||
t.Fatalf("scripts/setup.sh:%d grants a wildcard inside a command argument:\n %s\n"+
|
||||
"sudo-rs rejects this; name a fixed-verb helper instead (see EndpointSysctlHelper).", i+1, line)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ambientLine returns the unit's AmbientCapabilities= value, or "".
|
||||
func ambientLine(unit string) string {
|
||||
for line := range strings.SplitSeq(unit, "\n") {
|
||||
|
||||
@@ -89,6 +89,7 @@ func TestAmbientCapabilitiesReplaceSudo(t *testing.T) {
|
||||
{"iptables", "-A", "FORWARD", "-j", "ACCEPT"},
|
||||
{"arping", "-U", "-c", "2", "-I", "br-wan", "10.0.0.1"},
|
||||
{"sysctl", "-w", "net.ipv4.neigh.br-wan.proxy_delay=0"},
|
||||
{EndpointSysctlHelper, "ime-12345678", "rp_filter", "0"},
|
||||
}
|
||||
for _, argv := range unescalated {
|
||||
if NeedsPrivilege(argv[0], argv[1:]...) {
|
||||
@@ -124,6 +125,7 @@ func TestDaemonAmbientSetStillEscalatesIP(t *testing.T) {
|
||||
for _, argv := range [][]string{
|
||||
{"ip", "tuntap", "add", "tap0", "mode", "tap"},
|
||||
{"sysctl", "-qw", "net.ipv4.conf.tap0.rp_filter=0"},
|
||||
{EndpointSysctlHelper, "tap0", "rp_filter", "0"},
|
||||
} {
|
||||
if !NeedsPrivilege(argv[0], argv[1:]...) {
|
||||
t.Errorf("%v is not escalated, but the daemon holds no CAP_NET_ADMIN", argv)
|
||||
|
||||
Reference in New Issue
Block a user