Fix signature verification, check_download #23

dongsupark · 2023-10-30T15:43:41Z

In update-format-crau, we should check error from verify_rsa_pkcs to handle errors correctly.

Fix bugs when reading from File to buffer. We need to first create a BufReader for reading from the buffer, pass that into parsing functions. That would make the code much easier to maintain, instead of passing File itself. Then we can read data without having to first open the file and track read positions.

We need to introduce get_header_data() to read header and data, setting the begining of the stream including the whole data including delta update header as well as manifest. Doing that, signature verification works well.

Also introduce get_data_blob() to read only data without header, manifest.

We need to skip checking for existing downloads, if the file does not exist. Otherwise, check_download will simply fail in the beginning, due to missing files.

src/bin/download_sysext.rs

update-format-crau/src/delta_update.rs

pothos · 2023-11-03T10:16:10Z

The command from #13 (comment) still fails with

Error: "unable to verify signature \"oem-ami.gz\""

src/bin/download_sysext.rs

We should check error from verify_rsa_pkcs to handle errors correctly.

pothos · 2023-11-06T11:43:23Z

update-format-crau/src/delta_update.rs

    // sigmessages.signatures[] has a single element in case of dev update payloads,
    // while it could have multiple elements in case of production update payloads.
    // For now we assume only dev update payloads are supported.
    // Return the first valid signature, iterate into the next slot if invalid.
    sigmessage.signatures.iter()
        .find_map(|sig|
-            verify_sig_pubkey(testdata, sig, pubkeyfile)
+            verify_sig_pubkey(testdata.as_slice(), sig, pubkeyfile)
            .map(Vec::into_boxed_slice))
 }



Meta comment on the below function signature: What is the meaning of the return type? Could this be changed to Result with an error to be easier to understand and handle for the caller?

As I remember, the first version of the code had a full Result type like you say. After a code review, we chose to drop that for simplicity.

I don't understand what this returned array means, normally a verification either succeeds or gives an error.
@wrl Do you have a suggestion on how to improve this?

Currently it's:

// parse_signature_data takes a bytes slice for signature and public key file path. // Return only actual data, without version and special fields. pub fn parse_signature_data(testdata: &[u8], sigbytes: &[u8], pubkeyfile: &str) -> Option<Box<[u8]>> {

dongsupark · 2023-11-07T16:39:05Z

The command from #13 (comment) still fails with
Error: "unable to verify signature \"oem-ami.gz\""

Thanks for testing.
Now the signature verification works as expected.

The first issue was a wrong input buffer was being passed for signature verification. Fixed.

After that, the signature verification still did not work. Its main reason was that a signature of a Flatcar sysext image is generated from the whole data, from the beginning of delta update header to manifest and finally the data blob. Therefore in get_data_blobs we should not use header.translate_offset(0), but simply 0.

pothos · 2023-11-07T17:07:16Z

Running this I get outdir/oem-ami.gz written but that's still the payload, we need the extracted contents.

dongsupark · 2023-11-08T09:20:33Z

Running this I get outdir/oem-ami.gz written but that's still the payload, we need the extracted contents.

Fixed.
Now the extracted data blobs are written to outdir/oem-ami.raw.

src/bin/download_sysext.rs

pothos · 2023-11-08T11:22:30Z

Fixed.
Now the extracted data blobs are written to outdir/oem-ami.raw.

Something is wrong, please let the program check that the output matches the hash of new_partition_info.

For reference you can compare with the output of the bash decode_payload script which does things right.

PROTOPATH=src/update_engine/ ./decode_payload ~/flatcar/scripts/sdk_container/src/third_party/coreos-overlay/coreos-base/coreos-au-key/files/official-v2.pub.pem /var/tmp/oem-ami.gz /var/tmp/oem-ami.raw

dongsupark · 2023-11-08T17:10:46Z

Something is wrong, please let the program check that the output matches the hash of new_partition_info.

Done with reimplementing getting header and data parts.

pothos · 2023-11-09T13:25:32Z

Done with reimplementing getting header and data parts.

Thanks, that finally gives a working result

src/bin/download_sysext.rs

update-format-crau/src/delta_update.rs

We need to skip checking for existing downloads, if the file does not exist. Otherwise, check_download will simply fail in the beginning, due to missing files.

pothos · 2023-11-10T15:47:42Z

The last changes with the hardcoded tmp path make extraction fail:

code: 18, kind: CrossesDevices, message: "Invalid cross-device link"

dongsupark · 2023-11-10T17:13:21Z

The last changes with the hardcoded tmp path make extraction fail:
code: 18, kind: CrossesDevices, message: "Invalid cross-device link"

This is now fixed.

test/crau_verify.rs

update-format-crau/src/delta_update.rs

src/bin/download_sysext.rs

pothos

I get this error Error: Error { kind: UnexpectedEof, message: "failed to fill whole buffer" }
when running target/debug/download_sysext -p ~/flatcar/scripts/sdk_container/src/third_party/coreos-overlay/coreos-base/coreos-au-key/files/official-v2.pub.pem -m flatcar_production_update.gz -o /var/tmp/outdir/ -i /var/tmp/beta-response-genericsha256
with beta-response-genericsha256:

<?xml version="1.0" encoding="UTF-8"?>
<response protocol="3.0" server="nebraska"><daystart elapsed_seconds="0"></daystart><app appid="e96281a6-d1af-4bde-9a0a-97b76e56dc57" status="ok"><ping status="ok"></ping><updatecheck status="ok"><urls><url codebase="https://update.release.flatcar-linux.net/amd64-usr/3745.1.0/"></url></urls><manifest version="3745.1.0"><packages><package name="flatcar_production_update.gz" hash_sha256="07b07d57e4140675fc008ce59ae37bb976cc78a794a502551eefd712169ea427" hash="wu2kqzo1DdPfjLTKHsILXA/+NQc=" size="387642070" required="true"></package><package name="oem-ami.gz" hash="gwCrRdY+wOMO8xSU5CsvGEHsF4k=" hash_sha256="46a5f66202f89f9ca93b90476466bf8325976749a22aac2550e84cc4e3d61bb7" size="25669804" required="false"></package><package name="oem-azure.gz" hash="j0uFpiFGOGZxEtoQgMIJXFQhMpc=" hash_sha256="2d5d6ef2d61b05aadc41289f42cfbe08e2639717dabc09a3363f7f561d39318a" size="40969473" required="false"></package><package name="oem-qemu.gz" hash="DR/utcV9lnjq8f2+PHRl45Mm+hQ=" hash_sha256="a71ac69e59a66d20d6194db450bdc6bc05b27822eaa8890c6796586739bed0b9" size="2262" required="false"></package><package name="oem-vmware.gz" hash="JIjyTXkB+N7DmihOJP+LreXFyWI=" hash_sha256="38287be053ee9abbd1ea9c755235cf226eeaf30d03c94a8733f499955e4b4c65" size="1794357" required="false"></package></packages><actions><action event="postinstall" sha256="B7B9V+QUBnX8AIzlmuN7uXbMeKeUpQJVHu/XEhaepCc=" DisablePayloadBackoff="true"></action></actions></manifest></updatecheck><event status="ok"></event></app></response>

dongsupark · 2023-11-15T16:20:29Z

I get this error Error: Error { kind: UnexpectedEof, message: "failed to fill whole buffer" }

Fixed. freader.seek was needed.

src/bin/download_sysext.rs

Fix bugs when reading from File to buffer. We need to first create a BufReader for reading from the buffer, pass that into parsing functions. That would make the code much easier to maintain, instead of passing File itself. Then we can read data without having to first open the file and track read positions. We need to get length of header and data, reading from the begining of the stream including the whole data including delta update header as well as manifest. And pass the length to hash_on_disk to calculate the hash without having to read the while data into memory. Doing that, signature verification works well. Also introduce get_data_blob() to read only data without header, manifest.

dongsupark · 2023-11-16T08:29:18Z

Done with having seek only once.

Added more comments.

dongsupark · 2023-11-16T11:32:08Z

Thanks!

Update ue-rs to 0.1.0-r3, 2023-11-16. Pulls in flatcar/ue-rs#23 and flatcar/ue-rs#24.

dongsupark requested review from wrl and pothos October 30, 2023 15:43

dongsupark mentioned this pull request Oct 30, 2023

Verification and reporting #13

Closed

pothos reviewed Nov 2, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

dongsupark force-pushed the dongsu/fix-download-sig branch from 09de03e to c589a61 Compare November 3, 2023 09:53

pothos reviewed Nov 3, 2023

View reviewed changes

update-format-crau/src/delta_update.rs Outdated Show resolved Hide resolved

pothos reviewed Nov 3, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

update-format-crau: check result of verify_rsa_pkcs

384188a

We should check error from verify_rsa_pkcs to handle errors correctly.

pothos reviewed Nov 6, 2023

View reviewed changes

dongsupark force-pushed the dongsu/fix-download-sig branch from c589a61 to c38ec2a Compare November 7, 2023 16:31

dongsupark force-pushed the dongsu/fix-download-sig branch from c38ec2a to de20c55 Compare November 8, 2023 09:19

dongsupark force-pushed the dongsu/fix-download-sig branch from de20c55 to 1f69b54 Compare November 8, 2023 09:35

pothos reviewed Nov 8, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

dongsupark force-pushed the dongsu/fix-download-sig branch from 1f69b54 to b9c3665 Compare November 8, 2023 17:09

pothos reviewed Nov 9, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

dongsupark force-pushed the dongsu/fix-download-sig branch from b9c3665 to 3d8ef7a Compare November 9, 2023 14:28

pothos reviewed Nov 9, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

pothos reviewed Nov 9, 2023

View reviewed changes

update-format-crau/src/delta_update.rs Outdated Show resolved Hide resolved

download_sysext: skip checking for downloads if that does not exist

b64f479

We need to skip checking for existing downloads, if the file does not exist. Otherwise, check_download will simply fail in the beginning, due to missing files.

dongsupark force-pushed the dongsu/fix-download-sig branch from 3d8ef7a to 726cf6f Compare November 10, 2023 10:18

dongsupark force-pushed the dongsu/fix-download-sig branch from 726cf6f to 811afa0 Compare November 10, 2023 17:12

dongsupark force-pushed the dongsu/fix-download-sig branch from 811afa0 to 7f58af1 Compare November 10, 2023 18:46

pothos reviewed Nov 13, 2023

View reviewed changes

test/crau_verify.rs Outdated Show resolved Hide resolved

pothos reviewed Nov 13, 2023

View reviewed changes

update-format-crau/src/delta_update.rs Outdated Show resolved Hide resolved

dongsupark force-pushed the dongsu/fix-download-sig branch from 7f58af1 to 3a986a7 Compare November 14, 2023 16:27

pothos reviewed Nov 15, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

pothos reviewed Nov 15, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

pothos requested changes Nov 15, 2023

View reviewed changes

dongsupark force-pushed the dongsu/fix-download-sig branch from 3a986a7 to 2d602ef Compare November 15, 2023 16:19

pothos reviewed Nov 15, 2023

View reviewed changes

src/bin/download_sysext.rs Show resolved Hide resolved

pothos reviewed Nov 15, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

pothos reviewed Nov 15, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

pothos reviewed Nov 15, 2023

View reviewed changes

src/bin/download_sysext.rs Outdated Show resolved Hide resolved

dongsupark force-pushed the dongsu/fix-download-sig branch from 2d602ef to 0088652 Compare November 16, 2023 08:28

pothos approved these changes Nov 16, 2023

View reviewed changes

pothos linked an issue Nov 16, 2023 that may be closed by this pull request

Verification and reporting #13

Closed

dongsupark merged commit 58c7969 into trunk Nov 16, 2023
1 check passed

dongsupark deleted the dongsu/fix-download-sig branch November 16, 2023 11:32

dongsupark added a commit to flatcar/scripts that referenced this pull request Nov 16, 2023

overlay ue-rs: update to 0.1.0-r3, 2023-11-16

b5bb5c8

Update ue-rs to 0.1.0-r3, 2023-11-16. Pulls in flatcar/ue-rs#23 and flatcar/ue-rs#24.

dongsupark mentioned this pull request Nov 16, 2023

overlay ue-rs: update to 0.1.0-r3, 2023-11-16 flatcar/scripts#1397

Merged

pothos mentioned this pull request Nov 20, 2023

Improve error handling of update-format-crau #28

Closed

2 tasks

github-actions bot mentioned this pull request Nov 22, 2023

Monthly contributions report 2023-10-22 - 2023-11-21 flatcar/Flatcar#1259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix signature verification, check_download #23

Fix signature verification, check_download #23

dongsupark commented Oct 30, 2023 •

edited

Loading

pothos commented Nov 3, 2023

pothos Nov 6, 2023

dongsupark Nov 7, 2023

pothos Nov 7, 2023 •

edited

Loading

dongsupark commented Nov 7, 2023

pothos commented Nov 7, 2023

dongsupark commented Nov 8, 2023

pothos commented Nov 8, 2023

dongsupark commented Nov 8, 2023

pothos commented Nov 9, 2023

pothos commented Nov 10, 2023

dongsupark commented Nov 10, 2023

pothos left a comment

dongsupark commented Nov 15, 2023

dongsupark commented Nov 16, 2023

dongsupark commented Nov 16, 2023

Fix signature verification, check_download #23

Fix signature verification, check_download #23

Conversation

dongsupark commented Oct 30, 2023 • edited Loading

pothos commented Nov 3, 2023

pothos Nov 6, 2023

Choose a reason for hiding this comment

dongsupark Nov 7, 2023

Choose a reason for hiding this comment

pothos Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

dongsupark commented Nov 7, 2023

pothos commented Nov 7, 2023

dongsupark commented Nov 8, 2023

pothos commented Nov 8, 2023

dongsupark commented Nov 8, 2023

pothos commented Nov 9, 2023

pothos commented Nov 10, 2023

dongsupark commented Nov 10, 2023

pothos left a comment

Choose a reason for hiding this comment

dongsupark commented Nov 15, 2023

dongsupark commented Nov 16, 2023

dongsupark commented Nov 16, 2023

dongsupark commented Oct 30, 2023 •

edited

Loading

pothos Nov 7, 2023 •

edited

Loading