Remove the sign method
Exposing a function to sign arbitrary content is a massive security
hole, so remove it.
We are now signing compiled artifacts, so we no longer need to
manually sign. We lose a little test coverage, but e2e tests will
replace that.
Bug: 193241041
Test: atest ComposKeyTestCase
Change-Id: I8408c6d88ffbe744d45caca821816ce73ee47106
diff --git a/compos/aidl/com/android/compos/ICompOsService.aidl b/compos/aidl/com/android/compos/ICompOsService.aidl
index 194180b..395a09b 100644
--- a/compos/aidl/com/android/compos/ICompOsService.aidl
+++ b/compos/aidl/com/android/compos/ICompOsService.aidl
@@ -104,14 +104,4 @@
* @return whether the inputs are valid and correspond to each other.
*/
boolean verifySigningKey(in byte[] keyBlob, in byte[] publicKey);
-
- /**
- * Signs some data with the initialized key. The call will fail with EX_ILLEGAL_STATE if not
- * yet initialized.
- *
- * @param data The data to be signed. (Large data sizes may cause failure.)
- * @return the signature.
- */
- // STOPSHIP(b/193241041): We must not expose this from the PVM.
- byte[] sign(in byte[] data);
}
diff --git a/compos/compos_key_cmd/compos_key_cmd.cpp b/compos/compos_key_cmd/compos_key_cmd.cpp
index f8b3d16..f5c3510 100644
--- a/compos/compos_key_cmd/compos_key_cmd.cpp
+++ b/compos/compos_key_cmd/compos_key_cmd.cpp
@@ -405,121 +405,6 @@
return result;
}
-static Result<std::vector<uint8_t>> computeDigest(const std::string& file) {
- unique_fd fd(TEMP_FAILURE_RETRY(open(file.c_str(), O_RDONLY | O_CLOEXEC)));
- if (!fd.ok()) {
- return ErrnoError() << "Failed to open";
- }
-
- struct stat filestat;
- if (fstat(fd, &filestat) != 0) {
- return ErrnoError() << "Failed to fstat";
- }
-
- struct libfsverity_merkle_tree_params params = {
- .version = 1,
- .hash_algorithm = FS_VERITY_HASH_ALG_SHA256,
- .file_size = static_cast<uint64_t>(filestat.st_size),
- .block_size = 4096,
- };
-
- auto read_callback = [](void* file, void* buf, size_t count) {
- int* fd = static_cast<int*>(file);
- if (TEMP_FAILURE_RETRY(read(*fd, buf, count)) < 0) return -errno;
- return 0;
- };
-
- struct libfsverity_digest* digest;
- int ret = libfsverity_compute_digest(&fd, read_callback, ¶ms, &digest);
- if (ret < 0) {
- return Error(-ret) << "Failed to compute fs-verity digest";
- }
- std::unique_ptr<libfsverity_digest, decltype(&std::free)> digestOwner{digest, std::free};
-
- return std::vector(&digest->digest[0], &digest->digest[digest->digest_size]);
-}
-
-static std::string toHex(const std::vector<uint8_t>& digest) {
- std::stringstream ss;
- for (auto it = digest.begin(); it != digest.end(); ++it) {
- ss << std::setfill('0') << std::setw(2) << std::hex << static_cast<unsigned>(*it);
- }
- return ss.str();
-}
-
-static Result<void> signInfo(TargetVm& vm, const std::string& blob_file,
- const std::string& info_file, const std::vector<std::string>& files) {
- unique_fd info_fd(
- TEMP_FAILURE_RETRY(open(info_file.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC,
- S_IRUSR | S_IWUSR | S_IRGRP)));
- if (!info_fd.ok()) {
- return ErrnoError() << "Unable to create " << info_file;
- }
-
- std::string signature_file = info_file + ".signature";
- unique_fd signature_fd(TEMP_FAILURE_RETRY(open(signature_file.c_str(),
- O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC,
- S_IRUSR | S_IWUSR | S_IRGRP)));
- if (!signature_fd.ok()) {
- return ErrnoError() << "Unable to create " << signature_file;
- }
-
- auto cid = vm.resolveCid();
- if (!cid.ok()) {
- return cid.error();
- }
- auto service = getService(*cid);
- if (!service) {
- return Error() << "No service";
- }
-
- auto blob = readBytesFromFile(blob_file);
- if (!blob.ok()) {
- return blob.error();
- }
-
- auto initialized = service->initializeSigningKey(blob.value());
- if (!initialized.isOk()) {
- return Error() << "Failed to initialize signing key: " << initialized.getDescription();
- }
-
- std::map<std::string, std::string> file_digests;
-
- for (auto& file : files) {
- auto digest = computeDigest(file);
- if (!digest.ok()) {
- return digest.error();
- }
- file_digests.emplace(file, toHex(*digest));
- }
-
- OdsignInfo info;
- info.mutable_file_hashes()->insert(file_digests.begin(), file_digests.end());
-
- std::vector<uint8_t> serialized(info.ByteSizeLong());
- if (!info.SerializeToArray(serialized.data(), serialized.size())) {
- return Error() << "Failed to serialize protobuf";
- }
-
- if (!WriteFully(info_fd, serialized.data(), serialized.size()) ||
- close(info_fd.release()) != 0) {
- return Error() << "Failed to write info file";
- }
-
- std::vector<uint8_t> signature;
- auto status = service->sign(serialized, &signature);
- if (!status.isOk()) {
- return Error() << "Failed to sign: " << status.getDescription();
- }
-
- if (!WriteFully(signature_fd, signature.data(), signature.size()) ||
- close(signature_fd.release()) != 0) {
- return Error() << "Failed to write signature";
- }
-
- return {};
-}
-
static Result<void> initializeKey(TargetVm& vm, const std::string& blob_file) {
auto cid = vm.resolveCid();
if (!cid.ok()) {
@@ -628,17 +513,6 @@
} else {
std::cerr << result.error() << '\n';
}
- } else if (argc >= 5 && argv[1] == "sign-info"sv) {
- const std::string blob_file = argv[2];
- const std::string info_file = argv[3];
- const std::vector<std::string> files{&argv[4], &argv[argc]};
- auto result = signInfo(vm, blob_file, info_file, files);
- if (result.ok()) {
- std::cerr << "Info file generated and signed.\n";
- return 0;
- } else {
- std::cerr << result.error() << '\n';
- }
} else if (argc == 3 && argv[1] == "init-key"sv) {
auto result = initializeKey(vm, argv[2]);
if (result.ok()) {
@@ -662,9 +536,6 @@
<< " verify <blob file> <public key file> Verify that the content of the\n"
<< " specified private key blob and public key files are valid.\n "
<< " init-key <blob file> Initialize the service key.\n"
- << " sign-info <blob file> <info file> <files to be signed> Generate\n"
- << " an info file listing the paths and root digests of each of the files to\n"
- << " be signed, along with a signature of that file.\n"
<< "\n"
<< "OPTIONS: --log <log file> --debug --staged\n"
<< " (--cid <cid> | --start <image file>)\n"
diff --git a/compos/src/compsvc.rs b/compos/src/compsvc.rs
index 0d32841..8f1e205 100644
--- a/compos/src/compsvc.rs
+++ b/compos/src/compsvc.rs
@@ -176,10 +176,6 @@
true
})
}
-
- fn sign(&self, data: &[u8]) -> BinderResult<Vec<u8>> {
- to_binder_result(self.new_signer()?.sign(data))
- }
}
fn get_authfs_service() -> BinderResult<Strong<dyn IAuthFsService>> {
diff --git a/compos/tests/java/android/compos/test/ComposKeyTestCase.java b/compos/tests/java/android/compos/test/ComposKeyTestCase.java
index eacf3fb..aa43270 100644
--- a/compos/tests/java/android/compos/test/ComposKeyTestCase.java
+++ b/compos/tests/java/android/compos/test/ComposKeyTestCase.java
@@ -120,34 +120,6 @@
TEST_ROOT + "test_key.pubkey",
TEST_ROOT + "test_key2.blob");
assertThat(result.getStatus()).isEqualTo(CommandStatus.FAILED);
-
- // Now, continue to test the signing operation. It's the best to do this in a new test
- // method. Since we boot a VM for each test method, and booting a VM on cuttlefish/GCE is
- // very slow, a new test method unfortunately makes the whole test module to exceed the
- // timeout configured in the test infrastructure.
-
- // Generate key - should succeed
- android.run(
- COMPOS_KEY_CMD_BIN,
- "--cid " + mCid,
- "generate",
- TEST_ROOT + "test_key3.blob",
- TEST_ROOT + "test_key3.pubkey");
-
- // Generate some data to sign in a writable directory
- android.run("echo something > /data/local/tmp/something.txt");
-
- // Sign something - should succeed
- android.run(
- COMPOS_KEY_CMD_BIN,
- "--cid " + mCid,
- "sign-info",
- TEST_ROOT + "test_key3.blob",
- TEST_ROOT + "test.info",
- "/data/local/tmp/something.txt");
-
- // Check existence of the output signature - should succeed
- android.run("test -f " + TEST_ROOT + "test.info.signature");
}
private void startVm() throws Exception {