diff --git a/deploy/install-mtproxy.sh b/deploy/install-mtproxy.sh index 68a07b9..8701dda 100755 --- a/deploy/install-mtproxy.sh +++ b/deploy/install-mtproxy.sh @@ -40,10 +40,6 @@ if [[ ! -x "$source_directory/objs/bin/mtproto-proxy" ]] || test -x "$build_directory/objs/bin/mtproto-proxy" printf '%s\n' "$mtproxy_commit" > "$build_directory/.tproxy-commit" chown -R root:root "$build_directory" - # install.sh sets umask 077 before calling this script, so make's - # output is created 0700. The mtproxy.service unit execs this binary - # as the mtproxy user, which then can't read or run it. - chmod -R a+rX "$build_directory" if [[ -e "$source_directory" ]]; then mv "$source_directory" "$source_directory.before-tproxy.$(date +%Y%m%d%H%M%S)" fi @@ -52,6 +48,17 @@ if [[ ! -x "$source_directory/objs/bin/mtproto-proxy" ]] || rm -rf "$temporary" fi +# install.sh sets umask 077 before calling this script, so make's output +# (objs/, objs/bin/, the binary) keeps owner-only permissions; mtproxy.service +# runs this binary as User=mtproxy, which can then neither traverse nor exec +# it. Grant exactly the mtproxy group what it needs, matching the root:mtproxy +# 0750 scheme this installer already uses for /etc/mtproxy below, rather than +# opening the tree to every user on the host. This runs on every invocation, +# not only after a fresh build, so re-running the installer also repairs a +# host a previous interrupted run left in a bad state. +chown root:mtproxy "$source_directory/objs" "$source_directory/objs/bin" "$source_directory/objs/bin/mtproto-proxy" +chmod 0750 "$source_directory/objs" "$source_directory/objs/bin" "$source_directory/objs/bin/mtproto-proxy" + install -d -o root -g mtproxy -m 0750 /etc/mtproxy secret_temp="$(mktemp /etc/mtproxy/proxy-secret.XXXXXX)" config_temp="$(mktemp /etc/mtproxy/proxy-multi.conf.XXXXXX)" diff --git a/deploy/install.sh b/deploy/install.sh index 93c9653..ef33d39 100755 --- a/deploy/install.sh +++ b/deploy/install.sh @@ -143,10 +143,7 @@ if [[ -z "$go_binary" ]]; then go_binary="/opt/go${go_version}/bin/go" fi -# Run tests under a normal umask: this script's own umask 077 makes a -# config-package test fixture come out 0400 instead of the 0444 it -# writes, which then fails TestLoadAcceptsSystemdCredentialReadPermissions. -(umask 022; cd "$repository" && "$go_binary" test ./...) +(cd "$repository" && "$go_binary" test ./...) (cd "$repository" && "$go_binary" build -trimpath -ldflags='-s -w' -o /usr/local/bin/tproxy-server ./cmd/tproxy-server) chown root:root /usr/local/bin/tproxy-server chmod 0755 /usr/local/bin/tproxy-server diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d977312..0d33e48 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -234,6 +234,14 @@ func TestLoadAcceptsSystemdCredentialReadPermissions(t *testing.T) { if err := os.WriteFile(profiles, []byte(content), 0444); err != nil { t.Fatal(err) } + // os.WriteFile applies the test binary's process umask, so a restrictive + // umask (install.sh runs the suite under 077) silently strips the + // group/other bits the second Load call below must reject, making that + // assertion pass vacuously. Set the mode explicitly so the fixture means + // what it says regardless of the caller's umask. + if err := os.Chmod(profiles, 0444); err != nil { + t.Fatal(err) + } t.Setenv("CREDENTIALS_DIRECTORY", credentials) server := `{"public_hostname":"proxy.example.com","public_dir":"public","profiles_file":"credentials/profiles.json"}` path := filepath.Join(directory, "config.json")