From 625c588c0f1a77bf632f56aa1cf92bb746f6f02a Mon Sep 17 00:00:00 2001 From: xunchang Date: Fri, 22 Mar 2019 09:33:49 -0700 Subject: [PATCH 1/8] Move out the code to parse block map in MemMap We will reuse them to implement the fuse provider from block maps. Test: unit tests pass, sideload an OTA Change-Id: Iaa409d19569c4ccc0bb24e12518044fcddb45c69 --- otautil/include/otautil/sysutil.h | 51 ++++++++++ otautil/sysutil.cpp | 151 ++++++++++++++++-------------- tests/unit/sysutil_test.cpp | 61 ++++++++++++ 3 files changed, 192 insertions(+), 71 deletions(-) diff --git a/otautil/include/otautil/sysutil.h b/otautil/include/otautil/sysutil.h index 2eeb7c30..692a99e9 100644 --- a/otautil/include/otautil/sysutil.h +++ b/otautil/include/otautil/sysutil.h @@ -22,6 +22,57 @@ #include #include +#include "rangeset.h" + +// This class holds the content of a block map file. +class BlockMapData { + public: + // A "block map" which looks like this (from uncrypt/uncrypt.cpp): + // + // /dev/block/platform/msm_sdcc.1/by-name/userdata # block device + // 49652 4096 # file size in bytes, block size + // 3 # count of block ranges + // 1000 1008 # block range 0 + // 2100 2102 # ... block range 1 + // 30 33 # ... block range 2 + // + // Each block range represents a half-open interval; the line "30 33" reprents the blocks + // [30, 31, 32]. + static BlockMapData ParseBlockMapFile(const std::string& block_map_path); + + explicit operator bool() const { + return !path_.empty(); + } + + std::string path() const { + return path_; + } + uint64_t file_size() const { + return file_size_; + } + uint32_t block_size() const { + return block_size_; + } + RangeSet block_ranges() const { + return block_ranges_; + } + + private: + BlockMapData() = default; + + BlockMapData(const std::string& path, uint64_t file_size, uint32_t block_size, + RangeSet block_ranges) + : path_(path), + file_size_(file_size), + block_size_(block_size), + block_ranges_(std::move(block_ranges)) {} + + std::string path_; + uint64_t file_size_ = 0; + uint32_t block_size_ = 0; + RangeSet block_ranges_; +}; + /* * Use this to keep track of mapped segments. */ diff --git a/otautil/sysutil.cpp b/otautil/sysutil.cpp index d8969a0b..8366fa0a 100644 --- a/otautil/sysutil.cpp +++ b/otautil/sysutil.cpp @@ -18,12 +18,13 @@ #include // TEMP_FAILURE_RETRY #include -#include // SIZE_MAX +#include #include #include #include #include +#include #include #include @@ -34,6 +35,68 @@ #include #include +BlockMapData BlockMapData::ParseBlockMapFile(const std::string& block_map_path) { + std::string content; + if (!android::base::ReadFileToString(block_map_path, &content)) { + LOG(ERROR) << "Failed to read " << block_map_path; + return {}; + } + + std::vector lines = android::base::Split(android::base::Trim(content), "\n"); + if (lines.size() < 4) { + LOG(ERROR) << "Block map file is too short: " << lines.size(); + return {}; + } + + const std::string& block_dev = lines[0]; + + uint64_t file_size; + uint32_t blksize; + if (sscanf(lines[1].c_str(), "%" SCNu64 "%" SCNu32, &file_size, &blksize) != 2) { + LOG(ERROR) << "Failed to parse file size and block size: " << lines[1]; + return {}; + } + + if (file_size == 0 || blksize == 0) { + LOG(ERROR) << "Invalid size in block map file: size " << file_size << ", blksize " << blksize; + return {}; + } + + size_t range_count; + if (sscanf(lines[2].c_str(), "%zu", &range_count) != 1) { + LOG(ERROR) << "Failed to parse block map header: " << lines[2]; + return {}; + } + + uint64_t blocks = ((file_size - 1) / blksize) + 1; + if (blocks > std::numeric_limits::max() || range_count == 0 || + lines.size() != 3 + range_count) { + LOG(ERROR) << "Invalid data in block map file: size " << file_size << ", blksize " << blksize + << ", range_count " << range_count << ", lines " << lines.size(); + return {}; + } + + RangeSet ranges; + uint64_t remaining_blocks = blocks; + for (size_t i = 0; i < range_count; ++i) { + const std::string& line = lines[i + 3]; + uint64_t start, end; + if (sscanf(line.c_str(), "%" SCNu64 "%" SCNu64, &start, &end) != 2) { + LOG(ERROR) << "failed to parse range " << i << ": " << line; + return {}; + } + uint64_t range_blocks = end - start; + if (end <= start || range_blocks > remaining_blocks) { + LOG(ERROR) << "Invalid range: " << start << " " << end; + return {}; + } + ranges.PushBack({ start, end }); + remaining_blocks -= range_blocks; + } + + return BlockMapData(block_dev, file_size, blksize, std::move(ranges)); +} + bool MemMapping::MapFD(int fd) { struct stat sb; if (fstat(fd, &sb) == -1) { @@ -55,115 +118,61 @@ bool MemMapping::MapFD(int fd) { return true; } -// A "block map" which looks like this (from uncrypt/uncrypt.cpp): -// -// /dev/block/platform/msm_sdcc.1/by-name/userdata # block device -// 49652 4096 # file size in bytes, block size -// 3 # count of block ranges -// 1000 1008 # block range 0 -// 2100 2102 # ... block range 1 -// 30 33 # ... block range 2 -// -// Each block range represents a half-open interval; the line "30 33" reprents the blocks -// [30, 31, 32]. bool MemMapping::MapBlockFile(const std::string& filename) { - std::string content; - if (!android::base::ReadFileToString(filename, &content)) { - PLOG(ERROR) << "Failed to read " << filename; + auto block_map_data = BlockMapData::ParseBlockMapFile(filename); + if (!block_map_data) { return false; } - std::vector lines = android::base::Split(android::base::Trim(content), "\n"); - if (lines.size() < 4) { - LOG(ERROR) << "Block map file is too short: " << lines.size(); - return false; - } - - size_t size; - size_t blksize; - if (sscanf(lines[1].c_str(), "%zu %zu", &size, &blksize) != 2) { - LOG(ERROR) << "Failed to parse file size and block size: " << lines[1]; - return false; - } - - size_t range_count; - if (sscanf(lines[2].c_str(), "%zu", &range_count) != 1) { - LOG(ERROR) << "Failed to parse block map header: " << lines[2]; - return false; - } - - size_t blocks; - if (blksize != 0) { - blocks = ((size - 1) / blksize) + 1; - } - if (size == 0 || blksize == 0 || blocks > SIZE_MAX / blksize || range_count == 0 || - lines.size() != 3 + range_count) { - LOG(ERROR) << "Invalid data in block map file: size " << size << ", blksize " << blksize - << ", range_count " << range_count << ", lines " << lines.size(); + if (block_map_data.file_size() > std::numeric_limits::max()) { + LOG(ERROR) << "File size is too large for mmap " << block_map_data.file_size(); return false; } // Reserve enough contiguous address space for the whole file. + uint32_t blksize = block_map_data.block_size(); + uint64_t blocks = ((block_map_data.file_size() - 1) / blksize) + 1; void* reserve = mmap(nullptr, blocks * blksize, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0); if (reserve == MAP_FAILED) { PLOG(ERROR) << "failed to reserve address space"; return false; } - const std::string& block_dev = lines[0]; - android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(block_dev.c_str(), O_RDONLY))); + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(block_map_data.path().c_str(), O_RDONLY))); if (fd == -1) { - PLOG(ERROR) << "failed to open block device " << block_dev; + PLOG(ERROR) << "failed to open block device " << block_map_data.path(); munmap(reserve, blocks * blksize); return false; } ranges_.clear(); - unsigned char* next = static_cast(reserve); + auto next = static_cast(reserve); size_t remaining_size = blocks * blksize; - bool success = true; - for (size_t i = 0; i < range_count; ++i) { - const std::string& line = lines[i + 3]; - - size_t start, end; - if (sscanf(line.c_str(), "%zu %zu\n", &start, &end) != 2) { - LOG(ERROR) << "failed to parse range " << i << ": " << line; - success = false; - break; - } + for (const auto& [start, end] : block_map_data.block_ranges()) { size_t range_size = (end - start) * blksize; - if (end <= start || (end - start) > SIZE_MAX / blksize || range_size > remaining_size) { - LOG(ERROR) << "Invalid range: " << start << " " << end; - success = false; - break; - } - void* range_start = mmap(next, range_size, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd, static_cast(start) * blksize); if (range_start == MAP_FAILED) { - PLOG(ERROR) << "failed to map range " << i << ": " << line; - success = false; - break; + PLOG(ERROR) << "failed to map range " << start << ": " << end; + munmap(reserve, blocks * blksize); + return false; } ranges_.emplace_back(MappedRange{ range_start, range_size }); next += range_size; remaining_size -= range_size; } - if (success && remaining_size != 0) { + if (remaining_size != 0) { LOG(ERROR) << "Invalid ranges: remaining_size " << remaining_size; - success = false; - } - if (!success) { munmap(reserve, blocks * blksize); return false; } addr = static_cast(reserve); - length = size; + length = block_map_data.file_size(); - LOG(INFO) << "mmapped " << range_count << " ranges"; + LOG(INFO) << "mmapped " << block_map_data.block_ranges().size() << " ranges"; return true; } diff --git a/tests/unit/sysutil_test.cpp b/tests/unit/sysutil_test.cpp index 77625dbe..3466e8ee 100644 --- a/tests/unit/sysutil_test.cpp +++ b/tests/unit/sysutil_test.cpp @@ -17,8 +17,10 @@ #include #include +#include #include +#include "otautil/rangeset.h" #include "otautil/sysutil.h" TEST(SysUtilTest, InvalidArgs) { @@ -28,6 +30,65 @@ TEST(SysUtilTest, InvalidArgs) { ASSERT_FALSE(mapping.MapFile("")); } +TEST(SysUtilTest, ParseBlockMapFile_smoke) { + std::vector content = { + "/dev/abc", "49652 4096", "3", "1000 1008", "2100 2102", "30 33", + }; + + TemporaryFile temp_file; + ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path)); + + auto block_map_data = BlockMapData::ParseBlockMapFile(temp_file.path); + ASSERT_EQ("/dev/abc", block_map_data.path()); + ASSERT_EQ(49652, block_map_data.file_size()); + ASSERT_EQ(4096, block_map_data.block_size()); + ASSERT_EQ(RangeSet(std::vector{ + { 1000, 1008 }, + { 2100, 2102 }, + { 30, 33 }, + }), + block_map_data.block_ranges()); +} + +TEST(SysUtilTest, ParseBlockMapFile_invalid_line_count) { + std::vector content = { + "/dev/abc", "49652 4096", "2", "1000 1008", "2100 2102", "30 33", + }; + + TemporaryFile temp_file; + ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path)); + + auto block_map_data1 = BlockMapData::ParseBlockMapFile(temp_file.path); + ASSERT_FALSE(block_map_data1); +} + +TEST(SysUtilTest, ParseBlockMapFile_invalid_size) { + std::vector content = { + "/dev/abc", + "42949672950 4294967295", + "1", + "0 9", + }; + + TemporaryFile temp_file; + ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path)); + + auto block_map_data = BlockMapData::ParseBlockMapFile(temp_file.path); + ASSERT_EQ("/dev/abc", block_map_data.path()); + ASSERT_EQ(42949672950, block_map_data.file_size()); + ASSERT_EQ(4294967295, block_map_data.block_size()); + + content[1] = "42949672950 4294967296"; + ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path)); + auto large_block_size = BlockMapData::ParseBlockMapFile(temp_file.path); + ASSERT_FALSE(large_block_size); + + content[1] = "4294967296 1"; + ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path)); + auto too_many_blocks = BlockMapData::ParseBlockMapFile(temp_file.path); + ASSERT_FALSE(too_many_blocks); +} + TEST(SysUtilTest, MapFileRegularFile) { TemporaryFile temp_file1; std::string content = "abc"; From 908ad77af8731c0e8a7645575ea71fbbdd404d41 Mon Sep 17 00:00:00 2001 From: xunchang Date: Tue, 26 Mar 2019 09:54:34 -0700 Subject: [PATCH 2/8] Allow RSA 4096 key in package verification The RSA_verify sitll works for 4096 bits keys. And we just need to loose the check on modulus. Sample commands to generate the key & package: 1. openssl genrsa -out keypair.pem 4096 2. openssl pkcs8 -topk8 -inform PEM -outform DER -nocrypt \ -in keypair.pem -out private.pk8 3. openssl req -new -x509 -key keypair.pem -out public.x509.pem \ -days 365 4. java -Djava.library.path=prebuilts/sdk/tools/linux/lib64 -jar \ prebuilts/sdk/tools/lib/signapk.jar -w public.x509.pem private.pk8 \ unsigned.zip signed.zip Bug: 129163830 Test: unit tests pass Change-Id: I5a5ff539c9ff1955c02ec2ce4b17563cb92808a4 --- tests/component/verifier_test.cpp | 11 +++++++ tests/testdata/otasigned_4096bits.zip | Bin 0 -> 4055 bytes tests/testdata/testkey_4096bits.x509.pem | 35 +++++++++++++++++++++++ verifier.cpp | 4 +-- verifier.h | 3 +- 5 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 tests/testdata/otasigned_4096bits.zip create mode 100644 tests/testdata/testkey_4096bits.x509.pem diff --git a/tests/component/verifier_test.cpp b/tests/component/verifier_test.cpp index c904cd03..bdb8af23 100644 --- a/tests/component/verifier_test.cpp +++ b/tests/component/verifier_test.cpp @@ -158,6 +158,17 @@ TEST(VerifierTest, LoadCertificateFromBuffer_sha256_ec256bits) { VerifyPackageWithSingleCertificate("otasigned_v5.zip", std::move(cert)); } +TEST(VerifierTest, LoadCertificateFromBuffer_sha256_rsa4096_bits) { + Certificate cert(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr); + LoadKeyFromFile(from_testdata_base("testkey_4096bits.x509.pem"), &cert); + + ASSERT_EQ(SHA256_DIGEST_LENGTH, cert.hash_len); + ASSERT_EQ(Certificate::KEY_TYPE_RSA, cert.key_type); + ASSERT_EQ(nullptr, cert.ec); + + VerifyPackageWithSingleCertificate("otasigned_4096bits.zip", std::move(cert)); +} + TEST(VerifierTest, LoadCertificateFromBuffer_check_rsa_keys) { std::unique_ptr rsa(RSA_new()); std::unique_ptr exponent(BN_new(), BN_free); diff --git a/tests/testdata/otasigned_4096bits.zip b/tests/testdata/otasigned_4096bits.zip new file mode 100644 index 0000000000000000000000000000000000000000..5016dfc9a7b72782f7ba0392ed9dc78f7cf5b025 GIT binary patch literal 4055 zcmd6oc{tQv|Ho%D%-CXNnTRYAnP%+!z7J!h6e2SA>@z}^O!jP9#w|NdmQWEgkyH{< zL}jTEvSp7+wr5<=egE#B-|zQ4e?RAYUEj|+@9+C_u5*3QIiD9E3x>b}02TnF9aB4a z|J8s1H~_QP4FD((gBTi~VdMd@upBu5)hNhNEDX=+%9`SgPPzE`oO1Rh1p0XrPWcg? zU0ef+1r*ce$12=$%i&4Q#ai|)L+*Y!(A#Dq1d}pGSVnx#Td7{M9YwrTgxK%zO@6U;P5W)P)oQB?7RnDmZ zs@Y!kudsla^So49h{-wqn4-KGYZC+hyK8O+6Da1{$FO33TTou2z;tfsupP&zENEb| z!OrXBzIj3~+-dx+-bd@%x!5BvF3K5Q+nt$m8}RQTP5T$h{nQ`Zvy*%ph4eK6X{TKV zoML|$HSD@-$M&V%>DL?N_Xd`yv3RBVMV(N6^^h`~FN-=|ZAjN9Cu7|$zeP#9<>jx- z{`)VE?{OS-_hO?=B2CA6Y2W ziM3=m8?NEYHIzHauhvsq5py_~sN%qvd`Dk8N8QwLC|R_G#-c9#K<-&S*1N+#sL;nV zb*k`Mp~VqbE_m&0c%sztQ3CZ$=EB16(Aq{8O8#8I%z!?#dAaqy>IFBt{&J$gkK`AR zYJ!-{Vp~(M-r%x<8CXwIw7e%W1-IJMmg64eE*W%Ry=^}3Z-E;bY~tIctY*~ZWf4rg!V5Iie^@ z1GH;&>?PqTQoo4s`uqAj0%rX&ZnwCD`0veHbZ%+2YC`GQ0Ic(hbKUosxMs^nytvoG zCy^6{sc#ybwLso?Nsj`!Q&$l<*DI5xc6;SYK57~w;2||7Un0|``xz>Xu$=0zoMbMn z+Y$tw;XO<{*^Z4$k-K_gI8r5LID|Sq{#0bKSFmN>u(LK?G>$wAvh69>-Q!J0-)JWE+Y$~Ge|Iz3#LnHbE1UY`gT zpn?5LzrN_kOnIf#br&l({^pRWB7@dD;o>!sezJwTo|8eL(Z$DKyi8RP?vjQCthW_> zzW8KpS~nu21E6zFG2`dm+prlf+LPgG%YA~5M-|J^lTCZsbeAvqk9g{+`E%5R z72|=4HsSY1dorush27n`X_KuF^51GYHQ2tEvq7(wpoSomfyrD+l^rTLe;BQxLhdv7 zhLwCiBG`Htaxt7+yJY&Fl%M+^iHmQ>>nOD zY0i&V=Wgh25(D2&s{M@429ee=MxQDIJS9xh-il?1-~*laOoaW$Slqr|qKyh}#+jgu z%s0JVWy=*fTlK8yw|c&%yKVNoUU(>kxBuEu7)*@cJ6~-FctbNLREeD#C2bS0Xg8f- zBXqov)ocq;S%=Fz_aB++)a55f%z9>t)h17;K4vS3$#_XqHn<|C>If2&J(u8!TCG7S zrNMtRFTAt#@`xT=`9U|+(+nj*qp)K=)L8b2r_t*9!pClE#q>}AaeRJ71_Fx+xv}GB zr`57JG=zH+!TSF83`Y;ma3`93L+pfvrqD5 zb!|T&{_7-Fy!)BFN{qkgaz7JT8B%raM0?x0l98tTYYtn!SKYY{|`+ zG1iJma;a4^xqv*5T&`Y%mg=~c4jFBCmotN?HS0r>`vpJGqFyQOEOUS5#ABIRoK`Y_ zl`sJS7a8>(j|GC@jOzdIsPGpd{u%#;5kL?R;BQL&KIs7^{%$82-T%Z4JeCQ1;07_C z>x{_57)ir|Jl%a=2_lz5MXVV;U4L%?nhfKB!ldI&;^^!^W>7JiWgksu*#-fD7)~@h z6y_9X0(1pKKtLt{n)PoZu$T;mpvh21K6thm3<82cFu)%-G4><*uE>MDKW7B8GV-Fc zFsx`86nqu}MS#v)V-RSrg8>U0T;KV!r<-4(uctGH7tM1ZGPAK;_>p{x&Yr#^HlD6m zFo)0_2RfXMS@)k*{7Yf|KMMG7MHDUaKWl&ic@cjSRp-x$Vo(-+l#d;wf)Ar zMT{LEyMhv09;5u{@V|B0WRU3ZKbWyGU^0l4u@N9RhztS(TE}CGHI8U|sZ5<;1M^S9 zbVvFUPbzYZ^c%^ZZaDpik4|@_ld=51A^ybKtE8p%IY$VF!*VcqRd7MfzjD9VUt%JG zsi|@6CgquLrrG#$X0D$eU$ly-H@v3Dyw>UfBYFzWx#AT&4ZVSxIZlmQ@)e-KJ|kQJ z(;29|gLeB3;bjT>Jb_Z2;V<PetB8B5}tJZL^!b?;l5k1xcy$GGnZolqJg;z-m57 z$OWvlAw2qOMucE-tflow+)DXVwBWkr)5z>f9Ch{p0iXCIJ6L>tK54AtB zdFvz6M-{;Jvy!LT`;{(yJ``mvtbE4!46dlm*@st>%g{a+I>t8qvTw2F8~3&`CzTf5 zxiPA1yae~WP+1<+L$a-&>kvw(eE905ur^G&^p%pahi(sVb3#j0ewuX}Mg3H8AAFvk zZp#X$c8SiW_O}73kdvwB3b02bv52jqan-{$`2=lt?Mn|oyv+#JY%&iUToRpoF)^b; z@so55A#rIpqQir?kaB!^?e$4>gC(7YC5)Zi1peT#7u-&tXx*x0D*ZCE1?N5F?(brn zxT(&uLq@>W>1Xe6m+4m=-?bWIYQA5Sq<{T62n+-O%dF8>Xd%YM5#oUGLU=a0Xd_(# zB`#FG))UwUL|}^Fg%Gsx!DlfDKbrU2|8L+tFd%@yeFzg8%m}|9JI1|c96KQ3v02Bh z@=wpDy>GrN<8iN+Xs&ekVn2rWaUl>nn`*k%OTQ@$a5odrEf`(6OQpO4z3Gnmd9-BM zJ+;freRP6&D3Fwl@sX7b({E6fCuOig5=+U5W02_r)f1|{ z;}oGF`W_guU<4qhQlG@USgTjD>d076bE`b%hPpp;Ye-|Og)|dc&JnC$1=Xi^Fo3)Gq>z)PkRUtwEKShz(eH?`(h(w$KJX3*6Otibna1UJ)N>zmdNb-$snl8h6$5+ zCrJ-&r(snwR3;-6DYdf~;XXdK_RE!+xUCRv?TV6(8fPi}Hb*V+Q9pgFp3e61^MugG z(ZQC)ZjM`zY6HU-bCK7Tm?mK9cE^V7%c+(jQ%U{C)p1WF=+7!Wh{vMl^wPU$JB`7VBLyh#L|BdTCN%EBjGpOH4OGezivpDMNSh&tYMChf0$GTcVvNpE zmXTAv5isk?TK-&S0;?LBB8f8#sI0DSPB4yoY9qn(NWi@$OY70_>ATl;i*UaX*c-Io ZyyTYD&W~T}^NvWhUv}e%?e9y&{s9fSzaRhr literal 0 HcmV?d00001 diff --git a/tests/testdata/testkey_4096bits.x509.pem b/tests/testdata/testkey_4096bits.x509.pem new file mode 100644 index 00000000..cba30d61 --- /dev/null +++ b/tests/testdata/testkey_4096bits.x509.pem @@ -0,0 +1,35 @@ +-----BEGIN CERTIFICATE----- +MIIGADCCA+igAwIBAgIJAJiRMVvanGUaMA0GCSqGSIb3DQEBCwUAMIGUMQswCQYD +VQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNTW91bnRhaW4g +VmlldzEQMA4GA1UECgwHQW5kcm9pZDEQMA4GA1UECwwHQW5kcm9pZDEQMA4GA1UE +AwwHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTAe +Fw0xODEwMzAxMjEzNTFaFw00NjAzMTcxMjEzNTFaMIGUMQswCQYDVQQGEwJVUzET +MBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNTW91bnRhaW4gVmlldzEQMA4G +A1UECgwHQW5kcm9pZDEQMA4GA1UECwwHQW5kcm9pZDEQMA4GA1UEAwwHQW5kcm9p +ZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTCCAiIwDQYJKoZI +hvcNAQEBBQADggIPADCCAgoCggIBAL3ghKA8Gz9qOORY8gMY4wlB2tCJLDUO2tFG +LVK1UphtQMp+YEcz/0VQKVV7de7z6V4EMQ5P1HbxHOsjcKn/zXAl4YgFt7b5kZbC +bpNK4CYHEfho3j6fpYtq5d9q8rIA2kI0uZkkqPy1zXKTl2C2PjOoAnLQRk5xBVQG +M10/wYsf7yX36mSWoJJwKPp/EzVFpA+hX8HpljeIiZ6CFzKwJdqv9zO/xzfp6NsX +Tv5EGdkDxmw3qQqKgyl8dLMTZ/2zNfvVOMeZDusEPDF7A/lbU1byLWrKQdCzVb40 +yc7BCSRGYwM29R/byOcgD+lslwKSGzgzNmQXICt1tXz9bSJR8qh4tlAaiRc3ZKBe +hJWIFGkGtD/cDGtDE5DbNAOz6CdSDdE2XN0Qf0cfN1RHVE6fo2FtFicRRVuFBt8M +2cbQ7bzmEvtHD6W6dsf120FH7gppXKmnhMx1WazpxR2QltbiYDTy2ZZi4paS/jDB +fL9gMCWp3Ohg2y74NGfUw5CQWQsDpcki6I7RvwClBCyOV51LHn5LE/nY4DkVrZxk +Pw0/YrTWz5J5PbdMetTuIunE4ec4lm8nZnh1ET+2MHx2+RoyF5vBs4rp1KHHRaEA +veD2AfQOWxz7kOG9+akFot7n+QoWEGdwY0mJ9jsO/IITCjv3VbD7o0OoJv1R2AW5 +sK2KQ4PDAgMBAAGjUzBRMB0GA1UdDgQWBBT2EbrayXGhY6VCvSlLtRNyjW9ceDAf +BgNVHSMEGDAWgBT2EbrayXGhY6VCvSlLtRNyjW9ceDAPBgNVHRMBAf8EBTADAQH/ +MA0GCSqGSIb3DQEBCwUAA4ICAQC7SsWap9zDKmuR0qMUZ6wlualnag0hUG1jZHQP +t63KO6LmNNMSuXRX60Zcq6WWzgLOyoT4HqHZZ47Jamfb4XQQcnWMMW0tJ3pDtTkz +dZILBInHJO8QPYI8Du6XWsDLSvMajq6ueBtO3NdcgsNL7eiHf3WoOtajLZxFM94Z +MESkUQOIsqHolYeTMHLTsuGkX1CK2Zw3Xn18bUSTYwZCHa6mYH00ItUBfetGCnWh +Y7bth/R15Cc+hocSB7ZsOa/R5kDyDdFDIKrnV5nH5Yd7CryrYC6Ac5UarYrxSJTq +eKPwqUlJB/tJW/lvdLt8YaURbFGzf/ZqU12zZRafYjmMjcQvfpzMoDSnbvHTA9IR +ZGO7dwhwykoSaL4/8LWde49xQUq6F2pQBRmEr+7mTzml1MaM5cWEk5emkCMXgLog +k+c56CAk1EdM1teWik7wR0TIqkkYyYJHTSg61GkXUIXrZJ6iYx2ejDg1+QTPm9rU +Yr7nP52gVkQuUAX1+xB6wKLSDizQJw8SNiUGXl5+2vwV6+0BI3/CXlQ8I/nRPBC1 +oqOIkRSbE+IF7DP9QvYuNG/3bZZQ8LUVeHxqI5Mq8K2VIJZd95AIwPNMH34SaDGz +9xjG28Fq4ZkuDP0pCsHM9d2XEwK5PEVS18WW5fJ/QcJKMno4IPTB70ZBBjVzv6Y+ +MYjOrw== +-----END CERTIFICATE----- diff --git a/verifier.cpp b/verifier.cpp index 68a011e0..08d852b3 100644 --- a/verifier.cpp +++ b/verifier.cpp @@ -373,8 +373,8 @@ bool CheckRSAKey(const std::unique_ptr& rsa) { const BIGNUM* out_e; RSA_get0_key(rsa.get(), &out_n, &out_e, nullptr /* private exponent */); auto modulus_bits = BN_num_bits(out_n); - if (modulus_bits != 2048) { - LOG(ERROR) << "Modulus should be 2048 bits long, actual: " << modulus_bits; + if (modulus_bits != 2048 && modulus_bits != 4096) { + LOG(ERROR) << "Modulus should be 2048 or 4096 bits long, actual: " << modulus_bits; return false; } diff --git a/verifier.h b/verifier.h index 106b86b8..ef9feaff 100644 --- a/verifier.h +++ b/verifier.h @@ -88,7 +88,8 @@ class VerifierInterface { // VERIFY_FAILURE (if any error is encountered or no key matches the signature). int verify_file(VerifierInterface* package, const std::vector& keys); -// Checks that the RSA key has a modulus of 2048 bits long, and public exponent is 3 or 65537. +// Checks that the RSA key has a modulus of 2048 or 4096 bits long, and public exponent is 3 or +// 65537. bool CheckRSAKey(const std::unique_ptr& rsa); // Checks that the field size of the curve for the EC key is 256 bits. From fd7d835fd9eba3b87f9d5f287801082bf93b9962 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 27 Mar 2019 23:25:54 -0700 Subject: [PATCH 3/8] Remove the extern declaration of `sehandle` from roots.cpp. It has become obsolete since we replaced the call to make_ext4 with e2fsprogs (commit ded2dac082fd703f1cd7a5c3de59450cd3dc2530, which landed into P). Test: mmma -j bootable/recovery Change-Id: I09141322874213dcb0f1280bba239376e71a4d17 --- roots.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/roots.cpp b/roots.cpp index 7a922b8e..c42723f0 100644 --- a/roots.cpp +++ b/roots.cpp @@ -51,8 +51,6 @@ using android::fs_mgr::ReadDefaultFstab; static Fstab fstab; -extern struct selabel_handle* sehandle; - void load_volume_table() { if (!ReadDefaultFstab(&fstab)) { LOG(ERROR) << "Failed to read default fstab"; From 8bd6f455d261fa62a20321a426723c491903a82e Mon Sep 17 00:00:00 2001 From: Bernie Innocenti Date: Thu, 28 Mar 2019 15:48:08 +0900 Subject: [PATCH 4/8] Fix bogus error checking on unique_fd The expression "!fd" calls the implicit conversion to int, but comparing the raw fd against 0 does not work, since open() and other POSIX calls returning a file descriptor use -1 to signal an error. Test: m recovery Change-Id: I0847c276f39cb9dd09c7ffb96951276113418fc8 --- applypatch/applypatch.cpp | 2 +- fuse_sideload/fuse_sideload.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index f9383dde..90d8e860 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -76,7 +76,7 @@ static bool ReadPartitionToBuffer(const Partition& partition, FileContents* out, } android::base::unique_fd dev(open(partition.name.c_str(), O_RDONLY)); - if (!dev) { + if (dev == -1) { PLOG(ERROR) << "Failed to open eMMC partition \"" << partition << "\""; } else { std::vector buffer(partition.size); diff --git a/fuse_sideload/fuse_sideload.cpp b/fuse_sideload/fuse_sideload.cpp index b5b6ac15..3d948030 100644 --- a/fuse_sideload/fuse_sideload.cpp +++ b/fuse_sideload/fuse_sideload.cpp @@ -392,7 +392,7 @@ int run_fuse_sideload(std::unique_ptr&& provider, const char* } fd.ffd.reset(open("/dev/fuse", O_RDWR)); - if (!fd.ffd) { + if (fd.ffd == -1) { perror("open /dev/fuse"); result = -1; goto done; From 08ba1ad9b1a6406a67feddeff211387225ccd603 Mon Sep 17 00:00:00 2001 From: Bill Peckham Date: Thu, 28 Mar 2019 18:42:13 -0700 Subject: [PATCH 5/8] Use flags = 0 to avoid fd closing for child updater process If we use the default parameter we'll get O_CLOEXEC, which will close the fd that we pass to the child process, which will cause it to fail. Passing zero avoids the problem. Bug: 122813742 Test: Verify that non-A/B OTA update is successful. Change-Id: Ia61bc7260f17d9209715583e6ded69e1196ed3f6 --- install.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/install.cpp b/install.cpp index b7fb7887..ffa39e46 100644 --- a/install.cpp +++ b/install.cpp @@ -342,7 +342,9 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b // The updater in child process writes to the pipe to communicate with recovery. android::base::unique_fd pipe_read, pipe_write; - if (!android::base::Pipe(&pipe_read, &pipe_write)) { + // Explicitly disable O_CLOEXEC using 0 as the flags (last) parameter to Pipe + // so that the child updater process will recieve a non-closed fd. + if (!android::base::Pipe(&pipe_read, &pipe_write, 0)) { PLOG(ERROR) << "Failed to create pipe for updater-recovery communication"; return INSTALL_CORRUPT; } From 2478885f3ca47fe2c4073df1100f7bd6ad4931af Mon Sep 17 00:00:00 2001 From: xunchang Date: Fri, 22 Mar 2019 16:08:52 -0700 Subject: [PATCH 6/8] Move install to separate module Build libinstall as a shared library. Also drop the dependency on the global variables in common.h. Test: unit tests pass, sideload an OTA Change-Id: I30a20047768ce00689fc0e7851c1c5d712a365a0 --- Android.bp | 73 +---------------- common.h | 1 - fsck_unshare_blocks.cpp | 2 +- install/Android.bp | 78 +++++++++++++++++++ adb_install.cpp => install/adb_install.cpp | 14 ++-- asn1_decoder.cpp => install/asn1_decoder.cpp | 4 +- .../fuse_sdcard_install.cpp | 8 +- .../include/install/adb_install.h | 7 +- .../include/install/fuse_sdcard_install.h | 0 .../include/install/install.h | 12 ++- .../include/install/package.h | 0 .../include/install/verifier.h | 38 ++++----- .../include/private/asn1_decoder.h | 1 + .../include/private/setup_commands.h | 0 install.cpp => install/install.cpp | 32 ++++---- package.cpp => install/package.cpp | 2 +- verifier.cpp => install/verifier.cpp | 37 +++++---- logging.cpp | 2 +- otautil/Android.bp | 11 +++ roots.h => otautil/include/otautil/roots.h | 5 +- roots.cpp => otautil/roots.cpp | 8 +- recovery.cpp | 21 ++--- recovery_main.cpp | 2 +- tests/Android.bp | 7 +- tests/component/install_test.cpp | 4 +- tests/component/verifier_test.cpp | 4 +- tests/unit/asn1_decoder_test.cpp | 2 +- tests/unit/package_test.cpp | 2 +- 28 files changed, 194 insertions(+), 183 deletions(-) create mode 100644 install/Android.bp rename adb_install.cpp => install/adb_install.cpp (92%) rename asn1_decoder.cpp => install/asn1_decoder.cpp (98%) rename fuse_sdcard_install.cpp => install/fuse_sdcard_install.cpp (97%) rename adb_install.h => install/include/install/adb_install.h (86%) rename fuse_sdcard_install.h => install/include/install/fuse_sdcard_install.h (100%) rename install.h => install/include/install/install.h (92%) rename package.h => install/include/install/package.h (100%) rename verifier.h => install/include/install/verifier.h (80%) rename asn1_decoder.h => install/include/private/asn1_decoder.h (98%) rename private/install.h => install/include/private/setup_commands.h (100%) rename install.cpp => install/install.cpp (96%) rename package.cpp => install/package.cpp (99%) rename verifier.cpp => install/verifier.cpp (93%) rename roots.h => otautil/include/otautil/roots.h (96%) rename roots.cpp => otautil/roots.cpp (98%) diff --git a/Android.bp b/Android.bp index 7b67f407..a44a2c62 100644 --- a/Android.bp +++ b/Android.bp @@ -63,32 +63,18 @@ cc_defaults { "libbootloader_message", "libcrypto", "libcutils", - "libext4_utils", "libfs_mgr", - "libfusesideload", - "libhidl-gen-utils", - "libhidlbase", - "libhidltransport", "liblog", - "libpng", - "libselinux", - "libtinyxml2", - "libutils", - "libz", "libziparchive", ], static_libs: [ "librecovery_fastboot", "libminui", - "libpackage", - "libverifier", "libotautil", // external dependencies "libhealthhalutils", - "libvintf_recovery", - "libvintf", "libfstab", ], } @@ -102,69 +88,14 @@ cc_library_static { ], srcs: [ - "adb_install.cpp", "fsck_unshare_blocks.cpp", - "fuse_sdcard_install.cpp", - "install.cpp", "recovery.cpp", - "roots.cpp", ], shared_libs: [ + "libinstall", "librecovery_ui", ], - - include_dirs: [ - "system/vold", - ], -} - -cc_library_static { - name: "libverifier", - recovery_available: true, - - defaults: [ - "recovery_defaults", - ], - - srcs: [ - "asn1_decoder.cpp", - "verifier.cpp", - ], - - shared_libs: [ - "libbase", - "libcrypto", - "libcrypto_utils", - "libziparchive", - ], - - static_libs: [ - "libotautil", - ], -} - -cc_library_static { - name: "libpackage", - recovery_available: true, - - defaults: [ - "recovery_defaults", - ], - - srcs: [ - "package.cpp", - ], - - shared_libs: [ - "libbase", - "libcrypto", - "libziparchive", - ], - - static_libs: [ - "libotautil", - ], } cc_binary { @@ -172,6 +103,7 @@ cc_binary { recovery: true, defaults: [ + "libinstall_defaults", "librecovery_defaults", ], @@ -181,6 +113,7 @@ cc_binary { ], shared_libs: [ + "libinstall", "libminadbd_services", "librecovery_ui", ], diff --git a/common.h b/common.h index 38bdd526..22b2f0a0 100644 --- a/common.h +++ b/common.h @@ -31,7 +31,6 @@ struct selabel_handle; extern struct selabel_handle* sehandle; extern RecoveryUI* ui; -extern bool modified_flash; extern bool has_cache; // The current stage, e.g. "1/2". diff --git a/fsck_unshare_blocks.cpp b/fsck_unshare_blocks.cpp index ce6940a6..e74f8ba6 100644 --- a/fsck_unshare_blocks.cpp +++ b/fsck_unshare_blocks.cpp @@ -36,7 +36,7 @@ #include #include -#include "roots.h" +#include "otautil/roots.h" static constexpr const char* SYSTEM_E2FSCK_BIN = "/system/bin/e2fsck_static"; static constexpr const char* TMP_E2FSCK_BIN = "/tmp/e2fsck.bin"; diff --git a/install/Android.bp b/install/Android.bp new file mode 100644 index 00000000..aa47990a --- /dev/null +++ b/install/Android.bp @@ -0,0 +1,78 @@ +// Copyright (C) 2019 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +cc_defaults { + name: "libinstall_defaults", + + defaults: [ + "recovery_defaults", + ], + + shared_libs: [ + "libbase", + "libbootloader_message", + "libcrypto", + "libext4_utils", + "libfs_mgr", + "libfusesideload", + "libhidl-gen-utils", + "libhidlbase", + "libhidltransport", + "liblog", + "libselinux", + "libtinyxml2", + "libutils", + "libz", + "libziparchive", + ], + + static_libs: [ + "libotautil", + + // external dependencies + "libvintf_recovery", + "libvintf", + "libfstab", + ], +} + +cc_library { + name: "libinstall", + recovery_available: true, + + defaults: [ + "libinstall_defaults", + ], + + srcs: [ + "adb_install.cpp", + "asn1_decoder.cpp", + "fuse_sdcard_install.cpp", + "install.cpp", + "package.cpp", + "verifier.cpp", + ], + + shared_libs: [ + "librecovery_ui", + ], + + export_include_dirs: [ + "include", + ], + + export_shared_lib_headers: [ + "librecovery_ui", + ], +} diff --git a/adb_install.cpp b/install/adb_install.cpp similarity index 92% rename from adb_install.cpp rename to install/adb_install.cpp index 1d19fd3a..5296ff60 100644 --- a/adb_install.cpp +++ b/install/adb_install.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "adb_install.h" +#include "install/adb_install.h" #include #include @@ -29,12 +29,16 @@ #include #include -#include "common.h" #include "fuse_sideload.h" -#include "install.h" +#include "install/install.h" #include "recovery_ui/ui.h" -int apply_from_adb(bool* wipe_cache) { +static bool SetUsbConfig(const std::string& state) { + android::base::SetProperty("sys.usb.config", state); + return android::base::WaitForProperty("sys.usb.state", state); +} + +int apply_from_adb(bool* wipe_cache, RecoveryUI* ui) { // Save the usb state to restore after the sideload operation. std::string usb_state = android::base::GetProperty("sys.usb.state", "none"); // Clean up state and stop adbd. @@ -85,7 +89,7 @@ int apply_from_adb(bool* wipe_cache) { break; } } - result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, wipe_cache, false, 0); + result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, wipe_cache, false, 0, ui); break; } diff --git a/asn1_decoder.cpp b/install/asn1_decoder.cpp similarity index 98% rename from asn1_decoder.cpp rename to install/asn1_decoder.cpp index 285214f1..2d81a6e1 100644 --- a/asn1_decoder.cpp +++ b/install/asn1_decoder.cpp @@ -14,9 +14,7 @@ * limitations under the License. */ -#include "asn1_decoder.h" - -#include +#include "private/asn1_decoder.h" int asn1_context::peek_byte() const { if (length_ == 0) { diff --git a/fuse_sdcard_install.cpp b/install/fuse_sdcard_install.cpp similarity index 97% rename from fuse_sdcard_install.cpp rename to install/fuse_sdcard_install.cpp index 1f385093..dde289f8 100644 --- a/fuse_sdcard_install.cpp +++ b/install/fuse_sdcard_install.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "fuse_sdcard_install.h" +#include "install/fuse_sdcard_install.h" #include #include @@ -35,8 +35,8 @@ #include "bootloader_message/bootloader_message.h" #include "fuse_provider.h" #include "fuse_sideload.h" -#include "install.h" -#include "roots.h" +#include "install/install.h" +#include "otautil/roots.h" static constexpr const char* SDCARD_ROOT = "/sdcard"; // How long (in seconds) we wait for the fuse-provided package file to @@ -184,7 +184,7 @@ int ApplyFromSdcard(Device* device, bool* wipe_cache, RecoveryUI* ui) { } } - result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, wipe_cache, false, 0 /*retry_count*/); + result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, wipe_cache, false, 0 /*retry_count*/, ui); break; } diff --git a/adb_install.h b/install/include/install/adb_install.h similarity index 86% rename from adb_install.h rename to install/include/install/adb_install.h index cc3ca267..dbc82450 100644 --- a/adb_install.h +++ b/install/include/install/adb_install.h @@ -14,9 +14,8 @@ * limitations under the License. */ -#ifndef _ADB_INSTALL_H -#define _ADB_INSTALL_H +#pragma once -int apply_from_adb(bool* wipe_cache); +#include -#endif +int apply_from_adb(bool* wipe_cache, RecoveryUI* ui); diff --git a/fuse_sdcard_install.h b/install/include/install/fuse_sdcard_install.h similarity index 100% rename from fuse_sdcard_install.h rename to install/include/install/fuse_sdcard_install.h diff --git a/install.h b/install/include/install/install.h similarity index 92% rename from install.h rename to install/include/install/install.h index da8aa5e3..74fb3d17 100644 --- a/install.h +++ b/install/include/install/install.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef RECOVERY_INSTALL_H_ -#define RECOVERY_INSTALL_H_ +#pragma once #include @@ -26,6 +25,7 @@ #include #include "package.h" +#include "recovery_ui/ui.h" enum InstallResult { INSTALL_SUCCESS, @@ -45,12 +45,12 @@ enum class OtaType { // Installs the given update package. If INSTALL_SUCCESS is returned and *wipe_cache is true on // exit, caller should wipe the cache partition. -int install_package(const std::string& package, bool* wipe_cache, bool needs_mount, - int retry_count); +int install_package(const std::string& package, bool* wipe_cache, bool needs_mount, int retry_count, + RecoveryUI* ui); // Verifies the package by ota keys. Returns true if the package is verified successfully, // otherwise returns false. -bool verify_package(Package* package); +bool verify_package(Package* package, RecoveryUI* ui); // Reads meta data file of the package; parses each line in the format "key=value"; and writes the // result to |metadata|. Return true if succeed, otherwise return false. @@ -67,5 +67,3 @@ bool verify_package_compatibility(ZipArchiveHandle package_zip); // Mandatory checks: ota-type, pre-device and serial number(if presents) // AB OTA specific checks: pre-build version, fingerprint, timestamp. int CheckPackageMetadata(const std::map& metadata, OtaType ota_type); - -#endif // RECOVERY_INSTALL_H_ diff --git a/package.h b/install/include/install/package.h similarity index 100% rename from package.h rename to install/include/install/package.h diff --git a/verifier.h b/install/include/install/verifier.h similarity index 80% rename from verifier.h rename to install/include/install/verifier.h index ef9feaff..f9e94758 100644 --- a/verifier.h +++ b/install/include/install/verifier.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef _RECOVERY_VERIFIER_H -#define _RECOVERY_VERIFIER_H +#pragma once #include @@ -44,25 +43,20 @@ struct ECKEYDeleter { }; struct Certificate { - typedef enum { - KEY_TYPE_RSA, - KEY_TYPE_EC, - } KeyType; + typedef enum { + KEY_TYPE_RSA, + KEY_TYPE_EC, + } KeyType; - Certificate(int hash_len_, - KeyType key_type_, - std::unique_ptr&& rsa_, - std::unique_ptr&& ec_) - : hash_len(hash_len_), - key_type(key_type_), - rsa(std::move(rsa_)), - ec(std::move(ec_)) {} + Certificate(int hash_len_, KeyType key_type_, std::unique_ptr&& rsa_, + std::unique_ptr&& ec_) + : hash_len(hash_len_), key_type(key_type_), rsa(std::move(rsa_)), ec(std::move(ec_)) {} - // SHA_DIGEST_LENGTH (SHA-1) or SHA256_DIGEST_LENGTH (SHA-256) - int hash_len; - KeyType key_type; - std::unique_ptr rsa; - std::unique_ptr ec; + // SHA_DIGEST_LENGTH (SHA-1) or SHA256_DIGEST_LENGTH (SHA-256) + int hash_len; + KeyType key_type; + std::unique_ptr rsa; + std::unique_ptr ec; }; class VerifierInterface { @@ -103,7 +97,5 @@ bool LoadCertificateFromBuffer(const std::vector& pem_content, Certific // certificates. Returns an empty list if we fail to parse any of the entries. std::vector LoadKeysFromZipfile(const std::string& zip_name); -#define VERIFY_SUCCESS 0 -#define VERIFY_FAILURE 1 - -#endif /* _RECOVERY_VERIFIER_H */ +#define VERIFY_SUCCESS 0 +#define VERIFY_FAILURE 1 diff --git a/asn1_decoder.h b/install/include/private/asn1_decoder.h similarity index 98% rename from asn1_decoder.h rename to install/include/private/asn1_decoder.h index 3e992115..e5337d9c 100644 --- a/asn1_decoder.h +++ b/install/include/private/asn1_decoder.h @@ -17,6 +17,7 @@ #ifndef ASN1_DECODER_H_ #define ASN1_DECODER_H_ +#include #include class asn1_context { diff --git a/private/install.h b/install/include/private/setup_commands.h similarity index 100% rename from private/install.h rename to install/include/private/setup_commands.h diff --git a/install.cpp b/install/install.cpp similarity index 96% rename from install.cpp rename to install/install.cpp index ffa39e46..a1124c36 100644 --- a/install.cpp +++ b/install/install.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "install.h" +#include "install/install.h" #include #include @@ -46,19 +46,22 @@ #include #include -#include "common.h" +#include "install/package.h" +#include "install/verifier.h" #include "otautil/error_code.h" #include "otautil/paths.h" +#include "otautil/roots.h" #include "otautil/sysutil.h" #include "otautil/thermalutil.h" -#include "package.h" -#include "private/install.h" +#include "private/setup_commands.h" #include "recovery_ui/ui.h" -#include "roots.h" -#include "verifier.h" using namespace std::chrono_literals; +static constexpr int kRecoveryApiVersion = 3; +// Assert the version defined in code and in Android.mk are consistent. +static_assert(kRecoveryApiVersion == RECOVERY_API_VERSION, "Mismatching recovery API versions."); + // Default allocation of progress bar segments to operations static constexpr int VERIFICATION_PROGRESS_TIME = 60; static constexpr float VERIFICATION_PROGRESS_FRACTION = 0.25; @@ -323,7 +326,7 @@ static void log_max_temperature(int* max_temperature, const std::atomic& l // If the package contains an update binary, extract it and run it. static int try_update_binary(const std::string& package, ZipArchiveHandle zip, bool* wipe_cache, std::vector* log_buffer, int retry_count, - int* max_temperature) { + int* max_temperature, RecoveryUI* ui) { std::map metadata; if (!ReadMetadataFromPackage(zip, &metadata)) { LOG(ERROR) << "Failed to parse metadata in the zip file"; @@ -569,7 +572,7 @@ bool verify_package_compatibility(ZipArchiveHandle package_zip) { static int really_install_package(const std::string& path, bool* wipe_cache, bool needs_mount, std::vector* log_buffer, int retry_count, - int* max_temperature) { + int* max_temperature, RecoveryUI* ui) { ui->SetBackground(RecoveryUI::INSTALLING_UPDATE); ui->Print("Finding update package...\n"); // Give verification half the progress bar... @@ -596,7 +599,7 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo } // Verify package. - if (!verify_package(package.get())) { + if (!verify_package(package.get(), ui)) { log_buffer->push_back(android::base::StringPrintf("error: %d", kZipVerificationFailure)); return INSTALL_CORRUPT; } @@ -620,18 +623,19 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo ui->Print("Retry attempt: %d\n", retry_count); } ui->SetEnableReboot(false); - int result = try_update_binary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature); + int result = + try_update_binary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature, ui); ui->SetEnableReboot(true); ui->Print("\n"); return result; } -int install_package(const std::string& path, bool* wipe_cache, bool needs_mount, int retry_count) { +int install_package(const std::string& path, bool* wipe_cache, bool needs_mount, int retry_count, + RecoveryUI* ui) { CHECK(!path.empty()); CHECK(wipe_cache != nullptr); - modified_flash = true; auto start = std::chrono::system_clock::now(); int start_temperature = GetMaxValueFromThermalZone(); @@ -644,7 +648,7 @@ int install_package(const std::string& path, bool* wipe_cache, bool needs_mount, result = INSTALL_ERROR; } else { result = really_install_package(path, wipe_cache, needs_mount, &log_buffer, retry_count, - &max_temperature); + &max_temperature, ui); } // Measure the time spent to apply OTA update in seconds. @@ -702,7 +706,7 @@ int install_package(const std::string& path, bool* wipe_cache, bool needs_mount, return result; } -bool verify_package(Package* package) { +bool verify_package(Package* package, RecoveryUI* ui) { static constexpr const char* CERTIFICATE_ZIP_FILE = "/system/etc/security/otacerts.zip"; std::vector loaded_keys = LoadKeysFromZipfile(CERTIFICATE_ZIP_FILE); if (loaded_keys.empty()) { diff --git a/package.cpp b/install/package.cpp similarity index 99% rename from package.cpp rename to install/package.cpp index 6c7289f3..4402f485 100644 --- a/package.cpp +++ b/install/package.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "package.h" +#include "install/package.h" #include #include diff --git a/verifier.cpp b/install/verifier.cpp similarity index 93% rename from verifier.cpp rename to install/verifier.cpp index 08d852b3..6ba1d77c 100644 --- a/verifier.cpp +++ b/install/verifier.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "verifier.h" +#include "install/verifier.h" #include #include @@ -36,8 +36,8 @@ #include #include -#include "asn1_decoder.h" #include "otautil/print_sha1.h" +#include "private/asn1_decoder.h" /* * Simple version of PKCS#7 SignedData extraction. This extracts the @@ -82,10 +82,8 @@ static bool read_pkcs7(const uint8_t* pkcs7_der, size_t pkcs7_der_len, } std::unique_ptr signed_data_seq(signed_data_app->asn1_sequence_get()); - if (signed_data_seq == nullptr || - !signed_data_seq->asn1_sequence_next() || - !signed_data_seq->asn1_sequence_next() || - !signed_data_seq->asn1_sequence_next() || + if (signed_data_seq == nullptr || !signed_data_seq->asn1_sequence_next() || + !signed_data_seq->asn1_sequence_next() || !signed_data_seq->asn1_sequence_next() || !signed_data_seq->asn1_constructed_skip_all()) { return false; } @@ -96,11 +94,8 @@ static bool read_pkcs7(const uint8_t* pkcs7_der, size_t pkcs7_der_len, } std::unique_ptr sig_seq(sig_set->asn1_sequence_get()); - if (sig_seq == nullptr || - !sig_seq->asn1_sequence_next() || - !sig_seq->asn1_sequence_next() || - !sig_seq->asn1_sequence_next() || - !sig_seq->asn1_sequence_next()) { + if (sig_seq == nullptr || !sig_seq->asn1_sequence_next() || !sig_seq->asn1_sequence_next() || + !sig_seq->asn1_sequence_next() || !sig_seq->asn1_sequence_next()) { return false; } @@ -152,8 +147,8 @@ int verify_file(VerifierInterface* package, const std::vector& keys << " bytes from end"; if (signature_start > comment_size) { - LOG(ERROR) << "signature start: " << signature_start << " is larger than comment size: " - << comment_size; + LOG(ERROR) << "signature start: " << signature_start + << " is larger than comment size: " << comment_size; return VERIFY_FAILURE; } @@ -189,8 +184,8 @@ int verify_file(VerifierInterface* package, const std::vector& keys 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) { + 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 the verification if this // sequence occurs anywhere after the real one. @@ -203,8 +198,12 @@ int verify_file(VerifierInterface* package, const std::vector& keys bool need_sha256 = false; for (const auto& key : keys) { switch (key.hash_len) { - case SHA_DIGEST_LENGTH: need_sha1 = true; break; - case SHA256_DIGEST_LENGTH: need_sha256 = true; break; + case SHA_DIGEST_LENGTH: + need_sha1 = true; + break; + case SHA256_DIGEST_LENGTH: + need_sha256 = true; + break; } } @@ -247,8 +246,8 @@ int verify_file(VerifierInterface* package, const std::vector& keys const uint8_t* signature = eocd + eocd_size - signature_start; size_t signature_size = signature_start - FOOTER_SIZE; - LOG(INFO) << "signature (offset: " << std::hex << (length - signature_start) << ", length: " - << signature_size << "): " << print_hex(signature, signature_size); + LOG(INFO) << "signature (offset: " << std::hex << (length - signature_start) + << ", length: " << signature_size << "): " << print_hex(signature, signature_size); std::vector sig_der; if (!read_pkcs7(signature, signature_size, &sig_der)) { diff --git a/logging.cpp b/logging.cpp index 283d1150..48f9ec31 100644 --- a/logging.cpp +++ b/logging.cpp @@ -33,7 +33,7 @@ #include "common.h" #include "otautil/dirutil.h" #include "otautil/paths.h" -#include "roots.h" +#include "otautil/roots.h" static constexpr const char* LOG_FILE = "/cache/recovery/log"; static constexpr const char* LAST_INSTALL_FILE = "/cache/recovery/last_install"; diff --git a/otautil/Android.bp b/otautil/Android.bp index 41018dd2..b4936c08 100644 --- a/otautil/Android.bp +++ b/otautil/Android.bp @@ -42,12 +42,23 @@ cc_library_static { "dirutil.cpp", "mounts.cpp", "parse_install_logs.cpp", + "roots.cpp", "sysutil.cpp", "thermalutil.cpp", ], + include_dirs: [ + "system/vold", + ], + + static_libs: [ + "libfstab", + ], + shared_libs: [ "libcutils", + "libext4_utils", + "libfs_mgr", "libselinux", ], }, diff --git a/roots.h b/otautil/include/otautil/roots.h similarity index 96% rename from roots.h rename to otautil/include/otautil/roots.h index 7b031a18..482f3d05 100644 --- a/roots.h +++ b/otautil/include/otautil/roots.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef RECOVERY_ROOTS_H_ -#define RECOVERY_ROOTS_H_ +#pragma once #include @@ -58,5 +57,3 @@ int setup_install_mounts(); bool logical_partitions_mapped(); std::string get_system_root(); - -#endif // RECOVERY_ROOTS_H_ diff --git a/roots.cpp b/otautil/roots.cpp similarity index 98% rename from roots.cpp rename to otautil/roots.cpp index c42723f0..815d644e 100644 --- a/roots.cpp +++ b/otautil/roots.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "roots.h" +#include "otautil/roots.h" #include #include @@ -174,11 +174,9 @@ int format_volume(const std::string& volume, const std::string& directory) { PLOG(ERROR) << "format_volume: failed to open " << v->blk_device; return -1; } - length = - get_file_size(fd.get(), v->length ? -v->length : CRYPT_FOOTER_OFFSET); + length = get_file_size(fd.get(), v->length ? -v->length : CRYPT_FOOTER_OFFSET); if (length <= 0) { - LOG(ERROR) << "get_file_size: invalid size " << length << " for " - << v->blk_device; + LOG(ERROR) << "get_file_size: invalid size " << length << " for " << v->blk_device; return -1; } } diff --git a/recovery.cpp b/recovery.cpp index d9c1f22f..421bc12f 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -50,20 +50,20 @@ #include #include -#include "adb_install.h" #include "common.h" #include "fsck_unshare_blocks.h" -#include "fuse_sdcard_install.h" -#include "install.h" +#include "install/adb_install.h" +#include "install/fuse_sdcard_install.h" +#include "install/install.h" +#include "install/package.h" #include "logging.h" #include "otautil/dirutil.h" #include "otautil/error_code.h" #include "otautil/paths.h" +#include "otautil/roots.h" #include "otautil/sysutil.h" -#include "package.h" #include "recovery_ui/screen_ui.h" #include "recovery_ui/ui.h" -#include "roots.h" static constexpr const char* CACHE_LOG_DIR = "/cache/recovery"; static constexpr const char* COMMAND_FILE = "/cache/recovery/command"; @@ -79,7 +79,7 @@ static constexpr const char* METADATA_ROOT = "/metadata"; // into target_files.zip. Assert the version defined in code and in Android.mk are consistent. static_assert(kRecoveryApiVersion == RECOVERY_API_VERSION, "Mismatching recovery API versions."); -bool modified_flash = false; +static bool modified_flash = false; std::string stage; const char* reason = nullptr; @@ -439,7 +439,7 @@ static std::unique_ptr ReadWipePackage(size_t wipe_package_size) { // 1. verify the package. // 2. check metadata (ota-type, pre-device and serial number if having one). static bool CheckWipePackage(Package* wipe_package) { - if (!verify_package(wipe_package)) { + if (!verify_package(wipe_package, ui)) { LOG(ERROR) << "Failed to verify package"; return false; } @@ -693,7 +693,7 @@ static Device::BuiltinAction prompt_and_wait(Device* device, int status) { modified_flash = true; bool adb = (chosen_action == Device::APPLY_ADB_SIDELOAD); if (adb) { - status = apply_from_adb(&should_wipe_cache); + status = apply_from_adb(&should_wipe_cache, ui); } else { status = ApplyFromSdcard(device, &should_wipe_cache, ui); } @@ -1030,7 +1030,8 @@ Device::BuiltinAction start_recovery(Device* device, const std::vectorShowText(true); } - status = apply_from_adb(&should_wipe_cache); + status = apply_from_adb(&should_wipe_cache, ui); if (status == INSTALL_SUCCESS && should_wipe_cache) { if (!wipe_cache(false, device)) { status = INSTALL_ERROR; diff --git a/recovery_main.cpp b/recovery_main.cpp index 2f5a1845..56ebe16f 100644 --- a/recovery_main.cpp +++ b/recovery_main.cpp @@ -53,12 +53,12 @@ #include "logging.h" #include "minadbd/minadbd.h" #include "otautil/paths.h" +#include "otautil/roots.h" #include "otautil/sysutil.h" #include "recovery.h" #include "recovery_ui/device.h" #include "recovery_ui/stub_ui.h" #include "recovery_ui/ui.h" -#include "roots.h" static constexpr const char* COMMAND_FILE = "/cache/recovery/command"; static constexpr const char* LOCALE_FILE = "/cache/recovery/last_locale"; diff --git a/tests/Android.bp b/tests/Android.bp index ef5919eb..09ef716d 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -76,9 +76,9 @@ libapplypatch_static_libs = [ librecovery_static_libs = [ "librecovery", "librecovery_fastboot", + "libinstall", + "librecovery_ui", "libminui", - "libpackage", - "libverifier", "libotautil", "libhealthhalutils", @@ -116,10 +116,9 @@ cc_test { ], static_libs: libapplypatch_static_libs + [ + "libinstall", "librecovery_ui", "libminui", - "libpackage", - "libverifier", "libotautil", "libupdater", "libgtest_prod", diff --git a/tests/component/install_test.cpp b/tests/component/install_test.cpp index 969805b4..38513293 100644 --- a/tests/component/install_test.cpp +++ b/tests/component/install_test.cpp @@ -32,9 +32,9 @@ #include #include -#include "install.h" +#include "install/install.h" #include "otautil/paths.h" -#include "private/install.h" +#include "private/setup_commands.h" static void BuildZipArchive(const std::map& file_map, int fd, int compression_type) { diff --git a/tests/component/verifier_test.cpp b/tests/component/verifier_test.cpp index bdb8af23..ded23c52 100644 --- a/tests/component/verifier_test.cpp +++ b/tests/component/verifier_test.cpp @@ -35,9 +35,9 @@ #include #include "common/test_constants.h" +#include "install/package.h" +#include "install/verifier.h" #include "otautil/sysutil.h" -#include "package.h" -#include "verifier.h" using namespace std::string_literals; diff --git a/tests/unit/asn1_decoder_test.cpp b/tests/unit/asn1_decoder_test.cpp index b334a655..d94dd435 100644 --- a/tests/unit/asn1_decoder_test.cpp +++ b/tests/unit/asn1_decoder_test.cpp @@ -20,7 +20,7 @@ #include -#include "asn1_decoder.h" +#include "private/asn1_decoder.h" TEST(Asn1DecoderTest, Empty_Failure) { uint8_t empty[] = {}; diff --git a/tests/unit/package_test.cpp b/tests/unit/package_test.cpp index fa492d38..a735a699 100644 --- a/tests/unit/package_test.cpp +++ b/tests/unit/package_test.cpp @@ -26,7 +26,7 @@ #include #include "common/test_constants.h" -#include "package.h" +#include "install/package.h" class PackageTest : public ::testing::Test { protected: From bc982a4f8845e5fa0cd2630ddcc3d8f70066b083 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 29 Mar 2019 15:43:09 -0700 Subject: [PATCH 7/8] Remove ui_print(). This used to be a helper function that allows printing message to UI. We no longer have any active user in bootable/recovery. Device-specific code can achieve the same functionality by calling GetUI()->Print() instead. Test: mmma -j bootable/recovery Change-Id: If584fc8a51d1af466f1d94d8ea5faa262603a784 --- common.h | 9 +-------- recovery.cpp | 15 --------------- recovery_ui/include/recovery_ui/device.h | 4 ++-- 3 files changed, 3 insertions(+), 25 deletions(-) diff --git a/common.h b/common.h index 22b2f0a0..9cb44bd3 100644 --- a/common.h +++ b/common.h @@ -14,11 +14,7 @@ * limitations under the License. */ -#ifndef RECOVERY_COMMON_H -#define RECOVERY_COMMON_H - -#include -#include +#pragma once #include @@ -39,9 +35,6 @@ extern std::string stage; // The reason argument provided in "--reason=". extern const char* reason; -void ui_print(const char* format, ...) __printflike(1, 2); - bool is_ro_debuggable(); bool SetUsbConfig(const std::string& state); -#endif // RECOVERY_COMMON_H diff --git a/recovery.cpp b/recovery.cpp index 421bc12f..02cc53c2 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include @@ -746,20 +745,6 @@ static void print_property(const char* key, const char* name, void* /* cookie */ printf("%s=%s\n", key, name); } -void ui_print(const char* format, ...) { - std::string buffer; - va_list ap; - va_start(ap, format); - android::base::StringAppendV(&buffer, format, ap); - va_end(ap); - - if (ui != nullptr) { - ui->Print("%s", buffer.c_str()); - } else { - fputs(buffer.c_str(), stdout); - } -} - static bool is_battery_ok(int* required_battery_level) { using android::hardware::health::V1_0::BatteryStatus; using android::hardware::health::V2_0::get_health_service; diff --git a/recovery_ui/include/recovery_ui/device.h b/recovery_ui/include/recovery_ui/device.h index cfa914e7..3c44510c 100644 --- a/recovery_ui/include/recovery_ui/device.h +++ b/recovery_ui/include/recovery_ui/device.h @@ -93,8 +93,8 @@ class Device { // Performs a recovery action selected from the menu. 'menu_position' will be the index of the // selected menu item, or a non-negative value returned from HandleMenuKey(). The menu will be - // hidden when this is called; implementations can call ui_print() to print information to the - // screen. If the menu position is one of the builtin actions, you can just return the + // hidden when this is called; implementations can call GetUI()->Print() to print information to + // the screen. If the menu position is one of the builtin actions, you can just return the // corresponding enum value. If it is an action specific to your device, you actually perform it // here and return NO_ACTION. virtual BuiltinAction InvokeMenuItem(size_t menu_position); From e0cfab3de9d0e1b09799c7e488f171b31ddfe7bf Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 29 Mar 2019 15:53:23 -0700 Subject: [PATCH 8/8] recovery: Remove SetUsbConfig() out of common.h. libinstall now has its own copy. Test: mmma -j bootable/recovery Change-Id: Ibbe7084e15baeb7e744f2175d5944477092acc9e --- common.h | 2 -- recovery.cpp | 6 ------ recovery_main.cpp | 6 ++++++ 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/common.h b/common.h index 9cb44bd3..a524a418 100644 --- a/common.h +++ b/common.h @@ -36,5 +36,3 @@ extern std::string stage; extern const char* reason; bool is_ro_debuggable(); - -bool SetUsbConfig(const std::string& state); diff --git a/recovery.cpp b/recovery.cpp index 02cc53c2..03491849 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -271,12 +271,6 @@ static bool erase_volume(const char* volume) { return (result == 0); } -// Sets the usb config to 'state' -bool SetUsbConfig(const std::string& state) { - android::base::SetProperty("sys.usb.config", state); - return android::base::WaitForProperty("sys.usb.state", state); -} - static bool yes_no(Device* device, const char* question1, const char* question2) { std::vector headers{ question1, question2 }; std::vector items{ " No", " Yes" }; diff --git a/recovery_main.cpp b/recovery_main.cpp index 56ebe16f..b41368d7 100644 --- a/recovery_main.cpp +++ b/recovery_main.cpp @@ -178,6 +178,12 @@ static std::string load_locale_from_cache() { return android::base::Trim(content); } +// Sets the usb config to 'state'. +static bool SetUsbConfig(const std::string& state) { + android::base::SetProperty("sys.usb.config", state); + return android::base::WaitForProperty("sys.usb.state", state); +} + static void ListenRecoverySocket(RecoveryUI* ui, std::atomic& action) { android::base::unique_fd sock_fd(android_get_control_socket("recovery")); if (sock_fd < 0) {