fix: cert auth method watches cert file change and NewCreds() notific…

[jasonjoo2010]

Hi community and committees,

Recently, our team encountered TLS certificate reloading issue even after adding reload: true to config when using cert method. After checking, it looks like not functioning properly.

Although there was a previous PR(#19002 ) which added certificate reloading capability to auto auth, it doesn't work in the actual use case. After checking further a little bit, I found that the agent blocked at the line auth.go:526 while the server responded with 400 errors at transport layer due to expired certificate. In our case, we use certificates in cloud native applications with 5 days as expiry and refresh the certs every day.

At the same time, the lifetime loop also listens on a channel returned by NewCreds() but it's not implemented in cert method. So it may be the best and shortest path adding this support to make the reloading mechanism work.

Description

So in a nutshell, the reload option for cert method only guarantee AuthClient() return a different client instance when called. It doesn't guarantee the renewed certificate on disk can sooner or later be used in auth. I don't know whether people maintain a good understanding on what this option actually does but it would be better to be more straightforward and more automated in reloading it.

This PR adds implementation of existing NewCreds() method in the interface as well as a new optional option reload_period for the internal file watching loop. And they only take effect when reload is enabled. In common cases, reload is the only option people need to set when they need.

TODO only if you're a HashiCorp employee

• [ ] Backport Labels: If this PR is in the ENT repo and needs to be backported, backport
  to N, N-1, and N-2, using the backport/ent/x.x.x+ent labels. If this PR is in the CE repo, you should only backport to N, using the backport/x.x.x label, not the enterprise labels.
  • [ ] If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
• [ ] ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature of a public function, even if that change is in a CE file, double check that applying the patch for this PR to the ENT repo and running tests doesn't break any tests. Sometimes ENT only tests rely on public functions in CE files.
• [ ] Jira: If this change has an associated Jira, it's referenced either in the PR description, commit message, or branch name.
• [ ] RFC: If this change has an associated RFC, please link it in the description.
• [ ] ENT PR: If this change has an associated ENT PR, please link it in the description. Also, make sure the changelog is in this PR, not in your ENT PR.

[hashicorp-cla-app[bot]]

All committers have signed the CLA.

[hashicorp-cla-app[bot]]

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

_{Have you signed the CLA already but the status is still pending? Recheck it.}

[VioletHynes]

To add to the suggestions Scott and I made, this will also need a changelog (a file called 28126.txt in the changelog directory containing a discussion of the improvement) -- you can look at the other files for examples, and I'm happy to review.

You will also want to update the docs around this feature. In particular, there should be a new entry added for the new config option, and you should also update the reload option as appropriate. This file can be found at website/content/docs/agent-and-proxy/autoauth/methods/cert.mdx.

Thanks for the contribution! Excited to get this merged. Please let me know if you have any questions about any of the suggestions.

[jasonjoo2010]

Hi Violet and Scott,

After checking the existing code logic carefully, I realised there were special cases that the users may:

1. Don't give any cert/key files in the config.
2. Only set ca cert.
3. Only set client cert/key.

Although the most common use case is setting all 3 options, we need to cover them in the test cases and keep it backward compatible. So I adjusted the codes a little bit for this.

I also added the release note and relevant module docs, please take a look at them.

Any further suggestions, please let me know.

Cheers

[VioletHynes]

Closes https://github.com/hashicorp/vault/issues/12233 (as merging this will make enable_reauth_on_new_credentials a supported feature.

[jasonjoo2010]

I saw there were tiny red ones. No worry, let me solve the race condition in test cases, they are only for debugging purpose but we can do it in the right way.

[VioletHynes]

Thanks for the contribution, and the diligence in the added and improved tests.

Merging! 🚢

[jasonjoo2010]

Thanks for your kind and patient review, @VioletHynes and @sgmiller That's what makes a product great and greater, really appreciate it!