ports/esp32 add api support for ota updates #3576
Conversation
add the following APIs: esp.partition_find_first esp.ota_set_boot_partition esp.ota_get_boot_partition move the vfs to its own partition if it exists use an ota capable partition layout
|
Any plans to merge this? Pull request was created more than a month ago. |
|
Having OTA capability would be nice! Do I understand it correctly that it would update the partition with micropython, but not the one with the filesystem? |
ports/esp32/modules/_boot.py
Outdated
| @@ -5,6 +5,7 @@ | |||
| try: | |||
| if bdev: | |||
| uos.mount(bdev, '/') | |||
| print("Mounted /") | |||
There was a problem hiding this comment.
indentation looks wrong. tab? one space missing?
ports/esp32/modules/flashbdev.py
Outdated
| if not offset: | ||
| self.START_SEC = esp.flash_user_start() // self.SEC_SIZE | ||
| else: | ||
| self.START_SEC = offset // self.SEC_SIZE |
There was a problem hiding this comment.
again: indentation looks wrong.
It moves the fs to its own partition, right now micropython ignores the partition table and uses space after micropython. This would corrupt a 2nd partition. You can update the fs too, but you do not need any code in mp to do it, it can be done in pure python code so this pull doesn't contain any code for it
Depends on your esp32 module. Most I work with have 4mb flash, enought for two mp partitions with 1.7mb each and a 512kb fs partition. My folder struktur is two folders at / one named "factory" and one named "ota_0". My boot.py then moves in the folder named after the boot partition and executes the boot.py and main.py there. Like that i can update the fs too, using a downloaded tar. |
|
Thanks for explaining. C&P that to some docs file? |
|
Updated to current master, fixed indentation. I will try to get our internal ota updater written in python added to the docu, just have to get an ok for it first. What more is needed to merge this? |
app_update was included multiple times
|
I did not yet have time to publish our python side for this, but are there any plans to merge this? |
|
@andynd: can you fix the makefile error? It's the ULP which has been added to the makefile. |
|
Merged upstream master. Any idea where to put the examples? I don't want to add yet another readme file like I had to for the ulp. |
esp.ota_get_running_partition() esp.ota_get_next_update_partition(start_from=None)
|
I've added a README.ota.md on how to use this APIs. The code (in the README) is extracted from our product source code and some (irrelevant) parts removed. I have not tested it after I removed this parts but the production code is working so it should at most have some minor errors. |
|
I've integrated it into my micropython tree, works just fine (after fixing the makefile merge)! Ps, you can't use a 512K fatvfs! The minimum for 4K sectors is 512K, but.... an additional 16K is being used for the WL (wear level). Here's what espressif said:
|
|
Well the micropython implementation doesn't seem to have this limitation. Micropython doesn't use any of the esp-idf filesystem code. |
|
Merged upstream master again. What is missing to finally merge this? The original PR is 7 months old. |
|
@andynd guess it is the usual "many contributions, few reviewers/maintainers / little time" issue. |
ports/esp32/Makefile
Outdated
| @@ -60,6 +60,8 @@ INC += -I$(BUILD) | |||
|
|
|||
| INC_ESPCOMP += -I$(ESPCOMP)/bootloader_support/include | |||
| INC_ESPCOMP += -I$(ESPCOMP)/bootloader_support/include_priv | |||
| INC_ESPCOMP += -I$(ESPCOMP)/bootloader_support/include_bootloader | |||
| >>>>>>> upstream/master | |||
There was a problem hiding this comment.
Yes it was, thx for pointing it out. Funny how it still was able to built without errors. Fixed.
|
poke |
|
more poke |
|
Yes, because this pull request is over a year old. Unless somebody tells me he is going to merge it I will not again spend the time to fix it. |
|
@dpgeorge ^^^ |
|
@dpgeorge: Wake up call... |
|
@dpgeorge: what does it take to get this merged? |
|
@dpgeorge ^^^ |
|
I am watching this since quite a while and I think this project needs more core developers who are trusted enough to review and merge PRs into this project. I don't have personal interest to become a core dev (I don't like coding in C and also do not use micropython that much, I like micropython, but basically just played with it yet), but I know there are people who would qualify (just look at the PRs and forks). Also, considering that revision control makes it easy to also revert stuff in case something is found to be problematic, maybe PRs should be accepted more easily. New stuff can be marked experimental for a while, without any guarantees (not even for staying). Open a ticket about this? |
|
@ThomasWaldmann: I suspect that @dpgeorge wants to be in control and therefor doesn’t want to delegate the PR merge rights. Maybe that’s because of the breakup with pfalcon. |
|
@Lisa999: The lack of progress really is super frustrating and it's a shame that pfalcon is not working at this repo any more. Maybe you can team up with pfalcon (or someone else) on his repo or find a fork where there are more activity (maybe LoBoris)? |
|
Sorry all for neglecting this. I'll try to make some progress here. I think it's a good feature to have, so thanks @andynd for contributing it. But I think the way it's done, the way it is exposed at the Python level, needs to be changed. The reason is because it looks like the approach here cannot support encrypted partitions (but please correct me if I'm wrong). That's because encrypted read/write must go through As such my proposal would be to make the API more object-based, rather than pure function calls. In particular put emphasis on the concept of a
For example the API could be: class Partition:
@staticmethod
def find_first(type, subtype, label) -> Partition: ...
def metadata(self) -> (type, subtype, address, size, label, encrypted): ...
def readinto(self, offset, buf): ....
def write(self, offset, buf): ....
def erase(self, offset, size): ....
def ota_get_boot_partition() -> Partition: ...
def ota_get_running_partition() -> Partition: ...
def ota_set_boot_partition(part): ...
def ota_get_next_update_partition(part) -> Partition: ...By having a |
| import esp | ||
| import sys | ||
|
|
||
| def bytecompare(a,b): |
There was a problem hiding this comment.
Why is this function necessary, isn't it enough to just do a == b? Or do I miss something?
There was a problem hiding this comment.
Its not, (a == b) can be = false while bytecompare(a,b) returns true, because its compare by referenz and compare by value
There was a problem hiding this comment.
Can you give an example where it's not the same? MicroPython compares by value, eg:
>>> bytearray(b'\x01\x02\x03')==b'\x01\x02\x03'
True
>>> bytearray(b'\x01\x02\x03')==bytearray(b'\x01\x02\x03')
True
>>> b'\x01\x02\x03'==bytearray(b'\x01\x02\x03')
True|
See #4910 for a proposed |
|
Is it posible to resolve those conflicts? |
|
Where are we on this one? I got a bit lost with all those cross references... Is there documentation on how to make OTA possible? |
|
|
||
| debug() | ||
| run(host='0.0.0.0', port=8000) | ||
| ``` |
There was a problem hiding this comment.
The Python code style looks bad, isn't it?
There was a problem hiding this comment.
This is very small example server, it's fine for that but obviosly nobody should use that in production.
|
Why is this PR still open? Is it hoping for someone to turn pieces of the README.ota.md into docs? |
|
My guess this README.ota.md is outdated. @dpgeorge implemented the Partition Class which serves as a replacement for another Class which is used in this pull request. |
| @@ -0,0 +1,405 @@ | |||
| # Over the air updates | |||
|
|
|||
| This is an example of how to do ota-updates using micropython and a very simple bottle server application. It assumes you have some way of notifying your esp of updates and transmit the update hashe securely (for example using ssl with certificate pinning). | |||
There was a problem hiding this comment.
typo "hashe"
how about cryptographically signed updates rather? the esp-idf / esp32 support ecdsa signatures and can verify that on update (flashing) / on boot (kind of "secure boot"). they even support encrypted flash images, but there is no requirement to use encryption if you just want to have signatures.
that way, the security of the server having the update files and that of the transmission channel does not matter.
if the signature is ok, you know the file is authentic, complete, correct (as released / signed by the author).
at least as long as you manage to keep your private signing key private.
There was a problem hiding this comment.
Agreed. As long as there are no secrets or otherwise sensitive info embedded in the MP firmware being updated it should be fine to verify a hash over the firmware assuming the hash got transmitted securely. A signature is more flexible but much more of a hassle in many use-cases.
There was a problem hiding this comment.
this has nothing to do with whether it has secrets inside, but whether you can authenticate the image. you can't authenticate with just a hash from same source.
There was a problem hiding this comment.
It's save as long as you have a secure way to transfer this hash. We did not want signed updates because the signature verification was too complex and it was not needed for our usecase. We use certificate pinning and would have gained nothing except protection against a attacker controlled server, but that was not our attack scenario.
|
|
||
| def bytecompare(a,b): | ||
| if (len(a) != len(b)): | ||
| return False |
| def trackerprobe(): | ||
| if do_update: | ||
| res['update'] = {} | ||
| res['update']['partition'] = {'url': '{}/chunk/part'.format(otahost), 'size': int(os.stat(partfile).st_size), 'hash': hashlib.sha256(open(partfile, 'rb').read()).hexdigest()} |
There was a problem hiding this comment.
the problem with this is that if somebody hacks your server and puts some tampered update binaries on it (instead the original good ones), the code will compute the hash for the tampered ones and the updater code on the esp32 will happily flash the bad update.
There was a problem hiding this comment.
That was not the attack vector we cared about. We wanted verified updates without confidentiality, thats why no encryption.
There was a problem hiding this comment.
I'm very interested in OTA (or rather, will be when I'm through with my current TODO list :-). But I'm not understanding what this PR is proposing. It's great to have a long readme, but I would prefer to first have a concise statement about how OTA is supposed to work and which use-cases it covers.
For example, it seems this PR intends the entire MP filesystem to be swapped with the OTA, as opposed to separating the filesystem from the MP firmware. Also, it seems this PR intends there to be 3 slots for firmware: factory, ota1, and ota2. Yet only 2 are really required, the factory one is optional (I wouldn't want it, for example).
I have used the Arduino Update class on the esp32 successfully implementing my own update and thought it abstracted the minutiae nicely without adding too many constraints. It would be worth taking a look at what it does: https://github.com/espressif/arduino-esp32/blob/master/libraries/Update/src/Updater.cpp
| STATIC mp_obj_t mp_esp_ota_get_boot_partition (void) { | ||
| const esp_partition_t* part = esp_ota_get_boot_partition(); | ||
| void* partptr = ∂ | ||
| return mp_obj_new_bytes(partptr, sizeof(partptr)); |
There was a problem hiding this comment.
What is this returning in Python-speak? An opaque bunch of bytes?
| @@ -5,6 +5,7 @@ | |||
| try: | |||
| if bdev: | |||
| uos.mount(bdev, '/') | |||
| print("Mounted /") | |||
| nvs, data, nvs, 0x9000, 0x6000, | ||
| phy_init, data, phy, 0xf000, 0x1000, | ||
| factory, app, factory, 0x10000, 0x180000, | ||
| nvs, data, nvs, 0x9000, 0x4000 |
There was a problem hiding this comment.
Gratuitous whitespace change?
Why the size reduction?
|
|
||
| ## Folders | ||
|
|
||
| We are using the following folder structure: |
| └─ ota_1 | ||
| ``` | ||
|
|
||
| The folders "factory", "ota_o" and "ota_1" musst have corresponding partitions. "VERSION" contains our currently running software version identifier, for example a git commit hash. |
There was a problem hiding this comment.
Why 3 partitions, i.e. copies of the firmware? Or am I misunderstanding?
There was a problem hiding this comment.
in the product where this code is from, we used "factory" with the factory image that never got updated and was the fallback code for some specific scenarios. You can use only two partitions as well.
|
In general: please understand that this code was written for a specific product with specific tradeoffs and making it open source was just an afterthought. Now that there is a Please understand as well that we wanted secure (as in someone who controlles the network can not tamper with them) and not confidential (an attacker can read firmware images, they are not secret and we do not care if someone gets them). An attacker who attacks our server and steals the SSL certificate or replaces the binary was not our concern. For this scenario it is fine to transfere a hash of the update over a ssl protected connection with certificate pinning and not to do signature verification and to transmit the update chunks unencrypted. |
add the following APIs:
esp.partition_find_first
esp.ota_set_boot_partition
esp.ota_get_boot_partition
esp.ota_get_running_partition
esp.ota_get_next_update_partition
move the vfs to its own partition if it exists
use an ota capable partition layout