Skip to content
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

Attempting to authenticate to "login" endpoint for non-existent mount path and no X-Vault-Token erroneously calls fetchACLTokenEntryAndEntity, resulting in "missing client token" instead of "invalid mount path" #11152

Closed
lambertryanPFG opened this issue Mar 19, 2021 · 6 comments
Labels
bug Used to indicate a potential bug core/router core Issues and Pull-Requests specific to Vault Core

Comments

@lambertryanPFG
Copy link

lambertryanPFG commented Mar 19, 2021

Describe the bug
Vault returns { "error": [ "missing client token" ] } when you attempt to hit a login endpoint on a mount path that doesn't exist using an authentication method that doesn't require a pre-existing auth token.

request_handling.go has a method handleCancelableRequest that has this logic:

if c.router.LoginPath(ctx, req.Path) {
   resp, auth, err = c.handleLoginRequest(ctx, req)
} else {
   resp, auth, err = c.handleRequest(ctx, req)
}

router.LoginPath takes the endpoint path and uses the go-radix library to determine if it's a valid login path (as defined in the unit test as an existing mount path appending with one of the following).

n := &NoopBackend{
   Login: []string{
      "login",
      "oauth/*",
   },
}

If the mount path doesn't exist, the call to LongestPrefix fails to figure out if the path contains "login" or "oauth/*" and just returns false by default, causing LoginPath to return false. Here is the relevant code for LoginPath:

func (r *Router) LoginPath(ctx context.Context, path string) bool {
   ns, err := namespace.FromContext(ctx)
   if err != nil {
      return false
   }
   adjustedPath := ns.Path + path
   r.l.RLock()
   mount, raw, ok := r.root.LongestPrefix(adjustedPath)
   r.l.RUnlock()
   if !ok {
      return false
   }

The problem is that LoginPath returning false causes the above "if" clause in handleCancelableRequest to send the request to handleRequest instead of handleLoginRequest. Both methods call a checkToken method but with different parameters. handleLoginRequest calls it with a boolean flag "unauth" set to true, while handleRequest calls it with that flag set to false.

auth, te, ctErr := c.checkToken(ctx, req, false) // The call from handleRequest

Inside of checkToken is the following logic:

// Even if unauth, if a token is provided, there's little reason not to
// gather as much info as possible for the audit log and to e.g. control
// trace mode for EGPs.
if !unauth || (unauth && req.ClientToken != "") {
   acl, te, entity, identityPolicies, err = c.fetchACLTokenEntryAndEntity(ctx, req)
   // In the unauth case we don't want to fail the command, since it's
   // unauth, we just have no information to attach to the request, so
   // ignore errors...this was best-effort anyways
   if err != nil && !unauth {
      return nil, te, err
   }
}

In scenarios like this where "unauth" is false, the method fetchACLTokenEntryAndEntity is executed. Inside of this method is the following check for a client token, which in this scenario doesn't exist, thus we end up bubbling up the "missing client token" error instead of something more descriptive.

if req.ClientToken == "" {
   return nil, nil, nil, nil, &logical.StatusBadRequest{Err: "missing client token"}
}

Additionally, adding any random client token value to my requests (in my case an invalid one) does return a more descriptive error of "permission denied".

To Reproduce
Steps to reproduce the behavior:

  1. Configure a vault instance
  2. Enable cert authentication
  3. Generate a certificate
  4. Use certificate to send a POST to an invalid "login" end point (i.e. /auth/[invalid name]/login) with no X-Vault-Token parameter set in the headers.
  5. Observe "missing client token" error with 400 status code

Alternatively:

  1. Modify the test cases for TestRouter_LoginPath as follows:
       tcases := []tcase{
		{"random", false},
		{"auth/invalid/login", true},  //This new case should cause the tests to fail since it will return false instead of true
		{"auth/foo/login", true},
		{"auth/foo/bar", false},
		{"auth/foo/oauth", false},
		{"auth/foo/oauth/redirect", true},
	}
  1. Observe TestRouter_LoginPath fail on {"auth/invalid/login", true}

Expected behavior
Provide a clearer response message like "invalid target endpoint". Possibly by adding an additional processing logic to LoginPath to check for the presence of "login" or "oauth/*" even if LongestPrefix returns false.

Environment:

  • Vault Server Version (retrieve with vault status): Release v1.6.3
  • Vault CLI Version (retrieve with vault version): Release v1.6.3
  • Server Operating System/Architecture: macOS Catalina v10.15.7

Vault server configuration file(s): N/A

Additional context
N/A

@lambertryanPFG lambertryanPFG changed the title Attempting to authenticate to "login" endpoint for non-existent mount path and no X-Vault-Token throws "missing client token" instead "invalid path" Attempting to authenticate to "login" endpoint for non-existent mount path and no X-Vault-Token erroneously calls fetchACLTokenEntryAndEntity, resulting in "missing client token" instead of "invalid mount path" Mar 19, 2021
@HridoyRoy
Copy link
Contributor

Hi @lambertryanPFG ,
I'm a bit confused as to what the bug is. Am I correct in understanding that the token was not set in the call to the nonexistent mount, and resulted in an error stating that no token was supplied?
Thanks!

@lambertryanPFG
Copy link
Author

lambertryanPFG commented Mar 31, 2021

@HridoyRoy Correct. For cert or ldap authentication, I don't have a token yet and am trying to generate one by hitting the login end point while specifying a valid cert or ldap credential. If the mount point isn't valid it tells me I don't have a token. To which the response from engineers is usually something like "I know I don't have an auth token. Tell me what the actual problem is." The actual problem is that the mount path doesn't exist.

As described in the report, this seems to be because the logic for the end point pattern matching fails before it can determine if it's a "login" end point. loginPath() thus returns false and sends the request to handleRequest(). The systemic effect is that the system ends up assuming you've authenticated already, thus it hits additional logic to validate the auth token it thinks you should have, but don't. handleLoginRequest() avoids this validation by setting unauth = true, which would have been the correct flow in this case.

The end point pattern matching seems to be done with a suffix tree, but what happens if a section of the suffix prior to "/login" can't be matched? With a regex check before loginPath() returns false you can't search for the existence of "/login" or "/oauth/*" anywhere within the suffix as a secondary verification. Or you could just add a function that verifies the existence of the mount path before the call to loginPath().

NOTE: The question might come up as to why I'm trying to authenticate to a non-existent mount path. In a live scenario I encountered this when I was not the administrator of vault and someone had setup the auth method and given me a valid certificate but had not properly communicated the correct mount path to use, and I was unable to log into the vault namespace to check it myself, so I made the assumption that it was the default. It was not, but rather than telling me that in the error response it just told me that I didn't give it a valid token.

@HridoyRoy
Copy link
Contributor

HridoyRoy commented Apr 1, 2021

@lambertryanPFG I believe the default behavior when accessing a mount should be to check auth information. In the case where the mount is an auth mount, the auth information to be checked isn't related to a Vault token. An existence check should be secondary to whether one can access a particular vault path in the first place.

In other words, Vault should report nothing at all unless it first confirms the identity of the entity accessing it, unless the call is specifically to an endpoint which is unauthenticated.

Vault operators should have runbooks specifying access to the mounts that are in use. In the case where a user is unable to access Vault due to a user error, they would need to contact the Vault operator.

I'm going to keep this issue open to let others (tagging @sgmiller @raskchanky ) weigh in on the conversation, but I'm in favor of closing this issue.

Thanks!

@lambertryanPFG
Copy link
Author

@lambertryanPFG I believe the default behavior when accessing a mount should be to check auth information. In the case where the mount is an auth mount, the auth information to be checked isn't related to a Vault token. An existence check should be secondary to whether one can access a particular vault path in the first place.

An existence check should be primary because you shouldn't even bother checking if it's an auth path if the mount doesn't exist. Fail out immediately with a generic error instead of erroneously routing the request to the handleRequest().

In other words, Vault should report nothing at all unless it first confirms the identity of the entity accessing it, unless the call is specifically to an endpoint which is unauthenticated.

I'm not entirely sure what you mean by this. Vault is already reporting something. It's reporting that I'm "missing an auth token". I can deduce the existence of a mount through inference.

Take LDAP for instance. If I provide a valid auth path on an existing mount, but bad credentials, and an invalid X-Vault-Token value, I get Status Code 400 and "ldap operation failed: failed to bind as user" or "ldap operation failed: unable to retrieve user bind DN". This indicated that the path exists but my credentials are bad.

Alternatively, if I provide an auth path to a nonexistent mount, an invalid X-Vault-Token value, and the same bad credentials, I get 403 and "permission denied". Because of the difference in behavior I can infer which paths exist and which don't.

The "missing client token" error doesn't do anything at all to protect the names of the existing mounts.

@vishalnayak vishalnayak added bug Used to indicate a potential bug core/router labels Jun 8, 2021
@heatherezell heatherezell added core Issues and Pull-Requests specific to Vault Core and removed waiting-for-response labels Sep 2, 2021
@akshya96
Copy link
Contributor

Fixed this issue. Authentication to "login" endpoint for non-existent mount path returns permission denied with status code 403

@heatherezell
Copy link
Contributor

Thank you, @akshya96!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug core/router core Issues and Pull-Requests specific to Vault Core
Projects
None yet
Development

No branches or pull requests

5 participants