Add support for enable/disable multiple logins with the same OTP

[guidotripaldi]

• Why this change is necessary? OTP Tokens shouldn't be never used again after their first validation, to prevent security holes. If a TOTP token is allowed to be used multiple times during it's lifetime, a man-in-the-middle attacker could use the eavesdropped token at the same time with the legitimate user, gaining access to the service but without the user nor the administrator can notice the fact. An OTP already used should be rejected, and the user alerted with an informative message saying to wait for the next token before entering again. The service administrator could log all the multiple attempts, and point out those access with the same token but coming from different IP.
• How does it address the problem? It adds a new item named :allow_multiple_otp_logins to the configuration file. When its value is false (default), the database is checked every time the user try to login with a valid OTP token. If the otp token is already present in the database, the token is rejected and the login will fail. Instead if the valid token was never used before, it is stored in the database, and the login will succeed.
• Are there any side effects? The database is updated every time a token is valid and never used before.

[riverrun]

Sorry about the delay in getting back to you - been really busy lately. I just want to take a closer look at this change, and then I'll let you know what I think. Thanks.

[riverrun]

Do you have any links (academic papers, blog posts, etc.) to information about reusing OTP tokens?

[guidotripaldi]

Before pulling the request, I've searched for sources about that, but I didn't find any master references, neither RFC 4226 nor 6238, just some minor posts. One of them states the same concern: https://issues.jboss.org/browse/KEYCLOAK-2797 But I think it can be considered a best practice in the banking world, every service I usually use, refuses to accept again a just-validated (x)otp token.

[riverrun]

Thanks for the link. I also found this paper, which goes into some detail about several vulnerabilities, including replay attacks.

I'm definitely going to implement these changes, but I want to do a bit more research first, and then I'll let you know what changes we can make to the PR.

[riverrun]

There's more info about possible attacks here.

Here are my thoughts at the moment:

1. We need to distinguish between the type of attacks that will affect TOTP and those that will affect HOTP.
2. For TOTP, we need to record the last time interval count and then make sure that the interval count, which is returned by Comeonin.Otp.check_totp, of the person logging in is higher than the one stored in the database. There should be no need to store multiple tokens.
3. For HOTP, this check is not necessary (to be more exact, Comeonin already implicitly checks this), but we do need to make sure that the database is locked from the time the 'last count' is checked until it is updated (including the time that the token is being checked). An attacker could potentially gain entry within this small time window.
4. This should not be configurable. It's a basic security feature - which I should have implemented earlier :)

Before we do any more work on this, just let me know what your thoughts are on the points I've made, and then we can discuss how to proceed.

[guidotripaldi]

Thanks, I will read the documents you've linked, then I'll try to comment your thoughts. In the meantime, I've found this Digital Authentication Guideline by NIST: https://pages.nist.gov/800-63-3/sp800-63b.html and this post that comments the above document, humbling about the end of SMS-based 2FA. Maybe they are interesting for the future improvement of Openmaize. Also these links are a good references to quote in the Openmaize docs: https://stopthinkconnect.org/campaigns/two-steps-ahead-campaign https://nstic.blogs.govdelivery.com/tag/multi-factor-authentication/ Maybe can be useful to have a wiki page where to put all the references, to document the sources on which Openmaize is based upon, what do you think? I'll try to setup some pages to see how it sounds.

[riverrun]

A wiki is a good idea. There's already some information on the Comeonin wiki, but there will also be aspects that belong on an Openmaize wiki.

[guidotripaldi]

Reading better the RFC6238, I've noticed a sentence (at the end of section 5.2) that I've missed previously, it explicitly says that

The verifier MUST NOT accept the second attempt of the OTP after the successful validation has been issued for the first OTP, which ensures one-time only use of an OTP.

so as you said, it has definitely to be implemented and not to be optional. My implementation isn't very efficent because it stores forever in the database every token validated, but since our scope is not to have a permanent log but just to prevent the reuse of a token in its short window of validity, we surely need a different approach.

But I'm not sure to fully understand your suggestion:

For TOTP, we need to record the last time interval count and then make sure that the interval count, which is returned by Comeonin.Otp.check_totp, of the person logging in is higher than the one stored in the database. There should be no need to store multiple tokens.

Comeonin.Otp.check_totp will return the same count for all the checks done during the token lifetime, so I don't understand well how to recognize a second attempt just checking for the interval count. But maybe I've not understand well what do you meant.

[guidotripaldi]

Storing all the tokens (or interval counts, as you suggested) in the default repo database is inefficient because it is a waste of space (potentially billions of records stored that will never be checked again) and because it is relatively slow to access. So maybe is better to approach the problem in another way. Since we do not need to check the tokens again after they have expired, we could store them in a sort of cache in memory, using ETS or Mnesia. After the stored token expires, it will be removed from the cache. Memory comsumption will be low, as only the tokens validated during the interval of their validity will occupy memory space. What do you think?

[riverrun]

I think we only need to check for a token being used again during the token lifetime.

Initially, I was thinking of just storing the last token used and checking that, but that would not work in the following scenario:

A valid user logs in with token A, then immediately logs out and logs in again, this time with token B. At this time, token A is still valid, and an attacker can log in with token A.

In the above scenario, storing the last interval count would prevent the attacker from logging in. This is because the interval count will be less than token B's interval count. The attacker can also not use token B because the interval count is the same as the stored last interval count.

I still think that we just need to store the last interval count for TOTP. There is no need to store all the tokens.

[riverrun]

I'm closing this PR in favor of 3a7a781 and #64, which I hope to fix in the near future. Many thanks for your help in clearing up this issue.