HMAC error when `key` argument is MemoryView and not bytes

[justinmayer]

Steps to Reproduce

1. Log in and register a TOTP device.
2. Log out and log back in with username and password.
3. Enter the six-digit TOTP code when prompted and tap Submit.

Expected Result

The login process would complete successfully, re-directing to post-login URL.

Observed Result

An a TypeError exception is returned:

TypeError at /[…]/verify-second-factor/
key: expected bytes or bytearray, but got 'memoryview'

[tap to see full traceback]

~~~ [03/Oct/2019 10:51:07] "POST /sandbox/login/ HTTP/1.1" 302 0 [03/Oct/2019 10:51:07] "GET /sandbox/security/verify-second-factor/ HTTP/1.1" 200 19668 Internal Server Error: /sandbox/security/verify-second-factor/ Traceback (most recent call last): File "/sandbox/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner response = get_response(request) File "/sandbox/lib/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response response = self.process_exception_by_middleware(e, request) File "/sandbox/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) File "/usr/local/Cellar/python/3.7.4/Frameworks/Python.framework/Versions/3.7/lib/python3.7/contextlib.py", line 74, in inner return func(*args, **kwds) File "/sandbox/lib/python3.7/site-packages/django/views/generic/base.py", line 71, in view return self.dispatch(request, *args, **kwargs) File "/sandbox/lib/python3.7/site-packages/kagi/views/login.py", line 100, in dispatch return super(VerifySecondFactorView, self).dispatch(request, *args, **kwargs) File "/sandbox/lib/python3.7/site-packages/django/views/generic/base.py", line 97, in dispatch return handler(request, *args, **kwargs) File "/sandbox/lib/python3.7/site-packages/kagi/views/login.py", line 105, in post if form.is_valid() and form.validate_second_factor(): File "/sandbox/lib/python3.7/site-packages/kagi/forms.py", line 44, in validate_second_factor if device.validate_token(self.cleaned_data["token"]): File "/sandbox/lib/python3.7/site-packages/kagi/models.py", line 78, in validate_token if hmac.compare_digest(totp(self.key, t), token): File "/sandbox/lib/python3.7/site-packages/kagi/oath.py", line 53, in totp return hotp(key, T(t, step), digits) File "/sandbox/lib/python3.7/site-packages/kagi/oath.py", line 27, in hotp hs = hmac.new(key, msg, hashlib.sha1).digest() File "/sandbox/lib/python3.7/hmac.py", line 153, in new return HMAC(key, msg, digestmod) File "/sandbox/lib/python3.7/hmac.py", line 49, in __init__ raise TypeError("key: expected bytes or bytearray, but got %r" % type(key).__name__) TypeError: key: expected bytes or bytearray, but got 'memoryview' ~~~

Potential Solution / Workaround

Explicitly converting the key argument to bytes in this hmac.new() invocation resolves the error and allows the multi-factor authentication process to complete successfully:

hs = hmac.new(bytes(key), msg, hashlib.sha1).digest()

I can’t reproduce this in the Kagi test project, but it’s nonetheless a reproducible error in a (non-public) repository that otherwise exhibits no problems. One difference between the two projects is that the error occurs in a project in which KagiLoginView is not used, since there is an existing login view with custom logic. This custom login view was instead modified to behave like KagiLoginView and re-direct to the stock VerifySecondFactorView view when a key/TOTP device count above zero is detected. Since VerifySecondFactorView is not subclassed and is used in its original unmodified form, it’s not clear whether/how the aforementioned login view difference would result in this error.

Environment

• macOS 10.13.6
• Python 3.7.4
• Django 2.2.6

Related Links

• Python Bug (marked "Resolved"): hmac unnecessarily restricts input to "bytes"

[justinmayer]

@natim: This one is quite a head-scratcher. Does anything jump out at you as a possible cause? Do you foresee any issues with explicitly converting the key to bytes?

[Natim]

Yes we should convert it/ encode it to bytes

Le jeu. 3 oct. 2019 à 20:52, Justin Mayer notifications@github.com a écrit :

@Natim https://github.com/Natim: This one is quite a head-scratcher. Does anything jump out at you as a possible cause? Do you foresee any issues with explicitly converting the key to bytes?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/justinmayer/kagi/issues/38?email_source=notifications&email_token=AABYATMIOIGRMAHH2VZ3RK3QMY5PTA5CNFSM4I5G3UCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAJGTRA#issuecomment-538077636, or mute the thread https://github.com/notifications/unsubscribe-auth/AABYATNGYT7VYQEXYJFS7P3QMY5PTANCNFSM4I5G3UCA .

[justinmayer]

Perhaps only if needed? That is...

try:
    hs = hmac.new(key, msg, hashlib.sha1).digest()
except TypeError:
    hs = hmac.new(bytes(key), msg, hashlib.sha1).digest()

What do you think?

[Natim]

It should always be bytes. We should not guess.

Le jeu. 3 oct. 2019 à 21:11, Justin Mayer notifications@github.com a écrit :

Perhaps only if needed? That is...

try: hs = hmac.new(key, msg, hashlib.sha1).digest() except TypeError: hs = hmac.new(bytes(key), msg, hashlib.sha1).digest()

What do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/justinmayer/kagi/issues/38?email_source=notifications&email_token=AABYATJVISW4AWJDXBZMUX3QMY7XNA5CNFSM4I5G3UCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAJIJZY#issuecomment-538084583, or mute the thread https://github.com/notifications/unsubscribe-auth/AABYATPDOMCRX6TTN2TCHNTQMY7XNANCNFSM4I5G3UCA .

[justinmayer]

Many thanks for the feedback and quick response. I'll fix that now. 👍

[Natim]

Closed with #40