Andrew Gunnerson
a2fb807803
Implicit error propagation was originally used because it was convenient
and made it easy to just bubble up errors via the ? operator without
thinking. However, there have been too many situations where this
resulted in error messages that were completely useless in
troubleshooting the problem. "I/O error", even with a specific reason
attached, is useless where there are potentially hundreds of operations
where I/O can fail.
I no longer think implicit error propagation is a good idea, so this
commit removes every single use of #[from] in every custom error type.
There are now many more error variants, allowing more context to be
attached to the underlying errors.
Previously, it was easy to encounter error messages like:
Caused by:
0: Failed to patch payload: payload.bin
1: Failed to patch boot images: boot, init_boot, vendor_boot
2: Boot image error
3: I/O error
4: failed to fill whole buffer
This is a terrible error message because it doesn't mention which of the
3 boot images failed to parse, nor does it mention during which I/O
operation it encountered EOF. With this commit, this sort of information
is now included. For example, if the boot image happened to be truncated
in the middle of the ramdisk, the error message would now be:
Caused by:
0: Failed to patch payload: payload.bin
1: Failed to patch boot images: boot, init_boot, vendor_boot
2: Failed to load boot image: init_boot
3: Failed to read boot image data: Boot::V3::ramdisk
4: failed to fill whole buffer
Changes:
* Remove all uses of #[from] from thiserror-derived error types.
* Errors during parsing and serialization of RSA private keys, RSA
public keys, and X509 certificates now include the file path.
* Removed unnecessary uses of BufReader and BufWriter when reading and
writing RSA private keys, RSA public keys, and X509 certificates,
since they need to be fully read into memory anyway.
* Use ReadFixedSizeExt instead of read_exact() where possible.
* Use &'static str instead of String in error fields where all possible
values are known at compile-time to avoid unnecessary heap allocation.
* Use ok_or() instead of ok_or_else() to construct errors when the error
variant uses known data and does not require heap allocation.
* Using DebugString instead of String in error variants that store a
preformatted debug string.
While working on this commit, an unrelated bug was found and fixed:
* Fix vendor v4 boot images that were truncated within the padding
following the bootconfig section being treated as valid.
Signed-off-by: Andrew Gunnerson <accounts+github@chiller3.com>
End-to-end tests
avbroot's output file is reproducible for a given input file. e2e.toml lists some profiles for generating mock OTA images with unique properties and the expected checksums before and after patching. These tests use pregenerated, hardcoded test keys for signing. These keys should NEVER be used for any other purpose.
For each profile listed in the config, the test process will:
- Generate a mock OTA based on the specification
- Verify tha original OTA checksum
- Run avbroot against the OTA using
--magisk(with a mock Magisk APK) - Verify the patched OTA checksum
- Extract the AVB-related partitions from the patched OTA
- Run avbroot against the OTA again using
--prepatched - Verify the patched OTA checksum again
The default profiles shipped with the project mimic how various stock OTAs for Pixel devices are built. The generated mock OTAs have valid signatures and data structures for all components, but without any actual data where possible. For example, most files in the ramdisks are empty files. To ensure the mock OTAs cannot be mistakenly installed on a real device, the OTA metadata lists a fake device name in the preconditions section.
Running the tests
To test against the profiles listed in e2e.toml, run:
# To test all profiles
cargo run --release -- test -a
# Or to test against specific profiles
cargo run --release -- test -p pixel_v4_gki -p pixel_v4_non_gki