Simply error surfacing

This commit is contained in:
Theodor S. Midtlien
2026-09-25 11:14:34 +02:00
parent ec3a20e979
commit 1939f44bcd
22 changed files with 332 additions and 47 deletions
+1 -1
View File
@@ -143,7 +143,7 @@ func (g *AuthzGate) authorize(ctx context.Context, method string, msg any) (cont
if presentable := presentableHandleError(handle, handleErr); presentable != nil {
return ctx, presentable
}
return ctx, denyPolicyLevel(req, policy)
return ctx, denyPolicyLevel(req, policy, target)
}
for _, rule := range policy.Rules {
if err := rule(req); err != nil {
+23 -10
View File
@@ -64,17 +64,14 @@ func denyLevel(r Request, want AuthzLevel) error {
// denyPolicyLevel refuses a caller at the gate, where the policy is in hand.
//
// Requiring privilege is the one denial a caller can act on, so it carries the
// elevated command rather than a bare refusal. A privileged method that declares
// no action keeps the plain message. Rules deny through denyLevel instead: they
// cannot reach the policy table without an initialization cycle, and no rule
// requires privilege.
func denyPolicyLevel(r Request, p MethodPolicy) error {
// A privileged method that declares no action keeps the plain message. Rules
// deny through denyLevel instead: they cannot reach the policy table without an
// initialization cycle, and no rule requires privilege.
func denyPolicyLevel(r Request, p MethodPolicy, target Target) error {
switch p.Level {
case AuthzLevelPrivileged:
if p.Action != "" {
actor, command := RequiredActor(p.Command)
return PrivilegeError(PrivilegeSummary(p.Action, actor), command)
return denyPrivileged(p, target)
}
case AuthzLevelSessionHolder:
@@ -83,11 +80,27 @@ func denyPolicyLevel(r Request, p MethodPolicy) error {
if r.Level == AuthzLevelProfileOwner {
return SessionHeldError(p.Action)
}
return NotOwnerError(p.Action)
return denyOwnership(p.Action, r.Identity, target)
case AuthzLevelProfileOwner:
return NotOwnerError(p.Action)
return denyOwnership(p.Action, r.Identity, target)
}
return denyLevel(r, p.Level)
}
// denyPrivileged refuses a method that needs a privileged caller.
// Claiming a profile is currently the only privileged method
func denyPrivileged(p MethodPolicy, target Target) error {
actor, command := RequiredActor(ClaimCommand(target.Handle))
return PrivilegeError(PrivilegeSummary(p.Action, actor), command)
}
// denyOwnership refuses a caller with no standing on the profile. Only a profile
// nobody has claimed is theirs to put right, so only that one carries a command.
func denyOwnership(action string, id Identity, target Target) error {
if target.UnOwned {
return UnownedError(action, target.Handle, consoleLookup(id))
}
return NotOwnerError(action)
}
+4
View File
@@ -25,6 +25,10 @@ func IsConsoleUser(id Identity) bool {
return guardConsoleLookup(id, isConsoleUser)
}
// consoleLookup is a variable so a test can decide whether a caller is at the
// console without the machine running it having a seat of its own.
var consoleLookup = IsConsoleUser
// guardConsoleLookup runs a platform lookup and turns a panic out of it into
// "cannot confirm".
func guardConsoleLookup(id Identity, lookup func(Identity) bool) (atConsole bool) {
+3 -10
View File
@@ -45,13 +45,8 @@ type MethodPolicy struct {
Audit bool
TargetsProfile bool
// Action and Command turn a privilege denial into guidance the caller can
// act on. Action reads as the subject of a sentence ("claiming a profile"),
// Command is the same operation run with the privileges it needs. Only read
// when Level is AuthzLevelPrivileged, the one denial a caller can fix by
// running as somebody else.
Action string
Command string
// Action reads as the subject of a refusal message ("claiming a profile").
Action string
}
// RequireHolderForFullStatus escalates a StatusRequest that asks for peer detail
@@ -133,14 +128,12 @@ var methodPolicies = map[string]MethodPolicy{
servicePath + "TracePacket": {Level: AuthzLevelSessionHolder, Action: "tracing a packet"},
servicePath + "TriggerUpdate": {Level: AuthzLevelSessionHolder, Audit: true, Action: "starting an update"},
// Root or administrator only. Claiming names an arbitrary principal, so the
// caller asserts who a profile belongs to. Ownership does not enter it.
// Root or administrator only.
servicePath + "ClaimProfile": {
Level: AuthzLevelPrivileged,
TargetsProfile: true,
Audit: true,
Action: "claiming a profile",
Command: ElevatedCommand("netbird profile claim <profile>"),
},
}
+167 -20
View File
@@ -20,27 +20,24 @@ func TestPoliciesDeclareAnAction(t *testing.T) {
}
}
// A privileged method must also say how to satisfy it, since that is the one
// refusal the caller can act on.
func TestPrivilegedPoliciesDeclareGuidance(t *testing.T) {
// A refusal that names no action cannot say what was refused, and a privileged
// method drops to a bare message without one.
func TestPrivilegedPoliciesDeclareAnAction(t *testing.T) {
for method, policy := range methodPolicies {
if policy.Level != AuthzLevelPrivileged {
continue
}
assert.NotEmpty(t, policy.Command, "%s requires privilege but declares no Command", method)
assert.NotEmpty(t, policy.Action, "%s requires privilege but declares no Action", method)
}
}
// Only privilege is something the caller can run their way out of. The other
// refusals explain and stop there.
func TestOnlyPrivilegedPoliciesDeclareACommand(t *testing.T) {
for method, policy := range methodPolicies {
if policy.Level == AuthzLevelPrivileged {
continue
}
assert.Empty(t, policy.Command, "%s is not privileged but offers a command", method)
}
}
// ownedByAnother is the profile most of these refusals are about: it exists and
// records an owner, that owner is simply not this caller.
var ownedByAnother = Target{Path: "/profiles/someone-else.json"}
// unownedProfile records no owner, which is what a machine set up with nobody at
// its console leaves behind.
var unownedProfile = Target{Path: "/profiles/default.json", UnOwned: true, Handle: "default"}
func TestDenyPolicyLevelCarriesPrivilegeGuidance(t *testing.T) {
req := Request{
@@ -49,7 +46,7 @@ func TestDenyPolicyLevelCarriesPrivilegeGuidance(t *testing.T) {
Method: servicePath + "ClaimProfile",
}
err := denyPolicyLevel(req, methodPolicies[servicePath+"ClaimProfile"])
err := denyPolicyLevel(req, methodPolicies[servicePath+"ClaimProfile"], unownedProfile)
require.Error(t, err)
st := gstatus.Convert(err)
@@ -65,7 +62,8 @@ func TestDenyPolicyLevelCarriesPrivilegeGuidance(t *testing.T) {
assert.Equal(t, ErrorReasonPrivilegeRequired, info.GetReason())
assert.Equal(t, ErrorDomain, info.GetDomain())
assert.NotEmpty(t, info.GetMetadata()[ErrorMetaSummary])
assert.NotEmpty(t, info.GetMetadata()[ErrorMetaCommand])
assert.Contains(t, info.GetMetadata()[ErrorMetaCommand], "netbird profile claim default",
"the guidance names the profile the request resolved to, not a placeholder")
}
// A profile that belongs to somebody else is explained, not answered with sudo.
@@ -77,7 +75,7 @@ func TestDenyPolicyLevelExplainsAProfileOwnedByAnother(t *testing.T) {
State: stubState{},
}
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"SetConfig"]))
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"SetConfig"], ownedByAnother))
assert.Equal(t, ErrorReasonNotProfileOwner, info.GetReason())
assert.Contains(t, info.GetMetadata()[ErrorMetaSummary], "belongs to another user")
@@ -94,7 +92,7 @@ func TestDenyPolicyLevelWithoutGuidanceStaysBare(t *testing.T) {
Method: servicePath + "NotARealMethod",
}
err := denyPolicyLevel(req, methodPolicyFor(req.Method))
err := denyPolicyLevel(req, methodPolicyFor(req.Method), ownedByAnother)
require.Error(t, err)
assert.Equal(t, codes.PermissionDenied, gstatus.Convert(err).Code())
assert.Empty(t, gstatus.Convert(err).Details())
@@ -126,7 +124,7 @@ func TestDenyPolicyLevelExplainsAHeldSession(t *testing.T) {
Method: servicePath + "Up",
}
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"Up"]))
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"Up"], ownedByAnother))
assert.Equal(t, ErrorReasonSessionHeld, info.GetReason())
summary := info.GetMetadata()[ErrorMetaSummary]
@@ -148,7 +146,7 @@ func TestDenyPolicyLevelBelowProfileOwnerBlamesOwnership(t *testing.T) {
Method: servicePath + "Up",
}
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"Up"]))
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"Up"], ownedByAnother))
assert.Equal(t, ErrorReasonNotProfileOwner, info.GetReason())
assert.Contains(t, info.GetMetadata()[ErrorMetaSummary], "belongs to another user")
assert.NotContains(t, info.GetMetadata()[ErrorMetaSummary], "connected",
@@ -236,3 +234,152 @@ func TestDenialFromFallsBackToTheStatusMessage(t *testing.T) {
require.True(t, ok)
assert.Equal(t, "refused for reasons", denial.Summary)
}
// A profile nobody has claimed is the headless install: the caller is not being
// kept out of somebody else's profile, they are being told to record an owner.
func TestDenyPolicyLevelOffersTheClaimForAnUnownedProfile(t *testing.T) {
req := Request{
Identity: KnownForTest(Identity{UID: 1000}),
Level: AuthzLevelIdentified,
Method: servicePath + "Up",
}
stubConsoleLookup(t, false)
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"Up"], unownedProfile))
assert.Equal(t, ErrorReasonProfileUnowned, info.GetReason())
summary := info.GetMetadata()[ErrorMetaSummary]
assert.Contains(t, summary, "Connecting", "the summary names what was refused")
assert.Contains(t, summary, "no owner on record")
assert.NotContains(t, summary, "another user",
"nobody owns it, so blaming another user would be untrue")
// Without the sudo prefix, which RequiredActor drops for a daemon that is
// not itself privileged, as this test process is not.
assert.Contains(t, info.GetMetadata()[ErrorMetaCommand], "netbird profile claim default",
"the command names the profile that was refused")
}
// The same refusal reaches a method that only needs profile owner, so a settings
// read on a fresh headless machine explains itself the same way connecting does.
func TestDenyPolicyLevelOffersTheClaimBelowSessionHolderToo(t *testing.T) {
req := Request{
Identity: KnownForTest(Identity{UID: 1000}),
Level: AuthzLevelIdentified,
Method: servicePath + "GetConfig",
}
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"GetConfig"], unownedProfile))
assert.Equal(t, ErrorReasonProfileUnowned, info.GetReason())
assert.Contains(t, info.GetMetadata()[ErrorMetaCommand], "netbird profile claim default")
}
// A session somebody else holds outranks the profile having no owner: the
// connection is what is in the way, and ending it is the remedy.
func TestDenyPolicyLevelKeepsTheHeldSessionAheadOfOwnership(t *testing.T) {
req := Request{
Identity: KnownForTest(Identity{UID: 1000}),
Level: AuthzLevelProfileOwner,
Method: servicePath + "Up",
}
info := denialDetail(t, denyPolicyLevel(req, methodPolicies[servicePath+"Up"], unownedProfile))
assert.Equal(t, ErrorReasonSessionHeld, info.GetReason())
}
// The claim command names the profile it is going to act on.
func TestClaimCommandNamesTheProfile(t *testing.T) {
assert.Equal(t, ElevatedCommand("netbird profile claim default"), ClaimCommand("default"))
assert.Equal(t, ElevatedCommand("netbird profile claim <profile>"), ClaimCommand(""),
"with no profile to name the caller fills in the placeholder")
}
// stubConsoleLookup decides whether a caller counts as being at the console,
// without the machine running the test having a seat of its own.
func stubConsoleLookup(t *testing.T, atConsole bool) {
t.Helper()
orig := consoleLookup
consoleLookup = func(Identity) bool { return atConsole }
t.Cleanup(func() { consoleLookup = orig })
}
// A caller away from the console is told what normally claims a profile, since
// a machine set up without one is how it goes unclaimed.
func TestUnownedSummaryNamesTheConsoleAwayFromIt(t *testing.T) {
summary := unownedSummary("connecting", false)
assert.Contains(t, summary, "has no owner on record")
assert.Contains(t, summary, "console")
assert.Contains(t, summary, "An explicit claim of the profile is needed.")
}
// A caller at the console who still finds no owner got here another way, a
// migration that did not finish among them.
func TestUnownedSummaryStaysQuietAboutTheConsoleAtIt(t *testing.T) {
summary := unownedSummary("connecting", true)
assert.Contains(t, summary, "has no owner on record")
assert.NotContains(t, summary, "console")
assert.Contains(t, summary, "An explicit claim of the profile is needed.",
"the remedy is the same wherever the caller is sitting")
}
// The reason and the command do not move with the caller, only the explanation
// of how the profile came to be unowned does.
func TestDenyOwnershipKeepsTheClaimForAConsoleCaller(t *testing.T) {
stubConsoleLookup(t, true)
info := denialDetail(t, denyOwnership("connecting", KnownForTest(Identity{UID: 1000}), unownedProfile))
assert.Equal(t, ErrorReasonProfileUnowned, info.GetReason())
assert.Contains(t, info.GetMetadata()[ErrorMetaCommand], "netbird profile claim default")
assert.NotContains(t, info.GetMetadata()[ErrorMetaSummary], "console")
}
// Two commands put an authorization refusal right, ending the session and
// recording an owner. Whatever a denial hands the caller is one of them.
func TestDenialsOfferOnlyTheTwoRemedies(t *testing.T) {
_, down := RequiredActor(DownCommand())
_, claim := RequiredActor(ClaimCommand(unownedProfile.Handle))
for name, err := range map[string]error{
"privileged": denyPrivileged(methodPolicies[servicePath+"ClaimProfile"], unownedProfile),
"session held": SessionHeldError("connecting"),
"unowned": UnownedError("connecting", unownedProfile.Handle, false),
"not owner": NotOwnerError("connecting"),
} {
t.Run(name, func(t *testing.T) {
denial, ok := DenialFrom(err)
require.True(t, ok, "every refusal has to be machine readable")
if denial.Command == "" {
return
}
assert.Contains(t, []string{down, claim}, denial.Command,
"a refusal offered a command that is neither remedy")
})
}
}
// A sudo prefix already says who has to run the command, so repeating it in the
// summary would be noise.
func TestRemedyNoteStaysQuietBehindSudo(t *testing.T) {
assert.Empty(t, remedyNote("root", "sudo netbird down"))
}
// Windows has no sudo to prefix and neither does a delegating daemon, so the
// summary is the only place that can name who must run the command.
func TestRemedyNoteNamesTheActorWithoutSudo(t *testing.T) {
assert.Equal(t, " Running this requires administrator privileges.",
remedyNote("administrator privileges", "netbird down"))
}
// The refusal a user hits when somebody else holds the session has to say what
// running the command it offers takes.
func TestSessionHeldSaysWhatRunningTheCommandTakes(t *testing.T) {
denial, ok := DenialFrom(SessionHeldError("switching profile"))
require.True(t, ok)
actor, command := RequiredActor(DownCommand())
require.NotContains(t, command, "sudo ", "this test process runs a delegating daemon")
assert.Contains(t, denial.Summary, "Running this requires "+actor)
}
+50 -4
View File
@@ -36,6 +36,10 @@ const (
// not what the method asked for, so telling the caller to elevate would send
// them the wrong way.
ErrorReasonNotProfileOwner = "NOT_PROFILE_OWNER"
// ErrorReasonProfileUnowned identifies a refusal caused by the profile
// recording no owner. It carries the command that records one.
ErrorReasonProfileUnowned = "PROFILE_UNOWNED"
)
// The identity of the process evaluating callers, captured once because it cannot
@@ -210,7 +214,8 @@ func PrivilegeError(summary, command string) error {
// SessionHeldError refuses an operation because another user has the machine
// connected.
func SessionHeldError(action string) error {
return denialError(ErrorReasonSessionHeld, sessionHeldSummary(action), ElevatedCommand("netbird down"))
actor, command := RequiredActor(DownCommand())
return denialError(ErrorReasonSessionHeld, sessionHeldSummary(action)+remedyNote(actor, command), command)
}
// NotOwnerError refuses an operation because the profile it addresses belongs to
@@ -219,6 +224,36 @@ func NotOwnerError(action string) error {
return denialError(ErrorReasonNotProfileOwner, notOwnerSummary(action), "")
}
// UnownedError refuses an operation because the profile it addresses has no
// owner on record, and names the command that gives it one. atConsole is whether
// the caller sits at one of this machine's consoles.
func UnownedError(action, handle string, atConsole bool) error {
actor, command := RequiredActor(ClaimCommand(handle))
return denialError(ErrorReasonProfileUnowned, unownedSummary(action, atConsole)+remedyNote(actor, command), command)
}
// remedyNote names who has to run the command a refusal offers.
func remedyNote(actor, command string) string {
if strings.HasPrefix(command, "sudo ") {
return ""
}
return " Running this requires " + actor + "."
}
// DownCommand renders the elevated command that ends the live session.
func DownCommand() string {
return ElevatedCommand("netbird down")
}
// ClaimCommand renders the elevated command that records an owner for a profile.
// With no profile to name it keeps the placeholder for the caller to fill in.
func ClaimCommand(handle string) string {
if handle == "" {
handle = "<profile>"
}
return ElevatedCommand("netbird profile claim " + handle)
}
// sessionHeldSummary says whose the connection is and why that settles it.
func sessionHeldSummary(action string) string {
return refusedSubject(action) + " refused while another user has this machine connected. " +
@@ -233,6 +268,19 @@ func notOwnerSummary(action string) string {
"so use one of your own or ask an administrator to hand this one over."
}
// unownedSummary says the profile belongs to nobody yet and what changes that.
// Only a caller away from the console is told about it, since a profile left
// unclaimed while somebody is at one got that way for another reason.
func unownedSummary(action string, atConsole bool) string {
cause := ""
if !atConsole {
cause = "A profile is claimed by the user who sets it up at this machine's console (in front of the machine), " +
"and nobody has done that here. "
}
return refusedSubject(action) + " refused because the profile it addresses has no owner on record. " +
cause + "An explicit claim of the profile is needed."
}
// refusedSubject opens a refusal with what was refused, falling back to the
// command itself for a method that names no action.
func refusedSubject(action string) string {
@@ -266,9 +314,7 @@ func denialError(reason, summary, command string) error {
}
// RequiredActor names who may perform the operation and adjusts the command to
// match. A daemon that is not itself privileged delegates to its own identity, so
// telling that host's user to become root is wrong twice over: root is not what the
// daemon checks for, and a rootless container has neither root nor sudo.
// match.
func RequiredActor(command string) (string, string) {
self, delegates := SelfDelegatesTo()
if !delegates {
+5
View File
@@ -6,6 +6,11 @@ import "context"
type Target struct {
Path string
Owned bool
// UnOwned reports that the resolved profile records no owner.
UnOwned bool
// Handle is the resolved profile's ID, which a refusal names in the command
// it hands the caller. A request that named nothing still has one.
Handle string
}
type targetKey struct{}