pyauth / pyotp

Python One-Time Password Library
https://pyauth.github.io/pyotp/
Other
3k stars 324 forks source link

Using provisioning_uri occaisionally mixes up secret & issuer #59

Closed bshore closed 6 years ago

bshore commented 6 years ago

I've noticed that upon generating a new TOTP with provisioning_uri (for Google) the string occasionally switches the secret & issuer causing the QR generation to break (with qrious).

Working with qrious: otpauth://totp/Company:name%domain.com?secret=2345ABCD6789EFGH&issuer=Company

Not Working with qrious: otpauth://totp/Company:name%domain.com?issuer=Company&secret=2345ABCD6789EFGH

Should this be shared with the people working on qrious, is the format not important?

kislyuk commented 6 years ago

The order is not supposed to be important. Nothing in https://github.com/google/google-authenticator/wiki/Key-Uri-Format specifies the order, so it's presumed that this is a standard URI formatted query string, where order should not matter. Does qrious throw an error? What is the error?

bshore commented 6 years ago

Thanks for the quick reply! The error shows when scanning in the Google Authenticator app.

"Key not recognized."

The QR is visibly different when issuer and secret are swapped, so it has to do with how qrious draws then. I'll share this with qrious people and hopefully find a solution.

bshore commented 6 years ago

I think for now I'm going to write something within my project that will enforce the format. I've opened an issue on the qrious repo but they seem inactive.

tilkinsc commented 6 years ago

Please link the repository

bshore commented 6 years ago

Here's the qrious repo and here's how I'm enforcing the QR. If there's a better way let me know, I just need the auth string to be formatted a certain way for Google's app.

import pyotp
code = pyotp.random_base32()
auth_code = pyotp.totp.TOTP(code).provisioning_uri("an@email.com", issuer_name="Company")

# Verify the auth_code string is correct format
# otpauth://totp/Company:an%40email.com?issuer=Company&secret=2345ABCD6789EFGH

if "secret" not in (auth_code.split("?")[1])[:6]:
    split_code = auth_code.split("?")
    swap = split_code[1].split("&")
    auth_code= "".join([split_code[0]+"?", swap[1]+"&", swap[0]])

# otpauth://totp/Company:an%40email.com?secret=2345ABCD6789EFGH&issuer=Company
tilkinsc commented 6 years ago

Stop using google authentication. Looking into it deeper, you might want to let qrious know that no issue is present. You aren't writing a fully qualified RFC 4648 BASE32-formatted stream to the URI. The 1 in the QR code is invalid. So its undefined behavior. Also, remember your base32 key exists because you can use any character from the range of 0-255 0 non-inclusive (depending on the application chain compatibility with \0).

Request to close this issue.

bshore commented 6 years ago

That's an intentionally fake key for demonstration. I didn't want to publish a real key I am using and I figured "1234ABCD5678EFGH" would have been obviously non-random base32.

edit: I took the 1's out of my examples.

Jelle28 commented 6 years ago

@kislyuk you're saying the uri parameter order is not important, but that's not the issue here. The issue is that the PyOTP makes a different URI sometimes. @CoCaBoJangLeS code solved the issue.

kislyuk commented 6 years ago

@Jelle28 I'm not sure if I follow. Are you saying that non-determinism is the issue?

bshore commented 6 years ago

@kislyuk @Jelle28 The issue was that drawing a QR code with qrious is inconsistent because the provisioning_uri string varies. Maybe making the provisioning_uri format consistent might be good?

The real issue is with qrious not handling the provisioning_uri string properly, so I'm not sure PyOTP should tailor itself to solve that.

tilkinsc commented 6 years ago

I confirmed everything works fine when using a valid key with qrious.

Jelle28 commented 6 years ago

@tilkinsc Did you try it with the two kinds of provisioning_uri's that CoCaBoJangLes provided in his first comment? Or in other words: Did you try if qrious worked with a provisioning_uri that e.g. looks like: 'otpauth://totp/Secure%20App:alice%40google.com?secret=JBSWY3DPEHPK3PXP&issuer=Secure%20App' Or do you confirm that qrious works fine when the provisioning_uri's parameters are switched, which would look like this: 'otpauth://totp/Secure%20App:alice%40google.com?issuer=Secure%20App&secret=JBSWY3DPEHPK3PXP' Did you need to use the code snippet that CoCaBoJangLes send? Do u also use qrious 4.0.2?

tilkinsc commented 6 years ago

I can confirm that it works in master of qrious, swapped and unswapped. The order of parameters doesn't matter. Look at their source code too.

bshore commented 6 years ago

@tilkinsc So you are able to scan those QR codes just fine with Google Auth app?

tilkinsc commented 6 years ago

The QR codes generated are valid, yes. However, please stop using Google Auth, as it is outdated and deprecated by Authy.