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

Unable to read Vault KVv2 secrets metadata #1396

Closed
riptl opened this issue Jul 22, 2020 · 8 comments · Fixed by #1399
Closed

Unable to read Vault KVv2 secrets metadata #1396

riptl opened this issue Jul 22, 2020 · 8 comments · Fixed by #1399
Labels
bug hashicat-update-complete Completed porting changes to hashicat hashicat-update-required Changes that need to be ported to hashicat vault Related to the Vault integration
Milestone

Comments

@riptl
Copy link

riptl commented Jul 22, 2020

Consul Template version

Vault Agent 1.4.2

Configuration

vault-k8s annotations

annotations = {
  "vault.hashicorp.com/agent-inject" = true
  "vault.hashicorp.com/role"         = "api-deployment"

  "vault.hashicorp.com/agent-inject-secret-minitoken-mac-secret"   = "secret/data/minitoken/mac_secret"
  "vault.hashicorp.com/agent-inject-template-minitoken-mac-secret" = <<EOF
{{- with secret "secret/metadata/minitoken/mac_secret" -}}
{{ . }}
{{- end }}
EOF
}

Generated Vault Agent config

{
  "auto_auth": {
    "method": {
      "type": "kubernetes",
      "mount_path": "auth/kubernetes",
      "config": {
        "role": "api-deployment"
      }
    },
    "sink": [
      {
        "type": "file",
        "config": {
          "path": "/home/vault/.vault-token"
        }
      }
    ]
  },
  "exit_after_auth": false,
  "pid_file": "/home/vault/.pid",
  "vault": {
    "address": "<omitted>"
  },
  "template": [
    {
      "destination": "/vault/secrets/minitoken-mac-secret",
      "contents": "{{- with secret \"secret/metadata/minitoken/mac_secret\" -}}\n{{ . }}\n{{- end }}\n",
      "left_delimiter": "{{",
      "right_delimiter": "}}"
    }
  ]
}

Consul Template

{{- with secret "secret/metadata/minitoken/mac_secret" -}}
{{ . }}
{{- end }}

Sanity Check Template that works

{{- range secrets "secret/metadata/minitoken/" -}}
Found secret: {{ . }}
{{- end }}

Logs

Output of sanity check template

Command

Found secret: mac_secret

Vault Agent logs

2020/07/21 23:44:26.312073 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 2 after "500ms")
2020/07/21 23:44:27.083082 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 3 after "1s")
2020/07/21 23:44:28.336913 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 4 after "2s")
2020/07/21 23:44:30.612501 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 5 after "4s")
2020/07/21 23:44:34.917585 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 6 after "8s")
2020/07/21 23:44:43.190042 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 7 after "16s")
2020/07/21 23:44:59.461971 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 8 after "32s")
2020/07/21 23:45:31.791564 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 9 after "1m0s")
2020/07/21 23:46:32.121237 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 10 after "1m0s")
2020/07/21 23:47:32.420788 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 11 after "1m0s")
2020/07/21 23:48:32.768307 [WARN] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (retry attempt 12 after "1m0s")
2020/07/21 23:49:33.023411 [ERR] (view) vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret (exceeded maximum retries)
2020/07/21 23:49:33.023617 [ERR] (runner) watcher reported error: vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret
2020-07-21T23:49:33.023Z [ERROR] template.server: template server error: error="vault.read(secret/metadata/minitoken/mac_secret): no secret exists at secret/data/metadata/minitoken/mac_secret"
2020-07-21T23:49:33.023Z [INFO]  template.server: template server stopped

Expected behavior

The secret call should be able to read Vault KV v2 metadata directly from the secret/metadata/<path> value.

Actual behavior

It was reported that no secret exists when asking for metadata.
This is probably due to vault.read() injecting path segment /data: secret/data/metadata/<path>.

A sanity check listing the secrets in secret/metadata shows that the secret that we read exists (see Configuration section).

Steps to reproduce

  1. Issue a versioned secret with TTL in Vault KV v2
  2. Configure Vault Agent to read from secret/metadata endpoint referring to secret via template.
@riptl
Copy link
Author

riptl commented Jul 22, 2020

I have a suspicion the error occurs in addPrefixToVKVPath. @findkim @kyhavlov

} else if isKVv2 {
d.secretPath = addPrefixToVKVPath(d.rawPath, mountPath, "data")
} else {

func addPrefixToVKVPath(p, mountPath, apiPrefix string) string {
switch {
case p == mountPath, p == strings.TrimSuffix(mountPath, "/"):
return path.Join(mountPath, apiPrefix)
default:
p = strings.TrimPrefix(p, mountPath)
// Don't add /data/ to the path if it's been added manually.
apiPathPrefix := apiPrefix
if !strings.HasSuffix(apiPrefix, "/") {
apiPathPrefix += "/"
}
if strings.HasPrefix(p, apiPathPrefix) {
return path.Join(mountPath, p)
}

imo, the code should check specifically for existing /metadata or /data prefixes, and add in the /data prefix when neither is present.

@riptl
Copy link
Author

riptl commented Jul 22, 2020

I found a workaround. (and probably another bug?)
We can exploit that evaluating whether to add prefix or not is done before cleaning the path.

This Consul template fetches metadata from Vault KV v2.

{{- with secret "secret/data/../metadata/minitoken/mac_secret" -}}
{{ . }}
{{- end }}

addPrefixToVKVPath decides to not alter the path because it has the /data prefix.
Then, at some point the request path gets cleaned to secret/metadata/minitoken/mac_secret.

The metadata endpoint is useful for fetching available secrets versions. It allows logic like iterating through a list of versions:

{{- with secret "secret/data/../metadata/minitoken/mac_secret" -}}
{{- range $key, $value := .Data.versions -}}
{{- with secret (printf "secret/minitoken/mac_secret?version=%s" $key) -}}
{{ . }}
{{- end }}
{{- end }}
{{- end }}

The exact template doesn't work though (vault.read(secret/minitoken/mac_secret.v1): no secret exists), but this is probably a separate issue.

@eikenb eikenb added bug vault Related to the Vault integration labels Jul 22, 2020
@eikenb
Copy link
Contributor

eikenb commented Jul 22, 2020

Hey @terorie, thanks for the report.

I agree with your assessment that the addPrefixToVKVPath not handling metadata is the issue. Looks like this stopped working with changes to fix issues where it mistakenly saw random 'foodata' entries as matching 'data' and not fixing their path. This broke 'metadata' as it was a good case of that bug, but there was no test asserting that.

The fix needs to include both a way to recognize 'metadata' as a valid path and to be sure to add a test for it.

@findkim findkim added this to the 0.25.1 milestone Jul 23, 2020
@findkim
Copy link
Contributor

findkim commented Jul 23, 2020

Great find @terorie, this was an overlook on my part and not considering the vault usage of metadata.

@findkim
Copy link
Contributor

findkim commented Jul 24, 2020

After digging into this a bit more, I noticed reading metadata was also not working in v0.24.1 due to the prepended /data/metadata/ path. It leads me to think metadata has not been supported for KVv2. Thanks for shedding light to this!

@riptl
Copy link
Author

riptl commented Jul 24, 2020

Thank you all so much for looking into this! vault-k8s has helped with automating secrets management on our Kubernetes clusters immensely. The quick developer response is the cherry on top.

@findkim I'm going to test the changes included in #1399 and report my findings. I'll also test out iterating versioned secrets with this using the aforementioned template. I might have another small bug there, but this is probably just wrong configuration on my end.

@riptl
Copy link
Author

riptl commented Jul 27, 2020

I could confirm that #1399 fixes iterating over KVv2 secrets versions.

Vault data:

vault kv put secret/test_secret abc=def
vault kv get -version 1 secret/test_secret
vault kv put secret/test_secret abc=def
vault kv get -version 2 secret/test_secret

Consul Template config:

vault {
  address = "http://localhost:8200"
  token   = "token"
}

template {
  destination = "./version-1-test.txt"
  contents = <<EOF
{{- with secret "secret/data/test_secret?version=1" -}}
{{ .Data }}
{{- end }}
EOF
}

template {
  destination = "./version-iteration-test.txt"
  contents = <<EOF
{{- with secret "secret/metadata/test_secret" -}}
{{- range $key, $value := .Data.versions -}}
{{- with secret (printf "secret/data/test_secret?version=%s" $key) }}
{{ .Data }}
{{- end }}
{{- end }}
{{- end }}
EOF
}

Output: version-iteration-test.txt

map[data:map[abc:def] metadata:map[created_time:2020-07-27T00:02:46.006401578Z deletion_time: destroyed:false version:1]]
map[data:map[abc:def] metadata:map[created_time:2020-07-27T00:07:05.084323392Z deletion_time: destroyed:false version:2]]

Output: version-1-test.txt

map[data:map[abc:def] metadata:map[created_time:2020-07-27T00:02:46.006401578Z deletion_time: destroyed:false version:1]]

@findkim
Copy link
Contributor

findkim commented Jul 28, 2020

Glad to hear that #1399 works for you @terorie, and appreciate the testing on your end! It helped expedite the release for the fix v0.25.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug hashicat-update-complete Completed porting changes to hashicat hashicat-update-required Changes that need to be ported to hashicat vault Related to the Vault integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants