[CDM perm sync] End to end test fixes
1. Add signature verification
2. Commented unnecessary permission restore logic, which checks the
permissions that can't be bypassed by the system uid.
Bug: 231474219
Test: manually tested end to end, and confirmed the permissions are
synced.
Change-Id: I582f318a8fe04d4120596be480760714b8e4acf1
diff --git a/services/companion/java/com/android/server/companion/datatransfer/CompanionMessageProcessor.java b/services/companion/java/com/android/server/companion/datatransfer/CompanionMessageProcessor.java
index e83e634..98a00aa6 100644
--- a/services/companion/java/com/android/server/companion/datatransfer/CompanionMessageProcessor.java
+++ b/services/companion/java/com/android/server/companion/datatransfer/CompanionMessageProcessor.java
@@ -102,9 +102,11 @@
Math.min((i + 1) * MESSAGE_SIZE_IN_BYTES, data.length));
proto.write(CompanionMessage.DATA, currentData);
- Slog.i(LOG_TAG, "Sending " + currentData.length + " bytes to " + packageName);
+ byte[] message = proto.getBytes();
- mSecureCommsManager.sendSecureMessage(associationId, messageId, proto.getBytes());
+ Slog.i(LOG_TAG, "Sending [" + message.length + "] bytes to " + packageName);
+
+ mSecureCommsManager.sendSecureMessage(associationId, messageId, message);
}
}
@@ -115,6 +117,9 @@
*/
public CompanionMessageInfo onDecryptedMessageReceived(int messageId, int associationId,
byte[] message) {
+ Slog.i(LOG_TAG, "Partial message received, size [" + message.length
+ + "], reading from protobuf.");
+
ProtoInputStream proto = new ProtoInputStream(message);
try {
int id = 0;
@@ -178,7 +183,8 @@
mAssociationsMessagesMap.put(associationId, associationMessages);
// Check if all the messages with the same parentId are received.
if (childMessages.size() == total) {
- Slog.i(LOG_TAG, "All [" + total + "] messages are received. Processing.");
+ Slog.i(LOG_TAG, "All [" + total + "] messages are received for parentId ["
+ + parentId + "]. Processing.");
childMessages.sort(Comparator.comparing(CompanionMessageInfo::getPage));
ByteArrayOutputStream stream = new ByteArrayOutputStream();
diff --git a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java
index d39fa2a..395cf18 100644
--- a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java
+++ b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java
@@ -219,8 +219,9 @@
private void processPermissionSyncMessage(CompanionMessageInfo messageInfo) {
Slog.i(LOG_TAG, "Applying permissions.");
// Start applying permissions
- BackupHelper backupHelper = new BackupHelper(mContext, mContext.getUser());
+ final long callingIdentityToken = Binder.clearCallingIdentity();
try {
+ BackupHelper backupHelper = new BackupHelper(mContext, mContext.getUser());
XmlPullParser parser = Xml.newPullParser();
ByteArrayInputStream stream = new ByteArrayInputStream(
messageInfo.getData());
@@ -233,6 +234,9 @@
} catch (XmlPullParserException e) {
Slog.e(LOG_TAG, "Error parsing message: "
+ new String(messageInfo.getData()));
+ } finally {
+ Slog.i(LOG_TAG, "Permissions applied.");
+ Binder.restoreCallingIdentity(callingIdentityToken);
}
}
diff --git a/services/companion/java/com/android/server/companion/datatransfer/permbackup/BackupHelper.java b/services/companion/java/com/android/server/companion/datatransfer/permbackup/BackupHelper.java
index f41aa2f..5e3c4c7 100644
--- a/services/companion/java/com/android/server/companion/datatransfer/permbackup/BackupHelper.java
+++ b/services/companion/java/com/android/server/companion/datatransfer/permbackup/BackupHelper.java
@@ -49,6 +49,7 @@
import org.xmlpull.v1.XmlSerializer;
import java.io.IOException;
+import java.security.cert.CertificateException;
import java.util.ArrayList;
import java.util.List;
@@ -68,6 +69,7 @@
private static final String ATTR_HAS_MULTIPLE_SIGNERS = "multi-signers";
private static final String TAG_SIGNATURE = "sig";
+ private static final String ATTR_SIGNATURE_VALUE = "v";
private static final String TAG_PERMISSION = "perm";
private static final String ATTR_PERMISSION_NAME = "name";
@@ -584,7 +586,8 @@
skipToEndOfTag(parser);
break;
case TAG_SIGNATURE:
- signatureList.add(new Signature(parser.getText()));
+ signatureList.add(new Signature(
+ parser.getAttributeValue(null, ATTR_SIGNATURE_VALUE)));
skipToEndOfTag(parser);
break;
default:
@@ -681,7 +684,7 @@
String.valueOf(mHasMultipleSigners));
for (Signature signature : mSignatures) {
serializer.startTag(null, TAG_SIGNATURE);
- serializer.text(signature.toCharsString());
+ serializer.attribute(null, ATTR_SIGNATURE_VALUE, signature.toCharsString());
serializer.endTag(null, TAG_SIGNATURE);
}
@@ -700,6 +703,47 @@
* @param pkgInfo The package to restore.
*/
void restore(@NonNull Context context, @NonNull PackageInfo pkgInfo) {
+ Slog.e(LOG_TAG, "Restoring permissions for package [" + mPackageName + "]");
+
+ // Verify signature info
+ try {
+ if (mHasMultipleSigners && pkgInfo.signingInfo.hasMultipleSigners()) {
+ // If both packages are signed by multi signers, check if two signature sets are
+ // effectively matched.
+ if (!Signature.areEffectiveMatch(mSignatures,
+ pkgInfo.signingInfo.getApkContentsSigners())) {
+ Slog.e(LOG_TAG, "Multi-signers signatures don't match for package ["
+ + mPackageName + "], skipped.");
+ return;
+ }
+ } else if (!mHasMultipleSigners && !pkgInfo.signingInfo.hasMultipleSigners()) {
+ // If both packages are not signed by multi signers, check if two signature sets
+ // have overlaps.
+ Signature[] signatures = pkgInfo.signingInfo.getSigningCertificateHistory();
+ if (signatures == null) {
+ Slog.e(LOG_TAG, "The dest package is unsigned.");
+ return;
+ }
+ boolean isMatched = false;
+ for (int i = 0; i < mSignatures.length; i++) {
+ for (int j = 0; j < signatures.length; j++) {
+ isMatched = Signature.areEffectiveMatch(mSignatures[i], signatures[j]);
+ }
+ }
+ if (!isMatched) {
+ Slog.e(LOG_TAG, "Single signer signatures don't match for package ["
+ + mPackageName + "], skipped.");
+ return;
+ }
+ } else {
+ Slog.e(LOG_TAG, "Number of signers don't match.");
+ return;
+ }
+ } catch (CertificateException ce) {
+ Slog.e(LOG_TAG, "Either the source or the dest package's bounced cert length "
+ + "looks fishy, skipped package [" + pkgInfo.packageName + "]");
+ }
+
AppPermissions appPerms = new AppPermissions(context, pkgInfo, false, true, null);
ArraySet<String> affectedPermissions = new ArraySet<>();
diff --git a/services/companion/java/com/android/server/companion/datatransfer/permbackup/model/AppPermissionGroup.java b/services/companion/java/com/android/server/companion/datatransfer/permbackup/model/AppPermissionGroup.java
index 71d561e..cf146ac 100644
--- a/services/companion/java/com/android/server/companion/datatransfer/permbackup/model/AppPermissionGroup.java
+++ b/services/companion/java/com/android/server/companion/datatransfer/permbackup/model/AppPermissionGroup.java
@@ -37,7 +37,6 @@
import android.content.pm.PackageManager.NameNotFoundException;
import android.content.pm.PermissionGroupInfo;
import android.content.pm.PermissionInfo;
-import android.os.Binder;
import android.os.Build;
import android.os.UserHandle;
import android.permission.PermissionManager;
@@ -1438,29 +1437,30 @@
}
}
- int flags = (permission.isUserSet() ? PackageManager.FLAG_PERMISSION_USER_SET : 0)
- | (permission.isUserFixed() ? PackageManager.FLAG_PERMISSION_USER_FIXED : 0)
- | (permission.isRevokedCompat()
- ? PackageManager.FLAG_PERMISSION_REVOKED_COMPAT : 0)
- | (permission.isPolicyFixed() ? PackageManager.FLAG_PERMISSION_POLICY_FIXED : 0)
- | (permission.isReviewRequired()
- ? PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED : 0)
- | (permission.isOneTime() ? PackageManager.FLAG_PERMISSION_ONE_TIME : 0)
- | (permission.isSelectedLocationAccuracy()
- ? PackageManager.FLAG_PERMISSION_SELECTED_LOCATION_ACCURACY : 0);
+// int flags = (permission.isUserSet() ? PackageManager.FLAG_PERMISSION_USER_SET : 0)
+// | (permission.isUserFixed() ? PackageManager.FLAG_PERMISSION_USER_FIXED : 0)
+// | (permission.isRevokedCompat()
+// ? PackageManager.FLAG_PERMISSION_REVOKED_COMPAT : 0)
+// | (permission.isPolicyFixed() ?
+// PackageManager.FLAG_PERMISSION_POLICY_FIXED : 0)
+// | (permission.isReviewRequired()
+// ? PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED : 0)
+// | (permission.isOneTime() ? PackageManager.FLAG_PERMISSION_ONE_TIME : 0)
+// | (permission.isSelectedLocationAccuracy()
+// ? PackageManager.FLAG_PERMISSION_SELECTED_LOCATION_ACCURACY : 0);
- mPackageManager.updatePermissionFlags(permission.getName(),
- mPackageInfo.packageName,
- PackageManager.FLAG_PERMISSION_USER_SET
- | PackageManager.FLAG_PERMISSION_USER_FIXED
- | PackageManager.FLAG_PERMISSION_REVOKED_COMPAT
- | PackageManager.FLAG_PERMISSION_POLICY_FIXED
- | (permission.isReviewRequired()
- ? 0 : PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED)
- | PackageManager.FLAG_PERMISSION_ONE_TIME
- | PackageManager.FLAG_PERMISSION_AUTO_REVOKED // clear auto revoke
- | PackageManager.FLAG_PERMISSION_SELECTED_LOCATION_ACCURACY,
- flags, mUserHandle);
+// mPackageManager.updatePermissionFlags(permission.getName(),
+// mPackageInfo.packageName,
+// PackageManager.FLAG_PERMISSION_USER_SET
+// | PackageManager.FLAG_PERMISSION_USER_FIXED
+// | PackageManager.FLAG_PERMISSION_REVOKED_COMPAT
+// | PackageManager.FLAG_PERMISSION_POLICY_FIXED
+// | (permission.isReviewRequired()
+// ? 0 : PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED)
+// | PackageManager.FLAG_PERMISSION_ONE_TIME
+// | PackageManager.FLAG_PERMISSION_AUTO_REVOKED // clear auto revoke
+// | PackageManager.FLAG_PERMISSION_SELECTED_LOCATION_ACCURACY,
+// flags, mUserHandle);
if (permission.affectsAppOp()) {
if (!permission.isSystemFixed()) {
@@ -1486,18 +1486,18 @@
// mTriggerLocationAccessCheckOnPersist = false;
// }
- String packageName = mPackageInfo.packageName;
- if (areRuntimePermissionsGranted(null, true, false)) {
- // Required to read device config in Utils.getOneTimePermissions*().
- final long token = Binder.clearCallingIdentity();
- try {
+// String packageName = mPackageInfo.packageName;
+// if (areRuntimePermissionsGranted(null, true, false)) {
+// // Required to read device config in Utils.getOneTimePermissions*().
+// final long token = Binder.clearCallingIdentity();
+// try {
// if (SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
- mContext.getSystemService(PermissionManager.class)
- .startOneTimePermissionSession(packageName,
- Utils.getOneTimePermissionsTimeout(),
- Utils.getOneTimePermissionsKilledDelay(mIsSelfRevoked),
- ONE_TIME_PACKAGE_IMPORTANCE_LEVEL_TO_RESET_TIMER,
- ONE_TIME_PACKAGE_IMPORTANCE_LEVEL_TO_KEEP_SESSION_ALIVE);
+// mContext.getSystemService(PermissionManager.class)
+// .startOneTimePermissionSession(packageName,
+// Utils.getOneTimePermissionsTimeout(),
+// Utils.getOneTimePermissionsKilledDelay(mIsSelfRevoked),
+// ONE_TIME_PACKAGE_IMPORTANCE_LEVEL_TO_RESET_TIMER,
+// ONE_TIME_PACKAGE_IMPORTANCE_LEVEL_TO_KEEP_SESSION_ALIVE);
// } else {
// mContext.getSystemService(PermissionManager.class)
// .startOneTimePermissionSession(packageName,
@@ -1505,13 +1505,13 @@
// ONE_TIME_PACKAGE_IMPORTANCE_LEVEL_TO_RESET_TIMER,
// ONE_TIME_PACKAGE_IMPORTANCE_LEVEL_TO_KEEP_SESSION_ALIVE);
// }
- } finally {
- Binder.restoreCallingIdentity(token);
- }
- } else {
- mContext.getSystemService(PermissionManager.class)
- .stopOneTimePermissionSession(packageName);
- }
+// } finally {
+// Binder.restoreCallingIdentity(token);
+// }
+// } else {
+// mContext.getSystemService(PermissionManager.class)
+// .stopOneTimePermissionSession(packageName);
+// }
}
/**
diff --git a/services/companion/java/com/android/server/companion/proto/companion_message.proto b/services/companion/java/com/android/server/companion/proto/companion_message.proto
index 4e8027f..893a0db 100644
--- a/services/companion/java/com/android/server/companion/proto/companion_message.proto
+++ b/services/companion/java/com/android/server/companion/proto/companion_message.proto
@@ -22,7 +22,7 @@
/* Represents a message between companion devices */
message CompanionMessage {
- int64 id = 1;
+ int32 id = 1;
PaginationInfo paginationInfo = 2;
@@ -34,7 +34,7 @@
/* Message pagination info */
message PaginationInfo {
// id of the parent message, which should be the same for all the paginated messages
- int64 parentId = 1;
+ int32 parentId = 1;
// page number of the current message in [1, total]
int32 page = 2;