default random_base32 is only 80 bits

[dylantack]

Hi, thanks for the great module!

random_base32() defaults to 16 characters. At 5 bits per character, that's only 80 bits, which is below the RFC minimum of 128.

Possibly the simplest solution is to increase this to 26 characters (130 bits). It's the smallest value that will work with the existing random implementation.

Alternatively to get exactly 128 bits, you could use:

base64.b32encode(random.getrandbits(128).to_bytes(16, byteorder='big')), then remove the padding.

However, this isn't really compatible with the existing function signature that takes a length as an input.

[tilkinsc]

def random_base32(
        length: int = 16,
        chars: Sequence[str] = list('ABCDEFGHIJKLMNOPQRSTUVWXYZ234567')) -> str:
    if length < 16:
        raise Exception("Secrets should be at least 128 bits")

    return ''.join(
        random.choice(chars)
        for _ in range(length)
    )

This is the function. 16 characters of 8 bits each equate to 16*8=128 bits. They are not to be treated as 5 bits per character. Yes, it only takes 5 bits to represent the 32 characters, but if you were to store that as binary, you wouldn't receive a base32 string like shown in list. OTP RFC doesn't translate them further into a 0-31 value before using it. The purpose for base32 is to be a human readable format (thusly missing some characters like zero, one, and being uppercase to solve I vs L issues, etc)

[dylantack]

Thanks for the reply!

pyotp does indeed decode the string to binary via base64.b32decode() prior to hashing. (see the snippet below, it outputs "10" (bytes), or 80 bits). Google Authenticator (and any app that's interoperable with it) uses base32 format to exchange the secret via otpauth:// URI's, but the secret used for the actual HMAC computation is the underlying binary, not the ASCII representation.

>>> len(pyotp.TOTP(pyotp.random_base32()).byte_secret())
10

def byte_secret(self) -> bytes:
    missing_padding = len(self.secret) % 8
    if missing_padding != 0:
        self.secret += '=' * (8 - missing_padding)
    return base64.b32decode(self.secret, casefold=True)

[kislyuk]

Thanks for the report @dylantack, you are correct that RFC 4226 section 4 mandates 128 bits and recommends 160 bits, while our convenience function generates a 80 bit secret. There is a small usability tradeoff here since some applications involve copying the base32 string by hand, however in most applications it will be copied via QR code. I don't think there would be any compatibility issues with longer secrets, but I'm not sure. I'll go ahead and update the convenience function to mandate 26 characters as you suggest.