kratos icon indicating copy to clipboard operation
kratos copied to clipboard

No validation for totp_code when unlinking TOTP 2FA method

Open KajsaEklof opened this issue 3 years ago • 5 comments

Preflight checklist

Describe the bug

When unlinking the TOTP 2FA Method there is no validation on the 'totp_code' that is required to be passed in. We can pass in any value or an empty string and the response is still coming back as successful with status code 200.

We are using Vue and the SelfServiceFlows. Our UI has a button to disable 2FA and this submits a submitSelfServiceSettingsFlow with a TotpMethodBody.

  kratos
      .submitSelfServiceSettingsFlow(this.flowId, undefined, {
        csrf_token: this.csrfToken,
        method: 'totp',
        totp_unlink: true,
      })

We were getting an error with a message that the totp_code was missing so we added an input field in the UI to provide a verification code. So the submitSelfServiceSettingsFlow looks like this now:

  kratos
      .submitSelfServiceSettingsFlow(this.flowId, undefined, {
        csrf_token: this.csrfToken,
        method: 'totp',
        totp_unlink: true,
        totp_code: this.verificationCode,
      })

But we seem to be able to pass in any value for totp_code, the code doesn't seem to be validated?

Reproducing the bug

  1. Using the Settings Flow for client-side browser clients initialise the settings flow with kratos.initializeSelfServiceSettingsFlowForBrowsers()
  2. Set up 2FA with totp:
kratos
    .submitSelfServiceSettingsFlow(this.flowId, undefined, {
        csrf_token: this.csrfToken,
        method: 'totp',
        totp_code: this.verificationCode,
    });
  1. Try to unlink 2FA totp method but provide an empty string for the totp_code:
kratos
    .submitSelfServiceSettingsFlow(this.flowId, undefined, {
        csrf_token: this.csrfToken,
        method: 'totp',
        totp_unlink: true,
        totp_code: '',
    });
  1. Response is successful with status 200 when we expected this to fail with an invalid totp_code.

Relevant log output

No response

Relevant configuration

No response

Version

v0.9.0-alpha.3

On which operating system are you observing this issue?

Windows

In which environment are you deploying?

Docker Compose

Additional Context

No response

KajsaEklof avatar May 06 '22 11:05 KajsaEklof

Thank you for the report. If we would require the TOTP code to remove the device, it would never be possible to remove a TOTP device that you lost. Removing the TOTP code requires a privileged session though, which will be requested by the user if the session is no longer considered privileged enough.

If you agree with these findings I think we can close this issue.

aeneasr avatar May 06 '22 11:05 aeneasr

Thanks for looking at this! Yes I agree with you about that happy to close this, although I have another question now 😄

I've enable 2FA on my account and then wait for the privileged session to run out. When I try and disable 2FA and need to verify the action it only asks for one level of sign in, email + password for example, without my 2FA? Just wondering why I don't need to verify with 2FA at that stage, since it's not yet turned off?

KajsaEklof avatar May 09 '22 12:05 KajsaEklof

I'd like to understand "Removing the TOTP code requires a privileged session though". At the moment I can link/unlink a TOTP app at AAL1 - this isn't a privileged session (IMO). This feels wrong. It means that if an account is compromised then a hacker can simply unlink any established TOTP app and re-link their own. Surely this makes TOTP 2FA worthless? It is possible to hide the settings in a UI without AAL2 if the user has not set up TOTP but the public Kratos endpoint is still accessible.

I accept your point about removing a lost device but I would have thought that in the case of a lost device there should be an admin action which can force unlink TOTP credentials but for a user to do it they must be at AAL2 i.e. have supplied a TOTP code.

jdudmesh avatar Sep 01 '22 16:09 jdudmesh

You'll need to set your AAL requirements to "highest_available" for the settings flow

aeneasr avatar Sep 02 '22 07:09 aeneasr

Ah, that fixed it, thanks. I wonder if it would be possible for Ory to throw an error if the user has linked a TOTP app but the AAL requirement is not set to highest available? Some sort of safety net would be useful here.

jdudmesh avatar Sep 02 '22 08:09 jdudmesh