Giter Club home page Giter Club logo

Comments (17)

RobThree avatar RobThree commented on June 2, 2024

It's too bad there is not a method to return the matching time slot value when a verification is successful.

You're welcome to do a PR 😉

Maybe I'm missing something in the library.

It's not currently in it but I'm open to consider exposing this information; however there is a chance of race conditions (however small) when you ask for the timeslice just before/after validating the code so to prevent that we would have to change the VerifyCode method to return both the validation result (valid code true/false) and the timeslice it used to validate. However, another problem is that we allow for more than one timeslice since client devices may be off by some margin. This discrepancy argument allows someone to specify how many timeslices a client device is allowed to be off. That would further complicate the above return result.

An alternative would be to expose the GetTimeSlice(...) method by making it public so you can use it for your own calculations (again: with a small risk of race conditions but that would probably be acceptable).

I think the best thing to do is simply store the current DateTime (not timeslice, actual date + time) and use that to ensure that no new attempts are being made in whatever timespan you desire. To me that makes much more sense (and doesn't require one to know/understand/handle "internals" like timeslices) and doesn't require changes in TwoFactorAuth.Net.

from twofactorauth.net.

lellis1936 avatar lellis1936 commented on June 2, 2024

I think the best thing to do is simply store the current DateTime...

The problem is this: let's say you allow a 5-minute discrepancy. Once logged in you are not able to log in again for five minutes. That can be a major nuisance. That's why storing the slice is better, though still subject to quirks.

I did think about providing an enhancement, but with all the affected overloads the only simple thing I could think of was a hack to store the slot value when a match occurs and provide a separate method to retrieve it. I found that too unpalatable to think about submitting.

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

let's say you allow a 5-minute discrepancy

Which is 10 (default) timeslices... pretty much if you ask me.

That can be a major nuisance.

I wonder... You just logged in. I'm sure a small percentage will encounter this anyway but you could use a (Re)Captcha or something to be able to bypass the second-login-attempt.

That's why storing the slice is better

You could still allow for a 5 minute discrepancy but store the date/time + a 30 second timespan for the case you're describing? The options are not mutually exclusive?

though still subject to quirks.

I'm pretty sure there will always be 'quirks' in this setup. With clear instructions in the UI and an option to bypass the 'protection' by having to enter a (Re)Captcha or something (with also the 2FA code ofcourse) I'm pretty sure it'll be fine for most users.

I did think about providing an enhancement, but with all the affected overloads the only simple thing I could think of was a hack to store the slot value when a match occurs and provide a separate method to retrieve it. I found that too unpalatable to think about submitting.

Huh? Just changing Private to Public on the GetTimeSlice method would allow you (in your scenario) to store the timeslice (besides the small race-conditions). If you're not willing to take (again, a very small) risk on race-conditions then the VerifyCode would have to be modified as I described here to return a status and "affected" or "used" timeslice(s) when verifying the code. That would break backwards compatibility and is, IMHO, not an option for solving such a trivial problem.

from twofactorauth.net.

lellis1936 avatar lellis1936 commented on June 2, 2024

Which is 10 (default) timeslices... pretty much if you ask me.

Sure is, it's just an example. I log out and in frequently when testing so having to wait more than 30 s would be a nuisance for me.

Huh? Just changing Private to Public on the GetTimeSlice method

But I don't want to store the current timeslice, I want to store the timeslice that actually corresponded with the entered TOTP code. The idea is if we save that, we can just disallow that timeslot, or any older timeslot from being used. Well anyway that's the idea but your methods don't easily support use of this scheme, which admittedly has its faults.

The idea of course is to prevent the need for storing all used codes while still allowing a quick logoff/login use case.

Thanks for your suggestions and about Captcha (which might be the best I can do). I have to think this through some more for the best compromise in our app.

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

What if you store a timestamp and the code? That would allow for discrepancies either way (+ or - from current time) with a variable discrepancy but would still only result one actual timeslice that would match and thus be blocked to prevent replay attacks.

from twofactorauth.net.

lellis1936 avatar lellis1936 commented on June 2, 2024

I'm intrigued but not sure what I'd do with the timestamp. You must see an algorithm I don't. Seems like all we can do here is prevent the same code from being used twice. I must be missing it.

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

Codes are not unique; the code you get could, theoretically, pop-up again tomorrow. The chances may be slim but there's a chance. So if you keep track of the code used and a timestamp you'll know if it's a replay-attack / code reuse or just a repeated code. All codes older than, say, 24hrs, can be deleted from your database (nightly task for example) or you can keep keep them indefinitely. Whatever. Either way, if you want to prevent replay-attacks / code reuse you'll need the actual code used and a timestamp to figure out if it's a replay-attack / code reuse or just a duplicate code.

from twofactorauth.net.

lellis1936 avatar lellis1936 commented on June 2, 2024

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

That works to block the most recent code but not any prior.

In our homegrown library [...] I do this by storing the timeslot value after success and disallowing that or any earlier timeslot.

So then you store the current time / timestamp on login? I don't see the problem? You can keep track of earlier used codes just as I described (in a database or whatever).

Nitpicking, but the RFC is clear that a code for a given timeslot should only validate once.

There is only one code for a given timeslot so all you need to store is the time(slot/stamp/whatever) of a succesful login (and, optionally, the code used). Since you cannot get to the time>slot< (currenty, but as discussed we could make it public) you can store the timestamp (+ code). This is exactly the same (a timeslot is nothing else but a timestamp mod X seconds (30 by default)). If you see the same timeslot/stamp (for the same user) in a timespan of 30(...by default) seconds again (with an already used code) you can act upon it and deny access ("validate once").

[...] but the implementation would be a bit ugly. If I were to do such pull request it would probably be rejected ;)

I'm afraid it will be 😉

But I'll try to create an example for you tonight or soon to show you what I mean.

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

Here's my idea in pseudo-code

var date = DateTime.Now;
var result = VerifyCode(secret, code, discrepancy, date);

// If result == true ("OK") you know exactly what date was used (no race-conditions). Now 
// you store the date (+ code) somewhere (database?)

For next logins you check when the last login was. If last login is within X timespan (which happens to be 30 seconds for example) and the code equals previous code then there's (a possibility of) a replay attack.

from twofactorauth.net.

lellis1936 avatar lellis1936 commented on June 2, 2024

I see. But follow my thought:

code stream
t-2 = 123
t-1 = 456
t-0 = 789

Assume variance (discrepancy) of 5 timeslots, your default.

All in one 30 second interval:
Log in 1 happens at t-0 using code 789. We store 789 and the time.
Log in 2 happens at t+1 using code 456. We store 456 and the time.
Log in 3 happens at t+2 using code 789 again. Still works.

This may be a low-risk scenario, it's true.

Google libpam solves this by supporting a list of recently used slot timestamps (not codes). Your scheme could be adapted but the list would need to include both the code and time element. The list would be awkward for the client app to maintain, for sure.

Just want to make sure my understanding is correct.

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

Your scheme could be adapted but the list would need to include both the code and time element. The list would be awkward for the client app to maintain, for sure.

So then making the GetTimeSlice() method public (and offer an overload that accepts a DateTime) would solve it (in combination with this code to pass date to the GetTimeSlice() method(s)), right?

edit: ... ah, it won't since you won't know which timeslice was actually used because of the discrepancy allowed. Now I see. Let me think about that... We could expose a 'read-only' TimeSliceUsed property that holds the actual, used and correct, timeslice that is updated by the VerifyCode() method. But that would be problematic if the instance is used by more than one thread for example. Another option would be to add an overload(s) that use out parameters to return the timeslice.

Let me think about this for a while (or let me know your thoughts/suggestions) on how to best handle this.

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

Currently we have:

public bool VerifyCode(string secret, string code) { ... }
public bool VerifyCode(string secret, string code, int discrepancy) { ... }
public bool VerifyCode(string secret, string code, int discrepancy, DateTime dateTime) { ... }
public bool VerifyCode(string secret, string code, int discrepancy, long timestamp) { ... }

I could add one or more or all of these overloads:

public bool VerifyCode(string secret, string code, out long timeSlice) { ... }
public bool VerifyCode(string secret, string code, int discrepancy, out long timeSlice) { ... }
public bool VerifyCode(string secret, string code, int discrepancy, DateTime dateTime, out long timeSlice) { ... }
public bool VerifyCode(string secret, string code, int discrepancy, long timestamp, out long timeSlice) { ... }

Which would return the timeslice used using an out argument. Currently that looks to be the most compatible method but I'll think about it for a while. Let me know what you think.

from twofactorauth.net.

lellis1936 avatar lellis1936 commented on June 2, 2024

I think it would be lovely if you could do that.

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

Added the overloads.

Will need to write some unittests and then publish a new package. Give me a day or two to iron out all the kinks (especially since the VerifyCode() was susceptible to timing attacks if not altered correctly; I think my changes are correct but I will need to give this some thorough thought).

(By the way, the build is failing because of a test failing because convert-unix-time.com appears to be down / gone. Will fix that too) Fixed that. Dropped the ConvertUnixTimeDotCom timeprovider (which was a mess to begin with) and replaced it with an NTPTimeProvider while I was at it.

from twofactorauth.net.

RobThree avatar RobThree commented on June 2, 2024

I present to you: 1.3.2 🎉

from twofactorauth.net.

lellis1936 avatar lellis1936 commented on June 2, 2024

Fantastic. I am building a client for the library now.

from twofactorauth.net.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.