diff options
| -rw-r--r-- | minui/graphics_drm.cpp | 199 | ||||
| -rw-r--r-- | minui/graphics_drm.h | 36 | ||||
| -rw-r--r-- | tests/component/verifier_test.cpp | 116 | ||||
| -rw-r--r-- | verifier.cpp | 301 | ||||
| -rw-r--r-- | verifier.h | 6 |
5 files changed, 201 insertions, 457 deletions
diff --git a/minui/graphics_drm.cpp b/minui/graphics_drm.cpp index 81b49fd95..f81fd9d5c 100644 --- a/minui/graphics_drm.cpp +++ b/minui/graphics_drm.cpp @@ -24,74 +24,37 @@ #include <sys/types.h> #include <unistd.h> +#include <memory> + +#include <android-base/macros.h> +#include <android-base/stringprintf.h> +#include <android-base/unique_fd.h> #include <drm_fourcc.h> #include <xf86drm.h> #include <xf86drmMode.h> #include "minui/minui.h" -#define ARRAY_SIZE(A) (sizeof(A)/sizeof(*(A))) - -MinuiBackendDrm::MinuiBackendDrm() - : GRSurfaceDrms(), main_monitor_crtc(nullptr), main_monitor_connector(nullptr), drm_fd(-1) {} - -void MinuiBackendDrm::DrmDisableCrtc(int drm_fd, drmModeCrtc* crtc) { - if (crtc) { - drmModeSetCrtc(drm_fd, crtc->crtc_id, - 0, // fb_id - 0, 0, // x,y - nullptr, // connectors - 0, // connector_count - nullptr); // mode - } -} - -int MinuiBackendDrm::DrmEnableCrtc(int drm_fd, drmModeCrtc* crtc, GRSurfaceDrm* surface) { - int ret = drmModeSetCrtc(drm_fd, crtc->crtc_id, surface->fb_id, 0, 0, // x,y - &main_monitor_connector->connector_id, - 1, // connector_count - &main_monitor_crtc->mode); - - if (ret) { - printf("drmModeSetCrtc failed ret=%d\n", ret); +GRSurfaceDrm::~GRSurfaceDrm() { + if (mmapped_buffer_) { + munmap(mmapped_buffer_, row_bytes * height); } - return ret; -} - -void MinuiBackendDrm::Blank(bool blank) { - if (blank) { - DrmDisableCrtc(drm_fd, main_monitor_crtc); - } else { - DrmEnableCrtc(drm_fd, main_monitor_crtc, GRSurfaceDrms[current_buffer]); - } -} - -void MinuiBackendDrm::DrmDestroySurface(GRSurfaceDrm* surface) { - if (!surface) return; - - if (surface->mmapped_buffer_) { - munmap(surface->mmapped_buffer_, surface->row_bytes * surface->height); - } - - if (surface->fb_id) { - int ret = drmModeRmFB(drm_fd, surface->fb_id); - if (ret) { - printf("drmModeRmFB failed ret=%d\n", ret); + if (fb_id) { + if (drmModeRmFB(drm_fd_, fb_id) != 0) { + perror("Failed to drmModeRmFB"); + // Falling through to free other resources. } } - if (surface->handle) { + if (handle) { drm_gem_close gem_close = {}; - gem_close.handle = surface->handle; + gem_close.handle = handle; - int ret = drmIoctl(drm_fd, DRM_IOCTL_GEM_CLOSE, &gem_close); - if (ret) { - printf("DRM_IOCTL_GEM_CLOSE failed ret=%d\n", ret); + if (drmIoctl(drm_fd_, DRM_IOCTL_GEM_CLOSE, &gem_close) != 0) { + perror("Failed to DRM_IOCTL_GEM_CLOSE"); } } - - delete surface; } static int drm_format_to_bpp(uint32_t format) { @@ -111,10 +74,7 @@ static int drm_format_to_bpp(uint32_t format) { } } -GRSurfaceDrm* MinuiBackendDrm::DrmCreateSurface(int width, int height) { - GRSurfaceDrm* surface = new GRSurfaceDrm; - *surface = {}; - +std::unique_ptr<GRSurfaceDrm> GRSurfaceDrm::Create(int drm_fd, int width, int height) { uint32_t format; PixelFormat pixel_format = gr_pixel_format(); // PixelFormat comes in byte order, whereas DRM_FORMAT_* uses little-endian @@ -137,12 +97,12 @@ GRSurfaceDrm* MinuiBackendDrm::DrmCreateSurface(int width, int height) { create_dumb.bpp = drm_format_to_bpp(format); create_dumb.flags = 0; - int ret = drmIoctl(drm_fd, DRM_IOCTL_MODE_CREATE_DUMB, &create_dumb); - if (ret) { - printf("DRM_IOCTL_MODE_CREATE_DUMB failed ret=%d\n", ret); - DrmDestroySurface(surface); + if (drmIoctl(drm_fd, DRM_IOCTL_MODE_CREATE_DUMB, &create_dumb) != 0) { + perror("Failed to DRM_IOCTL_MODE_CREATE_DUMB"); return nullptr; } + + std::unique_ptr<GRSurfaceDrm> surface = std::make_unique<GRSurfaceDrm>(drm_fd); surface->handle = create_dumb.handle; uint32_t handles[4], pitches[4], offsets[4]; @@ -151,20 +111,16 @@ GRSurfaceDrm* MinuiBackendDrm::DrmCreateSurface(int width, int height) { pitches[0] = create_dumb.pitch; offsets[0] = 0; - ret = - drmModeAddFB2(drm_fd, width, height, format, handles, pitches, offsets, &(surface->fb_id), 0); - if (ret) { - printf("drmModeAddFB2 failed ret=%d\n", ret); - DrmDestroySurface(surface); + if (drmModeAddFB2(drm_fd, width, height, format, handles, pitches, offsets, &surface->fb_id, 0) != + 0) { + perror("Failed to drmModeAddFB2"); return nullptr; } drm_mode_map_dumb map_dumb = {}; map_dumb.handle = create_dumb.handle; - ret = drmIoctl(drm_fd, DRM_IOCTL_MODE_MAP_DUMB, &map_dumb); - if (ret) { - printf("DRM_IOCTL_MODE_MAP_DUMB failed ret=%d\n", ret); - DrmDestroySurface(surface); + if (drmIoctl(drm_fd, DRM_IOCTL_MODE_MAP_DUMB, &map_dumb) != 0) { + perror("Failed to DRM_IOCTL_MODE_MAP_DUMB"); return nullptr; } @@ -175,14 +131,44 @@ GRSurfaceDrm* MinuiBackendDrm::DrmCreateSurface(int width, int height) { auto mmapped = mmap(nullptr, surface->height * surface->row_bytes, PROT_READ | PROT_WRITE, MAP_SHARED, drm_fd, map_dumb.offset); if (mmapped == MAP_FAILED) { - perror("mmap() failed"); - DrmDestroySurface(surface); + perror("Failed to mmap()"); return nullptr; } surface->mmapped_buffer_ = static_cast<uint8_t*>(mmapped); return surface; } +void MinuiBackendDrm::DrmDisableCrtc(int drm_fd, drmModeCrtc* crtc) { + if (crtc) { + drmModeSetCrtc(drm_fd, crtc->crtc_id, + 0, // fb_id + 0, 0, // x,y + nullptr, // connectors + 0, // connector_count + nullptr); // mode + } +} + +bool MinuiBackendDrm::DrmEnableCrtc(int drm_fd, drmModeCrtc* crtc, + const std::unique_ptr<GRSurfaceDrm>& surface) { + if (drmModeSetCrtc(drm_fd, crtc->crtc_id, surface->fb_id, 0, 0, // x,y + &main_monitor_connector->connector_id, + 1, // connector_count + &main_monitor_crtc->mode) != 0) { + perror("Failed to drmModeSetCrtc"); + return false; + } + return true; +} + +void MinuiBackendDrm::Blank(bool blank) { + if (blank) { + DrmDisableCrtc(drm_fd, main_monitor_crtc); + } else { + DrmEnableCrtc(drm_fd, main_monitor_crtc, GRSurfaceDrms[current_buffer]); + } +} + static drmModeCrtc* find_crtc_for_connector(int fd, drmModeRes* resources, drmModeConnector* connector) { // Find the encoder. If we already have one, just use it. @@ -264,7 +250,7 @@ drmModeConnector* MinuiBackendDrm::FindMainMonitor(int fd, drmModeRes* resources do { main_monitor_connector = find_used_connector_by_type(fd, resources, kConnectorPriority[i]); i++; - } while (!main_monitor_connector && i < ARRAY_SIZE(kConnectorPriority)); + } while (!main_monitor_connector && i < arraysize(kConnectorPriority)); /* If we didn't find a connector, grab the first one that is connected. */ if (!main_monitor_connector) { @@ -298,60 +284,53 @@ void MinuiBackendDrm::DisableNonMainCrtcs(int fd, drmModeRes* resources, drmMode GRSurface* MinuiBackendDrm::Init() { drmModeRes* res = nullptr; + drm_fd = -1; /* Consider DRM devices in order. */ for (int i = 0; i < DRM_MAX_MINOR; i++) { - char* dev_name; - int ret = asprintf(&dev_name, DRM_DEV_NAME, DRM_DIR_NAME, i); - if (ret < 0) continue; + auto dev_name = android::base::StringPrintf(DRM_DEV_NAME, DRM_DIR_NAME, i); + android::base::unique_fd fd(open(dev_name.c_str(), O_RDWR)); + if (fd == -1) continue; - drm_fd = open(dev_name, O_RDWR, 0); - free(dev_name); - if (drm_fd < 0) continue; - - uint64_t cap = 0; /* We need dumb buffers. */ - ret = drmGetCap(drm_fd, DRM_CAP_DUMB_BUFFER, &cap); - if (ret || cap == 0) { - close(drm_fd); + if (uint64_t cap = 0; drmGetCap(fd.get(), DRM_CAP_DUMB_BUFFER, &cap) != 0 || cap == 0) { continue; } - res = drmModeGetResources(drm_fd); + res = drmModeGetResources(fd.get()); if (!res) { - close(drm_fd); continue; } /* Use this device if it has at least one connected monitor. */ if (res->count_crtcs > 0 && res->count_connectors > 0) { - if (find_first_connected_connector(drm_fd, res)) break; + if (find_first_connected_connector(fd.get(), res)) { + drm_fd = fd.release(); + break; + } } drmModeFreeResources(res); - close(drm_fd); res = nullptr; } - if (drm_fd < 0 || res == nullptr) { - perror("cannot find/open a drm device"); + if (drm_fd == -1 || res == nullptr) { + perror("Failed to find/open a drm device"); return nullptr; } uint32_t selected_mode; main_monitor_connector = FindMainMonitor(drm_fd, res, &selected_mode); - if (!main_monitor_connector) { - printf("main_monitor_connector not found\n"); + fprintf(stderr, "Failed to find main_monitor_connector\n"); drmModeFreeResources(res); close(drm_fd); return nullptr; } main_monitor_crtc = find_crtc_for_connector(drm_fd, res, main_monitor_connector); - if (!main_monitor_crtc) { - printf("main_monitor_crtc not found\n"); + fprintf(stderr, "Failed to find main_monitor_crtc\n"); drmModeFreeResources(res); close(drm_fd); return nullptr; @@ -366,21 +345,20 @@ GRSurface* MinuiBackendDrm::Init() { drmModeFreeResources(res); - GRSurfaceDrms[0] = DrmCreateSurface(width, height); - GRSurfaceDrms[1] = DrmCreateSurface(width, height); + GRSurfaceDrms[0] = GRSurfaceDrm::Create(drm_fd, width, height); + GRSurfaceDrms[1] = GRSurfaceDrm::Create(drm_fd, width, height); if (!GRSurfaceDrms[0] || !GRSurfaceDrms[1]) { - // GRSurfaceDrms and drm_fd should be freed in d'tor. return nullptr; } current_buffer = 0; // We will likely encounter errors in the backend functions (i.e. Flip) if EnableCrtc fails. - if (DrmEnableCrtc(drm_fd, main_monitor_crtc, GRSurfaceDrms[1]) != 0) { + if (!DrmEnableCrtc(drm_fd, main_monitor_crtc, GRSurfaceDrms[1])) { return nullptr; } - return GRSurfaceDrms[0]; + return GRSurfaceDrms[0].get(); } static void page_flip_complete(__unused int fd, @@ -393,12 +371,9 @@ static void page_flip_complete(__unused int fd, GRSurface* MinuiBackendDrm::Flip() { bool ongoing_flip = true; - - int ret = drmModePageFlip(drm_fd, main_monitor_crtc->crtc_id, - GRSurfaceDrms[current_buffer]->fb_id, - DRM_MODE_PAGE_FLIP_EVENT, &ongoing_flip); - if (ret < 0) { - printf("drmModePageFlip failed ret=%d\n", ret); + if (drmModePageFlip(drm_fd, main_monitor_crtc->crtc_id, GRSurfaceDrms[current_buffer]->fb_id, + DRM_MODE_PAGE_FLIP_EVENT, &ongoing_flip) != 0) { + perror("Failed to drmModePageFlip"); return nullptr; } @@ -408,9 +383,8 @@ GRSurface* MinuiBackendDrm::Flip() { .events = POLLIN }; - ret = poll(&fds, 1, -1); - if (ret == -1 || !(fds.revents & POLLIN)) { - printf("poll() failed on drm fd\n"); + if (poll(&fds, 1, -1) == -1 || !(fds.revents & POLLIN)) { + perror("Failed to poll() on drm fd"); break; } @@ -419,21 +393,18 @@ GRSurface* MinuiBackendDrm::Flip() { .page_flip_handler = page_flip_complete }; - ret = drmHandleEvent(drm_fd, &evctx); - if (ret != 0) { - printf("drmHandleEvent failed ret=%d\n", ret); + if (drmHandleEvent(drm_fd, &evctx) != 0) { + perror("Failed to drmHandleEvent"); break; } } current_buffer = 1 - current_buffer; - return GRSurfaceDrms[current_buffer]; + return GRSurfaceDrms[current_buffer].get(); } MinuiBackendDrm::~MinuiBackendDrm() { DrmDisableCrtc(drm_fd, main_monitor_crtc); - DrmDestroySurface(GRSurfaceDrms[0]); - DrmDestroySurface(GRSurfaceDrms[1]); drmModeFreeCrtc(main_monitor_crtc); drmModeFreeConnector(main_monitor_connector); close(drm_fd); diff --git a/minui/graphics_drm.h b/minui/graphics_drm.h index f3aad6bfc..02db89f05 100644 --- a/minui/graphics_drm.h +++ b/minui/graphics_drm.h @@ -18,6 +18,9 @@ #include <stdint.h> +#include <memory> + +#include <android-base/macros.h> #include <xf86drmMode.h> #include "graphics.h" @@ -25,6 +28,12 @@ class GRSurfaceDrm : public GRSurface { public: + explicit GRSurfaceDrm(int drm_fd) : drm_fd_(drm_fd) {} + ~GRSurfaceDrm() override; + + // Creates a GRSurfaceDrm instance. + static std::unique_ptr<GRSurfaceDrm> Create(int drm_fd, int width, int height); + uint8_t* data() override { return mmapped_buffer_; } @@ -32,30 +41,33 @@ class GRSurfaceDrm : public GRSurface { private: friend class MinuiBackendDrm; - uint32_t fb_id; - uint32_t handle; + const int drm_fd_; + + uint32_t fb_id{ 0 }; + uint32_t handle{ 0 }; uint8_t* mmapped_buffer_{ nullptr }; + + DISALLOW_COPY_AND_ASSIGN(GRSurfaceDrm); }; class MinuiBackendDrm : public MinuiBackend { public: + MinuiBackendDrm() = default; + ~MinuiBackendDrm() override; + GRSurface* Init() override; GRSurface* Flip() override; void Blank(bool) override; - ~MinuiBackendDrm() override; - MinuiBackendDrm(); private: void DrmDisableCrtc(int drm_fd, drmModeCrtc* crtc); - int DrmEnableCrtc(int drm_fd, drmModeCrtc* crtc, GRSurfaceDrm* surface); - GRSurfaceDrm* DrmCreateSurface(int width, int height); - void DrmDestroySurface(GRSurfaceDrm* surface); + bool DrmEnableCrtc(int drm_fd, drmModeCrtc* crtc, const std::unique_ptr<GRSurfaceDrm>& surface); void DisableNonMainCrtcs(int fd, drmModeRes* resources, drmModeCrtc* main_crtc); drmModeConnector* FindMainMonitor(int fd, drmModeRes* resources, uint32_t* mode_index); - GRSurfaceDrm* GRSurfaceDrms[2]; - int current_buffer; - drmModeCrtc* main_monitor_crtc; - drmModeConnector* main_monitor_connector; - int drm_fd; + std::unique_ptr<GRSurfaceDrm> GRSurfaceDrms[2]; + int current_buffer{ 0 }; + drmModeCrtc* main_monitor_crtc{ nullptr }; + drmModeConnector* main_monitor_connector{ nullptr }; + int drm_fd{ -1 }; }; diff --git a/tests/component/verifier_test.cpp b/tests/component/verifier_test.cpp index d110c37e0..480f3c96c 100644 --- a/tests/component/verifier_test.cpp +++ b/tests/component/verifier_test.cpp @@ -30,6 +30,9 @@ #include <android-base/test_utils.h> #include <android-base/unique_fd.h> #include <gtest/gtest.h> +#include <openssl/bn.h> +#include <openssl/ec.h> +#include <openssl/nid.h> #include <ziparchive/zip_writer.h> #include "common/test_constants.h" @@ -148,6 +151,35 @@ TEST(VerifierTest, LoadCertificateFromBuffer_sha256_ec256bits) { VerifyPackageWithSingleCertificate("otasigned_v5.zip", std::move(cert)); } +TEST(VerifierTest, LoadCertificateFromBuffer_check_rsa_keys) { + std::unique_ptr<RSA, RSADeleter> rsa(RSA_new()); + std::unique_ptr<BIGNUM, decltype(&BN_free)> exponent(BN_new(), BN_free); + BN_set_word(exponent.get(), 3); + RSA_generate_key_ex(rsa.get(), 2048, exponent.get(), nullptr); + ASSERT_TRUE(CheckRSAKey(rsa)); + + // Exponent is expected to be 3 or 65537 + BN_set_word(exponent.get(), 17); + RSA_generate_key_ex(rsa.get(), 2048, exponent.get(), nullptr); + ASSERT_FALSE(CheckRSAKey(rsa)); + + // Modulus is expected to be 2048. + BN_set_word(exponent.get(), 3); + RSA_generate_key_ex(rsa.get(), 1024, exponent.get(), nullptr); + ASSERT_FALSE(CheckRSAKey(rsa)); +} + +TEST(VerifierTest, LoadCertificateFromBuffer_check_ec_keys) { + std::unique_ptr<EC_KEY, ECKEYDeleter> ec(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1)); + ASSERT_EQ(1, EC_KEY_generate_key(ec.get())); + ASSERT_TRUE(CheckECKey(ec)); + + // Expects 256-bit EC key with curve NIST P-256 + ec.reset(EC_KEY_new_by_curve_name(NID_secp224r1)); + ASSERT_EQ(1, EC_KEY_generate_key(ec.get())); + ASSERT_FALSE(CheckECKey(ec)); +} + TEST(VerifierTest, LoadKeysFromZipfile_empty_archive) { TemporaryFile otacerts; BuildCertificateArchive({}, otacerts.release()); @@ -206,8 +238,9 @@ class VerifierTest : public testing::TestWithParam<std::vector<std::string>> { } 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)); + std::string public_key_file = from_testdata_base("testkey_" + *it + ".x509.pem"); + certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr); + LoadKeyFromFile(public_key_file, &certs.back()); } } @@ -221,70 +254,10 @@ class VerifierSuccessTest : public VerifierTest { class VerifierFailureTest : public VerifierTest { }; -TEST(VerifierTest, load_keys_multiple_keys) { - std::string testkey_v4; - ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v4.txt"), &testkey_v4)); - - std::string testkey_v3; - ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3)); - - std::string keys = testkey_v4 + "," + testkey_v3 + "," + testkey_v4; - TemporaryFile key_file1; - ASSERT_TRUE(android::base::WriteStringToFile(keys, key_file1.path)); - std::vector<Certificate> certs; - ASSERT_TRUE(load_keys(key_file1.path, certs)); - ASSERT_EQ(3U, certs.size()); -} - -TEST(VerifierTest, load_keys_invalid_keys) { - std::vector<Certificate> certs; - ASSERT_FALSE(load_keys("/doesntexist", certs)); - - // Empty file. - TemporaryFile key_file1; - ASSERT_FALSE(load_keys(key_file1.path, certs)); - - // Invalid contents. - ASSERT_TRUE(android::base::WriteStringToFile("invalid", key_file1.path)); - ASSERT_FALSE(load_keys(key_file1.path, certs)); - - std::string testkey_v4; - ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v4.txt"), &testkey_v4)); - - // Invalid key version: "v4 ..." => "v6 ...". - std::string invalid_key2(testkey_v4); - invalid_key2[1] = '6'; - TemporaryFile key_file2; - ASSERT_TRUE(android::base::WriteStringToFile(invalid_key2, key_file2.path)); - ASSERT_FALSE(load_keys(key_file2.path, certs)); - - // Invalid key content: inserted extra bytes ",2209831334". - std::string invalid_key3(testkey_v4); - invalid_key3.insert(invalid_key2.size() - 2, ",2209831334"); - TemporaryFile key_file3; - ASSERT_TRUE(android::base::WriteStringToFile(invalid_key3, key_file3.path)); - ASSERT_FALSE(load_keys(key_file3.path, certs)); - - // Invalid key: the last key must not end with an extra ','. - std::string invalid_key4 = testkey_v4 + ","; - TemporaryFile key_file4; - ASSERT_TRUE(android::base::WriteStringToFile(invalid_key4, key_file4.path)); - ASSERT_FALSE(load_keys(key_file4.path, certs)); - - // Invalid key separator. - std::string invalid_key5 = testkey_v4 + ";" + testkey_v4; - TemporaryFile key_file5; - ASSERT_TRUE(android::base::WriteStringToFile(invalid_key5, key_file5.path)); - ASSERT_FALSE(load_keys(key_file5.path, certs)); -} - TEST(VerifierTest, BadPackage_AlteredFooter) { - std::string testkey_v3; - ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3)); - TemporaryFile key_file1; - ASSERT_TRUE(android::base::WriteStringToFile(testkey_v3, key_file1.path)); std::vector<Certificate> certs; - ASSERT_TRUE(load_keys(key_file1.path, certs)); + certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr); + LoadKeyFromFile(from_testdata_base("testkey_v3.x509.pem"), &certs.back()); std::string package; ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("otasigned_v3.zip"), &package)); @@ -298,12 +271,9 @@ TEST(VerifierTest, BadPackage_AlteredFooter) { } TEST(VerifierTest, BadPackage_AlteredContent) { - std::string testkey_v3; - ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3)); - TemporaryFile key_file1; - ASSERT_TRUE(android::base::WriteStringToFile(testkey_v3, key_file1.path)); std::vector<Certificate> certs; - ASSERT_TRUE(load_keys(key_file1.path, certs)); + certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr); + LoadKeyFromFile(from_testdata_base("testkey_v3.x509.pem"), &certs.back()); std::string package; ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("otasigned_v3.zip"), &package)); @@ -324,13 +294,9 @@ TEST(VerifierTest, BadPackage_AlteredContent) { } TEST(VerifierTest, BadPackage_SignatureStartOutOfBounds) { - std::string testkey_v3; - ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3)); - - TemporaryFile key_file; - ASSERT_TRUE(android::base::WriteStringToFile(testkey_v3, key_file.path)); std::vector<Certificate> certs; - ASSERT_TRUE(load_keys(key_file.path, certs)); + certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr); + LoadKeyFromFile(from_testdata_base("testkey_v3.x509.pem"), &certs.back()); // Signature start is 65535 (0xffff) while comment size is 0 (Bug: 31914369). std::string package = "\x50\x4b\x05\x06"s + std::string(12, '\0') + "\xff\xff\xff\xff\x00\x00"s; diff --git a/verifier.cpp b/verifier.cpp index 2101dcb41..44bd4e180 100644 --- a/verifier.cpp +++ b/verifier.cpp @@ -308,144 +308,6 @@ int verify_file(const unsigned char* addr, size_t length, const std::vector<Cert return VERIFY_FAILURE; } -std::unique_ptr<RSA, RSADeleter> parse_rsa_key(FILE* file, uint32_t exponent) { - // Read key length in words and n0inv. n0inv is a precomputed montgomery - // parameter derived from the modulus and can be used to speed up - // verification. n0inv is 32 bits wide here, assuming the verification logic - // uses 32 bit arithmetic. However, BoringSSL may use a word size of 64 bits - // internally, in which case we don't have a valid n0inv. Thus, we just - // ignore the montgomery parameters and have BoringSSL recompute them - // internally. If/When the speedup from using the montgomery parameters - // becomes relevant, we can add more sophisticated code here to obtain a - // 64-bit n0inv and initialize the montgomery parameters in the key object. - uint32_t key_len_words = 0; - uint32_t n0inv = 0; - if (fscanf(file, " %i , 0x%x", &key_len_words, &n0inv) != 2) { - return nullptr; - } - - if (key_len_words > 8192 / 32) { - LOG(ERROR) << "key length (" << key_len_words << ") too large"; - return nullptr; - } - - // Read the modulus. - std::unique_ptr<uint32_t[]> modulus(new uint32_t[key_len_words]); - if (fscanf(file, " , { %u", &modulus[0]) != 1) { - return nullptr; - } - for (uint32_t i = 1; i < key_len_words; ++i) { - if (fscanf(file, " , %u", &modulus[i]) != 1) { - return nullptr; - } - } - - // Cconvert from little-endian array of little-endian words to big-endian - // byte array suitable as input for BN_bin2bn. - std::reverse((uint8_t*)modulus.get(), - (uint8_t*)(modulus.get() + key_len_words)); - - // The next sequence of values is the montgomery parameter R^2. Since we - // generally don't have a valid |n0inv|, we ignore this (see comment above). - uint32_t rr_value; - if (fscanf(file, " } , { %u", &rr_value) != 1) { - return nullptr; - } - for (uint32_t i = 1; i < key_len_words; ++i) { - if (fscanf(file, " , %u", &rr_value) != 1) { - return nullptr; - } - } - if (fscanf(file, " } } ") != 0) { - return nullptr; - } - - // Initialize the key. - std::unique_ptr<RSA, RSADeleter> key(RSA_new()); - if (!key) { - return nullptr; - } - - key->n = BN_bin2bn((uint8_t*)modulus.get(), - key_len_words * sizeof(uint32_t), NULL); - if (!key->n) { - return nullptr; - } - - key->e = BN_new(); - if (!key->e || !BN_set_word(key->e, exponent)) { - return nullptr; - } - - return key; -} - -struct BNDeleter { - void operator()(BIGNUM* bn) const { - BN_free(bn); - } -}; - -std::unique_ptr<EC_KEY, ECKEYDeleter> parse_ec_key(FILE* file) { - uint32_t key_len_bytes = 0; - if (fscanf(file, " %i", &key_len_bytes) != 1) { - return nullptr; - } - - std::unique_ptr<EC_GROUP, void (*)(EC_GROUP*)> group( - EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1), EC_GROUP_free); - if (!group) { - return nullptr; - } - - // Verify that |key_len| matches the group order. - if (key_len_bytes != BN_num_bytes(EC_GROUP_get0_order(group.get()))) { - return nullptr; - } - - // Read the public key coordinates. Note that the byte order in the file is - // little-endian, so we convert to big-endian here. - std::unique_ptr<uint8_t[]> bytes(new uint8_t[key_len_bytes]); - std::unique_ptr<BIGNUM, BNDeleter> point[2]; - for (int i = 0; i < 2; ++i) { - unsigned int byte = 0; - if (fscanf(file, " , { %u", &byte) != 1) { - return nullptr; - } - bytes[key_len_bytes - 1] = byte; - - for (size_t i = 1; i < key_len_bytes; ++i) { - if (fscanf(file, " , %u", &byte) != 1) { - return nullptr; - } - bytes[key_len_bytes - i - 1] = byte; - } - - point[i].reset(BN_bin2bn(bytes.get(), key_len_bytes, nullptr)); - if (!point[i]) { - return nullptr; - } - - if (fscanf(file, " }") != 0) { - return nullptr; - } - } - - if (fscanf(file, " } ") != 0) { - return nullptr; - } - - // Create and initialize the key. - std::unique_ptr<EC_KEY, ECKEYDeleter> key(EC_KEY_new()); - if (!key || !EC_KEY_set_group(key.get(), group.get()) || - !EC_KEY_set_public_key_affine_coordinates(key.get(), point[0].get(), - point[1].get())) { - return nullptr; - } - - return key; -} - static std::vector<Certificate> IterateZipEntriesAndSearchForKeys(const ZipArchiveHandle& handle) { void* cookie; ZipString suffix("x509.pem"); @@ -500,6 +362,48 @@ std::vector<Certificate> LoadKeysFromZipfile(const std::string& zip_name) { return result; } +bool CheckRSAKey(const std::unique_ptr<RSA, RSADeleter>& rsa) { + if (!rsa) { + return false; + } + + const BIGNUM* out_n; + 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; + return false; + } + + BN_ULONG exponent = BN_get_word(out_e); + if (exponent != 3 && exponent != 65537) { + LOG(ERROR) << "Public exponent should be 3 or 65537, actual: " << exponent; + return false; + } + + return true; +} + +bool CheckECKey(const std::unique_ptr<EC_KEY, ECKEYDeleter>& ec_key) { + if (!ec_key) { + return false; + } + + const EC_GROUP* ec_group = EC_KEY_get0_group(ec_key.get()); + if (!ec_group) { + LOG(ERROR) << "Failed to get the ec_group from the ec_key"; + return false; + } + auto degree = EC_GROUP_get_degree(ec_group); + if (degree != 256) { + LOG(ERROR) << "Field size of the ec key should be 256 bits long, actual: " << degree; + return false; + } + + return true; +} + bool LoadCertificateFromBuffer(const std::vector<uint8_t>& pem_content, Certificate* cert) { std::unique_ptr<BIO, decltype(&BIO_free)> content( BIO_new_mem_buf(pem_content.data(), pem_content.size()), BIO_free); @@ -538,22 +442,20 @@ bool LoadCertificateFromBuffer(const std::vector<uint8_t>& pem_content, Certific } int key_type = EVP_PKEY_id(public_key.get()); - // TODO(xunchang) check the rsa key has exponent 3 or 65537 with RSA_get0_key; and ec key is - // 256 bits. if (key_type == EVP_PKEY_RSA) { cert->key_type = Certificate::KEY_TYPE_RSA; cert->ec.reset(); cert->rsa.reset(EVP_PKEY_get1_RSA(public_key.get())); - if (!cert->rsa) { - LOG(ERROR) << "Failed to get the rsa key info from public key"; + if (!cert->rsa || !CheckRSAKey(cert->rsa)) { + LOG(ERROR) << "Failed to validate the rsa key info from public key"; return false; } } else if (key_type == EVP_PKEY_EC) { cert->key_type = Certificate::KEY_TYPE_EC; cert->rsa.reset(); cert->ec.reset(EVP_PKEY_get1_EC_KEY(public_key.get())); - if (!cert->ec) { - LOG(ERROR) << "Failed to get the ec key info from the public key"; + if (!cert->ec || !CheckECKey(cert->ec)) { + LOG(ERROR) << "Failed to validate the ec key info from the public key"; return false; } } else { @@ -563,114 +465,3 @@ bool LoadCertificateFromBuffer(const std::vector<uint8_t>& pem_content, Certific return true; } - -// Reads a file containing one or more public keys as produced by -// DumpPublicKey: this is an RSAPublicKey struct as it would appear -// as a C source literal, eg: -// -// "{64,0xc926ad21,{1795090719,...,-695002876},{-857949815,...,1175080310}}" -// -// For key versions newer than the original 2048-bit e=3 keys -// supported by Android, the string is preceded by a version -// identifier, eg: -// -// "v2 {64,0xc926ad21,{1795090719,...,-695002876},{-857949815,...,1175080310}}" -// -// (Note that the braces and commas in this example are actual -// characters the parser expects to find in the file; the ellipses -// indicate more numbers omitted from this example.) -// -// The file may contain multiple keys in this format, separated by -// commas. The last key must not be followed by a comma. -// -// A Certificate is a pair of an RSAPublicKey and a particular hash -// (we support SHA-1 and SHA-256; we store the hash length to signify -// which is being used). The hash used is implied by the version number. -// -// 1: 2048-bit RSA key with e=3 and SHA-1 hash -// 2: 2048-bit RSA key with e=65537 and SHA-1 hash -// 3: 2048-bit RSA key with e=3 and SHA-256 hash -// 4: 2048-bit RSA key with e=65537 and SHA-256 hash -// 5: 256-bit EC key using the NIST P-256 curve parameters and SHA-256 hash -// -// Returns true on success, and appends the found keys (at least one) to certs. -// Otherwise returns false if the file failed to parse, or if it contains zero -// keys. The contents in certs would be unspecified on failure. -bool load_keys(const char* filename, std::vector<Certificate>& certs) { - std::unique_ptr<FILE, decltype(&fclose)> f(fopen(filename, "re"), fclose); - if (!f) { - PLOG(ERROR) << "error opening " << filename; - return false; - } - - while (true) { - certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr); - Certificate& cert = certs.back(); - uint32_t exponent = 0; - - char start_char; - if (fscanf(f.get(), " %c", &start_char) != 1) return false; - if (start_char == '{') { - // a version 1 key has no version specifier. - cert.key_type = Certificate::KEY_TYPE_RSA; - exponent = 3; - cert.hash_len = SHA_DIGEST_LENGTH; - } else if (start_char == 'v') { - int version; - if (fscanf(f.get(), "%d {", &version) != 1) return false; - switch (version) { - case 2: - cert.key_type = Certificate::KEY_TYPE_RSA; - exponent = 65537; - cert.hash_len = SHA_DIGEST_LENGTH; - break; - case 3: - cert.key_type = Certificate::KEY_TYPE_RSA; - exponent = 3; - cert.hash_len = SHA256_DIGEST_LENGTH; - break; - case 4: - cert.key_type = Certificate::KEY_TYPE_RSA; - exponent = 65537; - cert.hash_len = SHA256_DIGEST_LENGTH; - break; - case 5: - cert.key_type = Certificate::KEY_TYPE_EC; - cert.hash_len = SHA256_DIGEST_LENGTH; - break; - default: - return false; - } - } - - if (cert.key_type == Certificate::KEY_TYPE_RSA) { - cert.rsa = parse_rsa_key(f.get(), exponent); - if (!cert.rsa) { - return false; - } - - LOG(INFO) << "read key e=" << exponent << " hash=" << cert.hash_len; - } else if (cert.key_type == Certificate::KEY_TYPE_EC) { - cert.ec = parse_ec_key(f.get()); - if (!cert.ec) { - return false; - } - } else { - LOG(ERROR) << "Unknown key type " << cert.key_type; - return false; - } - - // if the line ends in a comma, this file has more keys. - int ch = fgetc(f.get()); - if (ch == ',') { - // more keys to come. - continue; - } else if (ch == EOF) { - break; - } else { - LOG(ERROR) << "unexpected character between keys"; - return false; - } - } - return true; -} diff --git a/verifier.h b/verifier.h index b7924c71f..df9a4b648 100644 --- a/verifier.h +++ b/verifier.h @@ -70,7 +70,11 @@ struct Certificate { int verify_file(const 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); +// Checks that the RSA key has a modulus of 2048 bits long, and public exponent is 3 or 65537. +bool CheckRSAKey(const std::unique_ptr<RSA, RSADeleter>& rsa); + +// Checks that the field size of the curve for the EC key is 256 bits. +bool CheckECKey(const std::unique_ptr<EC_KEY, ECKEYDeleter>& ec_key); // Parses a PEM-encoded x509 certificate from the given buffer and saves it into |cert|. Returns // false if there is a parsing failure or the signature's encryption algorithm is not supported. |