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

Fix: Optionally reload x509 key-pair from disk on agent auto-auth #19002

Merged
merged 3 commits into from
Mar 22, 2023

Conversation

karelorigin
Copy link
Contributor

This PR fixes #18562. I came to the conclusion that solving it this way would be safest, as it won't affect any existing Vault Agent configurations. I have also updated the documentation to match the new configuration property.

@karelorigin karelorigin requested a review from yhyakuna as a code owner February 6, 2023 13:35
@cipherboy cipherboy requested a review from a team February 6, 2023 20:36
@karelorigin
Copy link
Contributor Author

Update: I've successfully been using this fork for weeks, so I consider it safe.

@heatherezell heatherezell added docs auth/cert Authentication - certificates labels Mar 17, 2023
@heatherezell
Copy link
Contributor

The docs component looks good to me; the code changes need additional review. Thanks @cipherboy!

Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @karelorigin, this looks good to me, would you mind adding an improvement changelog entry within the PR and I'll happily merge it in.

Something like https://github.com/hashicorp/vault/blob/main/changelog/18740.txt can be used as an example.

Thanks for the contribution!

@karelorigin
Copy link
Contributor Author

@stevendpclark will do! Are the text file numberings sequential?

@stevendpclark
Copy link
Contributor

Are the text file numberings sequential?

We use the GH issue number for the changelog filename, so in this case please create changelog/19002.txt. Thank you!

@karelorigin
Copy link
Contributor Author

Done! Let me know if that works :)

Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@stevendpclark stevendpclark added this to the 1.14 milestone Mar 22, 2023
@stevendpclark stevendpclark merged commit 5631e80 into hashicorp:main Mar 22, 2023
@stevendpclark
Copy link
Contributor

@karelorigin Thanks again for the contribution, it will appear within the next major release of Vault.

@celesteking
Copy link

doesn't work per #26367

@karelorigin
Copy link
Contributor Author

@celesteking, this PR works fine, and I know so because it solved the exact problem I had. What you've likely missed, is that alongside the TLS configuration in your auto auth stanza, you also need to explicitly set TLS in the Vault configuration block:

vault {
  # <other configuration trimmed>

  ca_cert      = "/opt/vault/tls/ca.crt.pem"
  client_cert = "/opt/vault/tls/vault.crt.pem"
  client_key  = "/opt/vault/tls/vault.key.pem"
}

auto_auth {
  method "cert" {
    config {
      name        = "vault-cluster"
      ca_cert      = "/opt/vault/tls/ca.crt.pem"
      client_cert = "/opt/vault/tls/vault.crt.pem"
      client_key  = "/opt/vault/tls/vault.key.pem"
      reload       = true
    }
  }
}

Why this is necessary isn't entirely clear, though I remember it being necessary for me, and it's been running fine for over a year now.

@celesteking
Copy link

ok, it intermittently solves the problem. Sometimes it just doesn't reload the cert upon renewal and keeps using the expired one.

As regarding the vault {} stanza, docs say It is strongly advised to provide TLS settings in the configuration stanza within the auth method to avoid ..., but on the other hand ... If TLS settings are not present in the config stanza, Agent and Proxy will fall back to using TLS settings from their respective vault Stanzas.

Confusing as hell. Had the vault { client_*} been necessary, my config would not have worked on startup, but it works, so they're optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent auth/cert Authentication - certificates docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vault Agent cert auth method never reloads x509 key pair
4 participants