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

SECURITY: Requests module HTTPS - no server certificate verification. #838

Open
jonfoster opened this issue Apr 1, 2024 · 4 comments
Open

Comments

@jonfoster
Copy link
Contributor

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

            context = tls.SSLContext(tls.PROTOCOL_TLS_CLIENT)
            context.verify_mode = tls.CERT_NONE
            s = context.wrap_socket(s, server_hostname=host)

Assuming that it has the same meaning in MicroPython as cPython (I haven't checked), that line in the middle totally disables TLS security. The attacker pretending to be the server can send any certificate they like, and the client will blindly accept it.

If people are using HTTPS as "the new HTTP", and are happy with the HTTP you-get-no-security model, that's fine. But anyone relying on HTTPS for security, and expecting the normal level of security you'd get from HTTPS, is going to be in trouble.

At a minimum this should be documented clearly on the MicroPython requests documentation ... which doesn't seem to exist anywhere?

Ideally, MicroPython should default to a proper secure HTTPS implementation, including certificate verification, and have a way to opt-out.

@sosi-deadeye
Copy link

sosi-deadeye commented Apr 3, 2024

The problem is, that a chain.der is required.
It must be loaded before the check could be done.

esp32@ganymed:~/micropython/ports/unix/build-standard$ ./micropython c.py
Traceback (most recent call last):
  File "c.py", line 18, in <module>
  File "ssl.py", line 1, in wrap_socket
OSError: (-30336, 'MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED')

With ctx.load_verify_locations(cafile="chain.der") I'm able to load the cafile and the check works.
The chain.der has a size of ~1 kB. I guess this is why this option is set to None.
Flash is limited on microcontrollers.

But it should be documented, that the default behavior is unsafe and the why.

EDIT: Tested on a RP Pico W and it works with letsencrypt.

@jonfoster
Copy link
Contributor Author

Yes, you need root certificates. Some embedded platforms have root certificates built-in as part of the standard manufacturer-supplied drivers. (Example for ESP32). Ideally, MicroPython should use those, if available. In cPython, that's done with the SSLContext.load_default_certs() method.

@sosi-deadeye
Copy link

I used this command to convert the existing ca-bundle on my system into certs.der:

openssl x509 -outform der -in /etc/ssl/cert.pem -out certs.der

It's on an Arch Linux and the resulting certs.der is 2007 Bytes big. I expected much more.
But if Micropython should include this, it must also get updates of the root certificates.

@Carglglz
Copy link
Contributor

Carglglz commented Apr 4, 2024

@jonfoster you may be interested in #633, see the README

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

3 participants