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/go-cleanhttp@v0.5.2/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

@hghaf099 hghaf099 merged commit a511f6e into main Nov 18, 2022
@hghaf099 hghaf099 deleted the issue-17013-fix-auth-renew branch November 18, 2022 15:38
jayant07-yb pushed a commit to jayant07-yb/hashicorp-vault-integrations that referenced this pull request Mar 15, 2023
* fix auth renew panic

* CL

* adding a test step to a cert test for pathLoginRenew
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

3 participants