Return error stack traces for the CLI client #2602

a-palchikov · 2021-08-12T13:05:13Z

Description

Revisit #2269 to implement conditional stack traces for HTTP APIs.
The original implementation removes the stack traces from HTTP APIs
errors unconditionally which results in reduced troubleshooting efficacy
when the errors contain important details about a failure from the CLI
client.
In order to counter this and provide useful information when necessary,
the stack traces are only stripped if the request does not contain a
custom marker identifying the gravity client.

Type of change

Regression fix (non-breaking change which fixes a regression)
Internal change (not necessarily a bug fix or a new feature)
This change has a user-facing impact

Linked tickets and other PRs

Refs Security: stack traces returned in API #2269.

TODOs

Self-review the change
Perform manual testing
Address review feedback

Implementation

Performance/Scaling

Testing done

$ curl -k -H "Content-Type: application/json" -d '{"username": "example", "password":"invalid"}' https://localhost:61009/proxy/v1/webapi/sessions
{
    "error": {
        "message": "access denied"
    }
}

Before the change:

$ ./gravity --debug resume
Thu Aug 12 13:02:34 UTC	Connecting to installer
Thu Aug 12 13:02:34 UTC	Connected to installer
Thu Aug 12 13:02:34 UTC	Successfully added "node" node on 172.17.0.2
Thu Aug 12 13:02:34 UTC	All agents have connected!
Thu Aug 12 13:02:35 UTC	Executing "/configure" locally
Thu Aug 12 13:02:36 UTC	Configuring cluster packages
Thu Aug 12 13:02:36 UTC	Configure packages for all nodes
Thu Aug 12 13:02:39 UTC	Executing operation finished in 4 seconds
Thu Aug 12 13:02:39 UTC	Saving debug report to /root/installer/crashreport.tgz
[ERROR]: failed to execute phase "/configure"

and the log:

Logs

2021-08-12T13:02:39Z WARN [INSTALLER] Failed to execute. error:[
ERROR REPORT:
Original Error: *internal.RawTrace 
Stack Trace:
        github.com/gravitational/teleport@v3.2.17+incompatible/lib/httplib/httplib.go:122 github.com/gravitational/teleport/lib/httplib.ConvertResponse
        github.com/gravitational/gravity@/lib/ops/opsclient/opsclient.go:1670 github.com/gravitational/gravity/lib/ops/opsclient.(*Client).PostJSON
        github.com/gravitational/gravity@/lib/ops/opsclient/opsclient.go:943 github.com/gravitational/gravity/lib/ops/opsclient.(*Client).ConfigurePackages
        github.com/gravitational/gravity@/lib/install/phases/configure.go:69 github.com/gravitational/gravity/lib/install/phases.(*configureExecutor).Execute
        github.com/gravitational/gravity@/lib/fsm/fsm.go:525 github.com/gravitational/gravity/lib/fsm.(*FSM).executeOnePhase
        github.com/gravitational/gravity@/lib/fsm/fsm.go:442 github.com/gravitational/gravity/lib/fsm.(*FSM).executePhaseLocally
        github.com/gravitational/gravity@/lib/fsm/fsm.go:402 github.com/gravitational/gravity/lib/fsm.(*FSM).executePhase
        github.com/gravitational/gravity@/lib/fsm/fsm.go:246 github.com/gravitational/gravity/lib/fsm.(*FSM).ExecutePhase
        github.com/gravitational/gravity@/lib/fsm/fsm.go:175 github.com/gravitational/gravity/lib/fsm.(*FSM).ExecutePlan
        github.com/gravitational/gravity@/lib/install/operation.go:81 github.com/gravitational/gravity/lib/install.(*Installer).ExecuteOperation
        github.com/gravitational/gravity@/lib/install/engine/cli/cli.go:107 github.com/gravitational/gravity/lib/install/engine/cli.(*Engine).execute
        github.com/gravitational/gravity@/lib/install/engine/cli/cli.go:79 github.com/gravitational/gravity/lib/install/engine/cli.(*Engine).Execute
        github.com/gravitational/gravity@/lib/install/install.go:258 github.com/gravitational/gravity/lib/install.(*Installer).execute
        github.com/gravitational/gravity@/lib/install/install.go:206 github.com/gravitational/gravity/lib/install.(*Installer).startExecuteLoop.func1
        runtime/asm_amd64.s:1357 runtime.goexit
User Message: failed to execute phase "/configure"

After the change:

$ ./gravity --debug resume
Thu Aug 12 12:17:04 UTC	Connecting to installer
Thu Aug 12 12:17:04 UTC	Connected to installer
Thu Aug 12 12:17:04 UTC	Successfully added "node" node on 172.17.0.2
Thu Aug 12 12:17:04 UTC	All agents have connected!
Thu Aug 12 12:17:05 UTC	Executing "/configure" locally
Thu Aug 12 12:17:06 UTC	Configuring cluster packages
Thu Aug 12 12:17:06 UTC	Configure packages for all nodes
Thu Aug 12 12:17:09 UTC	Saving debug report to /root/installer/crashreport.tgz
[ERROR]: failed to execute phase "/configure"
	failed to parse name: 'foo-type'
	1:14: expected operand, found 'type'

and the corresponding log:

Logs

2021-08-12T11:19:40Z WARN [INSTALLER] Failed to execute. error:[
ERROR REPORT:
Original Error: *internal.RawTrace 1:14: expected operand, found 'type'
Stack Trace:
        github.com/gravitational/configure@v0.0.0-20191213111049-fce91dea0d0d/schema/schema.go:323 github.com/gravitational/configure/schema.(*cparser).checkName
        github.com/gravitational/configure@v0.0.0-20191213111049-fce91dea0d0d/schema/schema.go:207 github.com/gravitational/configure/schema.(*cparser).parseParam
        github.com/gravitational/configure@v0.0.0-20191213111049-fce91dea0d0d/schema/schema.go:192 github.com/gravitational/configure/schema.(*cparser).parse
        github.com/gravitational/configure@v0.0.0-20191213111049-fce91dea0d0d/schema/schema.go:38 github.com/gravitational/configure/schema.ParseJSON
        github.com/gravitational/gravity@/lib/pack/manifest.go:190 github.com/gravitational/gravity/lib/pack.ParseManifestJSON
        github.com/gravitational/gravity@/lib/pack/manifest.go:175 github.com/gravitational/gravity/lib/pack.ReadManifest
        github.com/gravitational/gravity@/lib/pack/utils.go:136 github.com/gravitational/gravity/lib/pack.GetConfigPackage
        github.com/gravitational/gravity@/lib/ops/opsservice/configure.go:870 github.com/gravitational/gravity/lib/ops/opsservice.(*site).getPlanetConfigPackage
        github.com/gravitational/gravity@/lib/ops/opsservice/configure.go:1033 github.com/gravitational/gravity/lib/ops/opsservice.(*site).configurePlanetServer
        github.com/gravitational/gravity@/lib/ops/opsservice/configure.go:847 github.com/gravitational/gravity/lib/ops/opsservice.(*site).configurePlanetMaster
        github.com/gravitational/gravity@/lib/ops/opsservice/configure.go:378 github.com/gravitational/gravity/lib/ops/opsservice.(*site).configurePackages
        github.com/gravitational/gravity@/lib/ops/opsservice/configure.go:111 github.com/gravitational/gravity/lib/ops/opsservice.(*Operator).ConfigurePackages
        github.com/gravitational/gravity@/lib/ops/opsroute/forward.go:521 github.com/gravitational/gravity/lib/ops/opsroute.(*Router).ConfigurePackages
        github.com/gravitational/gravity@/lib/ops/operatoracl.go:664 github.com/gravitational/gravity/lib/ops.(*OperatorACL).ConfigurePackages
        github.com/gravitational/gravity@/lib/ops/opshandler/opshandler.go:1620 github.com/gravitational/gravity/lib/ops/opshandler.(*WebHandler).configurePackages
        github.com/gravitational/gravity@/lib/ops/opshandler/opshandler.go:2432 github.com/gravitational/gravity/lib/ops/opshandler.NeedsAuth.func1
        github.com/gravitational/gravity@/lib/ops/opshandler/opshandler.go:2439 github.com/gravitational/gravity/lib/ops/opshandler.NeedsAuth.func2
        @/github.com/julienschmidt/httprouter/router.go:299 github.com/julienschmidt/httprouter.(*Router).ServeHTTP
        github.com/gravitational/teleport@v3.2.17+incompatible/lib/auth/middleware.go:299 github.com/gravitational/teleport/lib/auth.(*AuthMiddleware).ServeHTTP
        github.com/gravitational/gravity@/lib/ops/opshandler/opshandler.go:344 github.com/gravitational/gravity/lib/ops/opshandler.(*WebHandler).ServeHTTP
github.com/gravitational/gravity@/lib/ops/opshandler/opshandler.go:344 github.com/gravitational/gravity/lib/ops/opshandler.(*WebHandler).ServeHTTP
        @/github.com/julienschmidt/httprouter/router.go:237 github.com/julienschmidt/httprouter.(*Router).Handler.func1
        @/github.com/julienschmidt/httprouter/router.go:299 github.com/julienschmidt/httprouter.(*Router).ServeHTTP
        github.com/gravitational/gravity@/lib/httplib/http.go:187 github.com/gravitational/gravity/lib/httplib.GRPCHandlerFunc.func1
        net/http/server.go:2007 net/http.HandlerFunc.ServeHTTP
        net/http/server.go:2802 net/http.serverHandler.ServeHTTP
        net/http/server.go:1890 net/http.(*conn).serve
        runtime/asm_amd64.s:1357 runtime.goexit
Caught:
        github.com/gravitational/teleport@v3.2.17+incompatible/lib/httplib/httplib.go:122 github.com/gravitational/teleport/lib/httplib.ConvertResponse
        github.com/gravitational/gravity@/lib/ops/opsclient/opsclient.go:1670 github.com/gravitational/gravity/lib/ops/opsclient.(*Client).PostJSON
        github.com/gravitational/gravity@/lib/ops/opsclient/opsclient.go:943 github.com/gravitational/gravity/lib/ops/opsclient.(*Client).ConfigurePackages
        github.com/gravitational/gravity@/lib/install/phases/configure.go:69 github.com/gravitational/gravity/lib/install/phases.(*configureExecutor).Execute
        github.com/gravitational/gravity@/lib/fsm/fsm.go:525 github.com/gravitational/gravity/lib/fsm.(*FSM).executeOnePhase
        github.com/gravitational/gravity@/lib/fsm/fsm.go:442 github.com/gravitational/gravity/lib/fsm.(*FSM).executePhaseLocally
        github.com/gravitational/gravity@/lib/fsm/fsm.go:402 github.com/gravitational/gravity/lib/fsm.(*FSM).executePhase
        github.com/gravitational/gravity@/lib/fsm/fsm.go:246 github.com/gravitational/gravity/lib/fsm.(*FSM).ExecutePhase
        github.com/gravitational/gravity@/lib/fsm/fsm.go:175 github.com/gravitational/gravity/lib/fsm.(*FSM).ExecutePlan
        github.com/gravitational/gravity@/lib/install/operation.go:81 github.com/gravitational/gravity/lib/install.(*Installer).ExecuteOperation
        github.com/gravitational/gravity@/lib/install/engine/cli/cli.go:107 github.com/gravitational/gravity/lib/install/engine/cli.(*Engine).execute
        github.com/gravitational/gravity@/lib/install/engine/cli/cli.go:79 github.com/gravitational/gravity/lib/install/engine/cli.(*Engine).Execute
        github.com/gravitational/gravity@/lib/install/install.go:258 github.com/gravitational/gravity/lib/install.(*Installer).execute
        github.com/gravitational/gravity@/lib/install/install.go:206 github.com/gravitational/gravity/lib/install.(*Installer).startExecuteLoop.func1
        runtime/asm_amd64.s:1357 runtime.goexit
User Message: failed to parse name: 'foo-type'
        1:14: expected operand, found 'type'

Additional information

a-palchikov · 2021-08-12T13:06:08Z

lib/install/fsmspec.go

@@ -119,10 +119,6 @@ func DefaultFSMSpec(config FSMConfig) fsm.FSMSpecFunc {
 config.Operator,
 config.OperationKey)

- case phases.ConfigurePhase:


Duplicate of the switch case from above.

implement conditional stack traces for HTTP APIs. The original implementation removes the stack traces from HTTP APIs errors unconditionally which results in reduced troubleshooting efficacy when the errors contain important details about a failure from the CLI client. In order to counter this and provide useful information when necessary, the stack traces are only stripped if the request does not contain a custom marker identifying the gravity client. Updates #2269.

wadells · 2021-09-01T16:18:45Z

Sorry I didn't see this (and thus provide feedback) earlier.

Did you consider gating this behind a server side debug flag? Scanning software won't have this header set, but with the current implementation it is solely up to the client whether these are exposed -- which feels like it weakens the hardening done in gravitational/teleport#5070.

a-palchikov · 2021-09-02T19:51:38Z

Sorry I didn't see this (and thus provide feedback) earlier.

Did you consider gating this behind a server side debug flag? Scanning software won't have this header set, but with the current implementation it is solely up to the client whether these are exposed -- which feels like it weakens the hardening done in gravitational/teleport#5070.

I have not thought about a debug flag but it seems to only further complicate things. Besides it's not going to help debug an issue when the gravity client is talking to an existing infra which might be difficult to restart with a debug flag.
I agree this is not the ideal solution to eliminate exposure but so far the troubleshooting benefits outweigh anything slightly more complex I've thought about.
The header value can be generated dynamically to make the guessing more challenging but I don't think it's worth it.

a-palchikov requested a review from a team August 12, 2021 13:05

a-palchikov commented Aug 12, 2021

View reviewed changes

bernardjkim approved these changes Aug 26, 2021

View reviewed changes

a-palchikov force-pushed the dmitri/fix-proxy-errors branch from 72334ed to be00922 Compare August 31, 2021 15:36

a-palchikov force-pushed the dmitri/fix-proxy-errors branch from be00922 to 9a77a11 Compare September 1, 2021 12:55

a-palchikov merged commit be5f827 into master Sep 1, 2021

a-palchikov deleted the dmitri/fix-proxy-errors branch September 1, 2021 15:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return error stack traces for the CLI client #2602

Return error stack traces for the CLI client #2602

a-palchikov commented Aug 12, 2021

a-palchikov Aug 12, 2021

wadells commented Sep 1, 2021 •

edited

Loading

a-palchikov commented Sep 2, 2021

Return error stack traces for the CLI client #2602

Return error stack traces for the CLI client #2602

Conversation

a-palchikov commented Aug 12, 2021

Description

Type of change

Linked tickets and other PRs

TODOs

Implementation

Performance/Scaling

Testing done

Additional information

a-palchikov Aug 12, 2021

Choose a reason for hiding this comment

wadells commented Sep 1, 2021 • edited Loading

a-palchikov commented Sep 2, 2021

wadells commented Sep 1, 2021 •

edited

Loading