Signature streaming from local file, property to disable incremental.
+Tests
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest
Bug: b/136132412 b/133435829
Change-Id: I826900e120c72e7cdd0549c70da28d817982dcd3
diff --git a/core/java/android/os/incremental/IncrementalManager.java b/core/java/android/os/incremental/IncrementalManager.java
index 74a4215..35518db 100644
--- a/core/java/android/os/incremental/IncrementalManager.java
+++ b/core/java/android/os/incremental/IncrementalManager.java
@@ -50,6 +50,8 @@
public final class IncrementalManager {
private static final String TAG = "IncrementalManager";
+ private static final String ALLOWED_PROPERTY = "incremental.allowed";
+
public static final int CREATE_MODE_TEMPORARY_BIND =
IIncrementalService.CREATE_MODE_TEMPORARY_BIND;
public static final int CREATE_MODE_PERMANENT_BIND =
@@ -288,13 +290,21 @@
}
/**
- * Checks if Incremental is enabled
+ * Checks if Incremental feature is enabled on this device.
*/
- public static boolean isEnabled() {
+ public static boolean isFeatureEnabled() {
return nativeIsEnabled();
}
/**
+ * Checks if Incremental installations are allowed.
+ * A developer can disable Incremental installations by setting the property.
+ */
+ public static boolean isAllowed() {
+ return isFeatureEnabled() && android.os.SystemProperties.getBoolean(ALLOWED_PROPERTY, true);
+ }
+
+ /**
* Checks if path is mounted on Incremental File System.
*/
public static boolean isIncrementalPath(@NonNull String path) {
diff --git a/core/java/android/os/incremental/V4Signature.java b/core/java/android/os/incremental/V4Signature.java
index 5fadee4..6450a67 100644
--- a/core/java/android/os/incremental/V4Signature.java
+++ b/core/java/android/os/incremental/V4Signature.java
@@ -16,8 +16,11 @@
package android.os.incremental;
+import java.io.ByteArrayOutputStream;
import java.io.DataInputStream;
import java.io.DataOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
import java.io.IOException;
/**
@@ -26,20 +29,36 @@
* @hide
*/
public class V4Signature {
+ public static final String EXT = ".idsig";
+
public final byte[] verityRootHash;
public final byte[] v3Digest;
public final byte[] pkcs7SignatureBlock;
- V4Signature(byte[] verityRootHash, byte[] v3Digest, byte[] pkcs7SignatureBlock) {
- this.verityRootHash = verityRootHash;
- this.v3Digest = v3Digest;
- this.pkcs7SignatureBlock = pkcs7SignatureBlock;
+ /**
+ * Construct a V4Signature from .idsig file.
+ */
+ public static V4Signature readFrom(File file) {
+ try (DataInputStream stream = new DataInputStream(new FileInputStream(file))) {
+ return readFrom(stream);
+ } catch (IOException e) {
+ return null;
+ }
}
- static byte[] readBytes(DataInputStream stream) throws IOException {
- byte[] result = new byte[stream.readInt()];
- stream.read(result);
- return result;
+ /**
+ * Store the V4Signature to a byte-array.
+ */
+ public byte[] toByteArray() {
+ try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) {
+ try (DataOutputStream steam = new DataOutputStream(byteArrayOutputStream)) {
+ this.writeTo(steam);
+ steam.flush();
+ }
+ return byteArrayOutputStream.toByteArray();
+ } catch (IOException e) {
+ return null;
+ }
}
static V4Signature readFrom(DataInputStream stream) throws IOException {
@@ -49,9 +68,10 @@
return new V4Signature(verityRootHash, v3Digest, pkcs7SignatureBlock);
}
- static void writeBytes(DataOutputStream stream, byte[] bytes) throws IOException {
- stream.writeInt(bytes.length);
- stream.write(bytes);
+ V4Signature(byte[] verityRootHash, byte[] v3Digest, byte[] pkcs7SignatureBlock) {
+ this.verityRootHash = verityRootHash;
+ this.v3Digest = v3Digest;
+ this.pkcs7SignatureBlock = pkcs7SignatureBlock;
}
void writeTo(DataOutputStream stream) throws IOException {
@@ -59,4 +79,15 @@
writeBytes(stream, this.v3Digest);
writeBytes(stream, this.pkcs7SignatureBlock);
}
+
+ private static byte[] readBytes(DataInputStream stream) throws IOException {
+ byte[] result = new byte[stream.readInt()];
+ stream.read(result);
+ return result;
+ }
+
+ private static void writeBytes(DataOutputStream stream, byte[] bytes) throws IOException {
+ stream.writeInt(bytes.length);
+ stream.write(bytes);
+ }
}
diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java
index 2fcc1de..578c0cc 100644
--- a/core/java/com/android/server/SystemConfig.java
+++ b/core/java/com/android/server/SystemConfig.java
@@ -1156,7 +1156,7 @@
addFeature(PackageManager.FEATURE_RAM_NORMAL, 0);
}
- if (IncrementalManager.isEnabled()) {
+ if (IncrementalManager.isFeatureEnabled()) {
addFeature(PackageManager.FEATURE_INCREMENTAL_DELIVERY, 0);
}
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 4ed4de7..0e554f8 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -101,6 +101,7 @@
import android.os.SystemProperties;
import android.os.UserHandle;
import android.os.incremental.IncrementalFileStorages;
+import android.os.incremental.IncrementalManager;
import android.os.storage.StorageManager;
import android.provider.Settings.Secure;
import android.stats.devicepolicy.DevicePolicyEnums;
@@ -538,18 +539,21 @@
throw new IllegalArgumentException(
"DataLoader installation of APEX modules is not allowed.");
}
- }
-
- if (isStreamingInstallation()) {
- if (!isIncrementalInstallationAllowed(mPackageName)) {
- throw new IllegalArgumentException(
- "Incremental installation of this package is not allowed.");
- }
if (this.params.dataLoaderParams.getComponentName().getPackageName()
== SYSTEM_DATA_LOADER_PACKAGE) {
assertShellOrSystemCalling("System data loaders");
}
}
+
+ if (isIncrementalInstallation()) {
+ if (!IncrementalManager.isAllowed()) {
+ throw new IllegalArgumentException("Incremental installation not allowed.");
+ }
+ if (!isIncrementalInstallationAllowed(mPackageName)) {
+ throw new IllegalArgumentException(
+ "Incremental installation of this package is not allowed.");
+ }
+ }
}
public SessionInfo generateInfo() {
@@ -2385,12 +2389,16 @@
throw new IllegalStateException(
"Cannot add files to non-data loader installation session.");
}
- if (!isIncrementalInstallation()) {
+ if (isStreamingInstallation()) {
if (location != LOCATION_DATA_APP) {
throw new IllegalArgumentException(
"Non-incremental installation only supports /data/app placement: " + name);
}
}
+ if (metadata == null) {
+ throw new IllegalArgumentException(
+ "DataLoader installation requires valid metadata: " + name);
+ }
// Use installer provided name for now; we always rename later
if (!FileUtils.isValidExtFilename(name)) {
throw new IllegalArgumentException("Invalid name: " + name);
diff --git a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java
index 3909fdf..c267cea 100644
--- a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java
+++ b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java
@@ -88,6 +88,7 @@
import android.os.SystemProperties;
import android.os.UserHandle;
import android.os.UserManager;
+import android.os.incremental.V4Signature;
import android.os.storage.StorageManager;
import android.permission.IPermissionManager;
import android.system.ErrnoException;
@@ -3024,9 +3025,15 @@
final File file = new File(inPath);
final String name = file.getName();
final long size = file.length();
- byte[] metadata = inPath.getBytes(StandardCharsets.UTF_8);
+ final byte[] metadata = inPath.getBytes(StandardCharsets.UTF_8);
- session.addFile(LOCATION_DATA_APP, name, size, metadata, null);
+ // Try to load a v4 signature for the APK.
+ final V4Signature v4signature = V4Signature.readFrom(
+ new File(inPath + V4Signature.EXT));
+ final byte[] v4signatureBytes =
+ (v4signature != null) ? v4signature.toByteArray() : null;
+
+ session.addFile(LOCATION_DATA_APP, name, size, metadata, v4signatureBytes);
}
return 0;
} finally {
diff --git a/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp b/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp
index d803c1d..7e6f79f 100644
--- a/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp
+++ b/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp
@@ -20,6 +20,9 @@
#include <android-base/unique_fd.h>
#include <nativehelper/JNIHelp.h>
+#include "android-base/file.h"
+
+#include <endian.h>
#include <core_jni_helpers.h>
@@ -32,6 +35,8 @@
namespace {
+using android::base::borrowed_fd;
+using android::base::ReadFully;
using android::base::unique_fd;
static constexpr int BUFFER_SIZE = 256 * 1024;
@@ -80,6 +85,27 @@
return ids;
}
+static inline int32_t readBEInt32(borrowed_fd fd) {
+ int32_t result;
+ ReadFully(fd, &result, sizeof(result));
+ result = int32_t(be32toh(result));
+ return result;
+}
+
+static inline std::vector<char> readBytes(borrowed_fd fd) {
+ int32_t size = readBEInt32(fd);
+ std::vector<char> result(size);
+ ReadFully(fd, result.data(), size);
+ return result;
+}
+
+static inline int32_t skipIdSigHeaders(borrowed_fd fd) {
+ readBytes(fd); // verityRootHash
+ readBytes(fd); // v3Digest
+ readBytes(fd); // pkcs7SignatureBlock
+ return readBEInt32(fd); // size of the verity tree
+}
+
static inline unique_fd convertPfdToFdAndDup(JNIEnv* env, const JniIds& jni, jobject pfd) {
if (!pfd) {
ALOGE("Missing In ParcelFileDescriptor.");
@@ -90,33 +116,78 @@
ALOGE("Missing In FileDescriptor.");
return {};
}
- return unique_fd{dup(jniGetFDFromFileDescriptor(env, managedFd))};
+ unique_fd result{dup(jniGetFDFromFileDescriptor(env, managedFd))};
+ // Can be closed after dup.
+ env->CallStaticVoidMethod(jni.ioUtils, jni.ioUtilsCloseQuietly, pfd);
+ return result;
}
-static inline std::pair<unique_fd, bool> openIncomingFile(JNIEnv* env, const JniIds& jni,
- jobject shellCommand,
- IncFsSpan metadata) {
- jobject pfd = nullptr;
- const bool stdin = (metadata.size == 0 || *metadata.data == '-');
- if (stdin) {
+struct InputDesc {
+ unique_fd fd;
+ IncFsSize size;
+ IncFsBlockKind kind;
+ bool waitOnEof;
+};
+using InputDescs = std::vector<InputDesc>;
+
+static inline InputDescs openInputs(JNIEnv* env, const JniIds& jni, jobject shellCommand,
+ IncFsSize size, IncFsSpan metadata) {
+ InputDescs result;
+ result.reserve(2);
+
+ const bool fromStdin = (metadata.size == 0 || *metadata.data == '-');
+ if (fromStdin) {
// stdin
- pfd = env->CallStaticObjectMethod(jni.packageManagerShellCommandDataLoader,
- jni.pmscdGetStdInPFD, shellCommand);
- } else {
- // file
- const std::string filePath(metadata.data, metadata.size);
- pfd = env->CallStaticObjectMethod(jni.packageManagerShellCommandDataLoader,
- jni.pmscdGetLocalFile, shellCommand,
- env->NewStringUTF(filePath.c_str()));
+ auto fd = convertPfdToFdAndDup(
+ env, jni,
+ env->CallStaticObjectMethod(jni.packageManagerShellCommandDataLoader,
+ jni.pmscdGetStdInPFD, shellCommand));
+ if (fd.ok()) {
+ result.push_back(InputDesc{
+ .fd = std::move(fd),
+ .size = size,
+ .kind = INCFS_BLOCK_KIND_DATA,
+ .waitOnEof = true,
+ });
+ }
+ return result;
}
- auto result = convertPfdToFdAndDup(env, jni, pfd);
- if (pfd) {
- // Can be closed after dup.
- env->CallStaticVoidMethod(jni.ioUtils, jni.ioUtilsCloseQuietly, pfd);
+ // local file and possibly signature
+ const std::string filePath(metadata.data, metadata.size);
+ const std::string idsigPath = filePath + ".idsig";
+
+ auto idsigFd = convertPfdToFdAndDup(
+ env, jni,
+ env->CallStaticObjectMethod(jni.packageManagerShellCommandDataLoader,
+ jni.pmscdGetLocalFile, shellCommand,
+ env->NewStringUTF(idsigPath.c_str())));
+ if (idsigFd.ok()) {
+ ALOGE("idsig found, skipping to the tree");
+ auto treeSize = skipIdSigHeaders(idsigFd);
+ result.push_back(InputDesc{
+ .fd = std::move(idsigFd),
+ .size = treeSize,
+ .kind = INCFS_BLOCK_KIND_HASH,
+ .waitOnEof = false,
+ });
}
- return {std::move(result), stdin};
+ auto fileFd = convertPfdToFdAndDup(
+ env, jni,
+ env->CallStaticObjectMethod(jni.packageManagerShellCommandDataLoader,
+ jni.pmscdGetLocalFile, shellCommand,
+ env->NewStringUTF(filePath.c_str())));
+ if (fileFd.ok()) {
+ result.push_back(InputDesc{
+ .fd = std::move(fileFd),
+ .size = size,
+ .kind = INCFS_BLOCK_KIND_DATA,
+ .waitOnEof = false,
+ });
+ }
+
+ return result;
}
static inline JNIEnv* GetJNIEnvironment(JavaVM* vm) {
@@ -187,9 +258,9 @@
blocks.reserve(BLOCKS_COUNT);
for (auto&& file : addedFiles) {
- auto [incomingFd, stdin] = openIncomingFile(env, jni, shellCommand, file.metadata);
- if (incomingFd < 0) {
- ALOGE("Failed to open an IncFS file for metadata: %.*s, final file name is: %s. "
+ auto inputs = openInputs(env, jni, shellCommand, file.size, file.metadata);
+ if (inputs.empty()) {
+ ALOGE("Failed to open an input file for metadata: %.*s, final file name is: %s. "
"Error %d",
int(file.metadata.size), file.metadata.data, file.name, errno);
return false;
@@ -205,46 +276,15 @@
return false;
}
- IncFsSize size = file.size;
- IncFsSize remaining = size;
- IncFsSize totalSize = 0;
- IncFsBlockIndex blockIdx = 0;
- while (remaining > 0) {
- constexpr auto capacity = BUFFER_SIZE;
- auto size = buffer.size();
- if (capacity - size < INCFS_DATA_FILE_BLOCK_SIZE) {
- if (!flashToIncFs(incfsFd, false, &blocks, &blockIdx, &buffer)) {
- return false;
- }
- continue;
- }
-
- auto toRead = std::min<IncFsSize>(remaining, capacity - size);
- buffer.resize(size + toRead);
- auto read = ::read(incomingFd, buffer.data() + size, toRead);
- if (read == 0) {
- if (stdin) {
- // eof of stdin, waiting...
- ALOGE("eof of stdin, waiting...: %d, remaining: %d, block: %d, read: %d",
- int(totalSize), int(remaining), int(blockIdx), int(read));
- using namespace std::chrono_literals;
- std::this_thread::sleep_for(10ms);
- continue;
- }
- break;
- }
- if (read < 0) {
- ALOGE("Underlying file read error: %.*s: %d", int(file.metadata.size),
- file.metadata.data, int(read));
+ for (auto&& input : inputs) {
+ if (!copyToIncFs(incfsFd, input.size, input.kind, input.fd, input.waitOnEof,
+ &buffer, &blocks)) {
+ ALOGE("Failed to copy data to IncFS file for metadata: %.*s, final file name "
+ "is: %s. "
+ "Error %d",
+ int(file.metadata.size), file.metadata.data, file.name, errno);
return false;
}
-
- buffer.resize(size + read);
- remaining -= read;
- totalSize += read;
- }
- if (!buffer.empty() && !flashToIncFs(incfsFd, true, &blocks, &blockIdx, &buffer)) {
- return false;
}
}
@@ -252,16 +292,60 @@
return true;
}
- bool flashToIncFs(int incfsFd, bool eof, std::vector<IncFsDataBlock>* blocks,
- IncFsBlockIndex* blockIdx, std::vector<char>* buffer) {
+ bool copyToIncFs(borrowed_fd incfsFd, IncFsSize size, IncFsBlockKind kind,
+ borrowed_fd incomingFd, bool waitOnEof, std::vector<char>* buffer,
+ std::vector<IncFsDataBlock>* blocks) {
+ IncFsSize remaining = size;
+ IncFsSize totalSize = 0;
+ IncFsBlockIndex blockIdx = 0;
+ while (remaining > 0) {
+ constexpr auto capacity = BUFFER_SIZE;
+ auto size = buffer->size();
+ if (capacity - size < INCFS_DATA_FILE_BLOCK_SIZE) {
+ if (!flashToIncFs(incfsFd, kind, false, &blockIdx, buffer, blocks)) {
+ return false;
+ }
+ continue;
+ }
+
+ auto toRead = std::min<IncFsSize>(remaining, capacity - size);
+ buffer->resize(size + toRead);
+ auto read = ::read(incomingFd.get(), buffer->data() + size, toRead);
+ if (read == 0) {
+ if (waitOnEof) {
+ // eof of stdin, waiting...
+ ALOGE("eof of stdin, waiting...: %d, remaining: %d, block: %d, read: %d",
+ int(totalSize), int(remaining), int(blockIdx), int(read));
+ using namespace std::chrono_literals;
+ std::this_thread::sleep_for(10ms);
+ continue;
+ }
+ break;
+ }
+ if (read < 0) {
+ return false;
+ }
+
+ buffer->resize(size + read);
+ remaining -= read;
+ totalSize += read;
+ }
+ if (!buffer->empty() && !flashToIncFs(incfsFd, kind, true, &blockIdx, buffer, blocks)) {
+ return false;
+ }
+ return true;
+ }
+
+ bool flashToIncFs(borrowed_fd incfsFd, IncFsBlockKind kind, bool eof, IncFsBlockIndex* blockIdx,
+ std::vector<char>* buffer, std::vector<IncFsDataBlock>* blocks) {
int consumed = 0;
const auto fullBlocks = buffer->size() / INCFS_DATA_FILE_BLOCK_SIZE;
for (int i = 0; i < fullBlocks; ++i) {
const auto inst = IncFsDataBlock{
- .fileFd = incfsFd,
+ .fileFd = incfsFd.get(),
.pageIndex = (*blockIdx)++,
.compression = INCFS_COMPRESSION_KIND_NONE,
- .kind = INCFS_BLOCK_KIND_DATA,
+ .kind = kind,
.dataSize = INCFS_DATA_FILE_BLOCK_SIZE,
.data = buffer->data() + consumed,
};
@@ -271,10 +355,10 @@
const auto remain = buffer->size() - fullBlocks * INCFS_DATA_FILE_BLOCK_SIZE;
if (remain && eof) {
const auto inst = IncFsDataBlock{
- .fileFd = incfsFd,
+ .fileFd = incfsFd.get(),
.pageIndex = (*blockIdx)++,
.compression = INCFS_COMPRESSION_KIND_NONE,
- .kind = INCFS_BLOCK_KIND_DATA,
+ .kind = kind,
.dataSize = static_cast<uint16_t>(remain),
.data = buffer->data() + consumed,
};