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

Backport of Fix panic caused by parsing json.Number values for TypeCommaStringSlice fields into release/1.8.x #14741

Merged
merged 3 commits into from Mar 29, 2022

Conversation

hc-github-team-secure-vault-core
Copy link
Collaborator

Backport

This PR is auto-generated from #14522 to be assessed for backporting due to the inclusion of the label backport/1.8.x.

WARNING automatic cherry-pick of commits failed. Commits will require human attention.

The below text is copied from the body of the original PR.


Vault will panic when it attempts to parse fields with a schema of TypeCommaStringSlice if the provided value is an integer specifically if provided within the JSON body of an HTTP request. The underlying cause is within the ParseCommaStringSlice function in go-secure-stdlib/parseutil. The issue has been fixed in go-secure-stdlib #29.

The changes introduced within this PR are:

  • Upgrading to v0.1.4 of go-secure-stdlib/parseutil
  • Adding a test specifically for json.Number input

Before fix:

❯ vault auth enable userpass
Success! Enabled userpass auth method at: userpass/

❯ curl -v -XPOST -H "X-Vault-Token: $VAULT_TOKEN" --data '{"password":"abc123", "token_bound_cidrs": 123}' "$VAULT_ADDR/v1/auth/userpass/users/jdoe"
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8200 (#0)
> POST /v1/auth/userpass/users/jdoe HTTP/1.1
> Host: 127.0.0.1:8200
> User-Agent: curl/7.64.1
> Accept: */*
> X-Vault-Token: hvs.ogFGaKQ3F4DUEh2OtQ39L1bf
> Content-Length: 47
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 47 out of 47 bytes
* Empty reply from server
* Connection #0 to host 127.0.0.1 left intact
curl: (52) Empty reply from server
* Closing connection 0

Panic in server logs (truncated):

2022-03-16T10:42:16.943-0400 [INFO]  http: panic serving 127.0.0.1:51571: interface conversion: interface {} is json.Number, not string
goroutine 854 [running]:
net/http.(*conn).serve.func1()
        /usr/local/bin/go/src/net/http/server.go:1802 +0xb9
panic({0x514d440, 0xc000cce810})
        /usr/local/bin/go/src/runtime/panic.go:1047 +0x266
github.com/mitchellh/mapstructure.StringToSliceHookFunc.func1(0x51a9ec0, 0xc000dfd720, {0x51a9ec0, 0xc000dfd720})
        /Users/ccapurso/go/pkg/mod/github.com/mitchellh/mapstructure@v1.4.3/decode_hooks.go:91 +0xd1
github.com/mitchellh/mapstructure.DecodeHookExec({0x50be8e0, 0xc0009cbbc0}, {0x51a9ec0, 0xc000dfd720, 0x1054e26}, {0x4cf4fe0, 0xc0009cbba8, 0x203000})
        /Users/ccapurso/go/pkg/mod/github.com/mitchellh/mapstructure@v1.4.3/decode_hooks.go:49 +0x1f9
github.com/mitchellh/mapstructure.(*Decoder).decode(0xc001482120, {0x0, 0xc000cca501}, {0x51a9ec0, 0xc000dfd720}, {0x4cf4fe0, 0xc0009cbba8, 0x48})
        /Users/ccapurso/go/pkg/mod/github.com/mitchellh/mapstructure@v1.4.3/mapstructure.go:440 +0x179
github.com/mitchellh/mapstructure.(*Decoder).Decode(0xc001482120, {0x51a9ec0, 0xc000dfd720})
        /Users/ccapurso/go/pkg/mod/github.com/mitchellh/mapstructure@v1.4.3/mapstructure.go:398 +0xd8
github.com/hashicorp/go-secure-stdlib/parseutil.ParseCommaStringSlice({0x51a9ec0, 0xc000dfd720})
        /Users/ccapurso/go/pkg/mod/github.com/hashicorp/go-secure-stdlib/parseutil@v0.1.3/parseutil.go:354 +0x11e
github.com/hashicorp/vault/sdk/framework.(*FieldData).getPrimitive(0x50594e0, {0xc001325788, 0xc001325788}, 0xc0014a2230)
        /Users/ccapurso/git/vault/sdk/framework/field_data.go:289 +0x71b
github.com/hashicorp/vault/sdk/framework.(*FieldData).Validate(0xc000dfd8c0)
        /Users/ccapurso/git/vault/sdk/framework/field_data.go:44 +0xf3
github.com/hashicorp/vault/sdk/framework.(*Backend).HandleExistenceCheck(0x0, {0x64dd010, 0xc000cce4e0}, 0xc0001e9e00)
        /Users/ccapurso/git/vault/sdk/framework/backend.go:174 +0x3b4
github.com/hashicorp/vault/builtin/plugin.(*PluginBackend).HandleExistenceCheck.func1()
        /Users/ccapurso/git/vault/builtin/plugin/backend.go:211 +0x43
github.com/hashicorp/vault/builtin/plugin.(*PluginBackend).lazyLoadBackend(0xc001081860, {0x64dd010, 0xc000cce4e0}, {0x64de8c8, 0xc0007b7780}, 0xc000fe01d0)
        /Users/ccapurso/git/vault/builtin/plugin/backend.go:162 +0x19d
github.com/hashicorp/vault/builtin/plugin.(*PluginBackend).HandleExistenceCheck(0xc0007e1ea0, {0x64dd010, 0xc000cce4e0}, 0xc000e2dcb9)
        /Users/ccapurso/git/vault/builtin/plugin/backend.go:209 +0x9f
github.com/hashicorp/vault/vault.(*Router).routeCommon(0xc00047f9f0, {0x64dd010, 0xc000cce4e0}, 0xc0001e9e00, 0x1)
        /Users/ccapurso/git/vault/vault/router.go:720 +0x18a9
github.com/hashicorp/vault/vault.(*Router).RouteExistenceCheck(0x505c120, {0x64dd010, 0xc000cce4e0}, 0xd)
        /Users/ccapurso/git/vault/vault/router.go:511 +0x28
github.com/hashicorp/vault/vault.(*Core).checkToken(0xc00089ea00, {0x64dd010, 0xc000cce4e0}, 0xc0001e9e00, 0x0)
        /Users/ccapurso/git/vault/vault/request_handling.go:318 +0x6ab

After fix:

❯ vault auth enable userpass
Success! Enabled userpass auth method at: userpass/

❯ curl -v -XPOST -H "X-Vault-Token: $VAULT_TOKEN" --data '{"password":"abc123", "token_bound_cidrs": 123}' "$VAULT_ADDR/v1/auth/userpass/users/jdoe"
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8200 (#0)
> POST /v1/auth/userpass/users/jdoe HTTP/1.1
> Host: 127.0.0.1:8200
> User-Agent: curl/7.64.1
> Accept: */*
> X-Vault-Token: hvs.8isydRBWneASw7nO8wYfISCt
> Content-Length: 47
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 47 out of 47 bytes
< HTTP/1.1 400 Bad Request
< Cache-Control: no-store
< Content-Type: application/json
< Strict-Transport-Security: max-age=31536000; includeSubDomains
< Date: Wed, 16 Mar 2022 14:44:29 GMT
< Content-Length: 117
<
{"errors":["error parsing address \"123\": Unable to convert \"123\" to an IPv4 or IPv6 address, or a UNIX Socket"]}
* Connection #0 to host 127.0.0.1 left intact
* Closing connection 0

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 28, 2022

CLA assistant check
All committers have signed the CLA.

@vercel vercel bot temporarily deployed to Preview – vault-storybook March 28, 2022 15:08 Inactive
@vercel vercel bot temporarily deployed to Preview – vault March 28, 2022 15:08 Inactive
Copy link
Contributor

@HridoyRoy HridoyRoy left a comment

Choose a reason for hiding this comment

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

The PR contains no diffs. Is it ok to close?

@vercel vercel bot temporarily deployed to Preview – vault March 28, 2022 16:58 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 28, 2022 16:58 Inactive
@vercel vercel bot temporarily deployed to Preview – vault March 28, 2022 17:02 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 28, 2022 17:02 Inactive
@ccapurso
Copy link
Contributor

ccapurso commented Mar 28, 2022

The PR contains no diffs. Is it ok to close?

@HridoyRoy, the issue was that the 1.8.x still uses sdk/helper while 1.9.x and on moved to use go-secure-stdlib. The parseutil changes needed to be ported over to sdk/helper directly from go-secure-stdlib, see hashicorp/go-secure-stdlib#29.

@ccapurso ccapurso force-pushed the backport/vault-4280/grossly-firm-mite branch from 446e527 to 2833dfe Compare March 28, 2022 18:19
@vercel vercel bot temporarily deployed to Preview – vault March 28, 2022 18:19 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 28, 2022 18:19 Inactive
@ccapurso ccapurso requested a review from HridoyRoy March 28, 2022 18:53
@ccapurso ccapurso force-pushed the backport/vault-4280/grossly-firm-mite branch from 2833dfe to 01fa209 Compare March 28, 2022 21:05
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 28, 2022 21:06 Inactive
@vercel vercel bot temporarily deployed to Preview – vault March 28, 2022 21:06 Inactive
@ccapurso ccapurso merged commit 9e05eb7 into release/1.8.x Mar 29, 2022
@ccapurso ccapurso deleted the backport/vault-4280/grossly-firm-mite branch March 29, 2022 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants