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

token create doesn't accept 'd' suffix for ttl #19815

Closed
nemethloci opened this issue Mar 29, 2023 · 6 comments
Closed

token create doesn't accept 'd' suffix for ttl #19815

nemethloci opened this issue Mar 29, 2023 · 6 comments
Labels
bug Used to indicate a potential bug core/token

Comments

@nemethloci
Copy link

Describe the bug
vault token create -policy=mypolicy -ttl=1d
fails with error:
invalid value "1d" for flag -ttl: time: unknown unit "ds" in duration "1ds"

The same command works fine if 'd' is replaced with 'h'.

To Reproduce
Steps to reproduce the behavior:

  1. Run vault token create -policy=mypolicy -ttl=1d (note: result is the same regardless if the policy exists or not)

Expected behavior
token gets created with a 1 day ttl.

Environment:

  • Vault Server Version (retrieve with vault status):
    Version 1.12.2
    Build Date 2022-11-23T12:53:46Z

  • Vault CLI Version (retrieve with vault version):
    Vault v1.13.0 (a4cf0dc), built 2023-03-01T14:58:13Z

  • Server Operating System/Architecture:
    Ubuntu 20.04.5 LTS

Vault server configuration file(s):

api_addr     = "https://10.100.12.8"
cluster_addr = "https://10.100.12.19:8201"

log_level = "trace"

ui = true

seal "gcpckms" {
  project    = "rd-vault-development"
  region     = "europe-west4"
  key_ring   = "vault-f5287fb63ff07b0c"
  crypto_key = "vault-init"
}

storage "gcs" {
  bucket     = "rd-vault-development-vault-storage"
  ha_enabled = "true"
}

listener "tcp" {
  address     = "127.0.0.1:8200"
  tls_disable = "true"

  telemetry {
    prometheus_retention_time = "1h"
    disable_hostname = true
    unauthenticated_metrics_access = true
  }
}

listener "tcp" {
  address       = "10.100.12.19:8200"
  tls_disable   = "true"

  telemetry {
    prometheus_retention_time = "1h"
    disable_hostname = true
    unauthenticated_metrics_access = true
  }
}

listener "tcp" {
  address       = "10.100.12.8:8200"
  tls_disable   = "true"

  telemetry {
    prometheus_retention_time = "1h"
    disable_hostname = true
    unauthenticated_metrics_access = true
  }
}

telemetry {
  prometheus_retention_time = "300s",
  disable_hostname = true
}
@nemethloci
Copy link
Author

Addition: the same seems to be valid for -explicit-max-ttl . The docs reference this page for valid suffixes, which includes 'd':
https://developer.hashicorp.com/vault/docs/concepts/duration-format

@maxb
Copy link
Contributor

maxb commented Mar 30, 2023

The inclusion of days on that page is incorrect :-/ Vault actually uses Go's duration string format, which only supports units up to hours. Go's documentation for this can be found at https://pkg.go.dev/time#ParseDuration

@VioletHynes
Copy link
Contributor

The inclusion of days on that page is not incorrect - Vault does use Go's duration string format, but with some modifications. We should be using parseutil.ParseDurationSecond in all cases, over the Go method. The exact method is here, if you're interested:
https://github.com/hashicorp/go-secure-stdlib/blob/main/parseutil/parseutil.go#L99

If we don't support d on a duration string endpoint, I would definitely consider that a bug we should fix.

@heatherezell heatherezell added bug Used to indicate a potential bug core/token labels Apr 6, 2023
@maxb
Copy link
Contributor

maxb commented Apr 7, 2023

Interesting. Vault certainly didn't support d in all the versions I have actual experience using.

I propose it would be useful to add an info-box to the page mentioning that support for d is new as of whichever Vault version it can be depended upon to be present.

@maxb
Copy link
Contributor

maxb commented Apr 17, 2023

I did a search on the Vault codebase and found quite a few uses of time.ParseDuration still being applied to user input.

In the specific case mentioned in this issue, the problem cases are the ones in command/base_flags.go.

@hghaf099
Copy link
Contributor

hghaf099 commented May 4, 2023

This issue has been fixed. Closing it for now.

@hghaf099 hghaf099 closed this as completed May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug core/token
Projects
None yet
Development

No branches or pull requests

5 participants