Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0ab1da2e authored by Guojing Yuan's avatar Guojing Yuan
Browse files

[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
parent e87bf5f5

services/companion/java/com/android/server/companion/datatransfer/CompanionMessageProcessor.java

+9 −3
Original line number Diff line number Diff line
@@ -102,9 +102,11 @@ public class CompanionMessageProcessor { 
                    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 class CompanionMessageProcessor { 
     */

    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 @@ public class CompanionMessageProcessor { 
                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();

services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java

+5 −1
Original line number Diff line number Diff line
@@ -219,8 +219,9 @@ public class SystemDataTransferProcessor { 
    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 @@ public class SystemDataTransferProcessor { 
        } catch (XmlPullParserException e) {

            Slog.e(LOG_TAG, "Error parsing message: "

                    + new String(messageInfo.getData()));

        } finally {

            Slog.i(LOG_TAG, "Permissions applied.");

            Binder.restoreCallingIdentity(callingIdentityToken);

        }

    }

services/companion/java/com/android/server/companion/datatransfer/permbackup/BackupHelper.java

+46 −2
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ import org.xmlpull.v1.XmlPullParserException; 
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 @@ public class BackupHelper { 
    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 @@ public class BackupHelper { 
                                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 @@ public class BackupHelper { 
                    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 @@ public class BackupHelper { 
         * @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<>();

services/companion/java/com/android/server/companion/datatransfer/permbackup/model/AppPermissionGroup.java

+42 −42
Original line number Diff line number Diff line
@@ -37,7 +37,6 @@ import android.content.pm.PackageManager; 
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 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
                }

            }



            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 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
//            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 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
//                                    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);

//        }

    }



    /**

services/companion/java/com/android/server/companion/proto/companion_message.proto

+2 −2
Original line number Diff line number Diff line
@@ -22,7 +22,7 @@ package com.android.server.companion.proto; 


/* Represents a message between companion devices */

message CompanionMessage {

  int64 id = 1;

  int32 id = 1;



  PaginationInfo paginationInfo = 2;
@@ -34,7 +34,7 @@ message CompanionMessage { 
  /* 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;