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

Raise legible error when issuer or accountname contains colon or URI-encoded colon #174

Open
TriVoxel opened this issue Dec 28, 2024 · 2 comments

Comments

@TriVoxel
Copy link

TriVoxel commented Dec 28, 2024

Issue copied from "Secrets", a KeepassXC-compatible GNOME app using this library for TOTP generation. See more about the original issue here.

Description

This library cannot generate an OTP from a URL containing %3A in the URL (the result of a user using a colon in the name field)

Example

pyauth fails to generate an OTP from the following string:

otpauth://totp/Text%3A%20More%20Text:Secret?secret=FFFFFFFAAAAAABBBBBBB&issuer=Text%3A%20More%20Text

This is due to the %3A in the URL.

Steps to repeat

  1. Create a new Entry
  2. Title it Text: More Text
  3. Enter OTP Secret, eg. FFFFFFFAAAAAABBBBBBB
  4. Save
    (you get otpauth://totp/Text%3A%20More%20Text:Secret?secret=FFFFFFFAAAAAABBBBBBB&issuer=Text%3A%20More%20Text as the URL generated by "Secrets")
  5. Close
  6. Open and find Entry (no OTP gen)

Debug info

OS: Fedora Atomic 40 x86_64 (GNOME 46)

Secrets version: 9.6 (Flathub)

Logs

04-09-24 21:19:37 | ERROR | Could not parse OTP
Traceback (most recent call last):
  File "/app/lib/python3.11/site-packages/gsecrets/safe_element.py", line 606, in __init__
    self._otp = parse_uri(otp_uri)  # type: ignore
                ^^^^^^^^^^^^^^^^^^
  File "/app/lib/python3.11/site-packages/pyotp/__init__.py", line 68, in parse_uri
    raise ValueError('If issuer is specified in both label and parameters, it should be equal.')
ValueError: If issuer is specified in both label and parameters, it should be equal.
04-09-24 21:19:37 | ERROR | Could not parse OTP
Traceback (most recent call last):
  File "/app/lib/python3.11/site-packages/gsecrets/safe_element.py", line 606, in __init__
    self._otp = parse_uri(otp_uri)  # type: ignore
                ^^^^^^^^^^^^^^^^^^
  File "/app/lib/python3.11/site-packages/pyotp/__init__.py", line 68, in parse_uri
    raise ValueError('If issuer is specified in both label and parameters, it should be equal.')
ValueError: If issuer is specified in both label and parameters, it should be equal.
04-09-24 21:19:37 | ERROR | Could not parse OTP
Traceback (most recent call last):
  File "/app/lib/python3.11/site-packages/gsecrets/safe_element.py", line 606, in __init__
    self._otp = parse_uri(otp_uri)  # type: ignore
                ^^^^^^^^^^^^^^^^^^
  File "/app/lib/python3.11/site-packages/pyotp/__init__.py", line 68, in parse_uri
    raise ValueError('If issuer is specified in both label and parameters, it should be equal.')
ValueError: If issuer is specified in both label and parameters, it should be equal.
04-09-24 21:19:37 | ERROR | Could not parse OTP
Traceback (most recent call last):
  File "/app/lib/python3.11/site-packages/gsecrets/safe_element.py", line 606, in __init__
    self._otp = parse_uri(otp_uri)  # type: ignore
                ^^^^^^^^^^^^^^^^^^
  File "/app/lib/python3.11/site-packages/pyotp/__init__.py", line 68, in parse_uri
    raise ValueError('If issuer is specified in both label and parameters, it should be equal.')
ValueError: If issuer is specified in both label and parameters, it should be equal.
@TriVoxel
Copy link
Author

TriVoxel commented Dec 28, 2024

On the original issue, maintainer Maximiliano said:

Thanks for the reproducer, the issue lies in the pyotp library

Python 3.12.5 (main, Nov 10 2011, 15:00:00) [GCC 14.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pyotp import TOTP, parse_uri
>>> uri = "otpauth://totp/Text%3A%20More%20Text:Secret?secret=FFFFFFFAAAAAABBBBBBB&issuer=Text%3A%20More%20Text"
>>> otp = parse_uri(uri)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/app/lib/python3.12/site-packages/pyotp/__init__.py", line 72, in parse_uri
    raise ValueError("If issuer is specified in both label and parameters, it should be equal.")
ValueError: If issuer is specified in both label and parameters, it should be equal.

Note that the error refers to this part of the OTP uri spec, but here they requirement is satisfied.

@kislyuk
Copy link
Member

kislyuk commented Dec 29, 2024

This behavior is correct. Per the OTP URI specification you linked, the issuer/accountname components encoded by the label may not contain a colon (whether literal or URI-encoded).

There is an issue here, which is that PyOTP does not explicitly enforce this constraint (it should raise a legible error indicating that the label contains more than one colon or URI-encoded colon). Apologies for the misleading error here, but the underlying rejection of the label is correct.

@kislyuk kislyuk changed the title [BUG] TOTP fails to read OTP property if the TOTP URL contains an HTML URL-encoded colon character (%3A) Raise legible error when issuer or accountname contains colon or URI-encoded colon Dec 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants