Drop the dependency on 'ui' in verify_file(). (5e535014) · Commits · e / tools / etwrp

install.cpp

+21 −19

Original line number	Diff line number	Diff line
		@@ -27,6 +27,7 @@
		#include <unistd.h>

		#include <chrono>
		#include <functional>
		#include <limits>
		#include <map>
		#include <string>
		@@ -588,7 +589,8 @@ bool verify_package(const unsigned char* package_data, size_t package_size) {
		// Verify package.
		ui->Print("Verifying update package...\n");
		auto t0 = std::chrono::system_clock::now();
		int err = verify_file(const_cast<unsigned char*>(package_data), package_size, loadedKeys);
		int err = verify_file(const_cast<unsigned char*>(package_data), package_size, loadedKeys,
		std::bind(&RecoveryUI::SetProgress, ui, std::placeholders::_1));
		std::chrono::duration<double> duration = std::chrono::system_clock::now() - t0;
		ui->Print("Update package verification took %.1f s (result %d).\n", duration.count(), err);
		if (err != VERIFY_SUCCESS) {

tests/component/verifier_test.cpp

+45 −104

Original line number	Diff line number	Diff line
		@@ -22,93 +22,34 @@
		#include <sys/stat.h>
		#include <sys/types.h>

		#include <memory>
		#include <string>
		#include <vector>

		#include <openssl/sha.h>

		#include <android-base/file.h>
		#include <android-base/stringprintf.h>
		#include <ziparchive/zip_archive.h>
		#include <android-base/test_utils.h>

		#include "common.h"
		#include "common/test_constants.h"
		#include "otautil/SysUtil.h"
		#include "ui.h"
		#include "verifier.h"

		RecoveryUI* ui = NULL;

		class MockUI : public RecoveryUI {
		bool Init(const std::string&) override {
		return true;
		}
		void SetStage(int, int) override {}
		void SetBackground(Icon /icon/) override {}
		void SetSystemUpdateText(bool /security_update/) override {}

		void SetProgressType(ProgressType /determinate/) override {}
		void ShowProgress(float /portion/, float /seconds/) override {}
		void SetProgress(float /fraction/) override {}

		void ShowText(bool /visible/) override {}
		bool IsTextVisible() override {
		return false;
		}
		bool WasTextEverVisible() override {
		return false;
		}
		void Print(const char* fmt, ...) override {
		va_list ap;
		va_start(ap, fmt);
		vfprintf(stderr, fmt, ap);
		va_end(ap);
		}
		void PrintOnScreenOnly(const char* fmt, ...) override {
		va_list ap;
		va_start(ap, fmt);
		vfprintf(stderr, fmt, ap);
		va_end(ap);
		}
		void ShowFile(const char*) override {}

		void StartMenu(const char* const* /headers/, const char* const* /items/,
		int /initial_selection/) override {}
		int SelectMenu(int /sel/) override {
		return 0;
		}
		void EndMenu() override {}
		};

		void
		ui_print(const char* format, ...) {
		va_list ap;
		va_start(ap, format);
		vfprintf(stdout, format, ap);
		va_end(ap);
		}

		class VerifierTest : public testing::TestWithParam<std::vector<std::string>> {
		public:
		MemMapping memmap;
		std::vector<Certificate> certs;

		virtual void SetUp() {
		protected:
		void SetUp() override {
		std::vector<std::string> args = GetParam();
		std::string package = from_testdata_base(args[0]);
		if (sysMapFile(package.c_str(), &memmap) != 0) {
		FAIL() << "Failed to mmap " << package << ": " << strerror(errno) << "\n";
		}

		for (auto it = ++(args.cbegin()); it != args.cend(); ++it) {
		for (auto it = ++args.cbegin(); it != args.cend(); ++it) {
		std::string public_key_file = from_testdata_base("testkey_" + *it + ".txt");
		ASSERT_TRUE(load_keys(public_key_file.c_str(), certs));
		}
		}

		static void SetUpTestCase() {
		ui = new MockUI();
		}
		MemMapping memmap;
		std::vector<Certificate> certs;
		};

		class VerifierSuccessTest : public VerifierTest {
		@@ -118,11 +59,11 @@ class VerifierFailureTest : public VerifierTest {
		};

		TEST_P(VerifierSuccessTest, VerifySucceed) {
		ASSERT_EQ(verify_file(memmap.addr, memmap.length, certs), VERIFY_SUCCESS);
		ASSERT_EQ(verify_file(memmap.addr, memmap.length, certs, nullptr), VERIFY_SUCCESS);
		}

		TEST_P(VerifierFailureTest, VerifyFailure) {
		ASSERT_EQ(verify_file(memmap.addr, memmap.length, certs), VERIFY_FAILURE);
		ASSERT_EQ(verify_file(memmap.addr, memmap.length, certs, nullptr), VERIFY_FAILURE);
		}

		INSTANTIATE_TEST_CASE_P(SingleKeySuccess, VerifierSuccessTest,

verifier.cpp

+161 −168

Original line number	Diff line number	Diff line
		@@ -21,6 +21,7 @@
		#include <stdlib.h>
		#include <string.h>

		#include <functional>
		#include <algorithm>
		#include <memory>

		@@ -108,24 +109,26 @@ static bool read_pkcs7(uint8_t* pkcs7_der, size_t pkcs7_der_len, uint8_t** sig_d
		return *sig_der != NULL;
		}

		// Look for an RSA signature embedded in the .ZIP file comment given
		// the path to the zip. Verify it matches one of the given public
		// keys.
		//
		// Return VERIFY_SUCCESS, VERIFY_FAILURE (if any error is encountered
		// or no key matches the signature).

		int verify_file(unsigned char* addr, size_t length,
		const std::vector<Certificate>& keys) {
		ui->SetProgress(0.0);
		/*
		* Looks for an RSA signature embedded in the .ZIP file comment given the path to the zip. Verifies
		* that it matches one of the given public keys. A callback function can be optionally provided for
		* posting the progress.
		*
		* Returns VERIFY_SUCCESS or VERIFY_FAILURE (if any error is encountered or no key matches the
		* signature).
		*/
		int verify_file(unsigned char* addr, size_t length, const std::vector<Certificate>& keys,
		const std::function<void(float)>& set_progress) {
		if (set_progress) {
		set_progress(0.0);
		}

		// An archive with a whole-file signature will end in six bytes:
		//
		// (2-byte signature start) $ff $ff (2-byte comment size)
		//
		// (As far as the ZIP format is concerned, these are part of the
		// archive comment.) We start by reading this footer, this tells
		// us how far back from the end we have to start reading to find
		// (As far as the ZIP format is concerned, these are part of the archive comment.) We start by
		// reading this footer, this tells us how far back from the end we have to start reading to find
		// the whole comment.

		#define FOOTER_SIZE 6
		@@ -154,8 +157,7 @@ int verify_file(unsigned char* addr, size_t length,

		#define EOCD_HEADER_SIZE 22

		// The end-of-central-directory record is 22 bytes plus any
		// comment length.
		// The end-of-central-directory record is 22 bytes plus any comment length.
		size_t eocd_size = comment_size + EOCD_HEADER_SIZE;

		if (length < eocd_size) {
		@@ -163,29 +165,24 @@ int verify_file(unsigned char* addr, size_t length,
		return VERIFY_FAILURE;
		}

		// Determine how much of the file is covered by the signature.
		// This is everything except the signature data and length, which
		// includes all of the EOCD except for the comment length field (2
		// bytes) and the comment data.
		// Determine how much of the file is covered by the signature. This is everything except the
		// signature data and length, which includes all of the EOCD except for the comment length field
		// (2 bytes) and the comment data.
		size_t signed_len = length - eocd_size + EOCD_HEADER_SIZE - 2;

		unsigned char* eocd = addr + length - eocd_size;

		// If this is really is the EOCD record, it will begin with the
		// magic number $50 $4b $05 $06.
		if (eocd[0] != 0x50 \|\| eocd[1] != 0x4b \|\|
		eocd[2] != 0x05 \|\| eocd[3] != 0x06) {
		// If this is really is the EOCD record, it will begin with the magic number $50 $4b $05 $06.
		if (eocd[0] != 0x50 \|\| eocd[1] != 0x4b \|\| eocd[2] != 0x05 \|\| eocd[3] != 0x06) {
		LOG(ERROR) << "signature length doesn't match EOCD marker";
		return VERIFY_FAILURE;
		}

		for (size_t i = 4; i < eocd_size-3; ++i) {
		if (eocd[i ] == 0x50 && eocd[i+1] == 0x4b &&
		eocd[i+2] == 0x05 && eocd[i+3] == 0x06) {
		// if the sequence $50 $4b $05 $06 appears anywhere after
		// the real one, libziparchive will find the later (wrong) one,
		// which could be exploitable. Fail verification if
		// this sequence occurs anywhere after the real one.
		if (eocd[i ] == 0x50 && eocd[i+1] == 0x4b && eocd[i+2] == 0x05 && eocd[i+3] == 0x06) {
		// If the sequence $50 $4b $05 $06 appears anywhere after the real one, libziparchive will
		// find the later (wrong) one, which could be exploitable. Fail the verification if this
		// sequence occurs anywhere after the real one.
		LOG(ERROR) << "EOCD marker occurs after start of EOCD";
		return VERIFY_FAILURE;
		}
		@@ -217,12 +214,14 @@ int verify_file(unsigned char* addr, size_t length,
		if (need_sha256) SHA256_Update(&sha256_ctx, addr + so_far, size);
		so_far += size;

		if (set_progress) {
		double f = so_far / (double)signed_len;
		if (f > frac + 0.02 \|\| size == so_far) {
		ui->SetProgress(f);
		set_progress(f);
		frac = f;
		}
		}
		}

		uint8_t sha1[SHA_DIGEST_LENGTH];
		SHA1_Final(sha1, &sha1_ctx);
		@@ -243,11 +242,8 @@ int verify_file(unsigned char* addr, size_t length,
		return VERIFY_FAILURE;
		}

		/*
		* Check to make sure at least one of the keys matches the signature. Since
		* any key can match, we need to try each before determining a verification
		* failure has happened.
		*/
		// Check to make sure at least one of the keys matches the signature. Since any key can match,
		// we need to try each before determining a verification failure has happened.
		size_t i = 0;
		for (const auto& key : keys) {
		const uint8_t* hash;
		@@ -265,11 +261,10 @@ int verify_file(unsigned char* addr, size_t length,
		continue;
		}

		// The 6 bytes is the "(signature_start) $ff $ff (comment_size)" that
		// the signing tool appends after the signature itself.
		// The 6 bytes is the "(signature_start) $ff $ff (comment_size)" that the signing tool appends
		// after the signature itself.
		if (key.key_type == Certificate::KEY_TYPE_RSA) {
		if (!RSA_verify(hash_nid, hash, key.hash_len, sig_der,
		sig_der_length, key.rsa.get())) {
		if (!RSA_verify(hash_nid, hash, key.hash_len, sig_der, sig_der_length, key.rsa.get())) {
		LOG(INFO) << "failed to verify against RSA key " << i;
		continue;
		}
		@@ -277,10 +272,8 @@ int verify_file(unsigned char* addr, size_t length,
		LOG(INFO) << "whole-file signature verified against RSA key " << i;
		free(sig_der);
		return VERIFY_SUCCESS;
		} else if (key.key_type == Certificate::KEY_TYPE_EC
		&& key.hash_len == SHA256_DIGEST_LENGTH) {
		if (!ECDSA_verify(0, hash, key.hash_len, sig_der,
		sig_der_length, key.ec.get())) {
		} else if (key.key_type == Certificate::KEY_TYPE_EC && key.hash_len == SHA256_DIGEST_LENGTH) {
		if (!ECDSA_verify(0, hash, key.hash_len, sig_der, sig_der_length, key.ec.get())) {
		LOG(INFO) << "failed to verify against EC key " << i;
		continue;
		}

verifier.h

+8 −6

Original line number	Diff line number	Diff line
		@@ -17,6 +17,7 @@
		#ifndef _RECOVERY_VERIFIER_H
		#define _RECOVERY_VERIFIER_H

		#include <functional>
		#include <memory>
		#include <vector>

		@@ -58,13 +59,14 @@ struct Certificate {
		std::unique_ptr<EC_KEY, ECKEYDeleter> ec;
		};

		/* addr and length define a an update package file that has been
		* loaded (or mmap'ed, or whatever) into memory. Verify that the file
		* is signed and the signature matches one of the given keys. Return
		* one of the constants below.
		/*
		* 'addr' and 'length' define an update package file that has been loaded (or mmap'ed, or
		* whatever) into memory. Verifies that the file is signed and the signature matches one of the
		* given keys. It optionally accepts a callback function for posting the progress to. Returns one
		* of the constants of VERIFY_SUCCESS and VERIFY_FAILURE.
		*/
		int verify_file(unsigned char* addr, size_t length,
		const std::vector<Certificate>& keys);
		int verify_file(unsigned char* addr, size_t length, const std::vector<Certificate>& keys,
		const std::function<void(float)>& set_progress = nullptr);

		bool load_keys(const char* filename, std::vector<Certificate>& certs);