Customizing num_uses and ttl per approle secret id #14390

kent007 · 2022-03-07T18:00:37Z

Is your feature request related to a problem? Please describe.
When I generate a new secret ID for an approle, I would like to customize the TTL and/or num_uses for that secret ID, similarly to how I can customize the cidr_list and token_bound_cidrs.

Describe the solution you'd like
Additional API parameters for the approle secret ID creation allowing custom values to be set for num_uses and ttl.

Describe alternatives you've considered
Since num_uses and ttl are currently tied to the role configuration, any use case for an approle that requires a different value for either parameter than the default means I have to create an entirely new approle with identical policies but different configuration. This is possible, but tedious.

Explain any additional use-cases
In particular, this can be useful for the recommended approle use pattern that involves wrapping. For example, it would be very convenient to specify the num_uses for a new secret ID being passed to automation, even if the approle configuration did not have num_uses set.

The text was updated successfully, but these errors were encountered:

RemcoBuddelmeijer · 2022-03-11T14:30:18Z

This seems like an interesting case. Did you have anything written for this yourself?

kent007 · 2022-03-11T16:24:05Z

This seems like an interesting case. Did you have anything written for this yourself?

I'm not sure I understand. I haven't opened the vault code and evaluated the feasibility of a PR for this feature, although I'd be happy to.

In our system, for some approles we maintain one or more secret-ids that are long lived (ttl 90+ days) and generate others that are supposed to be short lived (1+ hours) to hand off to various automation. We have a system set up for tagging the short-lived approles with custom metadata and a reaper program that runs periodically to purge them...since we can't set the TTL in vault all the "short-lived" secret-ids ttls are only being enforced by the reaper, which means Vault would still consider them valid.

RemcoBuddelmeijer · 2022-03-11T20:19:31Z

This seems like an interesting case. Did you have anything written for this yourself?

I'm not sure I understand. I haven't opened the vault code and evaluated the feasibility of a PR for this feature, although I'd be happy to.

Sorry for the confusion! When saying “written anything for this” I was referring to having worked on the code yourself. I have had a look at your use case and I would say it’s pretty feasible.
If you want to make a PR yourself please by all means go ahead, otherwise I am always happy to do so! 😄

kent007 · 2022-03-11T21:28:05Z

If you want to make a PR yourself please by all means go ahead, otherwise I am always happy to do so! 😄

Much obliged! Realistically speaking I wouldn't be able to work on this feature for a few weeks, although I'll be happy to test if it becomes available soon. Our enterprise release is back by a few versions anyway, we're still on 1.6.3 and I'm not sure what our upgrade schedule is

RemcoBuddelmeijer · 2022-03-11T23:38:06Z

Much obliged! Realistically speaking I wouldn't be able to work on this feature for a few weeks, although I'll be happy to test if it becomes available soon.

Since that's the case. I'll go ahead and put in the work and refer this issue for testing and tracing purposes! 😄

Our enterprise release is back by a few versions anyway, we're still on 1.6.3 and I'm not sure what our upgrade schedule is

Perhaps attempting to put this as something that can be back-ported would be beneficial?

RemcoBuddelmeijer · 2022-03-12T21:26:33Z

@kent007 I have gone ahead and added the fields. Please let me know if this fixes your issue!

kent007 · 2022-03-14T13:35:31Z

@RemcoBuddelmeijer I read through your changes, they look great and should hopefully do exactly what we need!

Is there a test binary published anywhere I can download to experiment with? I'm unfamiliar with circle-ci -- if it'll be faster to download your branch and build it locally, I can do that as well

RemcoBuddelmeijer · 2022-03-14T14:14:27Z

@kent007 I didn't build any test binary, best would to build it yourself using make dev. This would yield the same results.

Add fields 'ttl' and 'num_uses' when generating/obtaining a SecretID. Rather than just being able to use the Role's SecretID ttl and num uses. hashicorp#14390

kent007 · 2022-03-14T19:26:25Z

I kicked the tires on a test binary for a little bit, seems to be working great. Very excited to see this feature added.

As far as backporting, our enterprise support team seems to be open to upgrading to a significantly newer version. If it's not too much trouble to backport I think it would be valuable, but it's not a deal breaker.

I'd also like to recommend that the docs regarding the recommended approle pattern for CI environments be updated with a few details:

that secret_id_ttl and num_uses can be set individually for the wrapped secret-ids
if operating in an environment that generates lots of secret-ids, these are stored persistently inside vault and might need to be cleaned out periodically. As far as I can tell, there are two conditions where an expired secret-id is automatically purged:
- when the last of num_uses logins has been executed
- attempting to use the secret-id after the ttl has run out. There's no good reason for well tuned automation to attempt a second login after the ttl, so these secret-ids will likely build up over time

RemcoBuddelmeijer · 2022-03-14T20:23:50Z

I kicked the tires on a test binary for a little bit, seems to be working great. Very excited to see this feature added.

Great to hear!

As far as backporting, our enterprise support team seems to be open to upgrading to a significantly newer version. If it's not too much trouble to backport I think it would be valuable, but it's not a deal breaker.

I will discuss this in the PR and/or make another PR out of it to backport it. Shouldn't be too big of an issue since it doesn't contain any breaking API changes.

I'd also like to recommend that the docs regarding the recommended approle pattern for CI environments be updated with a few details:

After the PR has been merged, I will make an attempt to get all the other Learns, Best Practices, etc. to be updated. For now the API docs have been updated and will include the new changes.

kent007 · 2022-03-16T18:44:26Z

Stumbled into another question related to this feature today...

Once we have the ability to set TTL and num_uses for individual secret-ids, we would like to be able to require a TTL and/or num_uses via a policy. Will the functionality already present for setting required and allowed parameters work for this? For example, something like this would enforce a TTL of 1 hour and num_uses of 1:

path "auth/approle/role/<my approle here>/secret-id" {
  capabilities = ["create", "update"]
  allowed_parameters = {
    "ttl" = ["1h"]
    "num_uses" = ["1"]
  }
}

RemcoBuddelmeijer · 2022-03-18T23:40:56Z

Hey @kent007

My apologies for the late answer.
Yes this works out of the box! Just to be sure I have gone ahead and tested it for you. For both cases where I presented other parameters I was welcomed with a "permission denied".

Rather than implicitly overriding, error when the ttl is lower than and the num uses higher than the role's configuration. hashicorp#14390

* Add fields 'ttl' and 'num_uses' to SecretID generation. Add fields 'ttl' and 'num_uses' when generating/obtaining a SecretID. Rather than just being able to use the Role's SecretID ttl and num uses. #14390 * Add secret_id_num_uses response field to generating SecretID Add the response field secret_id_num_uses to the endpoints for generating SecretIDs. Used in testing but also to supply the vendor with this variable. * Add tests for new ttl and num_uses SecretID generation fields Add tests to assert the new TTL and NumUses option in the SecretID entry. Separate test for testing with just parameters vs a -force example. * Patch up test for ttl and num_uses fields * Add changelog entry for auth/approle 'ttl' and 'num_uses' fields * Add fields to API Docs and AppRole Auth Docs example * Correct error message for failing test on missing field. Change the error message produced when a test fails due to a missing field. Previous values did not map to correct fields. * Remove unnecessary int cast to int "secret_id_num_uses" field. Unnecessary cast to int where type already is int. * Move numUses field check to after assignment. * Remove metadata entry in sample payload to limit change to changes made. Remove metadata entry in sample payload for custom-secret-id. The metadata was not changed in the features pull request. * Bind fields 'ttl' and 'num_uses' to role's configuration. Rather than implicitly overriding, error when the ttl is lower than and the num uses higher than the role's configuration. #14390 * Update changelog 14474 with a more detailed description. More elaborate description for the changelog. Specifying the per-request based fields. * Elaborate more on the bounds of the 'ttl' and 'num_uses' field. Specify in both the api-docs and the CLI the limits of the fields. Specify that the role's configuration is still the leading factor. * Upper bound ttl with role secret id ttl Upper bound ttl with role secret id ttl when creating a secret id Adding test cases for infinite ttl and num uses Adding test cases for negative ttl and num uses Validation on infinite ttl and num uses * Formatting issues. Removed unnecessary newline * Update documentation for AppRole Secret ID and Role Changed that TTL is not allowed to be shorter to longer * Cleanup approle secret ID test and impl * Define ttl and num_uses in every test Define ttl and num_uses in every test despite them not being tested. This is to ensure that no unexpected behaviour comes to mind. * Rename test RoleSecretID -> RoleSecretIDWithoutFields * Test secret id generation defaults to Role's config Test secret id generation defaults to Role's configuration entries. * Change finit -> finite Co-authored-by: Josh Black <raskchanky@users.noreply.github.com> * Rephrase comments to the correct validation check * Rephrase role-secret-id option description * Remove "default" incorrect statement about ttl * Remove "default" incorrect statement about ttl for custom secret id * Touch up approle.mdx to align more with path_role documentation Co-authored-by: Remco Buddelmeijer <r.buddelmeijer@fullstaq.com> Co-authored-by: Josh Black <raskchanky@users.noreply.github.com>

sgmiller added the core/auth label Mar 7, 2022

swayne275 added feature-request auth/approle labels Mar 8, 2022

heatherezell added the enhancement label Mar 11, 2022

RemcoBuddelmeijer mentioned this issue Mar 12, 2022

Add fields 'ttl' and 'num_uses' to SecretID generation. #14474

Merged

HridoyRoy closed this as completed in #14474 Sep 2, 2022

HridoyRoy mentioned this issue Sep 2, 2022

limit custom max ttl to mount max lease ttl and allow 0 num uses and … #17001

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Customizing num_uses and ttl per approle secret id #14390

Customizing num_uses and ttl per approle secret id #14390

kent007 commented Mar 7, 2022

RemcoBuddelmeijer commented Mar 11, 2022

kent007 commented Mar 11, 2022

RemcoBuddelmeijer commented Mar 11, 2022 •

edited

Loading

kent007 commented Mar 11, 2022

RemcoBuddelmeijer commented Mar 11, 2022 •

edited

Loading

RemcoBuddelmeijer commented Mar 12, 2022 •

edited

Loading

kent007 commented Mar 14, 2022

RemcoBuddelmeijer commented Mar 14, 2022

kent007 commented Mar 14, 2022

RemcoBuddelmeijer commented Mar 14, 2022

kent007 commented Mar 16, 2022 •

edited

Loading

RemcoBuddelmeijer commented Mar 18, 2022

Customizing num_uses and ttl per approle secret id #14390

Customizing num_uses and ttl per approle secret id #14390

Comments

kent007 commented Mar 7, 2022

RemcoBuddelmeijer commented Mar 11, 2022

kent007 commented Mar 11, 2022

RemcoBuddelmeijer commented Mar 11, 2022 • edited Loading

kent007 commented Mar 11, 2022

RemcoBuddelmeijer commented Mar 11, 2022 • edited Loading

RemcoBuddelmeijer commented Mar 12, 2022 • edited Loading

kent007 commented Mar 14, 2022

RemcoBuddelmeijer commented Mar 14, 2022

kent007 commented Mar 14, 2022

RemcoBuddelmeijer commented Mar 14, 2022

kent007 commented Mar 16, 2022 • edited Loading

RemcoBuddelmeijer commented Mar 18, 2022

RemcoBuddelmeijer commented Mar 11, 2022 •

edited

Loading

RemcoBuddelmeijer commented Mar 11, 2022 •

edited

Loading

RemcoBuddelmeijer commented Mar 12, 2022 •

edited

Loading

kent007 commented Mar 16, 2022 •

edited

Loading