Skip to content

Commit

Permalink
Merge pull request #1438: [Mac kext] Allow privileged system service …
Browse files Browse the repository at this point in the history
…'amfid' to hydrate

[releases/shipped] Allow privileged system service 'amfid' to hydrate
  • Loading branch information
jeschu1 authored Aug 12, 2019
2 parents c0d705d + ae2b1c2 commit 8b1dcc2
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 28 deletions.
23 changes: 18 additions & 5 deletions ProjFS.Mac/PrjFSKext/KauthHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ static bool TryReadVNodeFileFlags(vnode_t vn, vfs_context_t _Nonnull context, ui
KEXT_STATIC_INLINE bool FileFlagsBitIsSet(uint32_t fileFlags, uint32_t bit);
KEXT_STATIC_INLINE bool TryGetFileIsFlaggedAsInRoot(vnode_t vnode, vfs_context_t _Nonnull context, bool* flaggedInRoot);
KEXT_STATIC_INLINE bool ActionBitIsSet(kauth_action_t action, kauth_action_t mask);
KEXT_STATIC bool CurrentProcessWasSpawnedByRegularUser();
KEXT_STATIC bool CurrentProcessIsAllowedToHydrate();
KEXT_STATIC bool IsFileSystemCrawler(const char* procname);

static void WaitForListenerCompletion();
Expand Down Expand Up @@ -1144,10 +1144,10 @@ static bool TryGetVirtualizationRoot(
return false;
}

if (callbackPolicy == CallbackPolicy_UserInitiatedOnly && !CurrentProcessWasSpawnedByRegularUser())
if (callbackPolicy == CallbackPolicy_UserInitiatedOnly && !CurrentProcessIsAllowedToHydrate())
{
// Prevent hydration etc. by system services
KextLog_Info("TryGetVirtualizationRoot: process %d restricted due to owner UID.", pidMakingRequest);
KextLog_Info("TryGetVirtualizationRoot: process %d is not allowed to hydrate.", pidMakingRequest);
perfTracer->IncrementCount(PrjFSPerfCounter_VnodeOp_GetVirtualizationRoot_UserRestriction);

*kauthResult = KAUTH_RESULT_DENY;
Expand All @@ -1159,7 +1159,7 @@ static bool TryGetVirtualizationRoot(
return true;
}

KEXT_STATIC bool CurrentProcessWasSpawnedByRegularUser()
KEXT_STATIC bool CurrentProcessIsAllowedToHydrate()
{
bool nonServiceUser = false;

Expand Down Expand Up @@ -1188,7 +1188,7 @@ KEXT_STATIC bool CurrentProcessWasSpawnedByRegularUser()
process = parentProcess;
if (parentProcess == nullptr)
{
KextLog_Error("CurrentProcessIsOwnedOrWasSpawnedByRegularUser: Failed to locate ancestor process %d for current process %d\n", parentPID, proc_selfpid());
KextLog_Error("CurrentProcessIsAllowedToHydrate: Failed to locate ancestor process %d for current process %d\n", parentPID, proc_selfpid());
break;
}
}
Expand All @@ -1199,6 +1199,19 @@ KEXT_STATIC bool CurrentProcessWasSpawnedByRegularUser()
proc_rele(process);
}

if (!nonServiceUser)
{
// When amfid is invoked to check the code signature of a library which has not been hydrated,
// blocking hydration would cause the launch of an application which depends on the library to fail,
// so amfid should always be allowed to hydrate.
char buffer[MAXCOMLEN + 1] = "";
proc_selfname(buffer, sizeof(buffer));
if (0 == strcmp(buffer, "amfid"))
{
nonServiceUser = true;
}
}

return nonServiceUser;
}

Expand Down
2 changes: 1 addition & 1 deletion ProjFS.Mac/PrjFSKext/KauthHandlerTestable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ KEXT_STATIC bool ShouldHandleFileOpEvent(
VirtualizationRootHandle* root,
int* pid);
KEXT_STATIC void UseMainForkIfNamedStream(vnode_t& vnode, bool& putVnodeWhenDone);
KEXT_STATIC bool CurrentProcessWasSpawnedByRegularUser();
KEXT_STATIC bool CurrentProcessIsAllowedToHydrate();
KEXT_STATIC bool InitPendingRenames();
KEXT_STATIC void CleanupPendingRenames();
KEXT_STATIC void ResizePendingRenames(uint32_t newMaxPendingRenames);
Expand Down
6 changes: 3 additions & 3 deletions ProjFS.Mac/PrjFSKextTests/HandleFileOpOperationTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ - (void) setUp
self->otherRepoHandle = result.root;

MockProcess_AddContext(context, 501 /*pid*/);
MockProcess_SetSelfPid(501);
MockProcess_SetSelfInfo(501, "Test");
MockProcess_AddProcess(501 /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "test" /*name*/);

ProvidermessageMock_ResetResultCount();
Expand Down Expand Up @@ -402,7 +402,7 @@ - (void)testFileopHardlinkOtherRepoProviderPID
{
MockProcess_Reset();
MockProcess_AddContext(context, self->dummyClientPid /*pid*/);
MockProcess_SetSelfPid(self->dummyClientPid);
MockProcess_SetSelfInfo(self->dummyClientPid, "Test");
MockProcess_AddProcess(self->dummyClientPid /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "GVFS.Mount" /*name*/);

testFileVnode->attrValues.va_flags = FileFlags_IsInVirtualizationRoot;
Expand Down Expand Up @@ -432,7 +432,7 @@ - (void)testFileopHardlinkOtherRepoOtherProviderPID
{
MockProcess_Reset();
MockProcess_AddContext(context, self->otherDummyClientPid /*pid*/);
MockProcess_SetSelfPid(self->otherDummyClientPid);
MockProcess_SetSelfInfo(self->otherDummyClientPid, "Test");
MockProcess_AddProcess(self->otherDummyClientPid /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "GVFS.Mount" /*name*/);

testFileVnode->attrValues.va_flags = FileFlags_IsInVirtualizationRoot;
Expand Down
2 changes: 1 addition & 1 deletion ProjFS.Mac/PrjFSKextTests/HandleOperationTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ - (void) setUp
self->dummyRepoHandle = result.root;

MockProcess_AddContext(context, 501 /*pid*/);
MockProcess_SetSelfPid(501);
MockProcess_SetSelfInfo(501, "Test");
MockProcess_AddProcess(501 /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "test" /*name*/);

ProvidermessageMock_ResetResultCount();
Expand Down
36 changes: 22 additions & 14 deletions ProjFS.Mac/PrjFSKextTests/KauthHandlerTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ - (void) setUp {
self->cacheWrapper.AllocateCache();
context = vfs_context_create(nullptr);
MockProcess_AddContext(context, 501 /*pid*/);
MockProcess_SetSelfPid(501);
MockProcess_SetSelfInfo(501, "Test");
MockProcess_AddProcess(501 /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "test" /*name*/);
}

Expand Down Expand Up @@ -247,7 +247,7 @@ - (void)testShouldHandleVnodeOpEvent {
// Test with file crawler trying to populate an empty file
testVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot;
MockProcess_Reset();
MockProcess_SetSelfPid(501);
MockProcess_SetSelfInfo(501, "Test");
MockProcess_AddContext(context, 501 /*pid*/);
MockProcess_AddProcess(501 /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "mds" /*name*/);
XCTAssertFalse(
Expand All @@ -265,7 +265,7 @@ - (void)testShouldHandleVnodeOpEvent {

// Test with finder trying to populate an empty file
MockProcess_Reset();
MockProcess_SetSelfPid(501);
MockProcess_SetSelfInfo(501, "Test");
MockProcess_AddContext(context, 501 /*pid*/);
MockProcess_AddProcess(501 /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "Finder" /*name*/);
XCTAssertTrue(
Expand All @@ -282,47 +282,55 @@ - (void)testShouldHandleVnodeOpEvent {
XCTAssertEqual(kauthResult, KAUTH_RESULT_DEFER);
}

- (void)testCurrentProcessWasSpawnedByRegularUser {
- (void)testCurrentProcessIsAllowedToHydrate {
// Defaults should pass for all tests
XCTAssertTrue(CurrentProcessWasSpawnedByRegularUser());
XCTAssertTrue(CurrentProcessIsAllowedToHydrate());
MockProcess_Reset();

// Process is a service user and does not have a parent
// Process and its parents are owned by a system user, and the executable is not a whitelisted service.
MockProcess_AddContext(context, 500 /*pid*/);
MockProcess_SetSelfPid(500);
MockProcess_SetSelfInfo(500, "Test");
MockProcess_AddCredential(1 /*credentialId*/, 1 /*UID*/);
MockProcess_AddProcess(500 /*pid*/, 1 /*credentialId*/, 501 /*ppid*/, "test" /*name*/);
XCTAssertFalse(CurrentProcessWasSpawnedByRegularUser());
XCTAssertFalse(CurrentProcessIsAllowedToHydrate());
MockProcess_Reset();

// The service "amfid" and its parents are owned by system users, but the process name is whitelisted for hydration.
MockProcess_AddContext(context, 500 /*pid*/);
MockProcess_SetSelfInfo(500, "amfid");
MockProcess_AddCredential(1 /*credentialId*/, 1 /*UID*/);
MockProcess_AddProcess(500 /*pid*/, 1 /*credentialId*/, 501 /*ppid*/, "test" /*name*/);
XCTAssertTrue(CurrentProcessIsAllowedToHydrate());
MockProcess_Reset();

// Test a process with a service UID, valid parent pid, but proc_find fails to find parent pid
MockCalls::Clear();
MockProcess_AddContext(context, 500 /*pid*/);
MockProcess_SetSelfPid(500);
MockProcess_SetSelfInfo(500, "Test");
MockProcess_AddCredential(1 /*credentialId*/, 1 /*UID*/);
MockProcess_AddProcess(500 /*pid*/, 1 /*credentialId*/, 501 /*ppid*/, "test" /*name*/);
XCTAssertFalse(CurrentProcessWasSpawnedByRegularUser());
XCTAssertFalse(CurrentProcessIsAllowedToHydrate());
XCTAssertTrue(MockCalls::DidCallFunction(KextMessageLogged, KEXTLOG_ERROR));
MockProcess_Reset();

// 'sudo' scenario: Root process with non-root parent
MockProcess_AddContext(context, 502 /*pid*/);
MockProcess_SetSelfPid(502);
MockProcess_SetSelfInfo(502, "Test");
MockProcess_AddCredential(1 /*credentialId*/, 1 /*UID*/);
MockProcess_AddCredential(2 /*credentialId*/, 501 /*UID*/);
MockProcess_AddProcess(502 /*pid*/, 1 /*credentialId*/, 501 /*ppid*/, "test" /*name*/);
MockProcess_AddProcess(501 /*pid*/, 2 /*credentialId*/, 1 /*ppid*/, "test" /*name*/);
XCTAssertTrue(CurrentProcessWasSpawnedByRegularUser());
XCTAssertTrue(CurrentProcessIsAllowedToHydrate());
MockProcess_Reset();

// Process and it's parent are service users
MockProcess_AddContext(context, 502 /*pid*/);
MockProcess_SetSelfPid(502);
MockProcess_SetSelfInfo(502, "Test");
MockProcess_AddCredential(1 /*credentialId*/, 1 /*UID*/);
MockProcess_AddCredential(2 /*credentialId*/, 2 /*UID*/);
MockProcess_AddProcess(502 /*pid*/, 1 /*credentialId*/, 501 /*ppid*/, "test" /*name*/);
MockProcess_AddProcess(501 /*pid*/, 2 /*credentialId*/, 1 /*ppid*/, "test" /*name*/);
XCTAssertFalse(CurrentProcessWasSpawnedByRegularUser());
XCTAssertFalse(CurrentProcessIsAllowedToHydrate());
}

- (void)testUseMainForkIfNamedStream {
Expand Down
9 changes: 8 additions & 1 deletion ProjFS.Mac/PrjFSKextTests/MockProc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ static map<uintptr_t /*credential ID*/, int /*UID*/> s_credentialMap;
static map<vfs_context_t /*context*/, int /*pid*/> s_contextMap;
static map<int /*process Id*/, proc> s_processMap;
static int s_selfPid;
static string s_selfName;
static uint16_t s_currentThreadIndex = 0;
static thread s_threadPool[MockProcess_ThreadPoolSize] = {};

Expand All @@ -25,9 +26,10 @@ void MockProcess_Reset()
MockProcess_SetCurrentThreadIndex(0);
}

void MockProcess_SetSelfPid(int selfPid)
void MockProcess_SetSelfInfo(int selfPid, const string& selfName)
{
s_selfPid = selfPid;
s_selfName = selfName;
}

int proc_pid(proc_t proc)
Expand Down Expand Up @@ -149,6 +151,11 @@ int proc_selfpid(void)
return s_selfPid;
}

void proc_selfname(char* buf, int size)
{
strlcpy(buf, s_selfName.c_str(), size);
}

void MockProcess_AddCredential(uintptr_t credentialId, uid_t UID)
{
s_credentialMap.insert(make_pair(credentialId, UID));
Expand Down
3 changes: 2 additions & 1 deletion ProjFS.Mac/PrjFSKextTests/MockProc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ extern "C"
int proc_rele(proc_t p);
int proc_selfpid(void);
kernel_thread_t current_thread(void);
void proc_selfname(char* buf, int size);
}

struct proc {
Expand All @@ -30,7 +31,7 @@ struct proc {
std::string name;
};

void MockProcess_SetSelfPid(int selfPid);
void MockProcess_SetSelfInfo(int selfPid, const std::string& selfName);
void MockProcess_AddCredential(uintptr_t credentialId, uid_t UID);
void MockProcess_AddContext(vfs_context_t context, int pid);
void MockProcess_AddProcess(int pid, uintptr_t credentialId, int ppid, std::string procName);
Expand Down
4 changes: 2 additions & 2 deletions ProjFS.Mac/PrjFSKextTests/ShouldHandleFileOpTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ - (void) setUp {
self->cacheWrapper.AllocateCache();
self->context = vfs_context_create(nullptr);
MockProcess_AddContext(self->context, 501 /*pid*/);
MockProcess_SetSelfPid(501);
MockProcess_SetSelfInfo(501, "Test");
MockProcess_AddProcess(501 /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "test" /*name*/);

self->testMount = mount::Create();
Expand Down Expand Up @@ -191,7 +191,7 @@ - (void)testProviderInitiatedIO {
// Fail when pid matches provider pid
MockProcess_Reset();
MockProcess_AddContext(self->context, 0 /*pid*/);
MockProcess_SetSelfPid(0);
MockProcess_SetSelfInfo(0, "Test");
MockProcess_AddProcess(0 /*pid*/, 1 /*credentialId*/, 1 /*ppid*/, "test" /*name*/);
int pid;
XCTAssertFalse(
Expand Down

0 comments on commit 8b1dcc2

Please sign in to comment.