Skip to content

Commit 8be305d

Browse files
Jeff SharkeyAndroid (Google) Code Review
Jeff Sharkey
authored and
Android (Google) Code Review
committed
Merge "Also check app-ops on path-permissions." into mnc-dev
2 parents d9f1a0c + 0e621c3 commit 8be305d

2 files changed

Lines changed: 71 additions & 41 deletions

File tree

‎core/java/android/content/ContentProvider.java‎

Lines changed: 68 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616

1717
package android.content;
1818

19-
import static android.content.pm.PackageManager.PERMISSION_GRANTED;
2019
import static android.Manifest.permission.INTERACT_ACROSS_USERS;
20+
import static android.app.AppOpsManager.MODE_ALLOWED;
21+
import static android.app.AppOpsManager.MODE_ERRORED;
22+
import static android.app.AppOpsManager.MODE_IGNORED;
23+
import static android.content.pm.PackageManager.PERMISSION_GRANTED;
2124

2225
import android.annotation.NonNull;
2326
import android.annotation.Nullable;
@@ -40,8 +43,8 @@
4043
import android.os.ParcelFileDescriptor;
4144
import android.os.Process;
4245
import android.os.UserHandle;
43-
import android.util.Log;
4446
import android.text.TextUtils;
47+
import android.util.Log;
4548

4649
import java.io.File;
4750
import java.io.FileDescriptor;
@@ -474,14 +477,9 @@ private void enforceFilePermission(String callingPkg, Uri uri, String mode,
474477

475478
private int enforceReadPermission(String callingPkg, Uri uri, IBinder callerToken)
476479
throws SecurityException {
477-
enforceReadPermissionInner(uri, callerToken);
478-
479-
final int permOp = AppOpsManager.permissionToOpCode(mReadPermission);
480-
if (permOp != AppOpsManager.OP_NONE) {
481-
final int mode = mAppOpsManager.noteProxyOp(permOp, callingPkg);
482-
if (mode != AppOpsManager.MODE_ALLOWED) {
483-
return mode;
484-
}
480+
final int mode = enforceReadPermissionInner(uri, callingPkg, callerToken);
481+
if (mode != MODE_ALLOWED) {
482+
return mode;
485483
}
486484

487485
if (mReadOp != AppOpsManager.OP_NONE) {
@@ -493,14 +491,9 @@ private int enforceReadPermission(String callingPkg, Uri uri, IBinder callerToke
493491

494492
private int enforceWritePermission(String callingPkg, Uri uri, IBinder callerToken)
495493
throws SecurityException {
496-
enforceWritePermissionInner(uri, callerToken);
497-
498-
final int permOp = AppOpsManager.permissionToOpCode(mWritePermission);
499-
if (permOp != AppOpsManager.OP_NONE) {
500-
final int mode = mAppOpsManager.noteProxyOp(permOp, callingPkg);
501-
if (mode != AppOpsManager.MODE_ALLOWED) {
502-
return mode;
503-
}
494+
final int mode = enforceWritePermissionInner(uri, callingPkg, callerToken);
495+
if (mode != MODE_ALLOWED) {
496+
return mode;
504497
}
505498

506499
if (mWriteOp != AppOpsManager.OP_NONE) {
@@ -518,26 +511,47 @@ boolean checkUser(int pid, int uid, Context context) {
518511
== PERMISSION_GRANTED;
519512
}
520513

514+
/**
515+
* Verify that calling app holds both the given permission and any app-op
516+
* associated with that permission.
517+
*/
518+
private int checkPermissionAndAppOp(String permission, String callingPkg,
519+
IBinder callerToken) {
520+
if (getContext().checkPermission(permission, Binder.getCallingPid(), Binder.getCallingUid(),
521+
callerToken) != PERMISSION_GRANTED) {
522+
return MODE_ERRORED;
523+
}
524+
525+
final int permOp = AppOpsManager.permissionToOpCode(permission);
526+
if (permOp != AppOpsManager.OP_NONE) {
527+
return mTransport.mAppOpsManager.noteProxyOp(permOp, callingPkg);
528+
}
529+
530+
return MODE_ALLOWED;
531+
}
532+
521533
/** {@hide} */
522-
protected void enforceReadPermissionInner(Uri uri, IBinder callerToken)
534+
protected int enforceReadPermissionInner(Uri uri, String callingPkg, IBinder callerToken)
523535
throws SecurityException {
524536
final Context context = getContext();
525537
final int pid = Binder.getCallingPid();
526538
final int uid = Binder.getCallingUid();
527539
String missingPerm = null;
540+
int strongestMode = MODE_ALLOWED;
528541

529542
if (UserHandle.isSameApp(uid, mMyUid)) {
530-
return;
543+
return MODE_ALLOWED;
531544
}
532545

533546
if (mExported && checkUser(pid, uid, context)) {
534547
final String componentPerm = getReadPermission();
535548
if (componentPerm != null) {
536-
if (context.checkPermission(componentPerm, pid, uid, callerToken)
537-
== PERMISSION_GRANTED) {
538-
return;
549+
final int mode = checkPermissionAndAppOp(componentPerm, callingPkg, callerToken);
550+
if (mode == MODE_ALLOWED) {
551+
return MODE_ALLOWED;
539552
} else {
540553
missingPerm = componentPerm;
554+
strongestMode = Math.max(strongestMode, mode);
541555
}
542556
}
543557

@@ -551,22 +565,23 @@ protected void enforceReadPermissionInner(Uri uri, IBinder callerToken)
551565
for (PathPermission pp : pps) {
552566
final String pathPerm = pp.getReadPermission();
553567
if (pathPerm != null && pp.match(path)) {
554-
if (context.checkPermission(pathPerm, pid, uid, callerToken)
555-
== PERMISSION_GRANTED) {
556-
return;
568+
final int mode = checkPermissionAndAppOp(pathPerm, callingPkg, callerToken);
569+
if (mode == MODE_ALLOWED) {
570+
return MODE_ALLOWED;
557571
} else {
558572
// any denied <path-permission> means we lose
559573
// default <provider> access.
560574
allowDefaultRead = false;
561575
missingPerm = pathPerm;
576+
strongestMode = Math.max(strongestMode, mode);
562577
}
563578
}
564579
}
565580
}
566581

567582
// if we passed <path-permission> checks above, and no default
568583
// <provider> permission, then allow access.
569-
if (allowDefaultRead) return;
584+
if (allowDefaultRead) return MODE_ALLOWED;
570585
}
571586

572587
// last chance, check against any uri grants
@@ -575,7 +590,13 @@ protected void enforceReadPermissionInner(Uri uri, IBinder callerToken)
575590
? maybeAddUserId(uri, callingUserId) : uri;
576591
if (context.checkUriPermission(userUri, pid, uid, Intent.FLAG_GRANT_READ_URI_PERMISSION,
577592
callerToken) == PERMISSION_GRANTED) {
578-
return;
593+
return MODE_ALLOWED;
594+
}
595+
596+
// If the worst denial we found above was ignored, then pass that
597+
// ignored through; otherwise we assume it should be a real error below.
598+
if (strongestMode == MODE_IGNORED) {
599+
return MODE_IGNORED;
579600
}
580601

581602
final String failReason = mExported
@@ -587,25 +608,27 @@ protected void enforceReadPermissionInner(Uri uri, IBinder callerToken)
587608
}
588609

589610
/** {@hide} */
590-
protected void enforceWritePermissionInner(Uri uri, IBinder callerToken)
611+
protected int enforceWritePermissionInner(Uri uri, String callingPkg, IBinder callerToken)
591612
throws SecurityException {
592613
final Context context = getContext();
593614
final int pid = Binder.getCallingPid();
594615
final int uid = Binder.getCallingUid();
595616
String missingPerm = null;
617+
int strongestMode = MODE_ALLOWED;
596618

597619
if (UserHandle.isSameApp(uid, mMyUid)) {
598-
return;
620+
return MODE_ALLOWED;
599621
}
600622

601623
if (mExported && checkUser(pid, uid, context)) {
602624
final String componentPerm = getWritePermission();
603625
if (componentPerm != null) {
604-
if (context.checkPermission(componentPerm, pid, uid, callerToken)
605-
== PERMISSION_GRANTED) {
606-
return;
626+
final int mode = checkPermissionAndAppOp(componentPerm, callingPkg, callerToken);
627+
if (mode == MODE_ALLOWED) {
628+
return MODE_ALLOWED;
607629
} else {
608630
missingPerm = componentPerm;
631+
strongestMode = Math.max(strongestMode, mode);
609632
}
610633
}
611634

@@ -619,28 +642,35 @@ protected void enforceWritePermissionInner(Uri uri, IBinder callerToken)
619642
for (PathPermission pp : pps) {
620643
final String pathPerm = pp.getWritePermission();
621644
if (pathPerm != null && pp.match(path)) {
622-
if (context.checkPermission(pathPerm, pid, uid, callerToken)
623-
== PERMISSION_GRANTED) {
624-
return;
645+
final int mode = checkPermissionAndAppOp(pathPerm, callingPkg, callerToken);
646+
if (mode == MODE_ALLOWED) {
647+
return MODE_ALLOWED;
625648
} else {
626649
// any denied <path-permission> means we lose
627650
// default <provider> access.
628651
allowDefaultWrite = false;
629652
missingPerm = pathPerm;
653+
strongestMode = Math.max(strongestMode, mode);
630654
}
631655
}
632656
}
633657
}
634658

635659
// if we passed <path-permission> checks above, and no default
636660
// <provider> permission, then allow access.
637-
if (allowDefaultWrite) return;
661+
if (allowDefaultWrite) return MODE_ALLOWED;
638662
}
639663

640664
// last chance, check against any uri grants
641665
if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_WRITE_URI_PERMISSION,
642666
callerToken) == PERMISSION_GRANTED) {
643-
return;
667+
return MODE_ALLOWED;
668+
}
669+
670+
// If the worst denial we found above was ignored, then pass that
671+
// ignored through; otherwise we assume it should be a real error below.
672+
if (strongestMode == MODE_IGNORED) {
673+
return MODE_IGNORED;
644674
}
645675

646676
final String failReason = mExported

‎core/java/android/provider/DocumentsProvider.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ public Bundle call(String method, String arg, Bundle extras) {
640640
final Bundle out = new Bundle();
641641
try {
642642
if (METHOD_CREATE_DOCUMENT.equals(method)) {
643-
enforceWritePermissionInner(documentUri, null);
643+
enforceWritePermissionInner(documentUri, getCallingPackage(), null);
644644

645645
final String mimeType = extras.getString(Document.COLUMN_MIME_TYPE);
646646
final String displayName = extras.getString(Document.COLUMN_DISPLAY_NAME);
@@ -654,7 +654,7 @@ public Bundle call(String method, String arg, Bundle extras) {
654654
out.putParcelable(DocumentsContract.EXTRA_URI, newDocumentUri);
655655

656656
} else if (METHOD_RENAME_DOCUMENT.equals(method)) {
657-
enforceWritePermissionInner(documentUri, null);
657+
enforceWritePermissionInner(documentUri, getCallingPackage(), null);
658658

659659
final String displayName = extras.getString(Document.COLUMN_DISPLAY_NAME);
660660
final String newDocumentId = renameDocument(documentId, displayName);
@@ -678,7 +678,7 @@ public Bundle call(String method, String arg, Bundle extras) {
678678
}
679679

680680
} else if (METHOD_DELETE_DOCUMENT.equals(method)) {
681-
enforceWritePermissionInner(documentUri, null);
681+
enforceWritePermissionInner(documentUri, getCallingPackage(), null);
682682
deleteDocument(documentId);
683683

684684
// Document no longer exists, clean up any grants

0 commit comments

Comments
 (0)