BastiaanJansen / otp-java

A small and easy-to-use one-time password generator library for Java implementing RFC 4226 (HOTP) and RFC 6238 (TOTP).
MIT License
186 stars 30 forks source link

Validation not works properly if timestep was given in hours or days #47

Closed nishanthvasu closed 3 years ago

nishanthvasu commented 3 years ago

Hi, If i give timestep as hours or days by converting to milliseconds, it was not validating properly(For example if i generate otp ar today 8 Am and it was expiring at day after tomorrow 5:30 AM (GMT time)) and also it takes the time zone calculation as well. Request you to update the code in this issue. I have used HMACSHA512 algorithm with 6 digits. Tried with 30 days but its validating upto 20 days only.

BastiaanJansen commented 3 years ago

Hi @nishanthvasu,

Can you explain in more detail what the problem you encountered is, including on which version you are? And, if possible, post some example code so I can help find out what the problem is.

Bas

nishanthvasu commented 3 years ago

Hi @BastiaanJansen ,

Scenario : [builder.withPasswordLength(6).withAlgorithm(HMACAlgorithm.SHA512).withPeriod (Duration.ofDays(30)); ] I changed the duration to 30 days. For Ex: I generated OTP by today (10-Aug-2021 4:15 PM) which needs to be valid for 30 days i.e. until 9-Sep-2021 4:15 PM. But in this case it was valid for only for 21 days i.e. Until (31-Aug-2021 - 5:30 AM). As a conclusion, i found 2 issues here, 1) Days validation is not proper. 2) Validation is based on timezone which should not.

Here is my code,

public class OTPMain extends AppCompatActivity { AppCompatButton btGenerate, bt_validate; AppCompatTextView tvOtpForValidation, tv_isValid; AppCompatEditText tvOtp; byte[] byteKey; @Override protected void onCreate(@Nullable Bundle savedInstanceState) { super.onCreate(savedInstanceState); setContentView(R.layout.activity_main); btGenerate = findViewById(R.id.bt_generate); bt_validate = findViewById(R.id.bt_validate); tvOtp = findViewById(R.id.tv_otp); tvOtpForValidation = findViewById(R.id.tv_otp_valiation); tv_isValid = findViewById(R.id.tv_isValid); byteKey = SecretGenerator.generate(); TOTPGenerator.Builder builder = new TOTPGenerator.Builder(byteKey); builder.withPasswordLength(6).withAlgorithm(HMACAlgorithm.SHA512).withPeriod (Duration.ofDays(30)); TOTPGenerator totpGenerator = builder.build(); btGenerate.setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { tvOtp.setText(totpGenerator.generate()); } }); bt_validate.setOnClickListener(new View.OnClickListener() {

@Override public void onClick(View v) { boolean isValid = totpGenerator.verify(tvOtp.getText().toString()); if (isValid) { tv_isValid.setText("Valid"); } else { tv_isValid.setText("Invalid"); } } }); } }

BastiaanJansen commented 3 years ago

Hi @nishanthvasu,

The verify function works as intended.

For each generate method call, a counter is generated from the period with the following code:

System.currentTimeMillis() / period.toMillis();

So this counter is the same for every 30 seconds. This counter with the same secret will always generate the same token. So after the 30 second time window is over, the counter will be 1 bigger than the previous. Because it depends on the current unix time, it is not the case that when a token generated on 10:00:05 will be invalid after 10:00:35.

Concrete example: For example, say (assume: different secrets for both generatorA and generatorB and the .withPeriod is for 30 seconds for both):

10:00:00 - call generatorA.generate() and get codeA 10:00:05 - call generatorB.generate() and get codeB

The milliseconds since epoch at 10:00:00 2021-07-30 is 1627632000000, which means the counter is 1627632000000 / 30000 = 54254400. At 10:00:30, milliseconds since epoch is 1627632030000, which means the counter is 1627632030000 / 30000 = 54254401

As you can see, the counter will grow by 1 every 30 seconds. So, at 10:00:30, the token is invalid and the verify method will return false. Exactly after your defined period of 30 seconds.

However, the token generated by generatorB will also be invalid at 10:00:30: The milliseconds since epoch at 10:00:05 2021-07-30 is 1627632005000, which means the counter is 1627632005000 / 30000 = 54254400. At 10:00:30, milliseconds since epoch is 1627632030000, which means the counter is 1627632030000 / 30000 = 54254401 and thus a different token will be generated.

This token is only valid for 25 seconds. So, a period of 30 seconds, does not mean each token is valid for 30 seconds after generating the token, but rather that every 30 seconds, starting from unix time 0, the previous tokens aren't valid anymore.

nishanthvasu commented 3 years ago

Hi @BastiaanJansen ,

Thanks for your valuable information.

Is there a way to modify anything programatically for my requirement ? I was struggling with this issue for long time. It would be helpful for me if you have some clues on this

Thanks in advance!

BastiaanJansen commented 3 years ago

You could possibly use a HOTPGenerator with your own generated counter.

nishanthvasu commented 3 years ago

Hi @BastiaanJansen ,

But in HOTP case, OTP will expire once it's validated right.

BastiaanJansen commented 3 years ago

Both HOTP and TOTP tokens should be invalid once validated. However, this is not enforced in the library, so this should not be a problem.

nishanthvasu commented 3 years ago

Yes, but in TOTP , OTP would be valid until the provided time even though if we validate again and again within the provided time. In HOTP, it will be invalid once it's validated right, even if the time is remaining. In HOTP if we are giving counter as 5 as per example code, what does it mean ? Like whether it's valid for 5 mins or something like that ?

BastiaanJansen commented 3 years ago

I recommend you read the Request for Comments for both HOTP and TOTP to get a better understanding of OTP's.