Failure Signing OTA #328
Comments
|
This seems awful similar to #325. Could you verify what version of avbroot you are running on what OS? |
|
Ah, yes using I'm guessing I should be using avbroot-3.41.-x86_64-unknown-linux-gnu.zip? |
I think that would be better, but I don't think that is the cause of your new issue. What device are you trying to run avbroot for? |
|
Generic AI Box from Carlinkit. This is the build.prop from the extracted system image: |
|
Any chance you know of an alternate place to download the OTA? It looks like they throttle downloads from https://www.carlinkit.com/download.html to exactly 200Kbps for me. (If not, no worries! I can take a look tomorrow when the download finishes.) |
|
Thanks, got it! 30 seconds much better than a few hours. Taking a look now. |
|
The issue is that the OTA doesn't contain a Since the OTA is incomplete, I don't think there's much you can do, unfortunately. |
|
Does it possibly use boot and system_ext (or something else) to make the recovery image? Here is the recovery partition as read using QFIL after applying the update: https://drive.google.com/file/d/14_zigxxg2mdyk7HFo2DvDP9XzYN_bglB/view?usp=drive_link |
|
Thanks for uploading the recovery image. Is it a completely unmodified image? If it's unmodified and recovery mode actually boots while the bootloader is locked, then I think signature verification is broken on your device. You wouldn't gain any additional security by using avbroot vs. just leaving the bootloader unlocked. Details: The AVB metadata of (I had to chop of a bunch of excess 0's at the end of the file. If those weren't added by QFIL and the partition is actually like that, then it's yet another way that things are broken on this device.) However, the AVB metadata of
It doesn't look like it. I searched for various strings from your recovery image, like |
|
As far as I'm aware, it should be unmodified. Here is a separate recovery image that was provided by the vendor for QFIL recovery: https://drive.google.com/file/d/14epN_MlxeS2t0QTb1UVEmo_NChECPJn8/view?usp=drive_link Not sure if related at all, but the device does have funky behavior where it states "device not activated" after flashing with QFIL until it gets an internet connection. |
|
Ah, yep, that file no longer has the extra 0's at the end of the file, but is otherwise identical to the previous recovery image you uploaded. I took a further look and it definitely seems like this OEM doesn't care about security at all.
These private keys are all publicly available and should never be used. Given that and the fact that the
This comes from setprop persist.chelianyi.activate.state 1 |
|
Yes, most the Chinese CarPlay based "AI Box" devices all use the keys from the Google repos. Nice find on the CarDataManager.apk, especially considering I just posted it yesterday. Could avbroot be tricked into working I placed the recovery.img into the |
I'm not familiar with this |
|
Oops, was looking at the wrong folder. There is no Is it possible the recovery partition isn't used since it's an A/B device (or tries to be), and that recovery partition is unused? I'm seeing in the Google A/B docs (https://source.android.com/docs/core/ota/ab/ab_implement#partitions), the recovery is now contained in boot.img. |
|
The recovery program ( |
|
Does it use |
|
|
|
By the way, I'm looking to see if I can easily implement |
|
What would be awesome!! |
Some devices have "full" OTAs where the payload is missing the recovery partition. These subcommands make it possible to manually add back the missing image. Given the strict requirements for how the OTA zip is laid out and signed, users can't just replace payload.bin in a zip and call it a day, but it's sufficient for feeding a modified input to `avbroot ota patch`. Issue: #328 Signed-off-by: Andrew Gunnerson <[email protected]>
Some devices have "full" OTAs where the payload is missing the recovery partition. These subcommands make it possible to manually add back the missing image. Given the strict requirements for how the OTA zip is laid out and signed, users can't just replace payload.bin in a zip and call it a day, but it's sufficient for feeding a modified input to `avbroot ota patch`. Issue: #328 Signed-off-by: Andrew Gunnerson <[email protected]>
|
I just merged #331 and released version 3.5.0 with the change. Can you give this a try?
|
|
I was able to run the avbroot commands without errors when using the unmodified contents, but if I do the following, then I get an error: Pack using avbroot ... Zip command runs and indicates: Patch command returns error: |
|
... also I tried the steps as recommended for adding the recovery.img partition (without other changes), but the device does not take the patched file. Here's the log file: |
Great!
Ah, by using resize2fs, you wiped out the AVB metadata. I'd recommend this instead: avbroot avb unpack -i system.imgThis will create Then, recreate avbroot avb pack -o system.img --recompute-size
Hmm, that's extremely odd. Any chance you could upload the patched OTA you were flashing? The error is: It's saying the first 4 bytes of |
|
Here’s the patched file with just recovery partition added:
https://drive.google.com/file/d/14quLMwMToMFFRFGul8TY8S7OTJEaUq8P/view?usp=drivesdk
…On Sun, Aug 18, 2024 at 11:49 Andrew Gunnerson ***@***.***> wrote:
I was able to run the avbroot commands without errors when using the
unmodified contents
Great!
but if I do the following, then I get an error:
Ah, by using resize2fs, you wiped out the AVB metadata. I'd recommend this
instead:
avbroot avb unpack -i system.img
This will create avb.toml and raw.img. Modify raw.img instead of
system.img with your commands.
Then, recreate system.img with:
avbroot avb pack -o system.img --recompute-size
... also I tried the steps as recommended for adding the recovery.img
partition (without other changes), but the device does not take the patched
file.
Hmm, that's extremely odd. Any chance you could upload the patched OTA you
were flashing?
The error is:
08-18 08:13:24.062 E/update_engine( 1246): [ERROR:payload_metadata.cc(64)] Bad payload format -- invalid delta magic: 33265343 Expected: 43724155
It's saying the first 4 bytes of payload.bin are 3&SC instead of CrAU. I
have no idea what could possibly screw that up.
—
Reply to this email directly, view it on GitHub
<#328 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCQGSMG3D6XSSRVYRRAV7DZSDUERAVCNFSM6AAAAABMOZ2BMSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJVGM2TKMBSHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Thanks! Unfortunately, this is probably not fixable because Details: This is a hex dump of the portion of the zip file where
The incorrect offset is exactly 64 bytes before where it should be reading. Looking at the zip, Every entry has a 16 byte data descriptor: So (4 files before I tried flashing your zip on one of my test devices with some safety checks removed (nobody reading this should ever attempt this!) and it was able to successfully read the file. While data descriptors are optional (and the original |
|
I thought the data descriptors were 12 bytes? Is the 4-byte descriptor a new(er) version of the spec? Also, I'm seeing data descriptors are used in the original file I have. They all have bit 3 set in the general purpose flag (0x0800). Does it have anything to do with the original zip using zip spec |
|
Oh huh, I missed that in the original file. It does indeed have a data descriptor, but only for The data descriptors can be either 12 or 16 bytes (or more for zip64) because the 4 byte 0x08074b50 marker is optional (section 4.3.9.3). AOSP supports parsing data descriptors with and without the marker (source). I don't think the zip spec field has anything to do with it. At least in AOSP, that field is completely ignored. |
|
Would it be a heavy lift to modify the signing logic to support zip files without data descriptors? I'd be happy to donate to the project to make it happen. |
This commit adds a new `--zip-mode` parameter to `avbroot ota patch` to control whether the patched OTA zip is written with data descriptors or not. By default, the `streaming` mode is used, which matches the current behavior where the zip is hashed for signing as it is being written. The new `seekable` mode fully writes the zip before rereading it to hash the contents. The new mode is useful for devices with broken zip parsers that fail to properly handle data descriptors. All of the end-to-end tests have been duplicated to test both modes. Adding the seekable mode necessitated a couple other changes: * BufWriter is no longer used. Type erasure is very painful in Rust, so we need to keep the writer types the same for both the streaming and seekable modes. BufWriter is unusable in the seekable mode because we need to be able to read back what was written, which isn't supported. * HolePunchingWriter has been removed. It was a simple way to produce sparse files by seeking whenever a write buffer consists fully of zeros. When combined with BufWriter, there was previously never a situation where this was undesirable. However, with the new seekable mode and the zip library's pattern of writing one field at a time, the final 2 zero bytes (representing an empty archive comment) is never written and the file size is not increased either. Removing this is not a big deal since we no longer use stripped OTAs for the end-to-end tests. Those were really the only OTAs that benefitted from sparse files. A real OTA has very few zero bytes due to compression. Issue: #328 Signed-off-by: Andrew Gunnerson <[email protected]>
|
I don't accept donations for my projects anymore, but thank you for offering! Please give #337 a try. Test builds can be found at: https://github.com/chenxiaolong/avbroot/actions/runs/10463414808?pr=337. When patching, you'll want to use the new |
|
You are an Android genius!!! Now on to checkout your Custota project. |
This commit adds a new `--zip-mode` parameter to `avbroot ota patch` to control whether the patched OTA zip is written with data descriptors or not. By default, the `streaming` mode is used, which matches the current behavior where the zip is hashed for signing as it is being written. The new `seekable` mode fully writes the zip before rereading it to hash the contents. The new mode is useful for devices with broken zip parsers that fail to properly handle data descriptors. All of the end-to-end tests have been duplicated to test both modes. Adding the seekable mode necessitated a couple other changes: * BufWriter is no longer used. Type erasure is very painful in Rust, so we need to keep the writer types the same for both the streaming and seekable modes. BufWriter is unusable in the seekable mode because we need to be able to read back what was written, which isn't supported. * HolePunchingWriter has been removed. It was a simple way to produce sparse files by seeking whenever a write buffer consists fully of zeros. When combined with BufWriter, there was previously never a situation where this was undesirable. However, with the new seekable mode and the zip library's pattern of writing one field at a time, the final 2 zero bytes (representing an empty archive comment) is never written and the file size is not increased either. Removing this is not a big deal since we no longer use stripped OTAs for the end-to-end tests. Those were really the only OTAs that benefitted from sparse files. A real OTA has very few zero bytes due to compression. Issue: #328 Signed-off-by: Andrew Gunnerson <[email protected]>
|
#337 has been merged and will be included in the upcoming 3.7.0 release! |
I'm getting the following error when trying to sign an OTA image on 3.4.1. Any idea how to resolve this?
The text was updated successfully, but these errors were encountered: