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

pico_w: implement ssl with caveats #6999

Merged
merged 15 commits into from Oct 6, 2022
Merged

pico_w: implement ssl with caveats #6999

merged 15 commits into from Oct 6, 2022

Conversation

jepler
Copy link
Member

@jepler jepler commented Oct 5, 2022

Huge caveats:

  1. This PR actually resizes the area for CIRCUITPY. This means that upgrading to this firmware, or subsequently downgrading to an older firmware, will erase CIRCUITPY. This means that switching between pico and pico_w firmware will erase CIRCUITPY. BACK UP YOUR CIRCUITPY BEFORE TESTING THIS PR -- I may change or fine-tune the split while working on this PR too, so back up your CIRCUITPY before testing any new uf2 generated from this PR
  2. This PR does not yet validate server certificates. This means that https URLs do not actually provide higher security than http URLs, because a "server in the middle" need not prove they are the site you intended to reach in order to successfully impersonate it. Server certificates are now validated, except that expired certificates are not detected. (same as espressif port, see wifi/ssl: automatically set time from ntp; verify certificate validity #7004)

This brings the firmware size up to 1159444 bytes used, 671468 bytes free in flash firmware space out of 1830912 bytes (1788.0kB). with a CIRCUITPY drive size of 260kB (usable size about 240kB). It should be possible to add a certificate store in the remaining flash space; I just haven't started on this work yet.

Testing performed: Fetched the quotes json file a couple of times

----------------------------------------
Fetching text from https://www.adafruit.com/api/quotes.php
----------------------------------------
Text Response:  [{"text":"Mistakes are always forgivable, if one has the courage to admit them","author":"Bruce Lee"}]
----------------------------------------

Working URLs:

Correctly fails:

Not working URLs:

NotImplementedErrors (none of these should block merging this PR IMO):

  • load_verify_locations (lets you load a certificate for connect to a self-signed site or one otherwise not in nina-fw certificate store)
  • bind
  • listen
  • accept

Other shortcomings:

Test code
import os
import wifi
import socketpool
import adafruit_requests

TEXT_URL = "http://wifitest.adafruit.com/testwifi/index.html"
TEXT_URL2 = "https://www.adafruit.com/api/quotes.php"
wifi.radio.connect(os.getenv('WIFI_SSID'), os.getenv('WIFI_PASSWORD'))

pool = socketpool.SocketPool(wifi.radio)
requests = adafruit_requests.Session(pool, ssl.create_default_context())



print("Fetching text from %s" % TEXT_URL)
response = requests.get(TEXT_URL)
print("-" * 40)

print("Text Response: ", response.text)
print("-" * 40)
response.close()

print("Fetching text from %s" % TEXT_URL2)
response = requests.get(TEXT_URL2)
print("-" * 40)

print("Text Response: ", response.text)
print("-" * 40)
response.close()

Before this, CIRCUITPY would start at 1MB anyway. This appeared to work
only because I hadn't checked the actual size of the CIRCUITPY drive,
and because until now the flash hadn't actually crossed that 1MB
boundary into CIRCUITPY storage.

WARNING: on pico_w, upgrading/downgrading CircuitPython across this commit
boundary will erase the CIRCUITPY filesystem. After this commit,
switching between pico and pico_w firmware will erase the CIRCUITPY
filesystem
dhalbert
dhalbert previously approved these changes Oct 5, 2022
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested but looks fine by me.

@dhalbert dhalbert self-requested a review October 5, 2022 17:08
@dhalbert dhalbert dismissed their stale review October 5, 2022 17:09

wait for runs to work

Note: at this time, the ssl module on pico_w never verifies the server
certificate. This means it does not actually provide a higher security
level than regular socket / http protocols.
@bill88t
Copy link

bill88t commented Oct 5, 2022

Already built it. And it's already out of date!
Ljinux had to be severly trimmed to fit on this tiney tiny storage.
image
I will now try outside of my ljinux. (After rebuilding)

@bill88t
Copy link

bill88t commented Oct 5, 2022

With adafruit_requests as a .py, not an .mpy

image

.. this gets rid of one of the steps of converting it
This is intended (but not entirely verified) to match our esp32 builds.
It does fix accessing https://circuitpython.org, which failed before with
"MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE".

It still doesn't work on a personal website of mine with valid letsencrypt
certificate but I haven't verified whether it works on esp32s2 with CP.
That site only allows TLS 1.3, while this mbedtls only supports up to
1.2.
The version of mbedtls we adopted based on micropython's use has no
TLS 1.3 support, but the one in espressif esp-idf does.
@jepler
Copy link
Member Author

jepler commented Oct 5, 2022

Now,

  • certificates are verified against the nina-fw store, same as espressif, so https://self-signed.badssl.com/ is rejected
  • but out of date certificates are not caught, https://expired.badssl.com/
  • tls1.3-only sites do not work. this mbedtls version doesn't have 1.3 support. Espressif's mbedtls has "experimental" 1.3 support in the source but I do not think it is enabled

I believe this should broadly work across websites, and it now works on https://circuitpython.org/

Size wise, here's how pico_w is:

1191372 bytes used, 639540 bytes free in flash firmware space out of 1830912 bytes (1788.0kB).

And regular pico:

719264 bytes used, 325216 bytes free in flash firmware space out of 1044480 bytes (1020.0kB).

So we could give back another ~256kB to CIRCUITPY and still have more remaining flash than regular pico. However, we do need to consider that we might want to enable some of these in the future:

CIRCUITPY_HASHLIB = 0
CIRCUITPY_WEB_WORKFLOW = 0
CIRCUITPY_MDNS = 0

the latter two are challenging since as of right now the picow doesn't have the separate heap for long-lived data like espressif does with the esp-idf heap. They are not currently targeted to implement.

@jepler jepler changed the title pico_w: implement ssl with heavy caveats pico_w: implement ssl with caveats Oct 6, 2022
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, looks good, and let's get it in!

@dhalbert dhalbert merged commit e0517c7 into adafruit:main Oct 6, 2022
@bill88t
Copy link

bill88t commented Oct 6, 2022

So we could give back another ~256kB to CIRCUITPY and still have more remaining flash than regular pico. However, we do need to consider that we might want to enable some of these in the future:

1194544 bytes used, 112080 bytes free in flash firmware space out of 1306624 bytes (1276.0kB).

As a note, it can build just fine with another 512kB given back to CIRCUITPY and there is plenty space free.

@jepler
Copy link
Member Author

jepler commented Oct 6, 2022

I'd like to leave at least much free flash space as the non-"W" variant so that in a year we're not facing the dilemma of which modules that are enabled on Pico must be disabled on Pico W for it to fit.

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

Successfully merging this pull request may close these issues.

None yet

3 participants