tadeck / onetimepass

One-time password library for HMAC-based (HOTP) and time-based (TOTP) passwords
http://otp.readthedocs.org/
MIT License
680 stars 101 forks source link

Some secrets simply crash with 'Incorrect padding' #20

Open WhyNotHugo opened 7 years ago

WhyNotHugo commented 7 years ago

All secrets generated by fastmail see to make the library crash. I can't find any common pattern for them:

>>> import onetimepass
>>> onetimepass.get_totp('7uzthj2u3te6dopflwqbwa5n6u', as_string=True, token_length=6)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.6/site-packages/onetimepass/__init__.py", line 169, in get_totp
    token_length=token_length,
  File "/usr/lib/python3.6/site-packages/onetimepass/__init__.py", line 113, in get_hotp
    key = base64.b32decode(secret, casefold=casefold)
  File "/usr/lib/python3.6/base64.py", line 205, in b32decode
    raise binascii.Error('Incorrect padding')
binascii.Error: Incorrect padding

The one included in the above example is safe to share since I never added confirmed adding that one to my account. :)

WhyNotHugo commented 7 years ago

On, this is onetimepass v1.0.1, python 3.6.1.

yrps commented 7 years ago

Base32 takes 8 output bytes to encode each 5 input bytes. 1-4 input bytes still require 8 output bytes, appending = characters as padding.

In other words, otp asserts that len(secret) % 8 == 0. Appending ====== to your example makes it correct.

Solution: otp should provide an option to autopad secrets. (Or just to it by default.)

tadeck commented 7 years ago

Sure, this could be done to make it more user friendly.

Solution: otp should provide an option to autopad secrets. (Or just to it by default.)

Do you think this solution is enough / correct: https://stackoverflow.com/a/9807138/548696?

yrps commented 7 years ago

Looks good. (The condition is unnecessary though.) Tests should be easy to write too.