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.

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

TOTP - shorten the provisioning URL, use random instead of urandom #113

Open
istr opened this issue Feb 4, 2023 · 2 comments
Open

TOTP - shorten the provisioning URL, use random instead of urandom #113

istr opened this issue Feb 4, 2023 · 2 comments
Labels
Needs Feedback from Author Not enough information

Comments

@istr
Copy link

istr commented Feb 4, 2023

Currently the provisioning URL for the TOTP includes the hostname and the string Mail-in-a-Box Control Panel.

This leads to a large format QR code that is not supported by some hardware TOTP devices.
Replacing this with the fixed string pmiab will produce a small format QR that has better support / better interoperability.

The random source for the shared secret could be switched to /dev/random using the flag os.GRND_RANDOM. The average miab box should always have enough entropy to read 20 bytes from there.

The provided inline patch (two trivial changes) could be used to solve this issue.

diff --git a/management/mfa.py b/management/mfa.py
index 5650010..a858980 100644
--- a/management/mfa.py
+++ b/management/mfa.py
@@ -94,13 +94,13 @@ def validate_totp_secret(secret):
 
 def provision_totp(email, env):
        # Make a new secret.
-       secret = base64.b32encode(os.urandom(20)).decode('utf-8')
+       secret = base64.b32encode(os.getrandom(20, os.GRND_RANDOM)).decode('utf-8')
        validate_totp_secret(secret)  # sanity check
 
        # Make a URI that we encode within a QR code.
        uri = pyotp.TOTP(secret).provisioning_uri(
                name=email,
-               issuer_name=env["PRIMARY_HOSTNAME"] + " Mail-in-a-Box Control Panel")
+               issuer_name='pmiab')
 
        # Generate a QR code as a base64-encode PNG image.
        qr = qrcode.make(uri)
@ddavness
Copy link
Owner

ddavness commented Feb 12, 2023

This leads to a large format QR code that is not supported by some hardware TOTP devices.

Usually the issuer name is for certain apps Not making light of this statement, but for curiosity here - are there hardware TOTP generators capable of reading QR codes? The issuer name should be descriptive enough so that a user of certain apps like Authy/Okta Verify/etc. can display it to the user (pmiab won't cut it, but I think I might be able to come up with a shorter name).

The random source for the shared secret could be switched to /dev/random using the flag os.GRND_RANDOM

What is the benefit of doing that? Unless you don't want to risk the machine generating "insecure" bytes (which might not be too much of a big deal), you're just adding the possibility of /dev/random blocking in the process of generating the secret.

@ddavness ddavness added the Needs Feedback from Author Not enough information label Feb 12, 2023
@istr
Copy link
Author

istr commented Feb 13, 2023

What is the benefit of doing that?

Not much -- just an idea.

but for curiosity here - are there hardware TOTP generators capable of reading QR codes?

Yes. At least there is the ReinerSCT authenticator series https://shop.reiner-sct.com/authenticator (I have one with exactly the problem mentioned here). It supports scanning QR codes and time sync QR codes and is completely offline (not even a USB interface - only power and a camera). I don't know if there are any other products/providers around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Feedback from Author Not enough information
Projects
None yet
Development

No branches or pull requests

2 participants