gravitational / gravity

Kubernetes application deployments for restricted, regulated, or remote environments
Apache License 2.0
1.08k stars 109 forks source link

[BUG] Fail to emit an audit event #1345

Closed r0mant closed 1 year ago

r0mant commented 4 years ago

Describe the bug

When attempting to complete the operation manually via plan complete command, an audit event fails to be emitted with the *trace.BadParameterError invalid role auth.LocalUser error. Full stack trace below.

The error is not critical but the event is missed nonetheless.

Some brief investigation showed that the error is thrown from the same Teleport's getServerID method that was added as a part of security-related changes last year and which has already caused problems before e.g. with session upload.

This time it fails on the following line:

https://github.com/gravitational/teleport/blob/branch/3.2-g/lib/auth/apiserver.go#L2435

func (s *APIServer) getServerID(r *http.Request) (string, error) {
    role, ok := r.Context().Value(ContextUser).(BuiltinRole)
    if !ok {
        return "", trace.BadParameter("invalid role %T", r.Context().Value(ContextUser))
    }

For some reason it asserts auth.BuiltinRole there but we're getting auth.LocalUser. Don't know enough about Teleport internals to conclude why currently.

To Reproduce

Try to complete the operation manually.

Expected behavior

Audit event should be emitted without issues.

Logs

Aforementioned stack trace:

2020-04-09T15:46:55Z ERRO [EVENTS]    "Failed to emit audit event {operation.failed G0004E} [map[cluster:dev.test id:5c19d345-4244-4615-8d51-7780126790ca type:operation_expand user:agent@dev.test]]: 
ERROR REPORT:
Original Error: *trace.BadParameterError invalid role auth.LocalUser
Stack Trace:
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth/apiserver.go:2437 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth.(*APIServer).getServerID
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth/apiserver.go:1850 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth.(*APIServer).emitAuditEvent
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth/apiserver.go:270 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth.(*APIServer).withAuth.func1
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/httplib/httplib.go:50 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/httplib.MakeHandler.func1
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/julienschmidt/httprouter/router.go:299 github.com/gravitational/gravity/vendor/github.com/julienschmidt/httprouter.(*Router).ServeHTTP
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/httplib/httplib.go:156 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/httplib.RewritePaths.func1
    /go/src/net/http/server.go:1995 net/http.HandlerFunc.ServeHTTP
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth/middleware.go:298 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth.(*AuthMiddleware).ServeHTTP
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/gravitational/oxy/ratelimit/tokenlimiter.go:118 github.com/gravitational/gravity/vendor/github.com/gravitational/oxy/ratelimit.(*TokenLimiter).ServeHTTP
    /gopath/src/github.com/gravitational/gravity/vendor/github.com/gravitational/oxy/connlimit/connlimit.go:71 github.com/gravitational/gravity/vendor/github.com/gravitational/oxy/connlimit.(*ConnLimiter).ServeHTTP
    /go/src/net/http/server.go:2774 net/http.serverHandler.ServeHTTP
    /go/src/net/http/server.go:1878 net/http.(*conn).serve
    /go/src/runtime/asm_amd64.s:1337 runtime.goexit
Caught:
    /home/deemok/go/_/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/httplib/httplib.go:114 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/httplib.ConvertResponse
    /home/deemok/go/_/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth/clt.go:241 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth.(*Client).PostJSON
    /home/deemok/go/_/src/github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth/clt.go:1666 github.com/gravitational/gravity/vendor/github.com/gravitational/teleport/lib/auth.(*Client).EmitAuditEvent
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/opsservice/service.go:1438 github.com/gravitational/gravity/lib/ops/opsservice.(*Operator).EmitAuditEvent
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/events/events.go:67 github.com/gravitational/gravity/lib/ops/events.emit
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/events/events.go:42 github.com/gravitational/gravity/lib/ops/events.Emit
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/events/events.go:55 github.com/gravitational/gravity/lib/ops/events.EmitForOperation
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/opsservice/operationgroup.go:117 github.com/gravitational/gravity/lib/ops/opsservice.(*operationGroup).emitAuditEvent
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/opsservice/operationgroup.go:254 github.com/gravitational/gravity/lib/ops/opsservice.(*operationGroup).compareAndSwapOperationState
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/opsservice/site.go:580 github.com/gravitational/gravity/lib/ops/opsservice.(*site).compareAndSwapOperationState
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/opsservice/service.go:1065 github.com/gravitational/gravity/lib/ops/opsservice.(*Operator).SetOperationState
    /home/deemok/go/_/src/github.com/gravitational/gravity/lib/ops/utils.go:291 github.com/gravitational/gravity/lib/ops.FailOperation
    /home/deemok/go/_/src/github.com/gravitational/gravity/tool/gravity/cli/install.go:575 github.com/gravitational/gravity/tool/gravity/cli.completeJoinPlanFromExistingNode
    /home/deemok/go/_/src/github.com/gravitational/gravity/tool/gravity/cli/install.go:555 github.com/gravitational/gravity/tool/gravity/cli.completeJoinPlan
    /home/deemok/go/_/src/github.com/gravitational/gravity/tool/gravity/cli/operation.go:188 github.com/gravitational/gravity/tool/gravity/cli.completeOperationPlan
    /home/deemok/go/_/src/github.com/gravitational/gravity/tool/gravity/cli/run.go:418 github.com/gravitational/gravity/tool/gravity/cli.Execute
    /home/deemok/go/_/src/github.com/gravitational/gravity/e/tool/gravity/cli/run.go:157 github.com/gravitational/gravity/e/tool/gravity/cli.execute
    /home/deemok/go/_/src/github.com/gravitational/gravity/e/tool/gravity/cli/run.go:38 github.com/gravitational/gravity/e/tool/gravity/cli.Run
    /home/deemok/go/_/src/github.com/gravitational/gravity/e/tool/gravity/main.go:43 main.run
    /home/deemok/go/_/src/github.com/gravitational/gravity/e/tool/gravity/main.go:27 main.main
    /home/deemok/dev/go/go/src/runtime/proc.go:200 runtime.main
    /home/deemok/dev/go/go/src/runtime/asm_amd64.s:1337 runtime.goexit
User Message: invalid role auth.LocalUser
." events/events.go:44

Environment (please complete the following information):

Additional context

r0mant commented 4 years ago

Talking to Teleport team, the issue seems to be is that when emitting an audit event Teleport currently always expects server_id to be present among the event fields and tries to validate it because in Teleport all events are emitted by servers:

https://github.com/gravitational/teleport/blob/branch/3.2/lib/auth/apiserver.go#L1848-L1860

Gravity, on the other hand, emits various cluster-level events as well (such as, operation completed/failed) on user's behalf so this field may not be present and as such server_id validation is not necessary.

The proposal is to update the above code block this the following way:

    // Only check events that have `server_id` defined on them. This is because Gravity sometimes
    // submits events as the user instead of the builtin role.
    if eventToCheck(req) {        
        // Validate serverID field in event matches server ID from x509 identity. This
        // check makes sure nodes can only submit events for themselves.
        serverID, err := s.getServerID(r)
        if err != nil {
            return nil, trace.Wrap(err)
        }
        err = events.ValidateEvent(req.Fields, serverID)
        if err != nil {
            log.Warnf("Rejecting audit event %v from %v: %v. System may be under attack, a "+
                "node is attempting to submit events for an identity other than its own.",
                req.Type, serverID, err)
            return nil, trace.AccessDenied("failed to validate event")
        }
    }