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.

Sign up for GitHub

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

Jump to bottom

Fix plugin list API when audit logging enabled #18173

Merged
merged 5 commits into from
Dec 1, 2022
Merged

Fix plugin list API when audit logging enabled #18173

merged 5 commits into from
Dec 1, 2022

Conversation

tomhjp
Copy link
Contributor

@tomhjp tomhjp commented Nov 30, 2022

Fixes #17722

From 1.12.0 onwards, when the GET /v1/sys/plugins/catalog endpoint is hit and audit logging is enabled, Vault panics while HMACing the response in order to audit log it. @jmthvt correctly diagnosed this was due to the SemanticVersion field in the struct included in the response.

The handler now takes a similar approach to mountInfo() and rebuilds the structs as maps composed of primitive values.

Logging with panic stack trace added 
2022-11-30T18:06:02.279Z [ERROR] vault.audit: panic during logging: request_path=sys/plugins/catalog error="reflect: reflect.Value.Set using value obtained using unexported field"
  stacktrace=
  | goroutine 944 [running]:
  | runtime/debug.Stack()
  | \t/usr/local/go/src/runtime/debug/stack.go:24 +0x64
  | github.com/hashicorp/vault/vault.(*AuditBroker).LogResponse.func2()
  | \t/Users/tom/code/vault/vault/audit_broker.go:180 +0x68
  | panic({0x106f0a8c0, 0x1400152f220})
  | \t/usr/local/go/src/runtime/panic.go:884 +0x204
  | reflect.flag.mustBeAssignableSlow(0xb1a0?)
  | \t/usr/local/go/src/reflect/value.go:257 +0xb4
  | reflect.flag.mustBeAssignable(...)
  | \t/usr/local/go/src/reflect/value.go:247
  | reflect.Value.Set({0x106f0a8c0?, 0x14001b33540?, 0x14001871b58?}, {0x106f0a8c0?, 0x1400152f210?, 0x1023246ac?})
  | \t/usr/local/go/src/reflect/value.go:2154 +0x54
  | github.com/hashicorp/vault/audit.(*hashWalker).Primitive(0x14001b34460, {0x106f0a8c0?, 0x14001b33540?, 0x14001533b00?})
  | \t/Users/tom/code/vault/audit/hashstructure.go:354 +0x384
  | github.com/mitchellh/reflectwalk.walkPrimitive(...)
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:270
  | github.com/mitchellh/reflectwalk.walk({0x106f0a8c0?, 0x14001b33540?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:197 +0x580
  | github.com/mitchellh/reflectwalk.walkStruct({0x1079dfa20?, 0x14001b33540?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:404 +0x340
  | github.com/mitchellh/reflectwalk.walk({0x107be4e00?, 0x14001528058?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:206 +0x5e8
  | github.com/mitchellh/reflectwalk.walkStruct({0x107b24120?, 0x14001528000?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:404 +0x340
  | github.com/mitchellh/reflectwalk.walk({0x107b24120?, 0x14001528000?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:206 +0x5e8
  | github.com/mitchellh/reflectwalk.walkSlice({0x106e2aa60?, 0x14001672828?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:302 +0x190
  | github.com/mitchellh/reflectwalk.walk({0x107195be0?, 0x1400152f110?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:203 +0x488
  | github.com/mitchellh/reflectwalk.walkMap({0x1072908c0?, 0x140015220c0?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:252 +0x228
  | github.com/mitchellh/reflectwalk.walk({0x1072908c0?, 0x140015220c0?, 0x14001b34460?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:200 +0x4e8
  | github.com/mitchellh/reflectwalk.Walk({0x1072908c0?, 0x140015220c0?}, {0x1079be0e0, 0x14001b34460})
  | \t/Users/tom/go/pkg/mod/github.com/mitchellh/[email protected]/reflectwalk.go:99 +0x158
  | github.com/hashicorp/vault/audit.HashStructure(...)
  | \t/Users/tom/code/vault/audit/hashstructure.go:177
  | github.com/hashicorp/vault/audit.hashMap(0x140012f6fc0, 0x140015220c0?, {0x0, 0x0, 0x0})
  | \t/Users/tom/code/vault/audit/hashstructure.go:97 +0x1dc
  | github.com/hashicorp/vault/audit.HashResponse(0x140016bb1a0, 0x14001b33400, 0x1?, {0x0, 0x0, 0x0})
  | \t/Users/tom/code/vault/audit/hashstructure.go:132 +0x260
  | github.com/hashicorp/vault/audit.(*AuditFormatter).FormatResponse(0x140002fdb10, {0x107f2cec0, 0x140012dd5c0}, {0x107ee8380?, 0x14001522090}, {0x2c?, 0x2c?, 0x0}, 0x140001fc310)
  | \t/Users/tom/code/vault/audit/format.go:206 +0x190
  | github.com/hashicorp/vault/builtin/audit/file.(*Backend).LogResponse(0x140002fdb00, {0x107f2cec0, 0x140012dd5c0}, 0x140012dcab0?)
  | \t/Users/tom/code/vault/builtin/audit/file/backend.go:264 +0x15c
  | github.com/hashicorp/vault/vault.(*AuditBroker).LogResponse(0x140018f4cf0, {0x107f2cec0, 0x140012dd5c0}, 0x140001fc310, 0x13?)
  | \t/Users/tom/code/vault/vault/audit_broker.go:210 +0x35c
  | github.com/hashicorp/vault/vault.(*Core).handleCancelableRequest(0x14000887500, {0x107f2cec0, 0x140012dd260}, 0x14001680480)
  | \t/Users/tom/code/vault/vault/request_handling.go:781 +0x1a00
  | github.com/hashicorp/vault/vault.(*Core).switchedLockHandleRequest(0x14000887500, {0x107f2cec0, 0x140012dcc30}, 0x14001680480, 0x80?)
  | \t/Users/tom/code/vault/vault/request_handling.go:472 +0x48c
  | github.com/hashicorp/vault/vault.(*Core).HandleRequest(...)
  | \t/Users/tom/code/vault/vault/request_handling.go:433
  | github.com/hashicorp/vault/http.request(0x1078a2180?, {0x107f1b718, 0x140012dcb40}, 0x140019a1700, 0x14001680480?)
  | \t/Users/tom/code/vault/http/handler.go:926 +0x6c
  | github.com/hashicorp/vault/http.handleLogicalInternal.func1({0x107f1b718, 0x140012dcb40}, 0x140019a1700)
  | \t/Users/tom/code/vault/http/logical.go:354 +0xa4
  | net/http.HandlerFunc.ServeHTTP(0x14000603298?, {0x107f1b718?, 0x140012dcb40?}, 0x140012b5471?)
  | \t/usr/local/go/src/net/http/server.go:2109 +0x38
  | github.com/hashicorp/vault/http.handleRequestForwarding.func1({0x107f1b718, 0x140012dcb40}, 0x140019a1700)
  | \t/Users/tom/code/vault/http/handler.go:851 +0x2f0
  | net/http.HandlerFunc.ServeHTTP(0x14000603338?, {0x107f1b718?, 0x140012dcb40?}, 0x0?)
  | \t/usr/local/go/src/net/http/server.go:2109 +0x38
  | net/http.(*ServeMux).ServeHTTP(0x140012cbf00?, {0x107f1b718, 0x140012dcb40}, 0x140019a1700)
  | \t/usr/local/go/src/net/http/server.go:2487 +0x140
  | github.com/hashicorp/vault/http.wrapHelpHandler.func1({0x107f1b718, 0x140012dcb40}, 0x140019a1700)
  | \t/Users/tom/code/vault/http/help.go:25 +0x148
  | net/http.HandlerFunc.ServeHTTP(0x140006033e8?, {0x107f1b718?, 0x140012dcb40?}, 0x140012cba40?)
  | \t/usr/local/go/src/net/http/server.go:2109 +0x38
  | github.com/hashicorp/vault/http.wrapCORSHandler.func1({0x107f1b718?, 0x140012dcb40?}, 0x14000b86480?)
  | \t/Users/tom/code/vault/http/cors.go:29 +0x1a4
  | net/http.HandlerFunc.ServeHTTP(0x14000887500?, {0x107f1b718?, 0x140012dcb40?}, 0x14000281380?)
  | \t/usr/local/go/src/net/http/server.go:2109 +0x38
  | github.com/hashicorp/vault/http.rateLimitQuotaWrapping.func1({0x107f1b718, 0x140012dcb40}, 0x140019a1700)
  | \t/Users/tom/code/vault/http/util.go:110 +0x9fc
  | net/http.HandlerFunc.ServeHTTP(0x140012dcab0?, {0x107f1b718?, 0x140012dcb40?}, 0xc0da04c290871828?)
  | \t/usr/local/go/src/net/http/server.go:2109 +0x38
  | github.com/hashicorp/vault/http.wrapGenericHandler.func1({0x107f2a058?, 0x140005b2fc0}, 0x140019a1400)
  | \t/Users/tom/code/vault/http/handler.go:438 +0xb70
  | net/http.HandlerFunc.ServeHTTP(0x140011e1924?, {0x107f2a058?, 0x140005b2fc0?}, 0xffffffffffffffff?)
  | \t/usr/local/go/src/net/http/server.go:2109 +0x38
  | github.com/hashicorp/go-cleanhttp.PrintablePathCheckHandler.func1({0x107f2a058, 0x140005b2fc0}, 0x140019a1400)
  | \t/Users/tom/go/pkg/mod/github.com/hashicorp/[email protected]/handlers.go:42 +0x88
  | net/http.HandlerFunc.ServeHTTP(0x0?, {0x107f2a058?, 0x140005b2fc0?}, 0x1025c45a8?)
  | \t/usr/local/go/src/net/http/server.go:2109 +0x38
  | net/http.serverHandler.ServeHTTP({0x140012dca80?}, {0x107f2a058, 0x140005b2fc0}, 0x140019a1400)
  | \t/usr/local/go/src/net/http/server.go:2947 +0x2c4
  | net/http.(*conn).serve(0x14001b341e0, {0x107f2cec0, 0x14000ba8600})
  | \t/usr/local/go/src/net/http/server.go:1991 +0x560
  | created by net/http.(*Server).Serve
  | \t/usr/local/go/src/net/http/server.go:3102 +0x444
  
2022-11-30T18:06:02.279Z [ERROR] vault.core: failed to audit response: request_path=sys/plugins/catalog
  error=
  | 1 error occurred:
  | \t* panic generating audit log
  |

@tomhjp tomhjp requested review from mpalmi, swenson and tvoran November 30, 2022 20:10
swenson
swenson approved these changes Nov 30, 2022
View reviewed changes
Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

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

LGTM

@tomhjp tomhjp changed the title Fix audit panic list plugins Fix plugin list API when audit logging enabled Nov 30, 2022
@tomhjp tomhjp mentioned this pull request Nov 30, 2022
Closed
@tomhjp tomhjp merged commit d6ed362 into main Dec 1, 2022
@tomhjp tomhjp deleted the fix-audit-panic-list-plugins branch December 1, 2022 10:44
Merged
@mladlow mladlow added this to the 1.12.3 milestone Dec 1, 2022
@tomhjp tomhjp mentioned this pull request Dec 1, 2022
Merged
Merged
AnPucel pushed a commit that referenced this pull request Jan 14, 2023
@tomhjp @AnPucel
Fix plugin list API when audit logging enabled (#18173)
3c721dc 
* Add test that fails due to audit log panic
* Rebuild VersionedPlugin as map of primitive types before adding to response
* Changelog
* Fix casting in external plugin tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpalmi mpalmi mpalmi approved these changes

@swenson swenson swenson approved these changes

@tvoran tvoran Awaiting requested review from tvoran

Assignees
No one assigned
Labels
bug Used to indicate a potential bug core/plugin ecosystem/plugin
Projects
None yet
Milestone
1.12.3
Development

Successfully merging this pull request may close these issues.

plugin list fails when audit is enabled
4 participants
@tomhjp @swenson @mpalmi @mladlow