Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check Yubikey OTP length before validating #3746

Closed
plettich opened this issue Aug 31, 2023 · 3 comments · Fixed by #3929
Closed

Check Yubikey OTP length before validating #3746

plettich opened this issue Aug 31, 2023 · 3 comments · Fixed by #3929
Assignees
@jona-samuel
Labels
customer care Prio: Low
Milestone
3.10

Comments

@plettich
Copy link
Member

According to the docs the Yubikey OTP is always 44 chars long (https://developers.yubico.com/OTP/OTPs_Explained.html).
We can check the length of the given OTP before doing any further validation (this is also true for HOTP/TOTP/Email/SMS token).
That way we can identify possible broken input before getting cryptic errors like "CRC checksum failed".
@cornelinux
Copy link
Member

cornelinux commented Sep 4, 2023 •
edited

Additional to all OTP values, where we know, how long the otp value should be, we could...

  • with the yubi OTP also check for lowercase / force lowercase
  • catch the CRC checksum and return "invalid OTP value".

This can only happen with otppin=none

@cornelinux cornelinux added this to the 3.10 milestone Sep 11, 2023
@plettich
Copy link
Member Author

I have created a little test for different errors from check_otp():
https://gist.github.com/plettich/d156931d0b0d439a252b17613a8d33f9
Basically we can prevent some of the errors by:

  1. convert the anOtpVal to lower case at the beginning of the function.
  2. check for the proper length of the otp value (and log it before returning a result < 0). This gives at least a hint that there are characters missing or added and not the CRC Checksum failed
  3. add a log message when modhex_decode() fails (due to a wrong character in the otp value

We have no way of telling a user what is wrong because we do not interpret the return value:

privacyidea/privacyidea/lib/token.py

Lines 2030 to 2035 in a1b187f

reply_dict = {}
tokenobject = get_one_token(serial=serial)
res = tokenobject.check_otp(otpval) >= 0
if not res:
reply_dict["message"] = _("OTP verification failed.")
return res, reply_dict

jona-samuel pushed a commit that referenced this issue May 6, 2024
fixing #3746
dd08fd2
@jona-samuel jona-samuel mentioned this issue May 6, 2024
Merged
@jona-samuel
Copy link
Contributor

should we implement this for totp, hotp, SMS and email too?

@plettich plettich mentioned this issue May 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jona-samuel jona-samuel

Labels
customer care Prio: Low
Projects
None yet
Milestone
3.10
Development

Successfully merging a pull request may close this issue.

Check Yubikey OTP length before validating privacyidea/privacyidea
3 participants
@cornelinux @plettich @jona-samuel