Record and verify WebAuthn RPIDs by codingllama · Pull Request #25238 · gravitational/teleport

codingllama · 2023-04-26T21:33:12Z

Record and verify the credential RPID in WebauthnDevice, so we can detect and warn against accidental RPID changes.

RPID changes are not allowed by WebAuthn, so there's little Teleport can do to mitigate them.

Users that have only "invalid" WebAuthn devices will get "invalid credentials" errors on login. While a bit opaque, this is likely to lead to an user reset, which is the correct fix if only a few users are affected.

codingllama · 2023-04-26T21:34:40Z

Tested manually in a cluster with a Yubi4 and Yubi5 registered (empty CredentialRpId to begin with).

tobiaszheller

lgtm, just wonder, maybe rpid change in cluster should be stored in audit log as system event? It will also simplify debugging, wdyt @codingllama ?

codingllama · 2023-04-27T13:40:59Z

@tobiaszheller yes, that would be nice. I won't do that here, but might chase it later. It's simple enough for "online" changes, teleport.yaml changes are a bit harder but I think it could be done.

codingllama · 2023-04-27T14:16:55Z

I'm getting some flakiness on TestUpdateHeadlessAuthenticationState, but that seems to happen in master too, so I don't think I made it worse. FYI @Joerger.

Logs:

=== FAIL: lib/auth TestUpdateHeadlessAuthenticationState/NOK_same_user_approved_without_mfa (1.12s)
    auth_with_roles_test.go:4478:
        	Error Trace:	/Users/alan/workspace/teleport/lib/auth/auth_with_roles_test.go:4478
        	            				/Users/alan/workspace/teleport/lib/auth/auth_with_roles_test.go:4576
        	Error:      	Should be true
        	Test:       	TestUpdateHeadlessAuthenticationState/NOK_same_user_approved_without_mfa
        	Messages:   	expected access denied error but got: rpc error: code = DeadlineExceeded desc = context deadline exceeded

=== FAIL: lib/auth TestUpdateHeadlessAuthenticationState/OK_same_user_denied (1.12s)
{"caller":"auth/middleware.go:269","component":"auth","error":"listener is closed\n\taccept tcp 127.0.0.1:62438: use of closed network connection","level":"warning","message":"Mux serve failed.","timestamp":"2023-04-27T11:14:56-03:00"}
    auth_with_roles_test.go:4576:
        	Error Trace:	/Users/alan/workspace/teleport/lib/auth/auth_with_roles_test.go:4576
        	Error:      	Received unexpected error:
        	            	rpc error: code = DeadlineExceeded desc = context deadline exceeded
        	Test:       	TestUpdateHeadlessAuthenticationState/OK_same_user_denied
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc003036b40 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=headless_authentication, prefixes=/headless_authentication/, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc0010d8600 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=auth, prefixes=/locks, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc004089380 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=headless_authentication, prefixes=/headless_authentication/, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"auth/middleware.go:269","component":"auth","error":"listener is closed\n\taccept tcp 127.0.0.1:62436: use of closed network connection","level":"warning","message":"Mux serve failed.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc004089440 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=auth, prefixes=/locks, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"auth/middleware.go:269","component":"auth","error":"listener is closed\n\taccept tcp 127.0.0.1:62435: use of closed network connection","level":"warning","message":"Mux serve failed.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc0035cd2c0 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=headless_authentication, prefixes=/headless_authentication/, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc004089500 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=auth, prefixes=/locks, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc001bebe00 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=headless_authentication, prefixes=/headless_authentication/, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"auth/middleware.go:269","component":"auth","error":"listener is closed\n\taccept tcp 127.0.0.1:62441: use of closed network connection","level":"warning","message":"Mux serve failed.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc003e40b40 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=auth, prefixes=/locks, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}

=== FAIL: lib/auth TestUpdateHeadlessAuthenticationState/OK_same_user_approved_with_mfa (1.15s)
    auth_with_roles_test.go:4576:
        	Error Trace:	/Users/alan/workspace/teleport/lib/auth/auth_with_roles_test.go:4576
        	Error:      	Received unexpected error:
        	            	rpc error: code = DeadlineExceeded desc = context deadline exceeded
        	Test:       	TestUpdateHeadlessAuthenticationState/OK_same_user_approved_with_mfa
{"caller":"auth/middleware.go:269","component":"auth","error":"listener is closed\n\taccept tcp 127.0.0.1:62439: use of closed network connection","level":"warning","message":"Mux serve failed.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc003e409c0 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=headless_authentication, prefixes=/headless_authentication/, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:280","component":"buffer","level":"debug","message":"Removing watcher 0xc003e40a80 via external close.","timestamp":"2023-04-27T11:14:56-03:00"}
{"caller":"backend/buffer.go:283","component":"buffer","level":"debug","message":"Could not find watcher Watcher(name=auth, prefixes=/locks, capacity=1024, size=0).","timestamp":"2023-04-27T11:14:56-03:00"}

public-teleport-github-review-bot · 2023-04-27T19:16:26Z

@codingllama See the table below for backport results.

Branch	Result
branch/v11	Failed
branch/v12	Failed
branch/v13	Create PR

Record and verify the credential RPID in `WebauthnDevice`, so we can detect and warn against accidental RPID changes. RPID changes are not allowed by WebAuthn, so there's little Teleport can do to mitigate them. Users that have only "invalid" WebAuthn devices will get ["invalid credentials"][1] errors on login. While a bit opaque, this is likely to lead to an user reset, which is the correct fix if only a few users are affected. [1]: https://github.com/gravitational/teleport/blob/7a90a0ff9943e4f536736372dbdc90d324f143a2/lib/web/apiserver.go#L2180

There was a backward incompatible change introduce in #25238 that was preventing mfa devices from being unmarshalled due to an unknown field: ``` tctl users reset norbert ERROR: rpc error: code = Unknown desc = unknown field "credentialRpId" in types.WebauthnDevice ``` The custom `json.Unmarshaler` implemented by `types.MFADevice` did not initialize the protojson decoder to allow unknown fields. This prevented any reads of `types.MFADevice` from a backend that was upgraded and then downgraded.

codingllama added backport/branch/v11 labels Apr 26, 2023

codingllama requested a review from tobiaszheller April 26, 2023 21:33

github-actions Bot requested a review from timothyb89 April 26, 2023 21:33

github-actions Bot added the size/md label Apr 26, 2023

codingllama force-pushed the codingllama/record-rpid branch from 938ddd9 to 2039e57 Compare April 26, 2023 21:40

tobiaszheller approved these changes Apr 27, 2023

View reviewed changes

codingllama force-pushed the codingllama/record-rpid branch from 5501247 to 5d10554 Compare April 27, 2023 14:04

zmb3 approved these changes Apr 27, 2023

View reviewed changes

Comment thread lib/auth/webauthn/login.go Outdated

public-teleport-github-review-bot Bot removed the request for review from timothyb89 April 27, 2023 14:05

codingllama enabled auto-merge April 27, 2023 14:17

codingllama force-pushed the codingllama/record-rpid branch 2 times, most recently from dae0131 to 68c5d81 Compare April 27, 2023 16:30

codingllama added 7 commits April 27, 2023 15:40

Add the credential_rp_id field to WebauthnDevice

95f180b

Update generated protos

d254fbc

Record credential RPID on registration and login

3e26503

React to unexpected RPIDs when issuing challenges

100d316

Log number of allowed credentials

521dbae

Fix TestServer_CreateAuthenticateChallenge_authPreference

11112f3

Use slices.Delete

2ba935f

codingllama force-pushed the codingllama/record-rpid branch from 68c5d81 to ddd796d Compare April 27, 2023 18:40

Try to appease the flaky test detector

6aa2736

codingllama force-pushed the codingllama/record-rpid branch from ddd796d to 6aa2736 Compare April 27, 2023 18:42

codingllama added this pull request to the merge queue Apr 27, 2023

Merged via the queue into master with commit b4e5d78 Apr 27, 2023

codingllama deleted the codingllama/record-rpid branch April 27, 2023 19:14

codingllama mentioned this pull request Apr 27, 2023

[v13] Record and verify WebAuthn RPIDs #25282

Merged

codingllama mentioned this pull request Apr 27, 2023

[v12] Record and verify WebAuthn RPIDs #25289

Merged

codingllama mentioned this pull request Apr 27, 2023

[v11] Record and verify WebAuthn RPIDs #25291

Merged

rosstimothy mentioned this pull request May 1, 2023

Allow unknown fields when unmarshaling types.MFADevice #25437

Merged

codingllama mentioned this pull request May 4, 2023

Move away from strict JSON decoding in storage #25644

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Record and verify WebAuthn RPIDs#25238

Record and verify WebAuthn RPIDs#25238
codingllama merged 8 commits intomasterfrom
codingllama/record-rpid

codingllama commented Apr 26, 2023 •

edited

Loading

Uh oh!

codingllama commented Apr 26, 2023

Uh oh!

tobiaszheller left a comment

Uh oh!

codingllama commented Apr 27, 2023 •

edited

Loading

Uh oh!

Uh oh!

codingllama commented Apr 27, 2023

Uh oh!

public-teleport-github-review-bot Bot commented Apr 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

codingllama commented Apr 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codingllama commented Apr 26, 2023

Uh oh!

tobiaszheller left a comment

Choose a reason for hiding this comment

Uh oh!

codingllama commented Apr 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codingllama commented Apr 27, 2023

Uh oh!

public-teleport-github-review-bot Bot commented Apr 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codingllama commented Apr 26, 2023 •

edited

Loading

codingllama commented Apr 27, 2023 •

edited

Loading