Skip to content
This repository was archived by the owner on Apr 22, 2020. It is now read-only.

Add library to read/write firmware update HEADER files #74

Merged
merged 13 commits into from
Feb 6, 2019

Conversation

jonathanhaigh-arm
Copy link
Contributor

@jonathanhaigh-arm jonathanhaigh-arm commented Jan 28, 2019

When mbl-cloud-client wants to know what version of firmware is running
on a device it runs a script "arm_update_active_details.sh" that passes
mbl-cloud-client a "HEADER" file that contains information about the
last firmware update.

Usually, this HEADER file is passed to the device along with a firmware
update from the Pelion cloud, but there are two cases where that doesn't
happen:

  • When an update is sent to the device by the mbl-cli tool rather than
    the Pelion cloud.
  • When the first version of software is initially written directly to
    the deivce's flash.

We therefore need a mechanism to create these HEADER files independently
of the Pelion Cloud and mbl-cloud-client.

This commit adds:

  • The mbl-firmware-update-header Python library for creating and reading
    HEADER files.
  • Tests for the mbl-firmware-update-header library.
  • An update to the mbl-firmware-update-manager library (which is used to
    install updates that do not arrive via the Pelion cloud) to create
    a HEADER file for incoming updates.

Synchronization
Must be merged before: PelionIoT/meta-mbl#367

Jenkins Link
http://jenkins.mbed-linux.arm.com/blue/organizations/jenkins/jh-test/detail/jh-test/34/pipeline (successful)
http://jenkins.mbed-linux.arm.com/blue/organizations/jenkins/jh-test/detail/jh-test/36/pipeline (successful; after addressing review comments).
http://jenkins.mbed-linux.arm.com/blue/organizations/jenkins/jh-test/detail/jh-test/37/pipeline (aborted; after addressing further review comments and fixing sanity check errors).
http://jenkins.mbed-linux.arm.com/blue/organizations/jenkins/jh-test/detail/jh-test/38/pipeline (successful; after more changes).
http://jenkins.mbed-linux.arm.com/blue/organizations/jenkins/jh-test/detail/jh-test/40/pipeline (in progress; after module rename).

Testing done

  • Unit tests for the new library have been added and pass.
  • I have manually checked that the versions (timestamps) in HEADER files created by mbl-firmware-update-manager are reasonable.
  • I have checked that the firmware versions in HEADER files generated by mbl-firmware-update-manager show up in the Pelion Device Management Web UI as the devices' "manifest timestamp" attributes and the "PkgVersion" resources (/10252/0/6).
  • I have manually checked that the firmware hashes in HEADER files created by mbl-firmware-update-manager match the firmware hashes in HEADER files created by installing the same payloads via the Pelion cloud.
  • I have manually checked that the firmware hashes in HEADER files generated by mbl-firmware-update manager show up correctly as the "firmware checksum" attributes of devices.

When mbl-cloud-client wants to know what version of firmware is running
on a device it runs a script "arm_update_active_details.sh" that passes
mbl-cloud-client a "HEADER" file that contains information about the
last firmware update.

Usually, this HEADER file is passed to the device along with a firmware
update from the Pelion cloud, but there are two cases where that doesn't
happen:
* When an update is sent to the device by the mbl-cli tool rather than
  the Pelion cloud.
* When the first version of software is initially written directly to
  the deivce's flash.

We therefore need a mechanism to create these HEADER files independently
of the Pelion Cloud and mbl-cloud-client.

This commit adds:
* The mbl-firmware-update-header Python library for creating and reading
  HEADER files.
* Tests for the mbl-firmware-update-header library.
* An update to the mbl-firmware-update-manager library (which is used to
  install updates that do not arrive via the Pelion cloud) to create
  a HEADER file for incoming updates.

if len(data) < self.__header_and_crc_size:
raise FormatError(
"Data is too short (expecting at least {}B)".format(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to add len(data) in the message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Will do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in latest code.

if header_magic != self.__HEADER_MAGIC:
raise FormatError(
"Invalid header magic value 0x{:x} (should be 0x{:x})".format(
header_magic, self.__HEADER_MAGIC
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this header magic confidential, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope.

Copy link
Contributor

Choose a reason for hiding this comment

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

LOL meant "NOT confidential"


if header_version != self.HEADER_FORMAT_VERSION:
raise FormatError(
"Unsupported header version {}".format(header_version)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to add self.HEADER_FORMAT_VERSION to the message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in latest code.

# Create a "HEADER" file for the update - this is a blob that contains
# information about the update
with open(HEADER_FILE, "wb") as header_file:
header_file.write(_create_header_data(firmware_update_file_path))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think your new script throws exceptions. Are you handling them here in some way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is sloppy, should be fixed in the latest code.

@jonathanhaigh-arm
Copy link
Contributor Author

Adding DO NOT MERGE label - I need to re-test after the changes for review comments.

@jonathanhaigh-arm
Copy link
Contributor Author

Regarding the remaining sanity check failures: they are spurious; see PelionIoT/mbl-tools#126

* Confusingly, the code was previously sometimes treating firmware
  hashes as 512-bit values and sometimes treating them as 256-bit
  values. It sort-of worked by chance, but let's be explicit about
  what's happenning: the firmware_hash field in HEADER blobs is 512 bits
  long, but we only use the first 256 bits of it to store sha256 hashes.

* Add a missing "self" parameter in _create_header_data.
@jonathanhaigh-arm
Copy link
Contributor Author

Even after PelionIoT/mbl-tools#126 was merged there are still sanity check failures... They're in unrelated code and are being dealt with by PelionIoT/mbl-tools#129.

I think I've addressed all review comments and I've re-tested, so, reviewers, please re-review.

@jonathanhaigh-arm
Copy link
Contributor Author

Reviewers, sorry for the extra hassle but I want to make another change before this is merged.

In another PR I'm going to create a recipe in meta-mbl to use the Python module added in this PR to actually create a firmware update HEADER file... It seems sensible to call this new recipe "mbl-firmware-update-header", so I want to rename the Python module to be called "mbl.firmware_update_header.util".

@jonathanhaigh-arm
Copy link
Contributor Author

I've (partially) retested after the module rename and added a new Jenkins link (build in progress).

rwalton-arm and others added 2 commits February 1, 2019 13:39
* Remove optional "0" in some slice syntax.
* Remove unnecessary line break.
…ed/mbl-core into jh-update-header-IOTMBL-1099
@rwalton-arm
Copy link
Contributor

Thanks for the changes.

@jonathanhaigh-arm jonathanhaigh-arm merged commit 7553159 into master Feb 6, 2019
@jonathanhaigh-arm jonathanhaigh-arm deleted the jh-update-header-IOTMBL-1099 branch February 6, 2019 11:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants