← index #668Issue #839
Off-topic · high · value 0.161
QUERY · ISSUE

urequests doesn't handle basic auth formatted URLs correctly

openby 0xVelesopened 2023-05-24updated 2024-11-18

Per RFC1738 you can supply a username and password for basic auth as part of the URL in the format: http(s)://username:password@example.com however urequests interprets any colon following the protocol to be delimiting a host and port, as seen here.

Obviously it's simple to provide basic auth as a header instead, but it's probably best to be RFC compliant when possible.

5 comments
massimosala · 2023-06-28

Probably it isn't implemented because

  • passing clear text credentials in the URL is a bad idea
  • there are modern authentication options available, this is becoming obsolete.

Just for curiosity: are there running servers using https://user:pass@... ?

massimosala · 2023-06-28

Hi

I have rewritten the library, with some improvements and basic auth.

Do you want to test it ?

After your feedback, I will open source it and propose the new version to the Micropython mantainers.

jonnor · 2024-08-25

I think that parsing username:password out of URLs can be a separate function, which extracts the relevant information and a cleaned URL. And those that the few that need it can copy-paste it into their project.

smithps · 2024-11-18

I just found this bug report as I am trying to implement DDNS on a PicoW - the URL specified by dyndns is as follows;

https://{user}:{updater client key}@members.dyndns.org/v3/update?hostname={hostname}&myip={IP Address}

Which although not using a password as such, is still passing a plaintext "key".

Out of curiosity and in order to be able to handle such formatted URLs (even if they aren't recommended - it's obvious that they are still in use)....would it be sensible to search backwards from the first '/' looking for a port number and/or if the value found after the ':' is not numeric to ignore it?

smithps · 2024-11-18

Probably it isn't implemented because

  • passing clear text credentials in the URL is a bad idea
  • there are modern authentication options available, this is becoming obsolete.

Just for curiosity: are there running servers using https://user:pass@... ?

Yes; DynDNS (Part of Oracle) use this for updating Dynamic DNS records see their help article here

CANDIDATE · ISSUE

SECURITY: Requests module leaks passwords & usernames for HTTP Basic Auth

closedby jonfosteropened 2024-04-01updated 2025-01-28

While looking at the MicroPython requests module (on the git HEAD), I noticed this:

If you make a request with HTTP basic auth (a username/password) and did not specify a headers dict, then I believe the username and password would be added to the default headers to be used for every subsequent HTTP request. Even if that request is to a completely different server, which you don't trust with your username and password. That's probably not a good idea.

I haven't verified this, it's just from reading the code, but someone should probably look into it.

This is because there is headers={} in the function prototype, specifying a default for the headers parameter. But (at least in cPython) that same dictionary will get reused for every call that doesn't explicitly specify a headers parameter. So if the function changes the headers dictionary - such as by adding an Authorization header - that change will be there for every future call of the function. This is a known dangerous part of the Python language, you're not the first people to write this kind of bug.

To fix this, you could keep the auth headers separate from the headers variable. Something like this (totally untested!) commit: https://github.com/jonfoster/micropython-lib/commit/92e9b2208814faa22ba3ff2a19cbc8e0c5210a47 - feel free to use that as a starting point.

1 comment
Gadgetoid · 2024-06-10

The MicroPython way aiui would be to mirror CPython's solution to this problem, which uses a None default value and then sets it to an empty dict at runtime:

https://github.com/psf/requests/blob/0e322af87745eff34caffe4df68456ebc20d9068/src/requests/models.py#L258-L276

And I see this is exactly what #823 does. That'll teach me not to look at PRs first 😆

Keyboard

j / / n
next pair
k / / p
previous pair
1 / / h
show query pane
2 / / l
show candidate pane
c
copy suggested comment
r
toggle reasoning
g i
go to index
?
show this help
esc
close overlays

press ? or esc to close

copied