public/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-10-10 00:30:07 +09:00
Code Issues Packages Projects Releases Wiki Activity

util, types: parse unix_socket_permission strictly

Browse Source 
Default "0o770" failed the base-8 parse and silently became 0700.
Invalid, unquoted or above-0777 values now fail config load.

Fixes #3529
This commit is contained in:
LuoChen
2026-10-03 18:37:56 +08:00
committed by Kristoffer Dalby
parent c9852c43da
commit aaf4ccd8c9
7 changed files with 205 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+2
View File
@@ -99,6 +99,7 @@ clients, and how to run the same setup without Nix.
- `derp.paths` files must end in `.yaml`, `.yml`, `.json` or `.hujson`; the extension picks the format
- `dns.extra_records_path` must end in `.json`, `.hujson`, `.yaml` or `.yml`; the extension picks the format
- A `derp.paths` file that decodes to no regions now stops headscale from starting instead of being silently ignored
- `unix_socket_permission` must be a quoted octal string no higher than `"0777"`, e.g. `"0770"`; an unquoted number or an invalid value now stops headscale from starting instead of silently using `0700` [#3540](https://github.com/juanfont/headscale/pull/3540)
#### CLI
@@ -138,6 +139,7 @@ clients, and how to run the same setup without Nix.
- Fix SSH check periods coming from the first check rule for a node pair instead of the rule for the login user [#3517](https://github.com/juanfont/headscale/pull/3517)
- Policy changes resend a node's SSH policy only when it changed, sparing clients a full netmap rebuild [#3517](https://github.com/juanfont/headscale/pull/3517)
- Fix every grant being fully resolved as if it had `via` when the policy has no `via` grants, slowing map generation on large tailnets [#3538](https://github.com/juanfont/headscale/pull/3538)
- Fix the unix socket being created `0700` instead of the documented `0770` when `unix_socket_permission` is unset. The socket grants full admin access without authentication, so members of its group gain that access; check who is in it before upgrading [#3540](https://github.com/juanfont/headscale/pull/3540)
## 0.29.5 (202x-xx-xx)
cmd/headscale/headscale_test.go
+6 -2
View File
@@ -41,7 +41,9 @@ func TestConfigFileLoading(t *testing.T) {
assert.Empty(t, viper.GetString("tls_letsencrypt_hostname"))
assert.Equal(t, ":http", viper.GetString("tls_letsencrypt_listen"))
assert.Equal(t, "HTTP-01", viper.GetString("tls_letsencrypt_challenge_type"))
assert.Equal(t, fs.FileMode(0o770), util.GetFileMode("unix_socket_permission"))
mode, err := util.ParseFileMode(viper.GetString("unix_socket_permission"))
require.NoError(t, err)
assert.Equal(t, fs.FileMode(0o770), mode)
assert.False(t, viper.GetBool("logtail.enabled"))
}
@@ -71,6 +73,8 @@ func TestConfigLoading(t *testing.T) {
assert.Empty(t, viper.GetString("tls_letsencrypt_hostname"))
assert.Equal(t, ":http", viper.GetString("tls_letsencrypt_listen"))
assert.Equal(t, "HTTP-01", viper.GetString("tls_letsencrypt_challenge_type"))
assert.Equal(t, fs.FileMode(0o770), util.GetFileMode("unix_socket_permission"))
mode, err := util.ParseFileMode(viper.GetString("unix_socket_permission"))
require.NoError(t, err)
assert.Equal(t, fs.FileMode(0o770), mode)
assert.False(t, viper.GetBool("logtail.enabled"))
}
config-example.yaml
+1
View File
@@ -361,6 +361,7 @@ dns:
# Unix socket used for the CLI to connect without authentication
# Note: for production you will want to set this to something like:
unix_socket: /var/run/headscale/headscale.sock
# Octal, quoted: YAML reads an unquoted 0770 as the number 504.
unix_socket_permission: "0770"
# OpenID Connect
hscontrol/types/config.go
+17 -1
View File
@@ -1314,6 +1314,22 @@ func LoadServerConfig() (*Config, error) {
})
}
// YAML reads an unquoted 0770 as the integer 504, which cannot be told
// apart from a literal 504, so only a string is accepted.
socketPermRaw := viper.Get("unix_socket_permission")
socketPermStr, _ := socketPermRaw.(string)
socketPerm, err := util.ParseFileMode(socketPermStr)
if err != nil {
v.Add(&ConfigError{
Reason: "unix_socket_permission is not a quoted octal file mode",
Current: []KV{{"unix_socket_permission", socketPermRaw}},
Maximum: `"0777"`,
Hint: `quote the value, e.g. "0770"; unquoted, YAML reads 0770 as the number 504`,
Cause: err,
})
}
dnsConfig, err := dns()
if err != nil {
v.Add(&ConfigError{
@@ -1426,7 +1442,7 @@ func LoadServerConfig() (*Config, error) {
ACMEURL: viper.GetString("acme_url"),
UnixSocket: viper.GetString("unix_socket"),
UnixSocketPermission: util.GetFileMode("unix_socket_permission"),
UnixSocketPermission: socketPerm,
OIDC: OIDCConfig{
OnlyStartIfOIDCIsAvailable: viper.GetBool(
hscontrol/types/config_test.go
+77
View File
@@ -3,6 +3,7 @@ package types
import (
"encoding/json"
"fmt"
"io/fs"
"net/netip"
"os"
"path/filepath"
@@ -11,6 +12,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/spf13/viper"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -681,6 +683,81 @@ dns:
}
}
// The unset case is the regression: the default is spelled "0o770", which a
// base-8 parse rejects, so the socket silently became 0700.
// https://github.com/juanfont/headscale/issues/3529
func TestUnixSocketPermission(t *testing.T) {
tests := []struct {
name string
yaml string
env string
want fs.FileMode
wantErr bool
}{
{name: "unset-uses-default", want: 0o770},
{name: "null-uses-default", yaml: `unix_socket_permission: null`, want: 0o770},
{name: "quoted-leading-zero", yaml: `unix_socket_permission: "0770"`, want: 0o770},
{name: "quoted-plain", yaml: `unix_socket_permission: "770"`, want: 0o770},
{name: "quoted-0o-prefix", yaml: `unix_socket_permission: "0o770"`, want: 0o770},
{name: "quoted-0o-prefix-owner-only", yaml: `unix_socket_permission: "0o700"`, want: 0o700},
{name: "quoted-zero", yaml: `unix_socket_permission: "0000"`, want: 0},
{name: "env", env: "0660", want: 0o660},
{name: "env-0o-prefix", env: "0o660", want: 0o660},
{name: "env-overrides-file", yaml: `unix_socket_permission: "0770"`, env: "0700", want: 0o700},
{name: "env-invalid-is-error", env: "bogus", wantErr: true},
{name: "empty-is-error", yaml: `unix_socket_permission: ""`, wantErr: true},
{name: "symbolic-is-error", yaml: `unix_socket_permission: "rwxrwx---"`, wantErr: true},
{name: "special-bits-are-error", yaml: `unix_socket_permission: "1770"`, wantErr: true},
{name: "out-of-range-is-error", yaml: `unix_socket_permission: "17777"`, wantErr: true},
// YAML reads unquoted 0770 as int 504, indistinguishable from a literal 504.
{name: "unquoted-leading-zero-is-error", yaml: `unix_socket_permission: 0770`, wantErr: true},
{name: "unquoted-0o-prefix-is-error", yaml: `unix_socket_permission: 0o770`, wantErr: true},
{name: "unquoted-plain-is-error", yaml: `unix_socket_permission: 770`, wantErr: true},
{name: "float-is-error", yaml: `unix_socket_permission: 770.0`, wantErr: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
viper.Reset()
if tt.env != "" {
t.Setenv("HEADSCALE_UNIX_SOCKET_PERMISSION", tt.env)
}
tmpDir := t.TempDir()
cfg := `---
server_url: https://example.com
listen_addr: 0.0.0.0:8080
prefixes:
v4: 100.64.0.0/10
noise:
private_key_path: noise_private.key
database:
type: sqlite3
dns:
magic_dns: false
override_local_dns: false
` + tt.yaml + "\n"
require.NoError(t, os.WriteFile(
filepath.Join(tmpDir, "config.yaml"), []byte(cfg), 0o600))
require.NoError(t, LoadConfig(tmpDir, false))
got, err := LoadServerConfig()
if tt.wantErr {
require.ErrorIs(t, err, util.ErrInvalidFileMode,
"invalid unix_socket_permission must not silently fall back")
assert.Contains(t, err.Error(), "unix_socket_permission")
return
}
require.NoError(t, err)
assert.Equal(t, tt.want, got.UnixSocketPermission,
"got %#o, want %#o", got.UnixSocketPermission, tt.want)
})
}
}
// OK
// server_url: headscale.com, base: clients.headscale.com
// server_url: headscale.com, base: headscale.net
hscontrol/util/file.go
+11 -7
View File
@@ -27,6 +27,9 @@ const (
// ErrDirectoryPermission is returned when creating a directory fails due to permission issues.
var ErrDirectoryPermission = errors.New("creating directory failed with permission error")
// ErrInvalidFileMode is returned by [ParseFileMode].
var ErrInvalidFileMode = errors.New("not an octal file mode between 0 and 0777")
// ErrUnknownFileFormat is returned for a file whose extension names no format
// [UnmarshalByExt] reads.
var ErrUnknownFileFormat = errors.New("unknown file format, want .json, .hujson, .yaml or .yml")
@@ -87,15 +90,16 @@ func AbsolutePathFromConfigPath(path string) string {
return path
}
func GetFileMode(key string) fs.FileMode {
modeStr := viper.GetString(key)
mode, err := strconv.ParseUint(modeStr, Base8, BitSize64)
if err != nil {
return PermissionFallback
// ParseFileMode parses an octal permission such as "0770", "770" or "0o770".
// Bits above [fs.ModePerm] are rejected: [os.Chmod] drops them, so "17777"
// would silently become 0777.
func ParseFileMode(s string) (fs.FileMode, error) {
mode, err := strconv.ParseUint(strings.TrimPrefix(strings.ToLower(s), "0o"), Base8, BitSize64)
if err != nil || mode > uint64(fs.ModePerm) {
return 0, fmt.Errorf("%w: %q", ErrInvalidFileMode, s)
}
return fs.FileMode(mode) //nolint:gosec // file mode is bounded by ParseUint
return fs.FileMode(mode), nil
}
func EnsureDir(dir string) error {
hscontrol/util/util_test.go
+91
View File
@@ -2,6 +2,8 @@ package util
import (
"errors"
"fmt"
"io/fs"
"net/netip"
"strings"
"testing"
@@ -948,3 +950,92 @@ func TestUnmarshalByExt(t *testing.T) {
})
}
}
func TestParseFileMode(t *testing.T) {
tests := []struct {
in string
want fs.FileMode
wantErr bool
}{
{in: "770", want: 0o770},
{in: "0770", want: 0o770},
{in: "0o770", want: 0o770},
{in: "0O770", want: 0o770},
{in: "0o700", want: 0o700},
{in: "0o0770", want: 0o770},
{in: "00770", want: 0o770},
{in: "0640", want: 0o640},
{in: "600", want: 0o600},
{in: "0", want: 0},
{in: "000", want: 0},
{in: "0o0", want: 0},
{in: "7", want: 0o7},
{in: "0777", want: 0o777},
{in: "0o777", want: 0o777},
{in: "", wantErr: true},
{in: "0o", wantErr: true},
{in: "not-a-mode", wantErr: true},
{in: "-770", wantErr: true},
{in: "0x1f8", wantErr: true},
{in: "0b111111000", wantErr: true},
{in: "0o0o770", wantErr: true},
{in: "0o-770", wantErr: true},
{in: "+770", wantErr: true},
{in: "0_770", wantErr: true},
{in: "778", wantErr: true},
{in: "0779", wantErr: true},
{in: " 0770", wantErr: true},
{in: "0770 ", wantErr: true},
{in: "0770\n", wantErr: true},
{in: "rwxrwx---", wantErr: true},
{in: "u=rwx,g=rwx", wantErr: true},
{in: "99999999999999999999999", wantErr: true},
// Bits above 0777 would be dropped by os.Chmod, widening to 0777.
{in: "1770", wantErr: true},
{in: "01770", wantErr: true},
{in: "0o1770", wantErr: true},
{in: "1777", wantErr: true},
{in: "07777", wantErr: true},
{in: "17777", wantErr: true},
{in: "0o1000", wantErr: true},
}
for _, tt := range tests {
t.Run(tt.in, func(t *testing.T) {
got, err := ParseFileMode(tt.in)
if tt.wantErr {
if !errors.Is(err, ErrInvalidFileMode) {
t.Fatalf("ParseFileMode(%q) error = %v, want %v", tt.in, err, ErrInvalidFileMode)
}
return
}
if err != nil {
t.Fatalf("ParseFileMode(%q) error = %v", tt.in, err)
}
if got != tt.want {
t.Errorf("ParseFileMode(%q) = %#o, want %#o", tt.in, got, tt.want)
}
})
}
}
// Every permission must parse back from each spelling it is commonly written in.
func TestParseFileModeAllPermissions(t *testing.T) {
for m := range fs.ModePerm + 1 {
for _, s := range []string{
fmt.Sprintf("%o", m),
fmt.Sprintf("%03o", m),
fmt.Sprintf("%04o", m),
fmt.Sprintf("0o%o", m),
fmt.Sprintf("0O%03o", m),
} {
got, err := ParseFileMode(s)
if err != nil || got != m {
t.Errorf("ParseFileMode(%q) = %#o, %v; want %#o", s, got, err, m)
}
}
}
}