silverstripe / silverstripe-mfa

MultiFactor Authentication for Silverstripe CMS
BSD 3-Clause "New" or "Revised" License
11 stars 24 forks source link

CMS 5-RC. Regression tests: Option to reset an MFA method doesn't work proper with YubiKey #494

Closed sabina-talipova closed 1 year ago

sabina-talipova commented 1 year ago

Description

After I did reset, I cannot use new MFA (YubiKey) method. Form shows loader, but then again require to insert key.
In Network section in browser I get "500 Internal Server Error" or "message: "Verification failed: Unable to load the data"". All works fine with MFA auth with authenticator app.

Steps to reproduce

PRs

maxime-rainville commented 1 year ago

When trying to use my YebiKey I get a 401 response with:

{"message":"Verification failed: Unable to load the data"}

I got this response on SC. I was able to register the YubiKey. But can't use it on login attempts afterwards.

emteknetnz commented 1 year ago

I wasn't able to replicate on SCPS or locally

I did replicate on SC - graylog wasn't too helpful though, all I got was this:

2023-04-24T17:13:51.357472+12:00 ip-11-22-33-44 SilverStripe_audit[22650]: INFO: "yubikey@example.com" (ID: 116) failed to verify using MFA method {"method":"SilverStripe\\WebAuthn\\Method","attempts":1,"attempt_limit":5} {"real_ip":null,"url":"/Security/login/default/mfa/verify/web-authn?SecurityID=12345678902c549e2fd6eaebe483a3e10085cc40","http_method":"POST","server":"somesite-uat.sites.silverstripe.com","referrer":"https://somesite-uat.sites.silverstripe.com/Security/login/default/mfa"}

emteknetnz commented 1 year ago

Tested theory that it was related to the removal of padding issue with the upgrade to web-auth/webauthn-lib 4.1.0+ - https://github.com/silverstripe/silverstripe-webauthn-authenticator/commit/0523fed85f2813852b903bc20e1663e3ebcb6972

I did try moving around where the padding removal was happening - i tried moving it to auth.js btoa(...) on my local - however while this did strip the trailing = for the request send to the server (normally it sends 1x =), not surprisingly I got back a {"errors":["Registration failed: Unable to load the data"]} - also as said before I can't replicate this issue locally so it seems very likely that it's not a javascript issue

This is a hard one, looks like we need to debug UAT, somehow

Places to look in particular are in the webauthn-lib dependency (would need to fork in order to debug) PublicKeyCredentialLoader::load() PublicKeyCredentialSource::createFromArray() ^ These two throw the "Unable to load the data" portion of the error

silverstripe/webauthn-authenticator VerifyHandler::verify() ^ This throws the 'Verification failed' portion of the error

The json message in the comment above comes from a silverstripe/auditor hook, so I wouldn't wouldn't put any attention in to it

GuySartorelli commented 1 year ago

PR merged