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

Offload IoT Cloud connection to Nina module #208

Merged
merged 26 commits into from
Jan 28, 2021
Merged

Conversation

facchinm
Copy link
Contributor

Requires nina-fw 1.4.2

The SHA256 on OTA should be retested on the field (could be too slow for real world usage). In that case we should vendor a software only, embedded friendly implementation.

TODO: add control code if fw is < 1.4.2 that HALTS the execution. Strings comparison (<=>) should be checked on megaAVR since it reports false results.

@codecov-io
Copy link

codecov-io commented Nov 16, 2020

Codecov Report

Merging #208 (8383de1) into master (d04bf5b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #208   +/-   ##
=======================================
  Coverage   95.27%   95.27%           
=======================================
  Files          25       25           
  Lines         889      889           
=======================================
  Hits          847      847           
  Misses         42       42           
Impacted Files Coverage Δ
src/property/Property.h 90.47% <ø> (ø)
src/property/Property.cpp 87.20% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 457d932...8383de1. Read the comment docs.

Copy link
Collaborator

@aentinger aentinger left a comment

Choose a reason for hiding this comment

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

Concerning 46aa5f6 : Why? Can't imagine writing all the data via I2C is faster than doing it all in software. An additional drawback regarding security is that the whole application is available on the I2C lines for sniffing on every startup. It doesn't make a difference right now, with no read-out protection fuses blown, but could be in the future with Arduino Cloud being used for industrial grade solutions.

@facchinm
Copy link
Contributor Author

Concerning 46aa5f6 : Why? Can't imagine writing all the data via I2C is faster than doing it all in software. An additional drawback regarding security is that the whole application is available on the I2C lines for sniffing on every startup. It doesn't make a difference right now, with no read-out protection fuses blown, but could be in the future with Arduino Cloud being used for industrial grade solutions.

The goal is to get rid of bearssl entirely in case we offload SSL operation to the Nina; this path is "free" since we already include ECCx08 library, any other strategy will add a dependency.

@aentinger
Copy link
Collaborator

aentinger commented Nov 17, 2020

I understand your motivation but we can get rid of 98% of BearSSL and still keep their software-hash implementation. I consider the drawbacks of exposing the binary (runtime, secrecy) to severe to justify doing it.

@facchinm
Copy link
Contributor Author

@aentinger fixed by 13fbaf5

@aentinger
Copy link
Collaborator

Hi @per1234 👋 Can you please extend this PR by adding CI build for Arduino Uno WiFi. Rev. 2? One additional needed dependency for Uno WiFi Rev. 2 build is ArduinoSTL (There might be others which I've not identified yet).

@per1234
Copy link
Contributor

per1234 commented Nov 17, 2020

Can you please extend this PR by adding CI build for Arduino Uno WiFi. Rev. 2?

OK, here you go;
#209

src/cbor/CBORDecoder.h Outdated Show resolved Hide resolved
src/cbor/CBOREncoder.h Outdated Show resolved Hide resolved
facchinm and others added 11 commits January 26, 2021 15:48
Temporarily remove OTA support since it depends on bearssl's SHA256 APIs
Need to add a patch to Arduino_ConnectionHandler:

diff --git a/src/Arduino_ConnectionHandler.h b/src/Arduino_ConnectionHandler.h
index 195ab13..8d80f2e 100644
--- a/src/Arduino_ConnectionHandler.h
+++ b/src/Arduino_ConnectionHandler.h
@@ -29,7 +29,8 @@
   #define WIFI_FIRMWARE_VERSION_REQUIRED WIFI_FIRMWARE_REQUIRED
 #endif

-#if defined(ARDUINO_SAMD_MKRWIFI1010) || defined(ARDUINO_SAMD_NANO_33_IOT)
+#if defined(ARDUINO_SAMD_MKRWIFI1010) || defined(ARDUINO_SAMD_NANO_33_IOT) || \
+  defined(ARDUINO_AVR_UNO_WIFI_REV2)
   #include <WiFiNINA.h>
   #include <WiFiUdp.h>
Loses 2KB of flash compared with 46aa5f6 but it's surely faster
…ROR/WARNING/... to avoid name clashes with ídentically named constants within Arduino_DebugUtils
…compiled in (which is the case when working with Uno WiFi Rev. 2
@arduino-libraries arduino-libraries deleted a comment from github-actions bot Jan 27, 2021
@arduino-libraries arduino-libraries deleted a comment from github-actions bot Jan 27, 2021
@arduino-libraries arduino-libraries deleted a comment from github-actions bot Jan 27, 2021
It's required by the offloaded ecc anyway, so it will be ok
@github-actions
Copy link

Memory usage change @ b57d900

Board flash % RAM for global variables %
arduino:mbed:envie_m4 ❔ -128 - +64 -0.01 - +0.01 0 - 0 0.0 - 0.0
arduino:mbed:envie_m7 ❔ -128 - +64 -0.02 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkr1000 ❔ -112 - +48 -0.04 - +0.02 0 - 0 0.0 - 0.0
arduino:samd:mkrgsm1400 ❔ -112 - +40 -0.04 - +0.02 0 - 0 0.0 - 0.0
arduino:samd:mkrnb1500 ❔ -112 - +48 -0.04 - +0.02 0 - 0 0.0 - 0.0
arduino:samd:mkrwan1300 🔺 +40 - +40 +0.02 - +0.02 0 - 0 0.0 - 0.0
arduino:samd:mkrwifi1010 💚 -51208 - 0 -19.53 - 0.0 💚 -15304 - 0 -46.7 - 0.0
arduino:samd:nano_33_iot 💚 -51208 - 0 -19.53 - 0.0 💚 -15296 - 0 -46.68 - 0.0
esp32:esp32:esp32 🔺 +60 - +60 0.0 - 0.0 0 - 0 0.0 - 0.0
esp8266:esp8266:huzzah 💚 -84 - -80 -0.01 - -0.01 🔺 +348 - +352 +0.42 - +0.43
Click for full report table
Board examples/ArduinoIoTCloud-Advanced
flash
% examples/ArduinoIoTCloud-Advanced
RAM for global variables
% examples/ArduinoIoTCloud-Basic
flash
% examples/ArduinoIoTCloud-Basic
RAM for global variables
% examples/utility/ArduinoIoTCloud_Travis_CI
flash
% examples/utility/ArduinoIoTCloud_Travis_CI
RAM for global variables
% examples/utility/Provisioning
flash
% examples/utility/Provisioning
RAM for global variables
% examples/utility/SelfProvisioning
flash
% examples/utility/SelfProvisioning
RAM for global variables
%
arduino:mbed:envie_m4 64 0.01 0 0.0 0 0.0 0 0.0 64 0.01 0 0.0 -128 -0.01 0 0.0
arduino:mbed:envie_m7 64 0.01 0 0.0 64 0.01 0 0.0 64 0.01 0 0.0 -128 -0.02 0 0.0
arduino:samd:mkr1000 40 0.02 0 0.0 48 0.02 0 0.0 48 0.02 0 0.0 -112 -0.04 0 0.0
arduino:samd:mkrgsm1400 40 0.02 0 0.0 40 0.02 0 0.0 40 0.02 0 0.0 -112 -0.04 0 0.0
arduino:samd:mkrnb1500 48 0.02 0 0.0 40 0.02 0 0.0 48 0.02 0 0.0 -112 -0.04 0 0.0
arduino:samd:mkrwan1300 40 0.02 0 0.0 40 0.02 0 0.0 40 0.02 0 0.0
arduino:samd:mkrwifi1010 -51208 -19.53 -15304 -46.7 -51208 -19.53 -15296 -46.68 -51208 -19.53 -15296 -46.68 -48116 -18.35 -15296 -46.68 0 0.0 0 0.0
arduino:samd:nano_33_iot -50960 -19.44 -15296 -46.68 -51208 -19.53 -15296 -46.68 -50960 -19.44 -15288 -46.66 -47876 -18.26 -15288 -46.66 0 0.0 0 0.0
esp32:esp32:esp32 60 0.0 0 0.0 60 0.0 0 0.0 60 0.0 0 0.0
esp8266:esp8266:huzzah -80 -0.01 352 0.43 -84 -0.01 348 0.42 -80 -0.01 352 0.43
Click for full report CSV
Board,examples/ArduinoIoTCloud-Advanced<br>flash,%,examples/ArduinoIoTCloud-Advanced<br>RAM for global variables,%,examples/ArduinoIoTCloud-Basic<br>flash,%,examples/ArduinoIoTCloud-Basic<br>RAM for global variables,%,examples/utility/ArduinoIoTCloud_Travis_CI<br>flash,%,examples/utility/ArduinoIoTCloud_Travis_CI<br>RAM for global variables,%,examples/utility/Provisioning<br>flash,%,examples/utility/Provisioning<br>RAM for global variables,%,examples/utility/SelfProvisioning<br>flash,%,examples/utility/SelfProvisioning<br>RAM for global variables,%
arduino:mbed:envie_m4,64,0.01,0,0.0,0,0.0,0,0.0,64,0.01,0,0.0,-128,-0.01,0,0.0
arduino:mbed:envie_m7,64,0.01,0,0.0,64,0.01,0,0.0,64,0.01,0,0.0,-128,-0.02,0,0.0
arduino:samd:mkr1000,40,0.02,0,0.0,48,0.02,0,0.0,48,0.02,0,0.0,-112,-0.04,0,0.0
arduino:samd:mkrgsm1400,40,0.02,0,0.0,40,0.02,0,0.0,40,0.02,0,0.0,-112,-0.04,0,0.0
arduino:samd:mkrnb1500,48,0.02,0,0.0,40,0.02,0,0.0,48,0.02,0,0.0,-112,-0.04,0,0.0
arduino:samd:mkrwan1300,40,0.02,0,0.0,40,0.02,0,0.0,40,0.02,0,0.0,,,,
arduino:samd:mkrwifi1010,-51208,-19.53,-15304,-46.7,-51208,-19.53,-15296,-46.68,-51208,-19.53,-15296,-46.68,-48116,-18.35,-15296,-46.68,0,0.0,0,0.0
arduino:samd:nano_33_iot,-50960,-19.44,-15296,-46.68,-51208,-19.53,-15296,-46.68,-50960,-19.44,-15288,-46.66,-47876,-18.26,-15288,-46.66,0,0.0,0,0.0
esp32:esp32:esp32,60,0.0,0,0.0,60,0.0,0,0.0,60,0.0,0,0.0,,,,,,,,
esp8266:esp8266:huzzah,-80,-0.01,352,0.43,-84,-0.01,348,0.42,-80,-0.01,352,0.43,,,,,,,,

@aentinger aentinger merged commit d8a3a98 into master Jan 28, 2021
@aentinger aentinger deleted the offload-ssl-nina-take2 branch January 28, 2021 11:03
@eclipse1985
Copy link
Contributor

As far as I understand this is a breaking change that requires the nina firmware to be updated. This would need a UI/UX works from our side, so please do not release a new version for now 😄

@aentinger
Copy link
Collaborator

@eclipse1985 - yes 😉

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.

5 participants