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

Commit e37ea612 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Fix document management permission enforcement.

Allow both explicit holders of the MANAGE_DOCUMENTS permission and
those holding Uri grants to perform management tasks.

Extend grants for newly created documents when caller doesn't have
permission.  Revoke grants when deleting documents.

Test now writes actual content into picked file.  Workaround updated
flags for Drive app.

Bug: 10623211
Change-Id: Ia8e90b33e0fac8294b2cacb96d083c43fdf75aab
parent 954be023
Loading
Loading
Loading
Loading
+31 −7
Original line number Diff line number Diff line
@@ -28,11 +28,13 @@ import android.content.ContentValues;
import android.content.Context;
import android.content.Intent;
import android.content.UriMatcher;
import android.content.pm.PackageManager;
import android.content.pm.ProviderInfo;
import android.content.res.AssetFileDescriptor;
import android.database.Cursor;
import android.graphics.Point;
import android.net.Uri;
import android.os.Binder;
import android.os.Bundle;
import android.os.CancellationSignal;
import android.os.ParcelFileDescriptor;
@@ -40,6 +42,8 @@ import android.os.ParcelFileDescriptor.OnCloseListener;
import android.provider.DocumentsContract.Document;
import android.util.Log;

import com.android.internal.util.ArrayUtils;

import libcore.io.IoUtils;

import java.io.FileNotFoundException;
@@ -328,16 +332,24 @@ public abstract class DocumentsProvider extends ContentProvider {
    @Override
    public final Bundle callFromPackage(
            String callingPackage, String method, String arg, Bundle extras) {
        final Context context = getContext();

        if (!method.startsWith("android:")) {
            // Let non-platform methods pass through
            return super.callFromPackage(callingPackage, method, arg, extras);
        }

        // Require that caller can manage given document
        final String documentId = extras.getString(Document.COLUMN_DOCUMENT_ID);
        final Uri documentUri = DocumentsContract.buildDocumentUri(mAuthority, documentId);

        // Require that caller can manage given document
        final boolean callerHasManage =
                context.checkCallingOrSelfPermission(android.Manifest.permission.MANAGE_DOCUMENTS)
                == PackageManager.PERMISSION_GRANTED;
        if (!callerHasManage) {
            getContext().enforceCallingOrSelfUriPermission(
                    documentUri, Intent.FLAG_GRANT_WRITE_URI_PERMISSION, method);
        }

        final Bundle out = new Bundle();
        try {
@@ -345,14 +357,26 @@ public abstract class DocumentsProvider extends ContentProvider {
                final String mimeType = extras.getString(Document.COLUMN_MIME_TYPE);
                final String displayName = extras.getString(Document.COLUMN_DISPLAY_NAME);

                // TODO: issue Uri grant towards calling package
                // TODO: enforce that package belongs to caller
                final String newDocumentId = createDocument(documentId, mimeType, displayName);
                out.putString(Document.COLUMN_DOCUMENT_ID, newDocumentId);

                // Extend permission grant towards caller if needed
                if (!callerHasManage) {
                    final Uri newDocumentUri = DocumentsContract.buildDocumentUri(
                            mAuthority, newDocumentId);
                    context.grantUriPermission(callingPackage, newDocumentUri,
                            Intent.FLAG_GRANT_READ_URI_PERMISSION
                            | Intent.FLAG_GRANT_WRITE_URI_PERMISSION
                            | Intent.FLAG_PERSIST_GRANT_URI_PERMISSION);
                }

            } else if (METHOD_DELETE_DOCUMENT.equals(method)) {
                final String docId = extras.getString(Document.COLUMN_DOCUMENT_ID);
                deleteDocument(docId);
                deleteDocument(documentId);

                // Document no longer exists, clean up any grants
                context.revokeUriPermission(documentUri, Intent.FLAG_GRANT_READ_URI_PERMISSION
                        | Intent.FLAG_GRANT_WRITE_URI_PERMISSION
                        | Intent.FLAG_PERSIST_GRANT_URI_PERMISSION);

            } else {
                throw new UnsupportedOperationException("Method not supported " + method);
+50 −21
Original line number Diff line number Diff line
@@ -33,10 +33,14 @@ import libcore.io.IoUtils;
import libcore.io.Streams;

import java.io.InputStream;
import java.io.OutputStream;

public class TestActivity extends Activity {
    private static final String TAG = "TestActivity";

    private static final int CODE_READ = 42;
    private static final int CODE_WRITE = 43;

    private TextView mResult;

    @Override
@@ -49,10 +53,10 @@ public class TestActivity extends Activity {
        view.setOrientation(LinearLayout.VERTICAL);

        final CheckBox multiple = new CheckBox(context);
        multiple.setText("\nALLOW_MULTIPLE\n");
        multiple.setText("ALLOW_MULTIPLE");
        view.addView(multiple);
        final CheckBox localOnly = new CheckBox(context);
        localOnly.setText("\nLOCAL_ONLY\n");
        localOnly.setText("LOCAL_ONLY");
        view.addView(localOnly);

        Button button;
@@ -70,7 +74,7 @@ public class TestActivity extends Activity {
                if (localOnly.isChecked()) {
                    intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                }
                startActivityForResult(intent, 42);
                startActivityForResult(intent, CODE_READ);
            }
        });
        view.addView(button);
@@ -89,7 +93,7 @@ public class TestActivity extends Activity {
                if (localOnly.isChecked()) {
                    intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                }
                startActivityForResult(intent, 42);
                startActivityForResult(intent, CODE_READ);
            }
        });
        view.addView(button);
@@ -108,7 +112,7 @@ public class TestActivity extends Activity {
                if (localOnly.isChecked()) {
                    intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                }
                startActivityForResult(intent, 42);
                startActivityForResult(intent, CODE_READ);
            }
        });
        view.addView(button);
@@ -129,7 +133,7 @@ public class TestActivity extends Activity {
                if (localOnly.isChecked()) {
                    intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                }
                startActivityForResult(intent, 42);
                startActivityForResult(intent, CODE_READ);
            }
        });
        view.addView(button);
@@ -146,7 +150,7 @@ public class TestActivity extends Activity {
                if (localOnly.isChecked()) {
                    intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                }
                startActivityForResult(intent, 42);
                startActivityForResult(intent, CODE_WRITE);
            }
        });
        view.addView(button);
@@ -165,7 +169,7 @@ public class TestActivity extends Activity {
                if (localOnly.isChecked()) {
                    intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                }
                startActivityForResult(Intent.createChooser(intent, "Kittens!"), 42);
                startActivityForResult(Intent.createChooser(intent, "Kittens!"), CODE_READ);
            }
        });
        view.addView(button);
@@ -178,20 +182,45 @@ public class TestActivity extends Activity {

    @Override
    protected void onActivityResult(int requestCode, int resultCode, Intent data) {
        mResult.setText("resultCode=" + resultCode + ", data=" + String.valueOf(data));
        mResult.setText(null);
        String result = "resultCode=" + resultCode + ", data=" + String.valueOf(data);

        if (requestCode == CODE_READ) {
            final Uri uri = data != null ? data.getData() : null;
            if (uri != null) {
                InputStream is = null;
                try {
                    is = getContentResolver().openInputStream(uri);
                    final int length = Streams.readFullyNoClose(is).length;
                Log.d(TAG, "read length=" + length);
                    result += "; read length=" + length;
                } catch (Exception e) {
                    result += "; ERROR";
                    Log.w(TAG, "Failed to read " + uri, e);
                } finally {
                    IoUtils.closeQuietly(is);
                }
            } else {
                result += "no uri?";
            }
        } else if (requestCode == CODE_WRITE) {
            final Uri uri = data != null ? data.getData() : null;
            if (uri != null) {
                OutputStream os = null;
                try {
                    os = getContentResolver().openOutputStream(uri);
                    os.write("THE COMPLETE WORKS OF SHAKESPEARE".getBytes());
                } catch (Exception e) {
                    result += "; ERROR";
                    Log.w(TAG, "Failed to write " + uri, e);
                } finally {
                    IoUtils.closeQuietly(os);
                }
            } else {
                result += "no uri?";
            }
        }

        Log.d(TAG, result);
        mResult.setText(result);
    }
}
+7 −0
Original line number Diff line number Diff line
@@ -52,6 +52,13 @@ public class RootInfo {
        root.summary = getCursorString(cursor, Root.COLUMN_SUMMARY);
        root.documentId = getCursorString(cursor, Root.COLUMN_DOCUMENT_ID);
        root.availableBytes = getCursorLong(cursor, Root.COLUMN_AVAILABLE_BYTES);

        // TODO: remove this hack
        if ("com.google.android.apps.docs.storage".equals(root.authority)) {
            root.flags &= ~(Root.FLAG_PROVIDES_AUDIO | Root.FLAG_PROVIDES_IMAGES
                    | Root.FLAG_PROVIDES_VIDEO);
        }

        return root;
    }