Clean up comments

This commit is contained in:
Theodor S. Midtlien
2026-07-24 14:57:21 +02:00
parent 57f9cbe5ff
commit 0137876618
23 changed files with 55 additions and 103 deletions
+3 -9
View File
@@ -12,10 +12,8 @@ type Ownership struct {
Shared bool
}
// GroupResolver resolves a Unix caller's effective group IDs (primary +
// supplementary, NSS-aware) and owner group names to GIDs. It is only consulted
// for Unix `gid:`/`group:` owners; Windows uses the SIDs carried in the Identity.
// A nil resolver disables group matching.
// GroupResolver resolves a Unix caller's effective group IDs and owner group
// names to GIDs. A nil resolver disables group matching.
type GroupResolver interface {
// CallerGIDs returns the set of group IDs the caller belongs to.
CallerGIDs(id Identity) map[uint32]struct{}
@@ -24,9 +22,7 @@ type GroupResolver interface {
}
// Authorize reports whether the identity may control a profile with the given
// ownership. Privileged callers (root / elevated-admin / LocalSystem) and shared
// profiles are always allowed; otherwise the identity must match one of the
// owner principals.
// ownership. Privileged callers and shared profiles are always allowed.
func Authorize(o Ownership, id Identity, r GroupResolver) bool {
if id.IsPrivileged() {
return true
@@ -76,8 +72,6 @@ func principalMatches(p Principal, id Identity, r GroupResolver) bool {
}
}
// callerHasGID reports whether gid is the caller's primary GID (from peercred,
// no lookup) or one of their supplementary groups (NSS-resolved via r).
func callerHasGID(gid uint32, id Identity, r GroupResolver) bool {
if id.GID == gid {
return true
+1 -2
View File
@@ -11,8 +11,7 @@ import (
)
// NewTransportCredentials returns nil on platforms without a peer-identity
// primitive. The daemon falls back to insecure credentials and skips per-RPC
// authorization (logging a warning), preserving pre-hardening behavior.
// primitive.
func NewTransportCredentials() credentials.TransportCredentials {
return nil
}
+3 -7
View File
@@ -17,10 +17,6 @@ func NewTransportCredentials() credentials.TransportCredentials {
return unixCreds{}
}
// unixCreds implements credentials.TransportCredentials over a Unix socket.
// The server side reads SO_PEERCRED/LOCAL_PEERCRED during the handshake; the
// client side is a no-op (the kernel supplies the peer identity to the server
// without any client cooperation), so an ordinary insecure client still works.
type unixCreds struct{}
func (unixCreds) ClientHandshake(_ context.Context, _ string, conn net.Conn) (net.Conn, credentials.AuthInfo, error) {
@@ -28,8 +24,8 @@ func (unixCreds) ClientHandshake(_ context.Context, _ string, conn net.Conn) (ne
}
// ConnIdentity extracts the caller's identity from an accepted local IPC
// connection. On Unix it reads peer credentials from the socket. It is shared by
// the gRPC transport credentials and the JSON gateway (which forwards it).
// connection. On Unix it reads peer credentials from the socket. It is shared
// by the gRPC transport credentials and the JSON gateway (which forwards it).
func ConnIdentity(conn net.Conn) (Identity, error) {
return PeerIdentity(conn)
}
@@ -47,7 +43,7 @@ func (unixCreds) ServerHandshake(conn net.Conn) (net.Conn, credentials.AuthInfo,
}
func (unixCreds) Info() credentials.ProtocolInfo {
return credentials.ProtocolInfo{SecurityProtocol: "netbird-ipc-peercred"}
return credentials.ProtocolInfo{SecurityProtocol: AuthInfo{}.AuthType()}
}
func (unixCreds) Clone() credentials.TransportCredentials { return unixCreds{} }
+3 -3
View File
@@ -17,8 +17,8 @@ var (
procImpersonateNamedPipeClient = modadvapi32.NewProc("ImpersonateNamedPipeClient")
)
// DefaultPipeSDDL keeps the daemon control pipe open to any LOCAL caller, on par
// with the Unix socket's 0666 mode.
// DefaultPipeSDDL keeps the daemon control pipe open to any LOCAL caller,
// like Unix socket with 0666 permissions.
//
// D:P protected DACL, no inheritance
// (D;;GA;;;NU) deny GENERIC_ALL to NETWORK (remote/SMB)
@@ -69,7 +69,7 @@ func (winpipeCreds) ServerHandshake(conn net.Conn) (net.Conn, credentials.AuthIn
}
func (winpipeCreds) Info() credentials.ProtocolInfo {
return credentials.ProtocolInfo{SecurityProtocol: "netbird-ipc-peercred"}
return credentials.ProtocolInfo{SecurityProtocol: AuthInfo{}.AuthType()}
}
func (winpipeCreds) Clone() credentials.TransportCredentials { return winpipeCreds{} }
+3 -8
View File
@@ -8,9 +8,7 @@ import (
)
// Metadata keys used by the local JSON gateway to forward the HTTP client's
// identity to the daemon. Trusted by the interceptor ONLY when the gRPC peer is
// itself the daemon (self/privileged) — i.e. the loopback gateway — so a direct
// gRPC caller cannot forge them.
// identity to the daemon.
const (
mdFwdUID = "x-netbird-fwd-uid" // Unix
mdFwdGID = "x-netbird-fwd-gid" // Unix
@@ -20,9 +18,7 @@ const (
)
// ForwardIdentityMetadata encodes an identity for the gateway to forward to the
// daemon — Unix uid/gid, or the Windows user SID + enabled group SIDs +
// elevation. Both are supported so the gateway works whether the JSON socket is
// a Unix socket or a named pipe.
// daemon.
func ForwardIdentityMetadata(id Identity) metadata.MD {
if id.IsWindows() {
md := metadata.MD{}
@@ -41,8 +37,7 @@ func ForwardIdentityMetadata(id Identity) metadata.MD {
)
}
// forwardedIdentity extracts a forwarded identity from incoming gRPC metadata,
// if present and well-formed. Windows (SID) takes precedence over Unix (uid).
// forwardedIdentity extracts a forwarded identity from incoming gRPC metadata
func forwardedIdentity(ctx context.Context) (Identity, bool) {
md, ok := metadata.FromIncomingContext(ctx)
if !ok {
+1 -2
View File
@@ -33,9 +33,8 @@ func TestForwardIdentityRoundTrip(t *testing.T) {
func TestForwardedIdentity_None(t *testing.T) {
_, ok := forwardedIdentity(context.Background())
assert.False(t, ok, "no metadata → no forwarded identity")
assert.False(t, ok, "no metadata, no forwarded identity")
// Empty metadata (no forwarding keys) → none.
ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs("other", "x"))
_, ok = forwardedIdentity(ctx)
assert.False(t, ok)
+4 -10
View File
@@ -5,8 +5,7 @@
// On Unix the identity is read from the kernel via SO_PEERCRED (Linux) or
// LOCAL_PEERCRED (Darwin/FreeBSD). On Windows it is derived from the named-pipe
// client token. Platforms without a peer-identity primitive get no credentials
// and therefore no enforcement (the daemon logs a warning and stays open,
// preserving today's behavior until the transport gains an identity primitive).
// and therefore no enforcement.
package ipcauth
import (
@@ -21,8 +20,7 @@ import (
const sidLocalSystem = "S-1-5-18"
// Identity is the kernel-authenticated identity of a local IPC caller. The zero
// value is not a valid identity; obtain one via IdentityFromContext (which
// reports presence) or PeerIdentity.
// value is not a valid identity.
type Identity struct {
// UID and GID are the caller's Unix user ID and primary group ID.
// Zero on Windows, where SID is authoritative instead.
@@ -49,10 +47,7 @@ func (i Identity) IsWindows() bool {
}
// IsPrivileged reports whether the caller is the platform's administrative
// principal — Unix root (uid 0), or on Windows an elevated token or LocalSystem.
// It deliberately requires actual elevation on Windows (a non-elevated member of
// Administrators has a filtered token and is NOT privileged), mirroring "must
// really be root" on Unix.
// principal.
func (i Identity) IsPrivileged() bool {
if i.IsWindows() {
return i.Elevated || i.SID == sidLocalSystem
@@ -80,8 +75,7 @@ func (AuthInfo) AuthType() string { return "netbird-ipc-peercred" }
// IdentityFromContext extracts the caller's kernel-authenticated identity from
// the gRPC peer context. The second return value is false when no IPC transport
// credentials were negotiated (e.g. an unsupported platform, or a caller that
// did not come through the daemon socket) — callers MUST fail closed in that case.
// credentials were negotiated, callers MUST fail closed in that case.
func IdentityFromContext(ctx context.Context) (Identity, bool) {
p, ok := peer.FromContext(ctx)
if !ok {
+12 -14
View File
@@ -10,16 +10,14 @@ import (
"google.golang.org/grpc/status"
)
// Interceptor enforces per-RPC authorization on the daemon control channel,
// keyed to the caller's kernel-authenticated identity. It is safe-by-default:
// Interceptor enforces per-RPC authorization on the daemon IPC, keyed to
// the caller's kernel-authenticated identity. It is safe-by-default:
// any RPC without a matching bypass is gated by the active profile's ownership,
// and a caller without a readable identity is denied.
type Interceptor struct {
policy ProfilePolicy
resolver GroupResolver
// selfUID is the daemon's own effective UID. A caller whose UID matches it
// (rootless container / foreground daemon running as the invoking user) is
// allowed: it already has full control of the daemon process. -1 on Windows.
// selfUID is the daemon's own effective UID. -1 on Windows.
selfUID int
}
@@ -51,22 +49,22 @@ func (i *Interceptor) StreamServerInterceptor() grpc.StreamServerInterceptor {
func (i *Interceptor) authorize(ctx context.Context, fullMethod string) error {
id, ok := IdentityFromContext(ctx)
if !ok {
log.Warnf("ipc authz: DENY %s — caller identity unavailable", fullMethod)
log.Warnf("ipc authz: DENY %s. caller identity unavailable", fullMethod)
return status.Error(codes.PermissionDenied, "caller identity could not be verified on the daemon control channel")
}
if i.isSelfOrPrivileged(id) {
// The local JSON gateway connects as the daemon itself (self/privileged)
// and forwards the real HTTP client's identity. Trust it here — and only
// here, where the transport peer is already the daemon — then authorize
// as the forwarded client. A direct non-privileged caller never reaches
// this branch, so it cannot forge the forwarding metadata.
// and forwards the real HTTP client's identity. Trust it here, where
// the transport peer is already the daemon, then authorize as the
// forwarded client. A direct non-privileged caller never reaches this
// branch, so it cannot forge the forwarding metadata.
fwd, hasFwd := forwardedIdentity(ctx)
if !hasFwd {
i.auditAllow(id, fullMethod)
return nil
}
log.Infof("ipc authz: honoring gateway-forwarded identity %s", fwd)
log.Infof("ipc authz: gateway-forwarded identity %s", fwd)
id = fwd
if i.isSelfOrPrivileged(id) {
i.auditAllow(id, fullMethod)
@@ -82,7 +80,7 @@ func (i *Interceptor) authorize(ctx context.Context, fullMethod string) error {
o := i.policy.ActiveProfileOwnership()
// Trust-on-first-use: an unowned, non-shared profile is claimed by the first
// caller. The claim is atomic; if we lose the race we re-read and authorize.
// caller. The claim is atomic, if we lose the race we re-read and authorize.
if len(o.Owners) == 0 && !o.Shared {
claimed, err := i.policy.ClaimActiveProfileOwnerIfUnowned(id)
if err != nil {
@@ -102,9 +100,9 @@ func (i *Interceptor) authorize(ctx context.Context, fullMethod string) error {
return nil
}
log.Warnf("ipc authz: DENY %s for %s — active profile owned by another principal", fullMethod, id)
log.Warnf("ipc authz: DENY %s for %s. active profile owned by another principal", fullMethod, id)
return status.Errorf(codes.PermissionDenied,
"not authorized to control the active profile (caller %s); ask an owner or run as root/administrator", id)
"not authorized to control the active profile (caller %s). ask an owner or run as root/administrator", id)
}
// isSelfOrPrivileged reports whether the caller is the platform administrator
+3 -4
View File
@@ -104,15 +104,14 @@ func TestInterceptorForwardedIdentity(t *testing.T) {
return metadata.NewIncomingContext(ctx, metadata.Pairs(mdFwdUID, itoa(fwdUID)))
}
// Gateway (peer == daemon-self) forwards a non-owner client → denied as that client.
// Gateway forwards a non-owner client: denied as that client.
i := &Interceptor{policy: &mockPolicy{o: owners}, selfUID: selfUID}
assert.Error(t, i.authorize(withFwd(selfUID, 2000), up))
// Gateway forwards the owner → allowed.
// Gateway forwards the owner: allowed.
assert.NoError(t, i.authorize(withFwd(selfUID, 1000), up))
// A non-privileged direct caller's forwarded metadata is IGNORED (can't forge):
// caller uid 2000 forwarding uid:1000 is still treated as 2000 → denied.
// A non-privileged direct caller's forwarded metadata: denied
assert.Error(t, i.authorize(withFwd(2000, 1000), up))
}
+4 -6
View File
@@ -5,7 +5,7 @@ import "sync"
const servicePath = "/daemon.DaemonService/"
// ProfilePolicy exposes the active profile's ownership to the interceptor. The
// daemon server implements it; ConfigAdapter bridges the gap because the gRPC
// daemon server implements it. ConfigAdapter bridges the gap because the gRPC
// server (and its interceptor) is constructed before the server instance exists.
type ProfilePolicy interface {
// ActiveProfileOwnership returns the active profile's ownership policy.
@@ -14,8 +14,7 @@ type ProfilePolicy interface {
// ClaimActiveProfileOwnerIfUnowned atomically claims the active profile for
// id when it has no owners and is not shared (trust-on-first-use), and
// reports whether id is now an owner. A false return means the profile was
// already owned/shared or another caller won the claim — the caller must
// re-read ownership and authorize normally.
// already owned/shared or another caller won the claim.
ClaimActiveProfileOwnerIfUnowned(id Identity) (bool, error)
}
@@ -30,8 +29,7 @@ var handlerAuthorizedMethods = map[string]bool{
servicePath + "RenameProfile": true,
}
// auditMethods are the Tier-C/H RPCs (threat model §3) whose successful
// authorization is worth an audit log line. Denials are always logged.
// auditMethods are worth an audit log line. Denials are always logged.
var auditMethods = map[string]bool{
servicePath + "GetConfig": true,
servicePath + "SetConfig": true,
@@ -56,7 +54,7 @@ var auditMethods = map[string]bool{
// ConfigAdapter is a ProfilePolicy whose backend is set lazily, once the daemon
// server instance is created. Until then it reports an unowned profile
// (Ownership zero value), so non-privileged callers are denied — fail closed.
// (Ownership zero value), so non-privileged callers are denied.
type ConfigAdapter struct {
mu sync.RWMutex
backend ProfilePolicy
+1 -2
View File
@@ -45,8 +45,7 @@ func UIDPrincipal(uid uint32) string {
func SIDPrincipal(sid string) string { return string(KindSID) + ":" + sid }
// OwnerPrincipalForIdentity returns the self-ownership principal for an identity:
// the user's UID on Unix, or the user's SID on Windows. Used to auto-isolate a
// new profile to its creator.
// the user's UID on Unix, or the user's SID on Windows.
func OwnerPrincipalForIdentity(id Identity) string {
if id.IsWindows() {
return SIDPrincipal(id.SID)
+1 -2
View File
@@ -12,8 +12,7 @@ import (
const groupCacheTTL = 30 * time.Second
// NewDefaultGroupResolver returns an NSS-aware group resolver backed by
// getent/`id -G` (via client/internal/shell), so `gid:`/`group:` owners resolve
// NewDefaultGroupResolver returns an NSS-aware group resolver, owners resolve
// correctly for LDAP/AD users under CGO_ENABLED=0. Results are cached briefly.
func NewDefaultGroupResolver() GroupResolver {
return &nssResolver{byUID: make(map[uint32]gidCacheEntry)}
+1 -1
View File
@@ -4,7 +4,7 @@ package ipcauth
// NewDefaultGroupResolver returns nil on Windows: group authorization uses the
// group SIDs carried in the client token (see the Windows transport
// credentials), not NSS/getent.
// credentials).
func NewDefaultGroupResolver() GroupResolver {
return nil
}
+1 -2
View File
@@ -192,8 +192,7 @@ type Config struct {
// Owners lists the principals allowed to control this profile over the local
// IPC, as typed strings: "uid:1000", "gid:1000", "group:netbird-admins"
// (Unix, NSS-resolved) or "sid:S-1-5-..." (Windows user or group SID). Empty
// with Shared=false means the profile is owned by nobody yet (privileged +
// daemon-self only, until claimed). Interpreted by client/internal/ipcauth.
// with Shared=false means the profile is owned by nobody yet, until claimed
Owners []string `json:"Owners,omitempty"`
// Shared, when true, lets any authenticated local caller control this profile
+1 -2
View File
@@ -5,7 +5,6 @@ package shell
import "os/user"
// LookupWithGetent on Windows just delegates to os/user.Lookup.
// Windows does not use NSS/getent; its user lookup works without CGO.
func LookupWithGetent(username string) (*user.User, error) {
return user.Lookup(username)
}
@@ -20,7 +19,7 @@ func LookupGroupWithGetent(name string) (*user.Group, error) {
return user.LookupGroup(name)
}
// GetShellFromGetent is a no-op on Windows; shell resolution uses PowerShell detection.
// GetShellFromGetent is a no-op on Windows.
func GetShellFromGetent(_ string) string {
return ""
}