-
Notifications
You must be signed in to change notification settings - Fork 13.3k
OTA support encapsulated to ArduinoOTA class #883
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
Conversation
2bd6107
to
a3beaa6
Compare
Thanks @hallard. It's exactly what I wanted to avoid too :) |
@s2kinfra, you already have one class for doing that HttpUpdate or something like that. |
@mangelajo yeah thanks, I noticed that after writing the suggestion. |
nice work; 👍 !SHIP IT! 💃 |
great work! |
Q: should the receiving of a udp packet on the 8266 port stop all other udp listeners? prior to validating of the message and/or client connectivity? or am I reading this wrong? void ArduinoOTA::handle() { if (!_udp_ota->parsePacket()) IPAddress remote = _udp_ota->remoteIP(); if (_serial_debug){ WiFiUDP::stopAll(); ... |
@grahamehorner I was just refactoring the example we have for OTA https://github.com/esp8266/Arduino/blob/esp8266/hardware/esp8266com/esp8266/libraries/ESP8266mDNS/examples/DNS_SD_Arduino_OTA/DNS_SD_Arduino_OTA.ino But I guess it can make sense to put it after the TCP connection back to updater is done? BTW, I'm not sure what's the real reason of it, I guess that if the UDP listeners stay open, and they keep receiving packets we end up in a deadlock because nobody takes them and we have no more buffers for the normal TCP firmware download. |
@mangelajo I think I'll do a fork of your work on this and some testing; may be add a mutex with in the handle function after the code has done the validation of the udp packet will prevent race conditions where a system send many udp packets on the network. Q: I'm wondering if the esp8266 is in sleep/deep sleep mode would a udp packet wake the processor? WOL (wake up on LAN) style? so OTA updates would work on devices that are battery powered without consuming power polling/waking on a time based interval? |
@grahamehorner sounds good, may be we could get this merged, and improve iteratively. As soon as the API stays stable I guess it should be good :) |
@mangelajo I think we should ask the forum about the UDP packet that triggers the OTA; how do they see it working; I guess it should have some type of security so the devices know it has come for an authorised source? don't what a packet triggering OTA from a hacker ;) IoT security :O |
@grahamehorner , I agree, but one step at a time, I'm just encapsulating the example in the mDNS directory, and using the protocol already provided by the espota.py. We can upgrade that later, to include a password for example. That's a good idea. The ones without password would start regardless of the password, and when password support is there it won't start without it. Btw, we're far far far from any IOT security without full certificate support in the TLS part, that'd be the only fully secure way to do things, for example when connecting to download the firmware, do it in an encrypted way, and, only from a server which has a certificate we trust. |
@mangelajo definitely one step at a time; I believe we are in agile; I think the ArduionOTA should be made a separate GitHub repo to allow the library to be developed as the first step; the community can then raise product feature request on that repo which would be developed/placed in the sprint according to priority and integrated accordingly into the release of the library. This would certainly allow for adding features like password, certificate validation and https/sftp/ftps downloading if the community ask/like/vote for those features. |
@grahamehorner, more important than ask/like/vote is implement ;-), and btw, to implement certificate validation we need the underlaying support for it, and AFAIK that is still not ready. You're asking to slow down something potentially very useful for the community in general, in favour of having "production" features which the Arduino IDE doesn't seem to be ready for. |
@grahamehorner , thinking of our conversation here, I'm not sure if you are unaware that support for this is already integrated into the IDE. If you select the OTA mode, and then select the remote device, you're able to flash it with the ArduinoOTA protocol. @grahamehorner: @pgollor and @igrr worked on it ;), I guess they should have a saying here. |
if i remember correctly it has safety reasons to stop all UDP connections. Other connections can interrupt the update and that could cause problems. |
@pgollor, yeah, I imagined that, I found errors where unattended connections fill the IP stack buffer, and other connections cannot receive anything more. I agree with @grahamehorner we should at least move it to the point we know TCP connection is established back to espota.py. |
exactly the opposite; the cadence of the ArduinoOTA shouldn't be coupled to that of the IDE, yes features like secured download https may be requested and voted for by the community, but these remain block until the feature or the library for TLS/SSL support is available.
I wasn't aware but as above and like other tools IMHO should not be tied to the cadence of the IDE; these are supporting tools/libraries and should have their own release cycle; which allows quicker implementation/bug fixes and easier regression should a developer wish/need. again IMHO the ESP8622/Arduino project should be focused on integration of the esp8266 tool chain with the IDE for compilation/uploading and serial monitor; not core libraries or supporting libraries these should be other projects that don't impact the IDE integration of the tool chain. but these are topics not directly relating to the ArduinoOTA, which BTW and I do like your work on this thusfar. |
@mangelajo Great work! The code looks good to me, aside for a few nitpicks which may be fixed later on. Regarding WiFiUDP::stopAll — that's actually important stuff. You need to add WiFiClient::stopAll there as well. |
@grahamehorner |
@igrr @mangelajo I understand the explanation of why WiFiUdp.stopAll and WiFiClient::stopAll are needed but surely these should be done at the latest opportunity after validatinf the UDP packet/OTA request and the validating for connection to the download server? |
@grahamehorner, I guess if we "stopAll" after connection we would be killing our own connection, that's not cool. Let's look for a compromise solution, I can add an optional password to the UDP, and to espota.py, and kill all connections right before connecting back to the updater. Then, if something fails, you always get the callback "onError", and you can reset the board, or reconnect whatever was disconnected. |
@igrr, I personally prefer to contribute to the work here, and make sure the work is widely available for everyone. You're doing an awesome work with this. :) |
OTA support encapsulated to ArduinoOTA class
Thanks for merging, I will follow up with the suggested changes: optional password support for a tiny bit of extra security, and stopping also TCP connections. I guess we could also integrate SPIFFS update support too as the changes are very minimal. |
Regarding password-protected OTA, the way I was originally planning to do On Tue, Oct 20, 2015 at 11:03 AM, Miguel Ángel Ajo <[email protected]
|
@igrr @mangelajo greatwork; @mangelajo it would also be great if a CRC or HASH of the update file was given so the receive device can get the file has downloaded correctly prior to update; not sure if that's a big ask or if its even possible? |
Actually HMAC will serve both authentication and integrity validation On Tue, Oct 20, 2015 at 11:09 AM, Grahame Horner [email protected]
|
@igrr indeed; just saw your post seconds after I posted 👍 HMAC |
@igrr @mangelajo oh and include the version of the update in the request along with the HMAC to project the request; so devices that have the current version don't need to do anything, they simply discard the request. I realise these are changes to features and may/may not be a big ask for the library, I do IMHO think these are worth investing in and should you need me to do any work or fork the code and implement, I'd be happy to do so. |
@mangelajo i added the SPIFFS update support here #802 |
@igrr, so HMAC-MD5 would validate the first UDP packet?, and we could also include an MD5 sum of the firmware itself? We must avoid the entry on the update loop if the UDP request is not authenticated (to avoid denial of service attacks). |
exactly @pgollor, I mean, leveraging that too from the ArduinoOTA class, if we receive a SPIFFS update, we handle it, if we receive a firmware update, we handle it as firmware... :) |
Okay. I am not sure if i understand your plan completely. But I'm very excited. :) |
@mangelajo you add firmware hash to the UDP packet, and then sign it with HMAC. |
@mangelajo @igrr the nonce could simply be the build version/date time of the firmware + the number of seconds since 01/01/1970 00:00:00 when the UDP was sent |
@grahamehorner ESP does not necessarily know local time (i.e. I don't want to add dependency on NTP). |
@igrr the ESP already 'knows' time, just call sntp_init If you like, find more in https://github.com/Juppit/esp8266-SNTPClock On 22.10.2015 at 16:32 wrote Ivan Grokhotkov:
|
Yes, you can get ESP to obtain time, but doing so uses some resources, On Thu, Oct 22, 2015, 18:36 Juppit [email protected] wrote:
|
For a simple fast "secure hash" and protection against message replay you might like to check out my page on http://www.forward.com.au/pfod/secureChallengeResponse/index.html |
@Juppit @igrr I would think that while NTP does consume some resources, so would two/more udp packet exchange; and NTP time packet can be more useful generally to the ESP that a exchange of udp packets for CRAM-MD5 or SCRAM. but if the OTA did a general callback; the developer could do this with out implementing something that was to restrictive. |
OTA support encapsulated to ArduinoOTA class
Hi @mangelajo, I'm starting to use encapsulated ArduinoOTA class. Unfortunately all my OTA setting was (and still) stored in EEP and I would like to be able to change them after OTA instantiation. But all parameters are passed on class init, would it be possible to also add these parameters to the What do you think ? I can do it if you want, not a problem |
@hallard I'm not fully sure if it is what you want, but you can set the OTA parameters by calling |
@hallard as a temporary solution, I think you should be able to do this: ArduinoOTA ota("esp8266", 8266);
void setup() {
// read config
// ota_host_name = ...
// ota_port = ...
ota = ArduinoOTA(ota_host_name, ota_port);
} |
@hallard, yes I guess that could be possible. Also you could do: ArduinoOTA *ota; void setup() {
} @igrr, we may need to look into the authentication thing, any idea of how to integrate that with the IDE? As an extra measure, we could include a hash of the firmware itself into the UDP packet, and at the end of reception, before triggering reset/update sequence, we could check the hash received in the UDP packet (and signed with pass) matches the hash of the firmware. |
Firmware hash should most certainly be included into handshake, that's the Regarding passwords and IDE integration, you might want to coordinate with It would also make sense to open a new issue to track these improvements. On Sat, Nov 7, 2015, 21:53 Miguel Ángel Ajo [email protected]
|
here is the IDE part: me-no-dev/Arduino-1@2b75aec #include <ESP8266WiFi.h>
#include <ESP8266mDNS.h>
const char* host = "esp8266-8m";
const char* ssid = "nbis-test";
const char* password = "1234567890";
const uint16_t ota_port = 8266;
const char* ota_pass = "1234";
WiFiUDP OTA;
void initOTA(){
MDNS.enableArduino(ota_port, true);
OTA.begin(ota_port);
}
void checkOTA(){
if (OTA.parsePacket()) {
IPAddress remote = OTA.remoteIP();
String pass = OTA.readStringUntil(' ');
int cmd = OTA.parseInt();
int port = OTA.parseInt();
int size = OTA.parseInt();
Serial.printf("Update Pass: %s\n", pass.c_str());
if(!pass.equals(String(ota_pass))){
Serial.println("ERROR: Password Failed");
return;
}
Serial.printf("Update Start: %d\n", size);
if(!Update.begin(size)){
Update.printError(Serial);
return;
}
WiFiUDP::stopAll();
WiFiClient::stopAll();
WiFiClient client;
if (client.connect(remote, port)) {
Serial.setDebugOutput(true);
uint32_t written;
while(!Update.isFinished()){
written = Update.write(client);
if(written > 0) client.print(written, DEC);
}
Serial.setDebugOutput(false);
if(Update.end()){
client.print("OK");
Serial.printf("Update Success\n");
ESP.restart();
} else {
Update.printError(client);
Update.printError(Serial);
}
} else {
Serial.printf("Connect Failed\n");
}
}
}
void setup() {
Serial.begin(115200);
Serial.setDebugOutput(true);
Serial.println();
Serial.println("booting...");
WiFi.mode(WIFI_STA);
WiFi.begin(ssid, password);
if(WiFi.waitForConnectResult() == WL_CONNECTED){
MDNS.begin(host);
initOTA();
Serial.println("ready");
} else {
Serial.println("failed");
delay(10000);
ESP.reset();
}
}
void loop() {
checkOTA();
} |
ESP Changes: #980 |
Update czech language file based on suggestion
This work encapsulates the basic OTA functionality to an ArduinoOTA class.
It also adds callbacks for OTA events so the application can react. SPIFFS
support to come in next patches.