tobozo / ESP32-targz

🗜️ An Arduino library to unpack/uncompress tar, gz, and tar.gz files on ESP32 and ESP8266
Other
123 stars 16 forks source link

gzStreamUpdater update finishes successfully, but MD5 checksums do not match #68

Open arkhipenko opened 1 year ago

arkhipenko commented 1 year ago

Hi @tobozo Thank you for this library! I know what it means to maintain an open-source library, so your effort is really appreciated!

I came across an issue that maybe you have some insight into: I am updating esp32 firmware using your library based on the gzip file delivered to the device over OTA. The update actually ends up being successful (and the fimrware works!), but the resulting MD5 from the Update class does not match the calculated MD5 of the binary inside the gzip file. Any idea why this might be the case?

Here is the code snippet - nothing fancy - straight from one of your examples.

  Log.notice("OtaDispatcher::finishOta (gz bin) - processing firmware.gz file (full size: %d)" CR, tarGzIO.output_size);

  GZUnpacker->haltOnError( false ); // stop on fail (manual restart/reset required)
  GZUnpacker->setupFSCallbacks( targzTotalBytesFn, targzFreeBytesFn ); // prevent the partition from exploding, recommended
  GZUnpacker->setGzProgressCallback( BaseUnpacker::targzNullProgressCallback ); // targzNullProgressCallback or defaultProgressCallback
  GZUnpacker->setLoggerCallback( BaseUnpacker::targzNullLoggerCallback );    // gz log verbosity

  if( !GZUnpacker->gzStreamUpdater( (Stream *)&mGzFile, tarGzIO.output_size, 0, false ) ) {
      Log.error("OtaDispatcher::finishOta (gz bin) -gzStreamUpdater failed with return code #%d" CR, GZUnpacker->tarGzGetError() );
      _failed();
  }
  else {
      Log.notice("OtaDispatcher::finishOta (gz bin) - OTA Update ended successfully" CR);
      Log.notice("OtaDispatcher::finishOta (gz bin) - Comparing MD5 strings" CR);

      Log.notice("OtaDispatcher::finishOta (gz bin) - provided: %S" CR, mMD5String);
      Log.notice("OtaDispatcher::finishOta (gz bin) - calculated: %S" CR, Update.md5String());

      if ( mMD5String != Update.md5String() ) {
          Log.error("OtaDispatcher::finishOta (gz bin) - MD5 strings do not match" CR);
          _failed();     
      }
  }

I browsed through your code, and you seem to have Update.begin() and Update.write() and Update.end(true) in exactly the same places I would put them, and yet:

00:00:09:23.433 N: OtaDispatcher::finishOta (gz bin) - processing firmware.gz file (full size: 1140960)
00:00:09:41.756 N: OtaDispatcher::finishOta (gz bin) - OTA Update ended successfully
00:00:09:41.758 N: OtaDispatcher::finishOta (gz bin) - Comparing MD5 strings
00:00:09:41.769 N: OtaDispatcher::finishOta (gz bin) - provided: 3cb229da659a4fb476f8ec3bf16c009a
00:00:09:41.770 N: OtaDispatcher::finishOta (gz bin) - calculated: 6f43ff9a6cac85429333ac8f22516515
00:00:09:41.782 E: OtaDispatcher::finishOta (gz bin) - MD5 strings do not match

Any ideas/suggestions would be greatly appreciated!

arkhipenko commented 1 year ago

I am particularly interested in why you are altering the stream size here:

      if( !Update.begin( ( ( update_size + SPI_FLASH_SEC_SIZE-1 ) & ~( SPI_FLASH_SEC_SIZE-1 ) ), partition ) ) {
tobozo commented 1 year ago

hi, thanks for your feedback :+1:

I'm not sure why tarGzIO.output_size is given as second argument, but does it improve when you use UPDATE_SIZE_UNKNOWN instead?

 GZUnpacker->gzStreamUpdater( (Stream *)&mGzFile, UPDATE_SIZE_UNKNOWN, 0, false );

An ota partition size must be a multiple of 4096, so it may be padded with zeroes depending on the remaining bytes in the last gzip buffer, as a result the effective update size may differ from the binary size.

arkhipenko commented 1 year ago

Thanks for a quick reply @tobozo !

tarGzIO.output_size is the size of the unzaipped file. Using UPDATE_SIZE_UNKNOWN did not help - that's where I started.

Are you padding the buffer with 0's in your code?

arkhipenko commented 1 year ago

The MD5 calculation does not need to be aligned to 4K.

From the Updater class:


in bool UpdateClass::begin();
_md5 = MD5Builder();
...
_md5.begin();
...

in bool UpdateClass::_writeBuffer(): 
_md5.add(_buffer, _bufferLen);
...
in bool UpdateClass::end():
 _md5.calculate();

So the difference might be coming from the fact that all buffer lengths do not add up to the file size md5.add(_buffer, _bufferLen);

arkhipenko commented 1 year ago

I also noticed that I am on 1.1.4 and your master is different. Will try that.

tobozo commented 1 year ago

( ( update_size + SPI_FLASH_SEC_SIZE-1 ) & ~( SPI_FLASH_SEC_SIZE-1 ) ) calculates the destination partition size so that it matches the "multiple of 4096" requirement.

btw the new GzUpdateClass can also do stream update:

https://github.com/tobozo/ESP32-targz/blob/master/src/ESP32-targz-lib.hpp#L311-L383