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

Commit a703d2f5 authored by Eric Biggers's avatar Eric Biggers
Browse files

UserDataPreparer: only delete contents of user's system CE and DE dirs 

The /data/system_ce/$userId and /data/system_de/$userId directories are
created by vold, so they should be deleted by vold as well, and in fact
that would already happen except that system_server deletes them
recursively before vold gets to it.  Change system_server to delete just
the contents of these directories.

This is a prerequisite to locking down the ability to create these
directories (https://r.android.com/2078213), which is needed to stop
subdirectories from accidentally being created too early.  Technically
we could achieve this goal without limiting delete access, as it's
create access that really matters, but having the operations be paired
properly is much cleaner.

Test: Created and deleted a user, and verified that all their
      directories still got deleted.
Test: atest UserDataPreparerTest
Bug: 156305599
Change-Id: Iec908e1bc15a00c7179fcd1d80321c315682d339
parent 14fff66f

services/core/java/com/android/server/pm/UserDataPreparer.java

+7 −3
Original line number Diff line number Diff line
@@ -150,14 +150,18 @@ class UserDataPreparer { 
            if (Objects.equals(volumeUuid, StorageManager.UUID_PRIVATE_INTERNAL)) {

                if ((flags & StorageManager.FLAG_STORAGE_DE) != 0) {

                    FileUtils.deleteContentsAndDir(getUserSystemDirectory(userId));

                    FileUtils.deleteContentsAndDir(getDataSystemDeDirectory(userId));

                    // Delete the contents of /data/system_de/$userId, but not the directory itself

                    // since vold is responsible for that and system_server isn't allowed to do it.

                    FileUtils.deleteContents(getDataSystemDeDirectory(userId));

                }

                if ((flags & StorageManager.FLAG_STORAGE_CE) != 0) {

                    FileUtils.deleteContentsAndDir(getDataSystemCeDirectory(userId));

                    // Likewise, delete the contents of /data/system_ce/$userId but not the

                    // directory itself.

                    FileUtils.deleteContents(getDataSystemCeDirectory(userId));

                }

            }



            // Data with special labels is now gone, so finish the job

            // All the user's data directories should be empty now, so finish the job.

            storage.destroyUserStorage(volumeUuid, userId, flags);



        } catch (Exception e) {

services/tests/servicestests/src/com/android/server/pm/UserDataPreparerTest.java

+18 −11
Original line number Diff line number Diff line
@@ -17,6 +17,8 @@ 
package com.android.server.pm;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;

import static org.mockito.Matchers.eq;

import static org.mockito.Matchers.isNull;

import static org.mockito.Mockito.verify;
@@ -129,22 +131,16 @@ public class UserDataPreparerTest { 
    }



    @Test

    public void testDestroyUserData() throws Exception {

        // Add file in CE

    public void testDestroyUserData_De_DoesNotDestroyCe() throws Exception {

        // Add file in CE storage

        File systemCeDir = mUserDataPreparer.getDataSystemCeDirectory(TEST_USER_ID);

        systemCeDir.mkdirs();

        File ceFile = new File(systemCeDir, "file");

        writeFile(ceFile, "-----" );

        testDestroyUserData_De();

        // CE directory should be preserved

        // Destroy DE storage, then verify that CE storage wasn't destroyed too.

        mUserDataPreparer.destroyUserData(TEST_USER_ID, StorageManager.FLAG_STORAGE_DE);

        assertEquals(Collections.singletonList(ceFile), Arrays.asList(FileUtils.listFilesOrEmpty(

                systemCeDir)));



        testDestroyUserData_Ce();



        // Verify that testDir is empty

        assertEquals(Collections.emptyList(), Arrays.asList(FileUtils.listFilesOrEmpty(

                mUserDataPreparer.testDir)));

    }



    @Test
@@ -163,7 +159,13 @@ public class UserDataPreparerTest { 
        verify(mStorageManagerMock).destroyUserStorage(isNull(String.class), eq(TEST_USER_ID),

                        eq(StorageManager.FLAG_STORAGE_DE));



        assertEquals(Collections.emptyList(), Arrays.asList(FileUtils.listFilesOrEmpty(systemDir)));

        // systemDir (normal path: /data/system/users/$userId) should have been deleted.

        assertFalse(systemDir.exists());

        // systemDeDir (normal path: /data/system_de/$userId) should still exist but be empty, since

        // UserDataPreparer itself is responsible for deleting the contents of this directory, but

        // it delegates to StorageManager.destroyUserStorage() for deleting the directory itself.

        // We've mocked out StorageManager, so StorageManager.destroyUserStorage() will be a no-op.

        assertTrue(systemDeDir.exists());

        assertEquals(Collections.emptyList(), Arrays.asList(FileUtils.listFilesOrEmpty(

                systemDeDir)));

    }
@@ -181,6 +183,11 @@ public class UserDataPreparerTest { 
        verify(mStorageManagerMock).destroyUserStorage(isNull(String.class), eq(TEST_USER_ID),

                eq(StorageManager.FLAG_STORAGE_CE));



        // systemCeDir (normal path: /data/system_ce/$userId) should still exist but be empty, since

        // UserDataPreparer itself is responsible for deleting the contents of this directory, but

        // it delegates to StorageManager.destroyUserStorage() for deleting the directory itself.

        // We've mocked out StorageManager, so StorageManager.destroyUserStorage() will be a no-op.

        assertTrue(systemCeDir.exists());

        assertEquals(Collections.emptyList(), Arrays.asList(FileUtils.listFilesOrEmpty(

                systemCeDir)));

    }