Skip to content

Commit

Permalink
enfore read-only folder via ACL on windows
Browse files Browse the repository at this point in the history 
Signed-off-by: Matthieu Gallien <[email protected]>
  • Loading branch information
@mgallien
mgallien committed Feb 22, 2024
1 parent c0e14d8 commit 64859e7
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 17 deletions.
9 changes: 7 additions & 2 deletions src/common/filesystembase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@

#include "config.h"

#include "csync/ocsynclib.h"

#include <QString>
#include <ctime>
#include <QFileInfo>
#include <QLoggingCategory>

#include <csync/ocsynclib.h>
#include <ctime>

class QFile;

Expand All @@ -42,6 +43,10 @@ OCSYNC_EXPORT Q_DECLARE_LOGGING_CATEGORY(lcFileSystem)
* @brief This file contains file system helper
*/
namespace FileSystem {
enum class FolderPermissions {
ReadOnly,
ReadWrite,
};

/**
* @brief Mark the file as hidden (only has effects on windows)
Expand Down
105 changes: 104 additions & 1 deletion src/libsync/filesystem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@
#include <QDirIterator>
#include <QCoreApplication>

#include <filesystem>

#ifdef Q_OS_WIN
#include <securitybaseapi.h>
#include <sddl.h>
#endif

namespace OCC {
Expand Down Expand Up @@ -193,9 +196,109 @@ bool FileSystem::getInode(const QString &filename, quint64 *inode)
return false;
}

bool FileSystem::setFolderPermissions(const QString &path)
bool FileSystem::setFolderPermissions(const QString &path,

Check warning on line 199 in src/libsync/filesystem.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/filesystem.cpp:199:18 [modernize-use-trailing-return-type] 

use a trailing return type for this function

Check warning on line 199 in src/libsync/filesystem.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/filesystem.cpp:199:39 [bugprone-easily-swappable-parameters] 

2 adjacent parameters of 'setFolderPermissions' of similar type are easily swapped by mistake
FileSystem::FolderPermissions permissions)
{
#ifdef Q_OS_WIN
SECURITY_INFORMATION info = OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION;
constexpr auto length = 512;
char securityDescriptor[length];
auto neededLength = 0ul;

if (!GetFileSecurityW(path.toStdWString().c_str(), info, &securityDescriptor, length, &neededLength)) {
if (neededLength > length) {
qCWarning(lcFileSystem) << "error when calling GetFileSecurityW" << path << "size is too small to get the result" << "need" << neededLength << "instead of" << length;
return false;
}
qCWarning(lcFileSystem) << "error when calling GetFileSecurityW" << path << GetLastError();
return false;
}

int daclPresent = false, daclDefault = false;
PACL resultDacl = nullptr;
if (!GetSecurityDescriptorDacl(&securityDescriptor, &daclPresent, &resultDacl, &daclDefault)) {
qCWarning(lcFileSystem) << "error when calling GetSecurityDescriptorDacl" << path << GetLastError();
return false;
}
if (!daclPresent) {
qCWarning(lcFileSystem) << "error when calling DACL needed to set a folder read-only or read-write is missing" << path;
return false;
}

PACL newDacl = reinterpret_cast<PACL>(new char[length]);
if (!InitializeAcl(newDacl, length, ACL_REVISION)) {
qCWarning(lcFileSystem) << "error when calling InitializeAcl" << path << GetLastError();
return false;
}

PSID sid = nullptr;
if (!ConvertStringSidToSidA("S-1-5-32-545", &sid))
{
qCWarning(lcFileSystem) << "error when calling ConvertStringSidToSidA" << path << GetLastError();
return false;
}

if (permissions == FileSystem::FolderPermissions::ReadOnly) {
if (!AddAccessDeniedAce(newDacl, ACL_REVISION , STANDARD_RIGHTS_WRITE, sid)) {
qCWarning(lcFileSystem) << "error when calling AddAccessDeniedAce << path" << GetLastError();
return false;
}
}

ACL_SIZE_INFORMATION aclSize;
if (!GetAclInformation(resultDacl, &aclSize, sizeof(aclSize), AclSizeInformation)) {
qCWarning(lcFileSystem) << "error when calling GetAclInformation" << path << GetLastError();
return false;
}

for (int i = 0; i < aclSize.AceCount; ++i) {
void *currentAce = nullptr;
if (!GetAce(resultDacl, i, &currentAce)) {
qCWarning(lcFileSystem) << "error when calling GetAce" << path << GetLastError();
return false;
}

const auto currentAceHeader = reinterpret_cast<PACE_HEADER>(currentAce);

if (permissions == FileSystem::FolderPermissions::ReadWrite && (ACCESS_DENIED_ACE_TYPE == (currentAceHeader->AceType & ACCESS_DENIED_ACE_TYPE))) {
qCWarning(lcFileSystem) << "AceHeader" << path << currentAceHeader->AceFlags << currentAceHeader->AceSize << currentAceHeader->AceType;
continue;
}

if (!AddAce(newDacl, ACL_REVISION, i + 1, currentAce, currentAceHeader->AceSize)) {
qCWarning(lcFileSystem) << "error when calling AddAce" << path << GetLastError();
return false;
}
}

SECURITY_DESCRIPTOR newSecurityDescriptor;
if (!InitializeSecurityDescriptor(&newSecurityDescriptor, SECURITY_DESCRIPTOR_REVISION)) {
qCWarning(lcFileSystem) << "error when calling InitializeSecurityDescriptor" << path << GetLastError();
return false;
}

if (!SetSecurityDescriptorDacl(&newSecurityDescriptor, true, newDacl, false)) {
qCWarning(lcFileSystem) << "error when calling SetSecurityDescriptorDacl" << path << GetLastError();
return false;
}

if (!SetFileSecurityW(path.toStdWString().c_str(), info, &newSecurityDescriptor)) {
qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << path << GetLastError();
return false;
}
#else
switch (permissions) {
case OCC::FileSystem::FolderPermissions::ReadOnly:
std::filesystem::permissions(path.toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write, std::filesystem::perm_options::remove);
break;
case OCC::FileSystem::FolderPermissions::ReadWrite:
std::filesystem::permissions(path.toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
break;
}
qCInfo(lcFileSystem) << "new permissions" << static_cast<int>(std::filesystem::status(path.toStdWString()).permissions());
#endif

return true;
}


Expand Down
11 changes: 6 additions & 5 deletions src/libsync/filesystem.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@

#include "config.h"

#include "owncloudlib.h"
#include "common/filesystembase.h"

#include <QString>

#include <ctime>
#include <functional>

#include <owncloudlib.h>
// Chain in the base include and extend the namespace
#include "common/filesystembase.h"

class QFile;

namespace OCC {
Expand Down Expand Up @@ -97,7 +97,8 @@ namespace FileSystem {
const std::function<void(const QString &path, bool isDir)> &onDeleted = nullptr,
QStringList *errors = nullptr);

bool OWNCLOUDSYNC_EXPORT setFolderPermissions(const QString &path);
bool OWNCLOUDSYNC_EXPORT setFolderPermissions(const QString &path,
FileSystem::FolderPermissions permissions);
}

/** @} */
Expand Down
19 changes: 18 additions & 1 deletion src/libsync/owncloudpropagator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1460,7 +1460,18 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
!_item->_remotePerm.hasPermission(RemotePermissions::CanMove) &&
!_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories)) {
try {
std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write, std::filesystem::perm_options::remove);
if (QFileInfo::exists(propagator()->fullLocalPath(_item->_file))) {
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_file), FileSystem::FolderPermissions::ReadOnly);
qCDebug(lcDirectory) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write, std::filesystem::perm_options::remove);
qCDebug(lcDirectory) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
}
if (QFileInfo::exists(propagator()->fullLocalPath(_item->_renameTarget))) {
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadOnly);
qCDebug(lcDirectory) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions());
std::filesystem::permissions(propagator()->fullLocalPath(_item->_renameTarget).toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write, std::filesystem::perm_options::remove);
qCDebug(lcDirectory) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions());
}
}
catch (const std::filesystem::filesystem_error &e)
{
Expand All @@ -1475,10 +1486,16 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
!_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories))) {
try {
if (QFileInfo::exists(propagator()->fullLocalPath(_item->_file))) {
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_file), FileSystem::FolderPermissions::ReadWrite);
qCDebug(lcDirectory) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
qCDebug(lcDirectory) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
}
if (QFileInfo::exists(propagator()->fullLocalPath(_item->_renameTarget))) {
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadWrite);
qCDebug(lcDirectory) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions());
std::filesystem::permissions(propagator()->fullLocalPath(_item->_renameTarget).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
qCDebug(lcDirectory) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions());
}
}
catch (const std::filesystem::filesystem_error &e)
Expand Down
4 changes: 2 additions & 2 deletions src/libsync/propagatedownload.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ void PropagateDownloadFile::startDownload()
auto parentFolderStatus = std::filesystem::status(_parentFolderPath);
_parentPermissions = parentFolderStatus.permissions();
if ((_parentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(_parentFolderPath, _parentPermissions | std::filesystem::perms::owner_write, std::filesystem::perm_options::replace);
FileSystem::setFolderPermissions(QString::fromStdWString(parentFolderPath), FileSystem::FolderPermissions::ReadWrite);
emit propagator()->touchedFile(QString::fromStdWString(_parentFolderPath.wstring()));
_needParentFolderRestorePermissions = true;
}
Expand Down Expand Up @@ -1292,7 +1292,7 @@ void PropagateDownloadFile::downloadFinished()

if (_needParentFolderRestorePermissions) {
try {
std::filesystem::permissions(_parentFolderPath, _parentPermissions, std::filesystem::perm_options::replace);
FileSystem::setFolderPermissions(QString::fromStdWString(_parentFolderPath), FileSystem::FolderPermissions::ReadWrite);
emit propagator()->touchedFile(QString::fromStdWString(_parentFolderPath.wstring()));
_needParentFolderRestorePermissions = false;
}
Expand Down
23 changes: 17 additions & 6 deletions src/libsync/propagatorjobs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ void PropagateLocalMkdir::startLocalMkdir()
auto parentFolderStatus = std::filesystem::status(parentFolderPath);
parentPermissions = parentFolderStatus.permissions();
if ((parentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(parentFolderPath, parentPermissions | std::filesystem::perms::owner_write, std::filesystem::perm_options::replace);
FileSystem::setFolderPermissions(QString::fromStdWString(parentFolderPath), FileSystem::FolderPermissions::ReadWrite);
emit propagator()->touchedFile(QString::fromStdWString(parentFolderPath.wstring()));
}
}
Expand All @@ -210,7 +210,7 @@ void PropagateLocalMkdir::startLocalMkdir()

try {
if ((parentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(parentFolderPath, parentPermissions, std::filesystem::perm_options::replace);
FileSystem::setFolderPermissions(QString::fromStdWString(parentFolderPath), FileSystem::FolderPermissions::ReadWrite);
emit propagator()->touchedFile(QString::fromStdWString(parentFolderPath.wstring()));
}
}
Expand All @@ -225,7 +225,7 @@ void PropagateLocalMkdir::startLocalMkdir()
!_item->_remotePerm.hasPermission(RemotePermissions::CanMove) &&
!_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories)) {
try {
std::filesystem::permissions(newDirStr.toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::owner_write, std::filesystem::perm_options::remove);
FileSystem::setFolderPermissions(newDirStr, FileSystem::FolderPermissions::ReadOnly);
}
catch (const std::filesystem::filesystem_error &e)
{
Expand All @@ -235,6 +235,17 @@ void PropagateLocalMkdir::startLocalMkdir()
}
}

try {
if ((parentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
FileSystem::setFolderPermissions(QString::fromStdWString(parentFolderPath), FileSystem::FolderPermissions::ReadOnly);
emit propagator()->touchedFile(QString::fromStdWString(parentFolderPath.wstring()));
}
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}

// Insert the directory into the database. The correct etag will be set later,
// once all contents have been propagated, because should_update_metadata is true.
// Adding an entry with a dummy etag to the database still makes sense here
Expand Down Expand Up @@ -312,7 +323,7 @@ void PropagateLocalRename::start()
auto parentFolderStatus = std::filesystem::status(targetParentFolderPath);
targetParentPermissions = parentFolderStatus.permissions();
if ((targetParentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(targetParentFolderPath, targetParentPermissions | std::filesystem::perms::owner_write, std::filesystem::perm_options::replace);
FileSystem::setFolderPermissions(QString::fromStdWString(targetParentFolderPath), FileSystem::FolderPermissions::ReadWrite);
emit propagator()->touchedFile(QString::fromStdWString(targetParentFolderPath.wstring()));
}
}
Expand All @@ -330,7 +341,7 @@ void PropagateLocalRename::start()
auto parentFolderStatus = std::filesystem::status(originParentFolderPath);
originParentPermissions = parentFolderStatus.permissions();
if ((originParentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(originParentFolderPath, originParentPermissions | std::filesystem::perms::owner_write, std::filesystem::perm_options::replace);
FileSystem::setFolderPermissions(QString::fromStdWString(originParentFolderPath), FileSystem::FolderPermissions::ReadWrite);
emit propagator()->touchedFile(QString::fromStdWString(originParentFolderPath.wstring()));
}
}
Expand All @@ -342,7 +353,7 @@ void PropagateLocalRename::start()
const auto restoreTargetPermissions = [this] (const auto &parentFolderPath, const auto &parentPermissions) {
try {
if ((parentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(parentFolderPath, parentPermissions, std::filesystem::perm_options::replace);
FileSystem::setFolderPermissions(QString::fromStdWString(parentFolderPath), FileSystem::FolderPermissions::ReadOnly);
emit propagator()->touchedFile(QString::fromStdWString(parentFolderPath.wstring()));
}
}
Expand Down

0 comments on commit 64859e7

Please sign in to comment.