From c1682710d7e9d832c818129f62a5ff3db11bb030 Mon Sep 17 00:00:00 2001 From: Victor Hsieh Date: Fri, 13 Jan 2023 11:02:57 -0800 Subject: [PATCH 1/3] Drop unused return value Also rename the method name. The comment already says it's to be called from a scheduled job, where the returned value is not actually used. Also remove an unused variable. Bug: 265244016 Test: m Change-Id: I67da31115da7ea84d1a404268288323d979dd524 --- .../internal/os/IBinaryTransparencyService.aidl | 2 +- .../server/BinaryTransparencyService.java | 17 ++--------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/core/java/com/android/internal/os/IBinaryTransparencyService.aidl b/core/java/com/android/internal/os/IBinaryTransparencyService.aidl index a1ad5d5d50390..0772c8495f3a3 100644 --- a/core/java/com/android/internal/os/IBinaryTransparencyService.aidl +++ b/core/java/com/android/internal/os/IBinaryTransparencyService.aidl @@ -28,5 +28,5 @@ interface IBinaryTransparencyService { List getApexInfo(); - List getMeasurementsForAllPackages(); + void recordMeasurementsForAllPackages(); } \ No newline at end of file diff --git a/services/core/java/com/android/server/BinaryTransparencyService.java b/services/core/java/com/android/server/BinaryTransparencyService.java index 8f594a5a378a2..eed1d769157fc 100644 --- a/services/core/java/com/android/server/BinaryTransparencyService.java +++ b/services/core/java/com/android/server/BinaryTransparencyService.java @@ -100,7 +100,6 @@ import java.util.stream.Collectors; */ public class BinaryTransparencyService extends SystemService { private static final String TAG = "TransparencyService"; - private static final String EXTRA_SERVICE = "service"; @VisibleForTesting static final String VBMETA_DIGEST_UNINITIALIZED = "vbmeta-digest-uninitialized"; @@ -261,12 +260,8 @@ public class BinaryTransparencyService extends SystemService { * - all mainline modules (introduced in Android T) * - all preloaded apps and their update(s) (new in Android U) * - dynamically installed mobile bundled apps (MBAs) (new in Android U) - * - * @return a {@code List}. Each Bundle item contains values as - * defined by the return value of {@link #measurePackage(PackageInfo)}. */ - public List getMeasurementsForAllPackages() { - List results = new ArrayList<>(); + public void recordMeasurementsForAllPackages() { PackageManager pm = mContext.getPackageManager(); Set packagesMeasured = new HashSet<>(); @@ -288,7 +283,6 @@ public class BinaryTransparencyService extends SystemService { packagesMeasured.add(packageInfo.packageName); Bundle apexMeasurement = measurePackage(packageInfo); - results.add(apexMeasurement); if (record) { // compute digests of signing info @@ -340,7 +334,6 @@ public class BinaryTransparencyService extends SystemService { Bundle packageMeasurement = measurePackage(packageInfo); - results.add(packageMeasurement); if (record && (mba_status == MBA_STATUS_UPDATED_PRELOAD)) { // compute digests of signing info @@ -377,7 +370,6 @@ public class BinaryTransparencyService extends SystemService { packagesMeasured.add(packageInfo.packageName); Bundle packageMeasurement = measurePackage(packageInfo); - results.add(packageMeasurement); if (record) { // compute digests of signing info @@ -432,8 +424,6 @@ public class BinaryTransparencyService extends SystemService { Slog.d(TAG, "Measured " + packagesMeasured.size() + " packages altogether in " + timeSpentMeasuring + "ms"); } - - return results; } /** @@ -1179,14 +1169,11 @@ public class BinaryTransparencyService extends SystemService { // where this operation might take longer than expected, and so that we don't block // system_server's main thread. Executors.defaultThreadFactory().newThread(() -> { - // we discard the return value of getMeasurementsForAllPackages() as the - // results of the measurements will be recorded, and that is what we're aiming - // for with this job. IBinder b = ServiceManager.getService(Context.BINARY_TRANSPARENCY_SERVICE); IBinaryTransparencyService iBtsService = IBinaryTransparencyService.Stub.asInterface(b); try { - iBtsService.getMeasurementsForAllPackages(); + iBtsService.recordMeasurementsForAllPackages(); } catch (RemoteException e) { Slog.e(TAG, "Taking binary measurements was interrupted.", e); return; From a562ab963f33d2f544ea1f744e85f203ee527148 Mon Sep 17 00:00:00 2001 From: Victor Hsieh Date: Thu, 12 Jan 2023 17:43:19 -0800 Subject: [PATCH 2/3] Organize MBA and APEX info with structs This change adds a data class (actually, a parcelable, for future testing purpose) for storing interesting MBA/APEX info. The code now stages the data in the struct first, then calls the new method to write the new object to the log. Bug: 265244016 Test: manual Change-Id: Iecc6933e6faf8b31a81ee4fae2443f3499bf099e --- .../os/IBinaryTransparencyService.aidl | 21 +++ .../server/BinaryTransparencyService.java | 133 +++++++++--------- 2 files changed, 88 insertions(+), 66 deletions(-) diff --git a/core/java/com/android/internal/os/IBinaryTransparencyService.aidl b/core/java/com/android/internal/os/IBinaryTransparencyService.aidl index 0772c8495f3a3..b4a0aac4d477b 100644 --- a/core/java/com/android/internal/os/IBinaryTransparencyService.aidl +++ b/core/java/com/android/internal/os/IBinaryTransparencyService.aidl @@ -29,4 +29,25 @@ interface IBinaryTransparencyService { List getApexInfo(); void recordMeasurementsForAllPackages(); + + parcelable ApexInfo { + String packageName; + long longVersion; + byte[] digest; + int digestAlgorithm; + String[] signerDigests; + } + + parcelable AppInfo { + String packageName; + long longVersion; + byte[] digest; + int digestAlgorithm; + String[] signerDigests; + int mbaStatus; + String initiator; + String[] initiatorSignerDigests; + String installer; + String originator; + } } \ No newline at end of file diff --git a/services/core/java/com/android/server/BinaryTransparencyService.java b/services/core/java/com/android/server/BinaryTransparencyService.java index eed1d769157fc..e21eb38fecfee 100644 --- a/services/core/java/com/android/server/BinaryTransparencyService.java +++ b/services/core/java/com/android/server/BinaryTransparencyService.java @@ -285,18 +285,16 @@ public class BinaryTransparencyService extends SystemService { Bundle apexMeasurement = measurePackage(packageInfo); if (record) { - // compute digests of signing info - String[] signerDigestHexStrings = computePackageSignerSha256Digests( - packageInfo.signingInfo); + var apexInfo = new IBinaryTransparencyService.ApexInfo(); + apexInfo.packageName = packageInfo.packageName; + apexInfo.longVersion = packageInfo.getLongVersionCode(); + apexInfo.digest = apexMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); + apexInfo.digestAlgorithm = + apexMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM); + apexInfo.signerDigests = + computePackageSignerSha256Digests(packageInfo.signingInfo); - // log to statsd - FrameworkStatsLog.write(FrameworkStatsLog.APEX_INFO_GATHERED, - packageInfo.packageName, - packageInfo.getLongVersionCode(), - HexEncoding.encodeToString(apexMeasurement.getByteArray( - BUNDLE_CONTENT_DIGEST), false), - apexMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM), - signerDigestHexStrings); + recordApexInfo(apexInfo); } } if (DEBUG) { @@ -313,11 +311,11 @@ public class BinaryTransparencyService extends SystemService { } packagesMeasured.add(packageInfo.packageName); - int mba_status = MBA_STATUS_PRELOADED; + int mbaStatus = MBA_STATUS_PRELOADED; if (packageInfo.signingInfo == null) { Slog.d(TAG, "Preload " + packageInfo.packageName + " at " + packageInfo.applicationInfo.sourceDir + " has likely been updated."); - mba_status = MBA_STATUS_UPDATED_PRELOAD; + mbaStatus = MBA_STATUS_UPDATED_PRELOAD; PackageInfo origPackageInfo = packageInfo; try { @@ -328,32 +326,24 @@ public class BinaryTransparencyService extends SystemService { Slog.e(TAG, "Failed to obtain an updated PackageInfo of " + origPackageInfo.packageName, e); packageInfo = origPackageInfo; - mba_status = MBA_STATUS_ERROR; + mbaStatus = MBA_STATUS_ERROR; } } + if (record && (mbaStatus == MBA_STATUS_UPDATED_PRELOAD)) { + Bundle packageMeasurement = measurePackage(packageInfo); - Bundle packageMeasurement = measurePackage(packageInfo); + var appInfo = new IBinaryTransparencyService.AppInfo(); + appInfo.packageName = packageInfo.packageName; + appInfo.longVersion = packageInfo.getLongVersionCode(); + appInfo.digest = packageMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); + appInfo.digestAlgorithm = + packageMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM); + appInfo.signerDigests = + computePackageSignerSha256Digests(packageInfo.signingInfo); + appInfo.mbaStatus = mbaStatus; - if (record && (mba_status == MBA_STATUS_UPDATED_PRELOAD)) { - // compute digests of signing info - String[] signerDigestHexStrings = computePackageSignerSha256Digests( - packageInfo.signingInfo); - - // now we should have all the bits for the atom - byte[] cDigest = packageMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); - FrameworkStatsLog.write(FrameworkStatsLog.MOBILE_BUNDLED_APP_INFO_GATHERED, - packageInfo.packageName, - packageInfo.getLongVersionCode(), - (cDigest != null) ? HexEncoding.encodeToString(cDigest, false) : null, - packageMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM), - signerDigestHexStrings, // signer_cert_digest - mba_status, // mba_status - null, // initiator - null, // initiator_signer_digest - null, // installer - null // originator - ); + writeAppInfoToLog(appInfo); } } if (DEBUG) { @@ -372,50 +362,36 @@ public class BinaryTransparencyService extends SystemService { Bundle packageMeasurement = measurePackage(packageInfo); if (record) { - // compute digests of signing info - String[] signerDigestHexStrings = computePackageSignerSha256Digests( - packageInfo.signingInfo); - - // then extract package's InstallSourceInfo if (DEBUG) { Slog.d(TAG, "Extracting InstallSourceInfo for " + packageInfo.packageName); } + var appInfo = new IBinaryTransparencyService.AppInfo(); + appInfo.packageName = packageInfo.packageName; + appInfo.longVersion = packageInfo.getLongVersionCode(); + appInfo.digest = packageMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); + appInfo.digestAlgorithm = + packageMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM); + appInfo.signerDigests = + computePackageSignerSha256Digests(packageInfo.signingInfo); + appInfo.mbaStatus = MBA_STATUS_NEW_INSTALL; + + // extract package's InstallSourceInfo InstallSourceInfo installSourceInfo = getInstallSourceInfo( packageInfo.packageName); - String initiator = null; - SigningInfo initiatorSignerInfo = null; - String[] initiatorSignerInfoDigest = null; - String installer = null; - String originator = null; - if (installSourceInfo != null) { - initiator = installSourceInfo.getInitiatingPackageName(); - initiatorSignerInfo = + appInfo.initiator = installSourceInfo.getInitiatingPackageName(); + SigningInfo initiatorSignerInfo = installSourceInfo.getInitiatingPackageSigningInfo(); if (initiatorSignerInfo != null) { - initiatorSignerInfoDigest = computePackageSignerSha256Digests( - initiatorSignerInfo); + appInfo.initiatorSignerDigests = + computePackageSignerSha256Digests(initiatorSignerInfo); } - installer = installSourceInfo.getInstallingPackageName(); - originator = installSourceInfo.getOriginatingPackageName(); + appInfo.installer = installSourceInfo.getInstallingPackageName(); + appInfo.originator = installSourceInfo.getOriginatingPackageName(); } - // we should now have all the info needed for the atom - byte[] cDigest = packageMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); - FrameworkStatsLog.write(FrameworkStatsLog.MOBILE_BUNDLED_APP_INFO_GATHERED, - packageInfo.packageName, - packageInfo.getLongVersionCode(), - (cDigest != null) ? HexEncoding.encodeToString(cDigest, false) - : null, - packageMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM), - signerDigestHexStrings, - MBA_STATUS_NEW_INSTALL, // mba_status - initiator, - initiatorSignerInfoDigest, - installer, - originator - ); + writeAppInfoToLog(appInfo); } } } @@ -426,6 +402,31 @@ public class BinaryTransparencyService extends SystemService { } } + private void recordApexInfo(IBinaryTransparencyService.ApexInfo apexInfo) { + FrameworkStatsLog.write(FrameworkStatsLog.APEX_INFO_GATHERED, + apexInfo.packageName, + apexInfo.longVersion, + (apexInfo.digest != null) ? HexEncoding.encodeToString(apexInfo.digest, false) + : null, + apexInfo.digestAlgorithm, + apexInfo.signerDigests); + } + + private void writeAppInfoToLog(IBinaryTransparencyService.AppInfo appInfo) { + FrameworkStatsLog.write(FrameworkStatsLog.MOBILE_BUNDLED_APP_INFO_GATHERED, + appInfo.packageName, + appInfo.longVersion, + (appInfo.digest != null) ? HexEncoding.encodeToString(appInfo.digest, false) + : null, + appInfo.digestAlgorithm, + appInfo.signerDigests, + appInfo.mbaStatus, + appInfo.initiator, + appInfo.initiatorSignerDigests, + appInfo.installer, + appInfo.originator); + } + /** * A wrapper around * {@link ApkSignatureVerifier#verifySignaturesInternal(ParseInput, String, int, boolean)}. From 256abd4133a3b15604e59be152e7015e2c067020 Mon Sep 17 00:00:00 2001 From: Victor Hsieh Date: Fri, 13 Jan 2023 11:13:47 -0800 Subject: [PATCH 3/3] Return early if no need to record measurement recordMeasurementsForAllPackages is guarded by the `record` boolean variable deep in the control flow. Now that the method only writes atoms, it can return early for simplicity. Bug: 265244016 Test: Manual Change-Id: Ic877c85a844d299cf71fa1d8d73d595ec1dad68d --- .../server/BinaryTransparencyService.java | 105 +++++++++--------- 1 file changed, 52 insertions(+), 53 deletions(-) diff --git a/services/core/java/com/android/server/BinaryTransparencyService.java b/services/core/java/com/android/server/BinaryTransparencyService.java index e21eb38fecfee..87870f6a7c278 100644 --- a/services/core/java/com/android/server/BinaryTransparencyService.java +++ b/services/core/java/com/android/server/BinaryTransparencyService.java @@ -253,7 +253,9 @@ public class BinaryTransparencyService extends SystemService { /** * Measures and records digests for *all* covered binaries/packages. * - * This method will be called in a Job scheduled to take measurements periodically. + * This method will be called in a Job scheduled to take measurements periodically. If the + * last measurement was performaned recently (less than RECORD_MEASUREMENT_COOLDOWN_MS + * ago), the measurement and recording will be skipped. * * Packages that are covered so far are: * - all APEXs (introduced in Android T) @@ -262,18 +264,19 @@ public class BinaryTransparencyService extends SystemService { * - dynamically installed mobile bundled apps (MBAs) (new in Android U) */ public void recordMeasurementsForAllPackages() { - PackageManager pm = mContext.getPackageManager(); - Set packagesMeasured = new HashSet<>(); - // check if we should record the resulting measurements long currentTimeMs = System.currentTimeMillis(); - boolean record = false; - if ((currentTimeMs - mMeasurementsLastRecordedMs) >= RECORD_MEASUREMENTS_COOLDOWN_MS) { - Slog.d(TAG, "Measurement was last taken at " + mMeasurementsLastRecordedMs - + " and is now updated to: " + currentTimeMs); - mMeasurementsLastRecordedMs = currentTimeMs; - record = true; + if ((currentTimeMs - mMeasurementsLastRecordedMs) < RECORD_MEASUREMENTS_COOLDOWN_MS) { + Slog.d(TAG, "Skip measurement since the last measurement was only taken at " + + mMeasurementsLastRecordedMs + " within the cooldown period"); + return; } + Slog.d(TAG, "Measurement was last taken at " + mMeasurementsLastRecordedMs + + " and is now updated to: " + currentTimeMs); + mMeasurementsLastRecordedMs = currentTimeMs; + + PackageManager pm = mContext.getPackageManager(); + Set packagesMeasured = new HashSet<>(); // measure all APEXs first if (DEBUG) { @@ -284,18 +287,16 @@ public class BinaryTransparencyService extends SystemService { Bundle apexMeasurement = measurePackage(packageInfo); - if (record) { - var apexInfo = new IBinaryTransparencyService.ApexInfo(); - apexInfo.packageName = packageInfo.packageName; - apexInfo.longVersion = packageInfo.getLongVersionCode(); - apexInfo.digest = apexMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); - apexInfo.digestAlgorithm = - apexMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM); - apexInfo.signerDigests = - computePackageSignerSha256Digests(packageInfo.signingInfo); + var apexInfo = new IBinaryTransparencyService.ApexInfo(); + apexInfo.packageName = packageInfo.packageName; + apexInfo.longVersion = packageInfo.getLongVersionCode(); + apexInfo.digest = apexMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); + apexInfo.digestAlgorithm = + apexMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM); + apexInfo.signerDigests = + computePackageSignerSha256Digests(packageInfo.signingInfo); - recordApexInfo(apexInfo); - } + recordApexInfo(apexInfo); } if (DEBUG) { Slog.d(TAG, "Measured " + packagesMeasured.size() @@ -330,7 +331,7 @@ public class BinaryTransparencyService extends SystemService { } } - if (record && (mbaStatus == MBA_STATUS_UPDATED_PRELOAD)) { + if (mbaStatus == MBA_STATUS_UPDATED_PRELOAD) { Bundle packageMeasurement = measurePackage(packageInfo); var appInfo = new IBinaryTransparencyService.AppInfo(); @@ -361,38 +362,36 @@ public class BinaryTransparencyService extends SystemService { Bundle packageMeasurement = measurePackage(packageInfo); - if (record) { - if (DEBUG) { - Slog.d(TAG, - "Extracting InstallSourceInfo for " + packageInfo.packageName); - } - var appInfo = new IBinaryTransparencyService.AppInfo(); - appInfo.packageName = packageInfo.packageName; - appInfo.longVersion = packageInfo.getLongVersionCode(); - appInfo.digest = packageMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); - appInfo.digestAlgorithm = - packageMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM); - appInfo.signerDigests = - computePackageSignerSha256Digests(packageInfo.signingInfo); - appInfo.mbaStatus = MBA_STATUS_NEW_INSTALL; - - // extract package's InstallSourceInfo - InstallSourceInfo installSourceInfo = getInstallSourceInfo( - packageInfo.packageName); - if (installSourceInfo != null) { - appInfo.initiator = installSourceInfo.getInitiatingPackageName(); - SigningInfo initiatorSignerInfo = - installSourceInfo.getInitiatingPackageSigningInfo(); - if (initiatorSignerInfo != null) { - appInfo.initiatorSignerDigests = - computePackageSignerSha256Digests(initiatorSignerInfo); - } - appInfo.installer = installSourceInfo.getInstallingPackageName(); - appInfo.originator = installSourceInfo.getOriginatingPackageName(); - } - - writeAppInfoToLog(appInfo); + if (DEBUG) { + Slog.d(TAG, + "Extracting InstallSourceInfo for " + packageInfo.packageName); } + var appInfo = new IBinaryTransparencyService.AppInfo(); + appInfo.packageName = packageInfo.packageName; + appInfo.longVersion = packageInfo.getLongVersionCode(); + appInfo.digest = packageMeasurement.getByteArray(BUNDLE_CONTENT_DIGEST); + appInfo.digestAlgorithm = + packageMeasurement.getInt(BUNDLE_CONTENT_DIGEST_ALGORITHM); + appInfo.signerDigests = + computePackageSignerSha256Digests(packageInfo.signingInfo); + appInfo.mbaStatus = MBA_STATUS_NEW_INSTALL; + + // extract package's InstallSourceInfo + InstallSourceInfo installSourceInfo = getInstallSourceInfo( + packageInfo.packageName); + if (installSourceInfo != null) { + appInfo.initiator = installSourceInfo.getInitiatingPackageName(); + SigningInfo initiatorSignerInfo = + installSourceInfo.getInitiatingPackageSigningInfo(); + if (initiatorSignerInfo != null) { + appInfo.initiatorSignerDigests = + computePackageSignerSha256Digests(initiatorSignerInfo); + } + appInfo.installer = installSourceInfo.getInstallingPackageName(); + appInfo.originator = installSourceInfo.getOriginatingPackageName(); + } + + writeAppInfoToLog(appInfo); } } if (DEBUG) {