Repository navigation
feat(security): add a --security "anonymous=..." posture, defaulting to full #9844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
matthewmcneely
merged 6 commits into
main
from
matthewmcneely/security-anonymous-posture
Oct 6, 2026
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
1704897
feat(security): add a --security "anonymous=..." posture, defaulting …
matthewmcneely 5b591e3
fix(test): alias the go/token import to avoid a collision under the i…
matthewmcneely e74554c
fix(test): replace deprecated parser.ParseDir in the HTTP identity test
matthewmcneely 9fbb6a6
fix(zero): correct the --security anonymous help text
matthewmcneely 2d779c2
fix(security): address review on the anonymous posture warnings and i…
matthewmcneely 79de59d
fix(security): gate every Alter on the posture, and resolve identity …
matthewmcneely File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package alpha | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/dgraph-io/dgraph/v25/x" | ||
| ) | ||
|
|
||
| // securityWarnings returns the startup warnings for a --security configuration | ||
| // whose parts do not add up, or nil when they do. | ||
| // | ||
| // Three controls gate Alpha's privileged operations -- the whitelist, the auth | ||
| // token, and ACL -- and each one passes when its own feature is unconfigured. The | ||
| // protection an operator gets is whatever they configured rather than the union of | ||
| // the three, and the two combinations below are the ones where that produces an | ||
| // outcome they almost certainly did not intend. | ||
| // | ||
| // It is pure and returns strings rather than logging, so the combinations can be | ||
| // pinned by a table test. | ||
| func securityWarnings(posture x.AnonymousPosture, whitelist string, ips []x.IPRange, | ||
| authToken string, aclEnabled bool, httpPort int) []string { | ||
|
|
||
| hasCredential := aclEnabled || authToken != "" | ||
| var out []string | ||
|
|
||
| // The shipped default is an empty whitelist, which admits loopback only, so the | ||
| // admin plane does not leave the host without an operator widening it. This | ||
| // warns at exactly that point: whitelisting answers where a request came from | ||
| // and has no credential in it, so a widened range with nothing else configured | ||
| // means every address inside it can run privileged operations anonymously. | ||
| if posture == x.AnonymousFull && len(ips) > 0 && !hasCredential { | ||
| out = append(out, fmt.Sprintf( | ||
| `SECURITY: --security "whitelist=%s" admits non-loopback callers, but neither ACL nor `+ | ||
| `an admin token is configured. Privileged operations (backup, restore, export, `+ | ||
| `shutdown, removeNode, moveTablet, assign, draining, config, namespace create and `+ | ||
| `drop) are reachable from that range WITHOUT ANY CREDENTIAL: anyone who can reach `+ | ||
| `port %d can read the whole database via backup or export, restore over it, or shut `+ | ||
| `the cluster down. Set --security "token=..." or enable ACL, and narrow the `+ | ||
| `whitelist to the addresses that actually administer this cluster. `+ | ||
| `--security "anonymous=data" additionally denies every administrative operation to `+ | ||
| `a caller that presents no credential.`, whitelist, httpPort)) | ||
| } | ||
|
|
||
| // A closed posture with nothing that can produce an identity. Every capability | ||
| // check will deny, including the operator's own, so the cluster cannot be | ||
| // administered at all. This is a misconfiguration rather than a hardening, and | ||
| // it is worth saying so loudly at boot instead of letting it surface as a | ||
| // permission error during an incident. | ||
| if posture.RequiresIdentityForCapability() && !hasCredential { | ||
| out = append(out, fmt.Sprintf( | ||
| `SECURITY: --security "anonymous=%s" requires an identified caller, but neither ACL nor `+ | ||
| `an admin token is configured, so no request can ever be identified. Every `+ | ||
| `administrative operation will be denied, including from loopback, and this cluster `+ | ||
| `cannot be administered. Set --security "token=..." or enable ACL.`, posture)) | ||
|
matthewmcneely marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| return out | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,205 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package alpha | ||
|
|
||
| import ( | ||
| "go/ast" | ||
| "go/parser" | ||
| gotoken "go/token" | ||
| "path/filepath" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/dgraph-io/dgraph/v25/x" | ||
| ) | ||
|
|
||
| func TestSecurityWarnings(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| posture x.AnonymousPosture | ||
| whitelist string | ||
| authToken string | ||
| acl bool | ||
| want []string // substrings that must appear, one per expected warning | ||
| }{{ | ||
| // The shipped default. An empty whitelist admits loopback only, so the admin | ||
| // plane has not left the host and there is nothing to say. | ||
| name: "stock alpha is quiet", | ||
| posture: x.AnonymousFull, | ||
| }, { | ||
| name: "widened whitelist with no credential", | ||
| posture: x.AnonymousFull, | ||
| whitelist: "0.0.0.0/0", | ||
| want: []string{"WITHOUT ANY CREDENTIAL"}, | ||
| }, { | ||
| // A plausible-looking CIDR is still every pod in the cluster. | ||
| name: "private CIDR with no credential", | ||
| posture: x.AnonymousFull, | ||
| whitelist: "10.0.0.0/8", | ||
| want: []string{"WITHOUT ANY CREDENTIAL"}, | ||
| }, { | ||
| name: "widened whitelist with a token", | ||
| posture: x.AnonymousFull, | ||
| whitelist: "0.0.0.0/0", | ||
| authToken: "s3cr3t", | ||
| }, { | ||
| name: "widened whitelist with ACL", | ||
| posture: x.AnonymousFull, | ||
| whitelist: "0.0.0.0/0", | ||
| acl: true, | ||
| }, { | ||
| // The closed posture answers the exposure, so the first warning must stand | ||
| // down rather than tell an operator to fix something they already fixed. | ||
| name: "widened whitelist with a closed posture", | ||
| posture: x.AnonymousData, | ||
| whitelist: "0.0.0.0/0", | ||
| authToken: "s3cr3t", | ||
| }, { | ||
| // A closed posture with nothing that can produce an identity. Every | ||
| // capability check denies, including the operator's own. | ||
| name: "closed posture with no credential at all", | ||
| posture: x.AnonymousData, | ||
| want: []string{"cannot be administered"}, | ||
| }, { | ||
| name: "anonymous=none with no credential at all", | ||
| posture: x.AnonymousNone, | ||
| whitelist: "0.0.0.0/0", | ||
| want: []string{"cannot be administered"}, | ||
| }, { | ||
| name: "closed posture with ACL is fine", | ||
| posture: x.AnonymousNone, | ||
| whitelist: "0.0.0.0/0", | ||
| acl: true, | ||
| }} | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| ips, err := getIPsFromString(tt.whitelist) | ||
| require.NoError(t, err) | ||
|
|
||
| got := securityWarnings(tt.posture, tt.whitelist, ips, tt.authToken, tt.acl, 8080) | ||
| require.Len(t, got, len(tt.want)) | ||
| for i, want := range tt.want { | ||
| require.Contains(t, got[i], "SECURITY:") | ||
| require.Contains(t, got[i], want) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestDefaultWhitelistAdmitsLoopbackOnly pins the model the v25 default rests on: | ||
| // a stock `dgraph alpha`, with no --security flag, serves admin operations to this | ||
| // host and nothing else. That is what makes anonymous=full a defensible default, | ||
| // and what makes widening the whitelist the moment the admin plane leaves the | ||
| // host. | ||
| func TestDefaultWhitelistAdmitsLoopbackOnly(t *testing.T) { | ||
| prev := x.WorkerConfig.WhiteListedIPRanges | ||
| t.Cleanup(func() { x.WorkerConfig.WhiteListedIPRanges = prev }) | ||
|
|
||
| ips, err := getIPsFromString("") | ||
| require.NoError(t, err) | ||
| x.WorkerConfig.WhiteListedIPRanges = ips | ||
|
|
||
| for _, ip := range []string{"127.0.0.1", "::1"} { | ||
| require.Truef(t, x.IsIpWhitelisted(ip), "loopback %s must reach admin operations", ip) | ||
| } | ||
| for _, ip := range []string{ | ||
| "203.0.113.7", // arbitrary remote host | ||
| "172.17.0.1", // Docker bridge, i.e. the host reaching a published port | ||
| "10.0.5.7", // private network peer | ||
| "192.168.1.20", // LAN peer | ||
| } { | ||
| require.Falsef(t, x.IsIpWhitelisted(ip), | ||
| "non-loopback %s must not reach admin operations by default", ip) | ||
| } | ||
| } | ||
|
|
||
| // identityExceptions names the handlers that deliberately assemble the request | ||
| // context by hand, and why. Everything else must go through | ||
| // x.AttachRequestIdentity. | ||
| var identityExceptions = map[string]string{ | ||
| // Login is how an access JWT is obtained, so it must not depend on one being | ||
| // present, and it needs no Principal of its own: edgraph.Login authorizes on | ||
| // hasAdminAuth, which reads the peer and the auth token straight out of the | ||
| // metadata this prelude attaches. | ||
| "loginHandler": "login must not require a credential it is the means of issuing", | ||
| } | ||
|
|
||
| // TestHTTPEdgeResolvesIdentityThroughOneHelper pins an invariant that a live test | ||
| // caught the hard way. | ||
| // | ||
| // x.AttachRequestIdentity is the whole HTTP-edge prelude: access JWT, remote IP, | ||
| // auth token, then identity resolution. A handler that reaches for the individual | ||
| // pieces instead gets some of them, and the one it is most likely to omit is the | ||
| // auth token -- which is invisible until a closed --security "anonymous=..." | ||
| // posture is configured, at which point a caller presenting the token arrives | ||
| // unidentified and is refused. /state and /health?all both had exactly that shape. | ||
| // | ||
| // So: no handler in this package resolves identity by hand unless it is listed | ||
| // above with a reason. | ||
| func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { | ||
| banned := map[string]string{ | ||
| "AttachAccessJwt": "attaches the access JWT but not the --security auth token", | ||
| "AttachAuthToken": "attaches the auth token but resolves no Principal", | ||
| "AttachRemoteIP": "attaches the peer but resolves no Principal", | ||
| } | ||
|
|
||
| // Every non-test file in the package, regardless of build tags. parser.ParseDir is | ||
| // deprecated, and ignoring tags is what this test wants anyway: a handler compiled | ||
| // only on one platform is still a handler. | ||
| paths, err := filepath.Glob("*.go") | ||
| require.NoError(t, err) | ||
|
|
||
| // Aliased: the integration-tagged run_test.go declares a package-level `token`. | ||
| fset := gotoken.NewFileSet() | ||
| seenExceptions := map[string]bool{} | ||
| for _, path := range paths { | ||
| if strings.HasSuffix(path, "_test.go") { | ||
| continue | ||
| } | ||
| file, err := parser.ParseFile(fset, path, nil, 0) | ||
| require.NoError(t, err) | ||
|
|
||
| for _, decl := range file.Decls { | ||
| fn, ok := decl.(*ast.FuncDecl) | ||
| if !ok { | ||
| continue | ||
| } | ||
| _, exempt := identityExceptions[fn.Name.Name] | ||
| ast.Inspect(fn, func(n ast.Node) bool { | ||
| sel, ok := n.(*ast.SelectorExpr) | ||
| if !ok { | ||
| return true | ||
| } | ||
| pkgIdent, ok := sel.X.(*ast.Ident) | ||
| if !ok || pkgIdent.Name != "x" { | ||
| return true | ||
| } | ||
| why, bad := banned[sel.Sel.Name] | ||
| if !bad { | ||
| return true | ||
| } | ||
| if exempt { | ||
| seenExceptions[fn.Name.Name] = true | ||
| return true | ||
| } | ||
| t.Errorf("%s: %s calls x.%s, which %s. Use x.AttachRequestIdentity, or add "+ | ||
| "it to identityExceptions with a reason.", | ||
| fset.Position(sel.Pos()), fn.Name.Name, sel.Sel.Name, why) | ||
| return true | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // A stale exception is its own problem: it reads as a documented carve-out for | ||
| // something that no longer exists. | ||
| for name := range identityExceptions { | ||
| require.Truef(t, seenExceptions[name], | ||
| "identityExceptions lists %q, but it no longer assembles the context by hand", name) | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.