From 207d16bf8bfa15af807176af696c2cb61c769fb3 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 8 Aug 2016 17:32:37 -0400 Subject: [PATCH] Don't allow root from authentication backends either. We've disabled this in the token store, but it makes no sense to have that disabled but have it enabled elsewhere. It's the same issue across all, so simply remove the ability altogether. --- vault/core_test.go | 29 +++++++++-- vault/request_handling.go | 60 +++++++++++++---------- website/source/docs/auth/app-id.html.md | 6 +-- website/source/docs/auth/github.html.md | 9 ++-- website/source/docs/auth/ldap.html.md | 4 +- website/source/docs/auth/userpass.html.md | 6 +-- 6 files changed, 73 insertions(+), 41 deletions(-) diff --git a/vault/core_test.go b/vault/core_test.go index 2363ba51b2..367d43ea4b 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1808,7 +1808,6 @@ func TestCore_RenewToken_SingleRegister(t *testing.T) { // Based on bug GH-203, attempt to disable a credential backend with leased secrets func TestCore_EnableDisableCred_WithLease(t *testing.T) { - // Create a badass credential backend that always logs in as armon noopBack := &NoopBackend{ Login: []string{"login"}, Response: &logical.Response{ @@ -1817,11 +1816,25 @@ func TestCore_EnableDisableCred_WithLease(t *testing.T) { }, }, } + c, _, root := TestCoreUnsealed(t) c.credentialBackends["noop"] = func(*logical.BackendConfig) (logical.Backend, error) { return noopBack, nil } + var secretWritingPolicy = ` +name = "admins" +path "secret/*" { + capabilities = ["update", "create", "read"] +} +` + + ps := c.policyStore + policy, _ := Parse(secretWritingPolicy) + if err := ps.SetPolicy(policy); err != nil { + t.Fatal(err) + } + // Enable the credential backend req := logical.TestRequest(t, logical.UpdateOperation, "sys/auth/foo") req.Data["type"] = "noop" @@ -1831,11 +1844,21 @@ func TestCore_EnableDisableCred_WithLease(t *testing.T) { t.Fatalf("err: %v", err) } - // Attempt to login + // Attempt to login -- should fail because we don't allow root to be returned lreq := &logical.Request{ Path: "auth/foo/login", } lresp, err := c.HandleRequest(lreq) + if err == nil || lresp == nil || !lresp.IsError() { + t.Fatalf("expected error trying to auth and receive root policy") + } + + // Fix and try again + noopBack.Response.Auth.Policies = []string{"admins"} + lreq = &logical.Request{ + Path: "auth/foo/login", + } + lresp, err = c.HandleRequest(lreq) if err != nil { t.Fatalf("err: %v", err) } @@ -1879,7 +1902,7 @@ func TestCore_EnableDisableCred_WithLease(t *testing.T) { // Disable the credential backend req = logical.TestRequest(t, logical.DeleteOperation, "sys/auth/foo") - req.ClientToken = lresp.Auth.ClientToken + req.ClientToken = root resp, err = c.HandleRequest(req) if err != nil { t.Fatalf("err: %v %#v", err, resp) diff --git a/vault/request_handling.go b/vault/request_handling.go index d2f99ae92c..7098239159 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -261,7 +261,7 @@ func (c *Core) handleRequest(req *logical.Request) (retResp *logical.Response, r // handleLoginRequest is used to handle a login request, which is an // unauthenticated request to the backend. -func (c *Core) handleLoginRequest(req *logical.Request) (*logical.Response, *logical.Auth, error) { +func (c *Core) handleLoginRequest(req *logical.Request) (retResp *logical.Response, retAuth *logical.Auth, retErr error) { defer metrics.MeasureSince([]string{"core", "handle_login_request"}, time.Now()) // Create an audit trail of the request, auth is not available on login requests @@ -271,6 +271,16 @@ func (c *Core) handleLoginRequest(req *logical.Request) (*logical.Response, *log return nil, nil, ErrInternalError } + // The token store uses authentication even when creating a new token, + // so it's handled in handleRequest. It should not be reached here. + if strings.HasPrefix(req.Path, "auth/token/") { + c.logger.Printf( + "[ERR] core: unexpected login request for token backend "+ + "(request path: %s)", req.Path) + retErr = multierror.Append(retErr, ErrInternalError) + return nil, nil, retErr + } + // Route the request resp, err := c.router.Route(req) if resp != nil { @@ -296,6 +306,10 @@ func (c *Core) handleLoginRequest(req *logical.Request) (*logical.Response, *log if resp != nil && resp.Auth != nil { auth = resp.Auth + if strutil.StrListSubset(auth.Policies, []string{"root"}) { + return logical.ErrorResponse("authentication backends cannot create root tokens"), nil, logical.ErrInvalidRequest + } + // Determine the source of the login source := c.router.MatchingMount(req.Path) source = strings.TrimPrefix(source, credentialRoutePrefix) @@ -311,8 +325,8 @@ func (c *Core) handleLoginRequest(req *logical.Request) (*logical.Response, *log return nil, nil, ErrInternalError } - // Set the default lease if non-provided, root tokens are exempt - if auth.TTL == 0 && !strutil.StrListContains(auth.Policies, "root") { + // Set the default lease if not provided + if auth.TTL == 0 { auth.TTL = sysView.DefaultLeaseTTL() } @@ -331,31 +345,27 @@ func (c *Core) handleLoginRequest(req *logical.Request) (*logical.Response, *log TTL: auth.TTL, } - if strutil.StrListSubset(te.Policies, []string{"root"}) { - te.Policies = []string{"root"} - } else { - // Use a map to filter out/prevent duplicates - policyMap := map[string]bool{} - for _, policy := range te.Policies { - if policy == "" { - // Don't allow a policy with no name, even though it is a valid - // slice member - continue - } - policyMap[policy] = true + // Use a map to filter out/prevent duplicates + policyMap := map[string]bool{} + for _, policy := range te.Policies { + if policy == "" { + // Don't allow a policy with no name, even though it is a valid + // slice member + continue } - - // Add the default policy - policyMap["default"] = true - - te.Policies = []string{} - for k, _ := range policyMap { - te.Policies = append(te.Policies, k) - } - - sort.Strings(te.Policies) + policyMap[policy] = true } + // Add the default policy + policyMap["default"] = true + + te.Policies = []string{} + for k, _ := range policyMap { + te.Policies = append(te.Policies, k) + } + + sort.Strings(te.Policies) + if err := c.tokenStore.create(&te); err != nil { c.logger.Printf("[ERR] core: failed to create token: %v", err) return nil, auth, ErrInternalError diff --git a/website/source/docs/auth/app-id.html.md b/website/source/docs/auth/app-id.html.md index e6e9169f28..ae2110322d 100644 --- a/website/source/docs/auth/app-id.html.md +++ b/website/source/docs/auth/app-id.html.md @@ -101,21 +101,21 @@ the set of App IDs, user IDs, and the mapping between them. An example is shown below, use `vault path-help` for more details. ``` -$ vault write auth/app-id/map/app-id/foo value=root display_name=foo +$ vault write auth/app-id/map/app-id/foo value=admins display_name=foo ... $ vault write auth/app-id/map/user-id/bar value=foo cidr_block=10.0.0.0/16 ... ``` -The above creates an App ID "foo" that associates with the policy "root". +The above creates an App ID "foo" that associates with the policy "admins". The `display_name` sets the display name for audit logs and secrets. Next, we configure the user ID "bar" and say that the user ID bar can be paired with "foo" but only if the client is in the "10.0.0.0/16" CIDR block. The `cidr_block` configuration is optional. This means that if a client authenticates and provide both "foo" and "bar", -then the app ID will authenticate that client with the policy "root". +then the app ID will authenticate that client with the policy "admins". In practice, both the user and app ID are likely hard-to-guess UUID-like values. diff --git a/website/source/docs/auth/github.html.md b/website/source/docs/auth/github.html.md index cc3bd9c375..4782fd9b75 100644 --- a/website/source/docs/auth/github.html.md +++ b/website/source/docs/auth/github.html.md @@ -50,7 +50,7 @@ The response will be in JSON. For example: "auth": { "client_token": "c4f280f6-fdb2-18eb-89d3-589e2e834cdb", "policies": [ - "root" + "admins" ], "metadata": { "org": "test_org", @@ -109,12 +109,11 @@ you will need to include it as: `some-amazing-team`. Example: ``` -$ vault write auth/github/map/teams/admins value=root +$ vault write auth/github/map/teams/admins value=admins Success! Data written to: auth/github/map/teams/admins ``` -The above would make anyone in the "admins" team a root user in Vault -(not recommended). +The above would make anyone in the "admins" team receive tokens with the policy `admins`. You can then auth with a user that is a member of the "admins" team using a Personal Access Token with the `read:org` scope. @@ -125,6 +124,6 @@ $ vault auth -method=github token=000000905b381e723b3d6a7d52f148a5d43c4b45 Successfully authenticated! The policies that are associated with this token are listed below: -root +admins ``` diff --git a/website/source/docs/auth/ldap.html.md b/website/source/docs/auth/ldap.html.md index 5fc735e090..e9a4362030 100644 --- a/website/source/docs/auth/ldap.html.md +++ b/website/source/docs/auth/ldap.html.md @@ -49,7 +49,7 @@ Password (will be hidden): Successfully authenticated! The policies that are associated with this token are listed below: -root +admins ``` #### Via the API @@ -74,7 +74,7 @@ The response will be in JSON. For example: "auth": { "client_token": "c4f280f6-fdb2-18eb-89d3-589e2e834cdb", "policies": [ - "root" + "admins" ], "metadata": { "username": "mitchellh" diff --git a/website/source/docs/auth/userpass.html.md b/website/source/docs/auth/userpass.html.md index 89fdb70441..01647cd1da 100644 --- a/website/source/docs/auth/userpass.html.md +++ b/website/source/docs/auth/userpass.html.md @@ -49,7 +49,7 @@ The response will be in JSON. For example: "auth": { "client_token": "c4f280f6-fdb2-18eb-89d3-589e2e834cdb", "policies": [ - "root" + "admins" ], "metadata": { "username": "mitchellh" @@ -85,12 +85,12 @@ Use `vault path-help` for more details. ``` $ vault write auth/userpass/users/mitchellh \ password=foo \ - policies=root + policies=admins ... ``` The above creates a new user "mitchellh" with the password "foo" that -will be associated with the "root" policy. This is the only configuration +will be associated with the "admins" policy. This is the only configuration necessary. ## API