Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verification and reporting #13

Closed
pothos opened this issue Oct 10, 2023 · 4 comments · Fixed by #23
Closed

Verification and reporting #13

pothos opened this issue Oct 10, 2023 · 4 comments · Fixed by #23
Labels
kind/feature A feature request. kind/invalid This doesn't seem right

Comments

@pothos
Copy link
Member

pothos commented Oct 10, 2023

  • The OUTPUTDIR/PACKAGENAME files should not be created unless verified (both Omaha checksum and the pub key signature) - use a .unverified/ subfolder first
  • The process exit code should be non-zero if an error occurred
  • The file path to the pub key should be passed in as command line parameter
@pothos pothos added kind/feature A feature request. and removed kind/feature A feature request. labels Oct 10, 2023
@dongsupark
Copy link
Member

Addressed mostly(?) in #12.

@dongsupark dongsupark added the kind/feature A feature request. label Oct 12, 2023
@pothos pothos added the kind/invalid This doesn't seem right label Oct 30, 2023
@pothos
Copy link
Member Author

pothos commented Oct 30, 2023

Thanks, I'll do some tests to see if it's ready for integration with update-engine/bootengine.
What we also need for that is #17

@pothos
Copy link
Member Author

pothos commented Oct 30, 2023

Currently there is a bug: When the .unverified folder is empty, the pkg.check_download call fails.
Then the next problem is that let res_data = fs::read_to_string(from_path); makes no sense, it should be something like upfile.read_to_end(&mut buffer)? and then reopening the file or directly passing the buffer for read_delta_update_header.

The major problem is that the signature isn't verified at all. This is wrong because it doesn't forward the error to the caller:
_ = verify_sig::verify_rsa_pkcs(testdata, sig.data(), get_public_key_pkcs_pem(pubkeyfile, KeyTypePkcs8));
This is why the caller of verify_sig_pubkey thinks all would be good even for failures.
It needs to be reworked so that verify_signature_on_disk either doesn't receive bad signatures or skips over them. Currently it would fail on the first wrong signature which is not what we want because it's a dummy entry.

It's not hard to test all this:

cargo build
rm -rf /var/tmp/outdir
mkdir /var/tmp/outdir
RUST_LOG=debug 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 oem-ami.gz -o /var/tmp/outdir/ -i /var/tmp/beta-response

Where /var/tmp/beta-response is

<?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="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>

P.S.: Here is the command to do the same for Alpha: #12 (comment)

@dongsupark
Copy link
Member

Thanks. Fixed several things in #23.

Note, in the example data /var/tmp/beta-response from above, flatcar_production_update.gz needs to have also a field hash_sha256. Otherwise, download will simply fail due to missing sha256 hash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A feature request. kind/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants