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

fix auth renew panic #18011

Merged
merged 3 commits into from
Nov 18, 2022
Merged

fix auth renew panic #18011

merged 3 commits into from
Nov 18, 2022

Conversation

hghaf099
Copy link
Contributor

@hghaf099 hghaf099 commented Nov 17, 2022

Addresses #17013

When handleAuthRenew is invoked, b.AuthRenew is called with nil framework.FieldData here. This request goes to pathLoginRenew and this causes a panic for the case of Okta engine which expects to get a nonce from the field data.
The proposed fix assumes that there could be other code paths that call pathLoginRenew.

2022-11-17T09:30:58.576-0800 [INFO]  http: panic serving 127.0.0.1:61385: runtime error: invalid memory address or nil pointer dereference
goroutine 830 [running]:
net/http.(*conn).serve.func1()
        /Users/hamid/.go/src/net/http/server.go:1850 +0xbf
panic({0x59d6c80, 0x9f71ec0})
        /Users/hamid/.go/src/runtime/panic.go:890 +0x262
github.com/hashicorp/vault/sdk/framework.(*FieldData).Get(0x0, {0x658e728, 0x5})
        /Users/hamid/repo/vault/sdk/framework/field_data.go:61 +0x27
github.com/hashicorp/vault/builtin/credential/okta.(*backend).pathLoginRenew(0xc000d1bf50, {0x7109638, 0xc001c5fb60}, 0xc001599500, 0x0)
        /Users/hamid/repo/vault/builtin/credential/okta/path_login.go:147 +0x246
github.com/hashicorp/vault/sdk/framework.(*Backend).handleAuthRenew(0x55bbaa0?, {0x7109638?, 0xc001c5fb60?}, 0x0?)
        /Users/hamid/repo/vault/sdk/framework/backend.go:620 +0xe5
github.com/hashicorp/vault/sdk/framework.(*Backend).handleRevokeRenew(0x0?, {0x7109638?, 0xc001c5fb60?}, 0x106bdf6?)
        /Users/hamid/repo/vault/sdk/framework/backend.go:561 +0x4e
github.com/hashicorp/vault/sdk/framework.(*Backend).HandleRequest(0xc001694000, {0x7109638, 0xc001c5fb60}, 0xc001599500)
        /Users/hamid/repo/vault/sdk/framework/backend.go:198 +0xe7
github.com/hashicorp/vault/builtin/plugin/v5.(*backend).HandleRequest(0xc000dc8780, {0x7109638, 0xc001c5fb60}, 0xc001599500)
        /Users/hamid/repo/vault/builtin/plugin/v5/backend.go:92 +0xce
github.com/hashicorp/vault/vault.(*Router).routeCommon(0xc0001c8af0, {0x7109638, 0xc001c5fb60}, 0xc001599500, 0x0)
        /Users/hamid/repo/vault/vault/router.go:764 +0x160b
github.com/hashicorp/vault/vault.(*Router).Route(...)
        /Users/hamid/repo/vault/vault/router.go:544
github.com/hashicorp/vault/vault.(*ExpirationManager).renewAuthEntry(0xc00001ab40, {0x7109638, 0xc001c5ec60}, 0xc001599200, 0xc000352b60, 0x34630b8a000)
        /Users/hamid/repo/vault/vault/expiration.go:1978 +0x36e
github.com/hashicorp/vault/vault.(*ExpirationManager).RenewToken(0xc00001ab40, {0x7109638, 0xc001c5ec60}, 0xc001a22420?, 0xc001c38b00, 0x659a2be?)
        /Users/hamid/repo/vault/vault/expiration.go:1352 +0x5ad
github.com/hashicorp/vault/vault.(*TokenStore).handleRenew(0xc0005f4780, {0x7109638, 0xc001c5ec60}, 0x9?, 0x18?)
        /Users/hamid/repo/vault/vault/token_store.go:3471 +0x2e5
github.com/hashicorp/vault/vault.(*TokenStore).handleUpdateRenewAccessor(0x0?, {0x7109638, 0xc001c5ec60}, 0x0?, 0xffffffffffffffff?)
        /Users/hamid/repo/vault/vault/token_store.go:2531 +0x33f
github.com/hashicorp/vault/sdk/framework.(*Backend).HandleRequest(0xc000cbb420, {0x7109638, 0xc001c5ec60}, 0xc001599200)
        /Users/hamid/repo/vault/sdk/framework/backend.go:291 +0xa82
github.com/hashicorp/vault/vault.(*Router).routeCommon(0xc0001c8af0, {0x7109638, 0xc001c5ec60}, 0xc001599200, 0x0)
        /Users/hamid/repo/vault/vault/router.go:764 +0x160b
github.com/hashicorp/vault/vault.(*Router).Route(...)
        /Users/hamid/repo/vault/vault/router.go:544
github.com/hashicorp/vault/vault.(*Core).doRouting(0xc00129c000?, {0x7109638?, 0xc001c5ec60?}, 0xc001c5e810?)
        /Users/hamid/repo/vault/vault/request_handling.go:817 +0x2c
github.com/hashicorp/vault/vault.(*Core).handleRequest(0xc00129c000, {0x7109638, 0xc001c5ec60}, 0xc001599200)
        /Users/hamid/repo/vault/vault/request_handling.go:1008 +0x11da
github.com/hashicorp/vault/vault.(*Core).handleCancelableRequest(0xc00129c000, {0x7109638, 0xc001c5e8d0}, 0xc001599200)
        /Users/hamid/repo/vault/vault/request_handling.go:665 +0x1545
github.com/hashicorp/vault/vault.(*Core).switchedLockHandleRequest(0xc00129c000, {0x7109638, 0xc001c5e2a0}, 0xc001599200, 0x80?)
        /Users/hamid/repo/vault/vault/request_handling.go:472 +0x539
github.com/hashicorp/vault/vault.(*Core).HandleRequest(...)
        /Users/hamid/repo/vault/vault/request_handling.go:433
github.com/hashicorp/vault/http.request(0x5f53c80?, {0x70f81e0, 0xc001c5e1b0}, 0xc000345600, 0xc001599200?)
        /Users/hamid/repo/vault/http/handler.go:910 +0x86
github.com/hashicorp/vault/http.handleLogicalInternal.func1({0x70f81e0, 0xc001c5e1b0}, 0xc000345600)
        /Users/hamid/repo/vault/http/logical.go:354 +0xb6
net/http.HandlerFunc.ServeHTTP(0xc001651595?, {0x70f81e0?, 0xc001c5e1b0?}, 0xc001325d40?)
        /Users/hamid/.go/src/net/http/server.go:2109 +0x2f
github.com/hashicorp/vault/http.handleRequestForwarding.func1({0x70f81e0, 0xc001c5e1b0}, 0xc000345600)
        /Users/hamid/repo/vault/http/handler.go:835 +0x39d
net/http.HandlerFunc.ServeHTTP(0xc00188f398?, {0x70f81e0?, 0xc001c5e1b0?}, 0x0?)
        /Users/hamid/.go/src/net/http/server.go:2109 +0x2f
net/http.(*ServeMux).ServeHTTP(0xe?, {0x70f81e0, 0xc001c5e1b0}, 0xc000345600)
        /Users/hamid/.go/src/net/http/server.go:2487 +0x149
github.com/hashicorp/vault/http.wrapHelpHandler.func1({0x70f81e0, 0xc001c5e1b0}, 0xc000345600)
        /Users/hamid/repo/vault/http/help.go:25 +0x129
net/http.HandlerFunc.ServeHTTP(0xc0006d9280?, {0x70f81e0?, 0xc001c5e1b0?}, 0x2559440?)
        /Users/hamid/.go/src/net/http/server.go:2109 +0x2f
github.com/hashicorp/vault/http.wrapCORSHandler.func1({0x70f81e0?, 0xc001c5e1b0?}, 0xc00188f510?)
        /Users/hamid/repo/vault/http/cors.go:29 +0x1e5
net/http.HandlerFunc.ServeHTTP(0xc00129c000?, {0x70f81e0?, 0xc001c5e1b0?}, 0xc000a295c0?)
        /Users/hamid/.go/src/net/http/server.go:2109 +0x2f
github.com/hashicorp/vault/http.rateLimitQuotaWrapping.func1({0x70f81e0, 0xc001c5e1b0}, 0xc000345600)
        /Users/hamid/repo/vault/http/util.go:110 +0xc30
net/http.HandlerFunc.ServeHTTP(0xc001c5e120?, {0x70f81e0?, 0xc001c5e1b0?}, 0xc001a20110?)
        /Users/hamid/.go/src/net/http/server.go:2109 +0x2f
github.com/hashicorp/vault/http.wrapGenericHandler.func1({0x7106b20?, 0xc000350a80}, 0xc000345300)
        /Users/hamid/repo/vault/http/handler.go:422 +0xdb3
net/http.HandlerFunc.ServeHTTP(0xc001651595?, {0x7106b20?, 0xc000350a80?}, 0x9fdb060?)
        /Users/hamid/.go/src/net/http/server.go:2109 +0x2f
github.com/hashicorp/go-cleanhttp.PrintablePathCheckHandler.func1({0x7106b20, 0xc000350a80}, 0xc000345300)
        /Users/hamid/go/pkg/mod/github.com/hashicorp/[email protected]/handlers.go:42 +0x93
net/http.HandlerFunc.ServeHTTP(0x0?, {0x7106b20?, 0xc000350a80?}, 0x1317a74?)
        /Users/hamid/.go/src/net/http/server.go:2109 +0x2f
net/http.serverHandler.ServeHTTP({0x70f0690?}, {0x7106b20, 0xc000350a80}, 0xc000345300)
        /Users/hamid/.go/src/net/http/server.go:2947 +0x30c
net/http.(*conn).serve(0xc001a89400, {0x7109638, 0xc000cf42a0})
        /Users/hamid/.go/src/net/http/server.go:1991 +0x607
created by net/http.(*Server).Serve
        /Users/hamid/.go/src/net/http/server.go:3102 +0x4db

@hghaf099 hghaf099 requested review from a team and cipherboy November 17, 2022 17:59
Copy link
Contributor

@ccapurso ccapurso left a comment

Choose a reason for hiding this comment

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

Looks good to me

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.

3 participants