Use 2nd stack for update signature verification (#7149)

* Use 2nd stack for update signature verification Fixes #7145 When doing a signed update, the signature calculation can use a lot of stack, so move it silently to the BearSSL second stack. Also fix a memory leak of signature-bytes found by @JiriBilek * Reset state on any error condition in Updater::end
esp8266 · Mar 14, 2020 · e252873 · e252873
1 parent afb9921
commit e252873
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 4 deletions.
diff --git a/cores/esp8266/Updater.cpp b/cores/esp8266/Updater.cpp
@@ -1,6 +1,7 @@
 #include "Updater.h"
 #include "eboot_command.h"
 #include <esp8266_peri.h>
+#include "StackThunk.h"
 
 //#define DEBUG_UPDATER Serial
 
@@ -40,6 +41,14 @@ UpdaterClass::UpdaterClass()
 {
 #if ARDUINO_SIGNING
  installSignature(&esp8266::updaterSigningHash, &esp8266::updaterSigningVerifier);
+ stack_thunk_add_ref();
+#endif
+}
+
+UpdaterClass::~UpdaterClass()
+{
+#if ARDUINO_SIGNING
+ stack_thunk_del_ref();
 #endif
 }
 
@@ -199,14 +208,14 @@ bool UpdaterClass::end(bool evenIfRemaining){
 #ifdef DEBUG_UPDATER
  DEBUG_UPDATER.println(F("no update"));
 #endif
+ _reset();
  return false;
  }
 
  if(hasError() || (!isFinished() && !evenIfRemaining)){
 #ifdef DEBUG_UPDATER
  DEBUG_UPDATER.printf_P(PSTR("premature end: res:%u, pos:%zu/%zu\n"), getError(), progress(), _size);
 #endif
-
  _reset();
  return false;
  }
@@ -226,6 +235,7 @@ bool UpdaterClass::end(bool evenIfRemaining){
 #endif
  if (sigLen != _verify->length()) {
  _setError(UPDATE_ERROR_SIGN);
+ _reset();
  return false;
  }
 
@@ -251,6 +261,7 @@ bool UpdaterClass::end(bool evenIfRemaining){
  uint8_t *sig = (uint8_t*)malloc(sigLen);
  if (!sig) {
  _setError(UPDATE_ERROR_SIGN);
+ _reset();
  return false;
  }
  ESP.flashRead(_startAddress + binSize, (uint32_t *)sig, sigLen);
@@ -262,9 +273,12 @@ bool UpdaterClass::end(bool evenIfRemaining){
  DEBUG_UPDATER.printf("\n");
 #endif
  if (!_verify->verify(_hash, (void *)sig, sigLen)) {
+ free(sig);
  _setError(UPDATE_ERROR_SIGN);
+ _reset();
  return false;
  }
+ free(sig);
 #ifdef DEBUG_UPDATER
  DEBUG_UPDATER.printf_P(PSTR("[Updater] Signature matches\n"));
 #endif

diff --git a/cores/esp8266/Updater.h b/cores/esp8266/Updater.h
@@ -53,6 +53,7 @@ class UpdaterClass {
  typedef std::function<void(size_t, size_t)> THandlerFunction_Progress;
 
  UpdaterClass();
+ ~UpdaterClass();
 
  /* Optionally add a cryptographic signature verification hash and method */
  void installSignature(UpdaterHashClass *hash, UpdaterVerifyClass *verify) { _hash = hash; _verify = verify; }

diff --git a/libraries/ESP8266WiFi/src/BearSSLHelpers.cpp b/libraries/ESP8266WiFi/src/BearSSLHelpers.cpp
@@ -870,9 +870,9 @@ uint32_t SigningVerifier::length()
  }
 }
 
-bool SigningVerifier::verify(UpdaterHashClass *hash, const void *signature, uint32_t signatureLen) {
- if (!_pubKey || !hash || !signature || signatureLen != length()) return false;
-
+// We need to use the 2nd stack to do a verification, so do the thunk
+// directly inside the class function for ease of use.
+extern "C" bool SigningVerifier_verify(PublicKey *_pubKey, UpdaterHashClass *hash, const void *signature, uint32_t signatureLen) {
  if (_pubKey->isRSA()) {
  bool ret;
  unsigned char vrf[hash->len()];
@@ -890,6 +890,20 @@ bool SigningVerifier::verify(UpdaterHashClass *hash, const void *signature, uint
  }
 };
 
+#if !CORE_MOCK
+make_stack_thunk(SigningVerifier_verify);
+extern "C" bool thunk_SigningVerifier_verify(PublicKey *_pubKey, UpdaterHashClass *hash, const void *signature, uint32_t signatureLen);
+#endif
+
+bool SigningVerifier::verify(UpdaterHashClass *hash, const void *signature, uint32_t signatureLen) {
+ if (!_pubKey || !hash || !signature || signatureLen != length()) return false;
+#if !CORE_MOCK
+ return thunk_SigningVerifier_verify(_pubKey, hash, signature, signatureLen);
+#else
+ return SigningVerifier_verify(_pubKey, hash, signature, signatureLen);
+#endif
+}
+
 #if !CORE_MOCK
 
 // Second stack thunked helpers