Bug 1289718 - Implement mkdir.
MozReview-Commit-ID: 6r6sqOVekVu
--- a/security/sandbox/linux/SandboxBrokerClient.cpp
+++ b/security/sandbox/linux/SandboxBrokerClient.cpp
@@ -200,10 +200,17 @@ SandboxBrokerClient::Chmod(const char* a
int
SandboxBrokerClient::Link(const char* aOldPath, const char* aNewPath)
{
Request req = {SANDBOX_FILE_LINK, 0};
return DoCall(&req, aOldPath, aNewPath, nullptr, false);
}
+int
+SandboxBrokerClient::Mkdir(const char* aPath, int aMode)
+{
+ Request req = {SANDBOX_FILE_MKDIR, aMode};
+ return DoCall(&req, aPath, nullptr, nullptr, false);
+}
+
} // namespace mozilla
--- a/security/sandbox/linux/SandboxBrokerClient.h
+++ b/security/sandbox/linux/SandboxBrokerClient.h
@@ -30,16 +30,17 @@ class SandboxBrokerClient final : privat
~SandboxBrokerClient();
int Open(const char* aPath, int aFlags);
int Access(const char* aPath, int aMode);
int Stat(const char* aPath, struct stat* aStat);
int LStat(const char* aPath, struct stat* aStat);
int Chmod(const char* aPath, int aMode);
int Link(const char* aPath, const char* aPath2);
+ int Mkdir(const char* aPath, int aMode);
private:
int mFileDesc;
int DoCall(const Request* aReq,
const char* aPath,
const char* aPath2,
struct stat* aStat,
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -418,16 +418,23 @@ class ContentSandboxPolicy : public Sand
static intptr_t LinkTrap(ArgsRef aArgs, void *aux) {
auto broker = static_cast<SandboxBrokerClient*>(aux);
auto path = reinterpret_cast<const char*>(aArgs.args[0]);
auto path2 = reinterpret_cast<const char*>(aArgs.args[1]);
return broker->Link(path, path2);
}
+ static intptr_t MkdirTrap(ArgsRef aArgs, void* aux) {
+ auto broker = static_cast<SandboxBrokerClient*>(aux);
+ auto path = reinterpret_cast<const char*>(aArgs.args[0]);
+ auto mode = static_cast<mode_t>(aArgs.args[1]);
+ return broker->Mkdir(path, mode);
+ }
+
static intptr_t GetPPidTrap(ArgsRef aArgs, void* aux) {
// In a pid namespace, getppid() will return 0. We will return 0 instead
// of the real parent pid to see what breaks when we introduce the
// pid namespace (Bug 1151624).
return 0;
}
public:
@@ -519,16 +526,18 @@ public:
CASES_FOR_lstat:
return Trap(LStatTrap, mBroker);
CASES_FOR_fstatat:
return Trap(StatAtTrap, mBroker);
case __NR_chmod:
return Trap(ChmodTrap, mBroker);
case __NR_link:
return Trap(LinkTrap, mBroker);
+ case __NR_mkdir:
+ return Trap(MkdirTrap, mBroker);
}
} else {
// No broker; allow the syscalls directly. )-:
switch(sysno) {
case __NR_open:
case __NR_openat:
case __NR_access:
case __NR_faccessat:
@@ -541,17 +550,16 @@ public:
switch (sysno) {
#ifdef DESKTOP
case __NR_getppid:
return Trap(GetPPidTrap, nullptr);
// Filesystem syscalls that need more work to determine who's
// using them, if they need to be, and what we intend to about it.
- case __NR_mkdir:
case __NR_rmdir:
case __NR_getcwd:
CASES_FOR_statfs:
CASES_FOR_fstatfs:
case __NR_rename:
case __NR_symlink:
case __NR_quotactl:
case __NR_unlink:
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -286,16 +286,22 @@ AllowOperation(int aReqFlags, int aPerms
{
int needed = 0;
if (aReqFlags & R_OK) {
needed |= SandboxBroker::MAY_READ;
}
if (aReqFlags & W_OK) {
needed |= SandboxBroker::MAY_WRITE;
}
+ // We don't really allow executing anything,
+ // so in true unix tradition we hijack this
+ // for directories.
+ if (aReqFlags & X_OK) {
+ needed |= SandboxBroker::MAY_CREATE;
+ }
return (aPerms & needed) == needed;
}
static bool
AllowAccess(int aReqFlags, int aPerms)
{
if (aReqFlags & ~(R_OK|W_OK|F_OK)) {
return false;
@@ -370,16 +376,22 @@ DoChmod(const char* aPath, int aMode)
}
static int
DoLink(const char* aPath, const char* aPath2)
{
return link(aPath, aPath2);
}
+static int
+DoMkdir(const char* aPath, int aMode)
+{
+ return mkdir(aPath, aMode);
+}
+
size_t
SandboxBroker::ConvertToRealPath(char* aPath, size_t aBufSize, size_t aPathLen)
{
if (strstr(aPath, "..") != NULL) {
char* result = realpath(aPath, NULL);
if (result != NULL) {
strncpy(aPath, result, aBufSize);
aPath[aBufSize - 1] = '\0';
@@ -567,33 +579,43 @@ SandboxBroker::ThreadMain(void)
ios[1].iov_len = sizeof(statBuf);
} else {
resp.mError = errno;
}
break;
case SANDBOX_FILE_CHMOD:
if (permissive || AllowOperation(W_OK, perms)) {
- if (DoChmod(pathBuf,req.mFlags) == 0) {
+ if (DoChmod(pathBuf, req.mFlags) == 0) {
resp.mError = 0;
} else {
resp.mError = errno;
}
}
break;
case SANDBOX_FILE_LINK:
if (permissive || AllowOperation(W_OK, perms)) {
if (DoLink(pathBuf, pathBuf2) == 0) {
resp.mError = 0;
} else {
resp.mError = errno;
}
}
break;
+
+ case SANDBOX_FILE_MKDIR:
+ if (permissive || AllowOperation(W_OK | X_OK, perms)) {
+ if (DoMkdir(pathBuf, req.mFlags) == 0) {
+ resp.mError = 0;
+ } else {
+ resp.mError = errno;
+ }
+ }
+ break;
}
} else {
MOZ_ASSERT(perms == 0);
AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
}
const size_t numIO = ios[1].iov_len > 0 ? 2 : 1;
DebugOnly<const ssize_t> sent = SendWithFd(respfd, ios, numIO, openedFd);
--- a/security/sandbox/linux/broker/SandboxBrokerCommon.h
+++ b/security/sandbox/linux/broker/SandboxBrokerCommon.h
@@ -26,16 +26,17 @@ namespace mozilla {
class SandboxBrokerCommon {
public:
enum Operation {
SANDBOX_FILE_OPEN,
SANDBOX_FILE_ACCESS,
SANDBOX_FILE_STAT,
SANDBOX_FILE_CHMOD,
SANDBOX_FILE_LINK,
+ SANDBOX_FILE_MKDIR,
};
struct Request {
Operation mOp;
// For open, flags; for access, "mode"; for stat, O_NOFOLLOW for lstat.
int mFlags;
// The rest of the packet is the pathname.
// SCM_RIGHTS for response socket attached.
--- a/security/sandbox/linux/gtest/TestBroker.cpp
+++ b/security/sandbox/linux/gtest/TestBroker.cpp
@@ -61,16 +61,19 @@ protected:
return mClient->LStat(aPath, aStat);
}
int Chmod(const char* aPath, int aMode) {
return mClient->Chmod(aPath, aMode);
}
int Link(const char* aPath, const char* bPath) {
return mClient->Link(aPath, bPath);
}
+ int Mkdir(const char* aPath, int aMode) {
+ return mClient->Mkdir(aPath, aMode);
+ }
virtual void SetUp() {
ipc::FileDescriptor fd;
mServer = SandboxBroker::Create(GetPolicy(), getpid(), fd);
ASSERT_NE(mServer, nullptr);
ASSERT_TRUE(fd.IsValid());
auto rawFD = fd.ClonePlatformHandle();
@@ -217,16 +220,17 @@ TEST_F(SandboxBrokerTest, LStat)
EXPECT_EQ(0, LStat("/proc/self", &brokeredStat));
EXPECT_TRUE(S_ISLNK(brokeredStat.st_mode));
}
TEST_F(SandboxBrokerTest, Chmod)
{
unlink("/tmp/blublu");
+ rmdir("/tmp/blublu");
int fd = Open("/tmp/blublu", O_WRONLY | O_CREAT);
ASSERT_GE(fd, 0) << "Opening /tmp/blublu for writing failed.";
close(fd);
// Set read only. SandboxBroker enforces 0600 mode flags.
ASSERT_EQ(0, Chmod("/tmp/blublu", S_IRUSR));
// SandboxBroker doesn't use real access(), it just checks against
// the policy. So it can't see the change in permisions here.
// This won't work:
@@ -239,28 +243,44 @@ TEST_F(SandboxBrokerTest, Chmod)
EXPECT_EQ(0, stat("/tmp/blublu", &realStat));
EXPECT_EQ((mode_t)(S_IRUSR | S_IWUSR), realStat.st_mode & 0777);
EXPECT_EQ(0, unlink("/tmp/blublu"));
}
TEST_F(SandboxBrokerTest, Link)
{
unlink("/tmp/blublu");
+ rmdir("/tmp/blublu");
unlink("/tmp/blublublu");
int fd = Open("/tmp/blublu", O_WRONLY | O_CREAT);
ASSERT_GE(fd, 0) << "Opening /tmp/blublu for writing failed.";
close(fd);
ASSERT_EQ(0, Link("/tmp/blublu", "/tmp/blublublu"));
EXPECT_EQ(0, Access("/tmp/blublublu", F_OK));
// Not whitelisted target path
EXPECT_EQ(-EACCES, Link("/tmp/blublu", "/tmp/nope"));
EXPECT_EQ(0, unlink("/tmp/blublublu"));
EXPECT_EQ(0, unlink("/tmp/blublu"));
}
+TEST_F(SandboxBrokerTest, Mkdir)
+{
+ unlink("/tmp/blublu");
+ rmdir("/tmp/blublu");
+ unlink("/tmp/nope");
+ rmdir("/tmp/nope");
+ ASSERT_EQ(mkdir("/tmp/blublu", 0600), 0)
+ << "Creating dir /tmp/blublu failed.";
+ EXPECT_EQ(0, Access("/tmp/blublu", F_OK));
+ // Not whitelisted target path
+ EXPECT_EQ(-EACCES, Mkdir("/tmp/nope", 0600))
+ << "Creating dir without MAY_CREATE succeed.";
+ EXPECT_EQ(0, rmdir("/tmp/blublu"));
+}
+
TEST_F(SandboxBrokerTest, MultiThreadOpen) {
RunOnManyThreads<SandboxBrokerTest,
&SandboxBrokerTest::MultiThreadOpenWorker>();
}
void SandboxBrokerTest::MultiThreadOpenWorker() {
static const int kNumLoops = 10000;
for (int i = 1; i <= kNumLoops; ++i) {