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

bao login calls the token helper with get and expects that to succeed #312

Closed
ruuda opened this issue Apr 29, 2024 · 0 comments · Fixed by #313
Closed

bao login calls the token helper with get and expects that to succeed #312

ruuda opened this issue Apr 29, 2024 · 0 comments · Fixed by #313
Labels
bug Something isn't working

Comments

@ruuda
Copy link
Contributor

ruuda commented Apr 29, 2024

Describe the bug

When I run bao login, and a token helper is configured, Bao will call the token helper with get. This get might fail, the token helper might not have a token. The reason for running bao login in the first place, is to obtain a token and store it using the token helper. But even if the token helper has no token yet, Bao forces it to exit with exit code 0. If it exits with a nonzero exit code, then bao login fails, and we never get to the store step.

We can of course make the token helper exit with exit code 0, but then if you run e.g. bao kv get, and the token helper has no token, then the token helper no longer has the ability to make the kv get fail. Instead the kv get will fail with a misleading “permission denied” error. Permission is denied, because no X-Vault-Token was included in the request. Bao did not include a token, because the token helper did not return 1 but signalled success either way.

To Reproduce
Steps to reproduce the behavior:

  1. Configure a token helper in ~/.bao by putting e.g. token_helper = "/tmp/token_helper.sh" there.
  2. In that file, put the following and make the file executable:
    #!/usr/bin/bash
    echo "$@" >&2
    exit 1
  3. Run bao login.
  4. Observe how the script got called with get.
  5. Observe how returning 1 from that get prevented bao login from doing any login.

Expected behavior
I would expect bao login to not call the token helper with get. It should only call the token helper with store.

Once bao login does not call the token helper with get, it becomes possible to return 1 when a get lookup fails (because the token does not exist), and that enables the token helper to print a helpful error message in that case, instead of leaving the user with a misleading permission denied error.

Environment:

  • OpenBao Server Version (retrieve with bao status): irrelevant, we don’t even get tot he point where bao talks to a server
  • OpenBao CLI Version (retrieve with bao version): OpenBao v2.0.0-alpha20240329 ('74c2dddb0612b9a3da79384c20638266aa7de407'), built 2024-04-26T10:19:19Z
  • Server Operating System/Architecture: Linux 6.8.7 / x86-64

OpenBao server configuration file(s): irrelevant

Additional context

This bug report is an adaptation of hashicorp/vault#23194.

@ruuda ruuda added the bug Something isn't working label Apr 29, 2024
ruuda added a commit to ruuda/openbao that referenced this issue Apr 29, 2024
`vault login` used to call the token helper in `get` mode, because it
constructs an HTTP client, and the client automatically loads the token.
For almost everything that is the right thing to do, but for login, that
is the thing that is supposed to retrieve the token, and login itself
does not require the token. In fact, `vault login` would erase the token
on the client later on.

Calling the token helper in `get` mode is a problem, because if the
token helper fails, that blocks the login. But the token helper might
fail because it doesn't have a token yet.

This fixes openbao#312.

Signed-off-by: Ruud van Asseldonk <ruud@chorus.one>
ruuda added a commit to ruuda/openbao that referenced this issue Apr 29, 2024
`bao login` used to call the token helper in `get` mode, because it
constructs an HTTP client, and the client automatically loads the token.
For almost everything that is the right thing to do, but for login, that
is the thing that is supposed to retrieve the token, and login itself
does not require the token. In fact, `bao login` would erase the token
on the client later on.

Calling the token helper in `get` mode is a problem, because if the
token helper fails, that blocks the login. But the token helper might
fail because it doesn't have a token yet.

This fixes openbao#312.

Signed-off-by: Ruud van Asseldonk <ruud@chorus.one>
naphelps pushed a commit to ruuda/openbao that referenced this issue May 3, 2024
`bao login` used to call the token helper in `get` mode, because it
constructs an HTTP client, and the client automatically loads the token.
For almost everything that is the right thing to do, but for login, that
is the thing that is supposed to retrieve the token, and login itself
does not require the token. In fact, `bao login` would erase the token
on the client later on.

Calling the token helper in `get` mode is a problem, because if the
token helper fails, that blocks the login. But the token helper might
fail because it doesn't have a token yet.

This fixes openbao#312.

Signed-off-by: Ruud van Asseldonk <ruud@chorus.one>
naphelps pushed a commit that referenced this issue May 3, 2024
`bao login` used to call the token helper in `get` mode, because it
constructs an HTTP client, and the client automatically loads the token.
For almost everything that is the right thing to do, but for login, that
is the thing that is supposed to retrieve the token, and login itself
does not require the token. In fact, `bao login` would erase the token
on the client later on.

Calling the token helper in `get` mode is a problem, because if the
token helper fails, that blocks the login. But the token helper might
fail because it doesn't have a token yet.

This fixes #312.

Signed-off-by: Ruud van Asseldonk <ruud@chorus.one>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant