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

agent: auth.handler authenticate doesn't receive sigint #13600

Closed
dekimsey opened this issue Jan 7, 2022 · 4 comments
Closed

agent: auth.handler authenticate doesn't receive sigint #13600

dekimsey opened this issue Jan 7, 2022 · 4 comments
Labels
agent devex Developer Experience ecosystem

Comments

@dekimsey
Copy link
Collaborator

dekimsey commented Jan 7, 2022

Describe the bug
When running the vault agent, if authentication is misconfigured, the agent cannot be exited with a SIGINT until the timeout to connect to vault expires.

This issue is best reproduced running the process interactively.

To Reproduce
Steps to reproduce the behavior:

  1. Start agent
  2. CTRL+C
  3. Forced to wait till auth backoff/timeout (30s?) before application exists
$ vault agent -config=vault.hcl -log-level=trace
...
2022-01-07T18:33:08.892Z [INFO] (runner) creating watcher
2022-01-07T18:33:08.892Z [INFO]  auth.handler: starting auth handler
2022-01-07T18:33:08.892Z [INFO]  auth.handler: authenticating
2022-01-07T18:33:08.892Z [TRACE] auth.aws: beginning authentication
2022-01-07T18:33:08.893Z [INFO]  sink.server: starting sink server
^C==> Vault agent shutdown triggered
2022-01-07T18:33:15.478Z [INFO] (runner) stopping
2022-01-07T18:33:15.478Z [DEBUG] (runner) stopping watcher
2022-01-07T18:33:15.478Z [DEBUG] (watcher) stopping all views
2022-01-07T18:33:15.478Z [INFO]  template.server: template server stopped
2022-01-07T18:33:15.478Z [INFO]  sink.server: sink server stopped
2022-01-07T18:33:15.478Z [INFO]  sinks finished, exiting
^C^C^C^C^C
2022-01-07T18:34:08.893Z [ERROR] auth.handler: error authenticating: error="context deadline exceeded" backoff=1s
2022-01-07T18:34:08.893Z [TRACE] auth.aws: checking for new credentials
2022-01-07T18:34:08.893Z [INFO]  auth.handler: auth handler stopped

Expected behavior
SIGINT should interrupt and exit more quickly

Environment:

  • Vault Server Version (retrieve with vault status): v1.8.7
  • Vault CLI Version (retrieve with vault version): v1.9.2
  • Server Operating System/Architecture: CentOS 7 x86_64

Vault server configuration file(s):

auto_auth {
  method "aws" {
    mount_path = "auth/aws"
    config = {
      type = "iam"
      role = "redacted-role-name"
    }
  }
}
cache {
  use_auto_auth_token   = true
  force_auto_auth_token = true
}
listener "tcp" {
  address     = "127.0.0.1:8100"
  tls_disable = true
}

Additional context
I haven't tried v1.9 since there are currently other issues with it.

@hsimon-hashicorp
Copy link
Contributor

Really interesting, thanks for this!

@tvoran
Copy link
Member

tvoran commented Jan 12, 2022

I did a some testing with this too, and it looks like it's the Vault Write() call that's hanging:

secret, err = clientToUse.Logical().Write(path, data)
// Check errors/sanity
if err != nil {
ah.logger.Error("error authenticating", "error", err, "backoff", backoff)
backoffOrQuit(ctx, backoff)
continue
}

From my understanding, there's a ShutdownCh that is closed on SIGINT, which triggers canceling the context in the auth handler. But since vault's Logical().Write() doesn't take a context argument (and creates its own) that write call hangs until it times out. I did a quick test to pass a context into the Write() call, and agent then responded immediately to a SIGINT.

I'm not sure why Logical().Write() doesn't accept a context, so it's something we may want to revisit.

@averche averche added the devex Developer Experience label Mar 30, 2022
@averche
Copy link
Contributor

averche commented Mar 30, 2022

Hi @dekimsey,

Thank you for pointing out this issue! We have recently added context-aware functions to vault/api/* in #14388. I am now converting the Vault agent auth method to use the context functions in #14775. Once the PR is merged, the issue should be fixed!

@averche
Copy link
Contributor

averche commented Apr 6, 2022

The PR has been merged, thank you again for contributing to HashiCorp!

@averche averche closed this as completed Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent devex Developer Experience ecosystem
Projects
None yet
Development

No branches or pull requests

4 participants