Skip to content

Commit

Permalink
Fix review comments.
Browse files Browse the repository at this point in the history 
Signed-off-by: alex-z <[email protected]>
  • Loading branch information
@allexzander
allexzander committed Jan 29, 2024
1 parent db15833 commit 170f0be
Show file tree
Hide file tree
Showing 17 changed files with 282 additions and 218 deletions.
1 change: 0 additions & 1 deletion src/csync/csync.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ enum class ItemEncryptionStatus : int {

Q_ENUM_NS(ItemEncryptionStatus)

OCSYNC_EXPORT Q_NAMESPACE
enum class JournalDbEncryptionStatus : int {
NotEncrypted = 0,
Encrypted = 1,
Expand Down
2 changes: 1 addition & 1 deletion src/gui/filedetails/ShareView.qml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ ColumnLayout {
Layout.rightMargin: root.horizontalPadding

visible: root.userGroupSharingPossible
enabled: visible && !root.loading && !root.shareModel.isShareDisabledFolder && !shareeSearchField.isShareeFetchOngoing
enabled: visible && !root.loading && !root.shareModel.isShareDisabledEncryptedFolder && !shareeSearchField.isShareeFetchOngoing

accountState: root.accountState
shareItemIsFolder: root.fileDetails && root.fileDetails.isFolder
Expand Down
32 changes: 18 additions & 14 deletions src/gui/filedetails/sharemodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ void ShareModel::resetData()
{
beginResetModel();

_folder = nullptr;
_synchronizationFolder = nullptr;
_sharePath.clear();
_maxSharingPermissions = {};
_numericFileId.clear();
Expand Down Expand Up @@ -241,23 +241,23 @@ void ShareModel::updateData()
return;
}

_folder = FolderMan::instance()->folderForPath(_localPath);
_synchronizationFolder = FolderMan::instance()->folderForPath(_localPath);

if (!_folder) {
if (!_synchronizationFolder) {
qCWarning(lcShareModel) << "Could not update share model data for" << _localPath << "no responsible folder found";
resetData();
return;
}

qCDebug(lcShareModel) << "Updating share model data now.";

const auto relPath = _localPath.mid(_folder->cleanPath().length() + 1);
_sharePath = _folder->remotePathTrailingSlash() + relPath;
const auto relPath = _localPath.mid(_synchronizationFolder->cleanPath().length() + 1);
_sharePath = _synchronizationFolder->remotePathTrailingSlash() + relPath;

SyncJournalFileRecord fileRecord;
auto resharingAllowed = true; // lets assume the good

if (_folder->journalDb()->getFileRecord(relPath, &fileRecord) && fileRecord.isValid() && !fileRecord._remotePerm.isNull()
if (_synchronizationFolder->journalDb()->getFileRecord(relPath, &fileRecord) && fileRecord.isValid() && !fileRecord._remotePerm.isNull()
&& !fileRecord._remotePerm.hasPermission(RemotePermissions::CanReshare)) {
qCInfo(lcShareModel) << "File record says resharing not allowed";
resharingAllowed = false;
Expand All @@ -278,9 +278,13 @@ void ShareModel::updateData()
_sharedItemType = fileRecord.isE2eEncrypted() ? SharedItemType::SharedItemTypeEncryptedFile : SharedItemType::SharedItemTypeFile;
}

_isShareDisabledFolder = fileRecord.isE2eEncrypted()
const auto prevIsShareDisabledEncryptedFolder = _isShareDisabledEncryptedFolder;
_isShareDisabledEncryptedFolder = fileRecord.isE2eEncrypted()
&& (_sharedItemType != SharedItemType::SharedItemTypeEncryptedTopLevelFolder
|| fileRecord._e2eEncryptionStatus < SyncJournalFileRecord::EncryptionStatus::EncryptedMigratedV2_0);
if (prevIsShareDisabledEncryptedFolder != _isShareDisabledEncryptedFolder) {
emit isShareDisabledEncryptedFolderChanged();

Check warning on line 286 in src/gui/filedetails/sharemodel.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/filedetails/sharemodel.cpp:286:14 [modernize-use-trailing-return-type] 

use a trailing return type for this function
}

// Will get added when shares are fetched if no link shares are fetched
_placeholderLinkShare.reset(new Share(_accountState->account(),
Expand Down Expand Up @@ -442,14 +446,14 @@ void ShareModel::slotPropfindReceived(const QVariantMap &result)
}

const auto privateLinkUrl = result["privatelink"].toString();
_folderId = result["fileid"].toByteArray();
_fileRemoteId = result["fileid"].toByteArray();

if (!privateLinkUrl.isEmpty()) {
qCInfo(lcShareModel) << "Received private link url for" << _sharePath << privateLinkUrl;
_privateLinkUrl = privateLinkUrl;
} else if (!_folderId.isEmpty()) {
qCInfo(lcShareModel) << "Received numeric file id for" << _sharePath << _folderId;
_privateLinkUrl = _accountState->account()->deprecatedPrivateLinkUrl(_folderId).toString(QUrl::FullyEncoded);
} else if (!_fileRemoteId.isEmpty()) {
qCInfo(lcShareModel) << "Received numeric file id for" << _sharePath << _fileRemoteId;
_privateLinkUrl = _accountState->account()->deprecatedPrivateLinkUrl(_fileRemoteId).toString(QUrl::FullyEncoded);
}

setupInternalLinkShare();
Expand Down Expand Up @@ -1145,7 +1149,7 @@ void ShareModel::createNewUserGroupShare(const ShareePtr &sharee)
}

if (isSecureFileDropSupportedFolder()) {
if (!_folder) {
if (!_synchronizationFolder) {
qCWarning(lcShareModel) << "Could not share an E2EE folder" << _localPath << "no responsible folder found";
return;
}
Expand Down Expand Up @@ -1307,9 +1311,9 @@ bool ShareModel::serverAllowsResharing() const
&& _accountState->account()->capabilities().shareResharing();
}

bool ShareModel::isShareDisabledFolder() const
bool ShareModel::isShareDisabledEncryptedFolder() const

Check warning on line 1314 in src/gui/filedetails/sharemodel.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/filedetails/sharemodel.cpp:1314:18 [modernize-use-trailing-return-type] 

use a trailing return type for this function
{
return _isShareDisabledFolder;
return _isShareDisabledEncryptedFolder;
}

QVariantList ShareModel::sharees() const
Expand Down
12 changes: 6 additions & 6 deletions src/gui/filedetails/sharemodel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ShareModel : public QAbstractListModel
Q_PROPERTY(bool publicLinkSharesEnabled READ publicLinkSharesEnabled NOTIFY publicLinkSharesEnabledChanged)
Q_PROPERTY(bool userGroupSharingEnabled READ userGroupSharingEnabled NOTIFY userGroupSharingEnabledChanged)
Q_PROPERTY(bool canShare READ canShare NOTIFY sharePermissionsChanged)
Q_PROPERTY(bool isShareDisabledFolder READ isShareDisabledFolder NOTIFY isShareDisabledFolderChanged)
Q_PROPERTY(bool isShareDisabledEncryptedFolder READ isShareDisabledEncryptedFolder NOTIFY isShareDisabledEncryptedFolderChanged)
Q_PROPERTY(bool fetchOngoing READ fetchOngoing NOTIFY fetchOngoingChanged)
Q_PROPERTY(bool hasInitialShareFetchCompleted READ hasInitialShareFetchCompleted NOTIFY hasInitialShareFetchCompletedChanged)
Q_PROPERTY(bool serverAllowsResharing READ serverAllowsResharing NOTIFY serverAllowsResharingChanged)
Expand Down Expand Up @@ -119,7 +119,7 @@ class ShareModel : public QAbstractListModel
[[nodiscard]] bool userGroupSharingEnabled() const;
[[nodiscard]] bool canShare() const;
[[nodiscard]] bool serverAllowsResharing() const;
[[nodiscard]] bool isShareDisabledFolder() const;
[[nodiscard]] bool isShareDisabledEncryptedFolder() const;

[[nodiscard]] bool fetchOngoing() const;
[[nodiscard]] bool hasInitialShareFetchCompleted() const;
Expand All @@ -136,7 +136,7 @@ class ShareModel : public QAbstractListModel
void publicLinkSharesEnabledChanged();
void userGroupSharingEnabledChanged();
void sharePermissionsChanged();
void isShareDisabledFolderChanged();
void isShareDisabledEncryptedFolderChanged();
void lockExpireStringChanged();
void fetchOngoingChanged();
void hasInitialShareFetchCompletedChanged();
Expand Down Expand Up @@ -230,13 +230,13 @@ private slots:
bool _hasInitialShareFetchCompleted = false;
bool _sharePermissionsChangeInProgress = false;
bool _hideDownloadEnabledChangeInProgress = false;
bool _isShareDisabledFolder = false;
bool _isShareDisabledEncryptedFolder = false;
SharePtr _placeholderLinkShare;
SharePtr _internalLinkShare;
SharePtr _secureFileDropPlaceholderLinkShare;

QPointer<AccountState> _accountState;
QPointer<Folder> _folder;
QPointer<Folder> _synchronizationFolder;

QString _localPath;
QString _sharePath;
Expand All @@ -245,7 +245,7 @@ private slots:
SharedItemType _sharedItemType = SharedItemType::SharedItemTypeUndefined;
SyncJournalFileLockInfo _filelockState;
QString _privateLinkUrl;
QByteArray _folderId;
QByteArray _fileRemoteId;

QSharedPointer<ShareManager> _manager;

Expand Down
2 changes: 2 additions & 0 deletions src/libsync/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ set(libsync_SRCS
clientsideencryption.cpp
clientsideencryptionjobs.h
clientsideencryptionjobs.cpp
clientsideencryptionprimitives.h
clientsideencryptionprimitives.cpp
datetimeprovider.h
datetimeprovider.cpp
rootencryptedfolderinfo.h
Expand Down
76 changes: 19 additions & 57 deletions src/libsync/clientsideencryption.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class X509Certificate {

X509Certificate& operator=(X509Certificate&& other) = delete;

static X509Certificate readCertificate(ClientSideEncryption::Bio &bio)
static X509Certificate readCertificate(Bio &bio)
{
X509Certificate result;
result._certificate = PEM_read_bio_X509(bio, nullptr, nullptr, nullptr);
Expand All @@ -188,7 +188,7 @@ class X509Certificate {
X509* _certificate = nullptr;
};

QByteArray BIO2ByteArray(ClientSideEncryption::Bio &b)
QByteArray BIO2ByteArray(Bio &b)
{
auto pending = static_cast<int>(BIO_ctrl_pending(b));
QByteArray res(pending, '\0');
Expand All @@ -198,18 +198,14 @@ QByteArray BIO2ByteArray(ClientSideEncryption::Bio &b)

QByteArray handleErrors()
{
ClientSideEncryption::Bio bioErrors;
Bio bioErrors;
ERR_print_errors(bioErrors); // This line is not printing anything.
return BIO2ByteArray(bioErrors);
}
}


namespace EncryptionHelper {




QByteArray generateRandomFilename()
{
return QUuid::createUuid().toRfc4122().toHex();
Expand Down Expand Up @@ -508,11 +504,11 @@ QByteArray decryptStringSymmetric(const QByteArray& key, const QByteArray& data)
}

QByteArray privateKeyToPem(const QByteArray key) {
ClientSideEncryption::Bio privateKeyBio;
Bio privateKeyBio;
BIO_write(privateKeyBio, key.constData(), key.size());
auto pkey = ClientSideEncryption::PKey::readPrivateKey(privateKeyBio);
auto pkey = PKey::readPrivateKey(privateKeyBio);

ClientSideEncryption::Bio pemBio;
Bio pemBio;
PEM_write_bio_PKCS8PrivateKey(pemBio, pkey, nullptr, nullptr, 0, nullptr, nullptr);
QByteArray pem = BIO2ByteArray(pemBio);

Expand All @@ -526,10 +522,10 @@ QByteArray encryptStringAsymmetric(const QSslKey key, const QByteArray &data)
qCDebug(lcCse) << "Public key is null. Could not encrypt.";
return {};
}
ClientSideEncryption::Bio publicKeyBio;
Bio publicKeyBio;
const auto publicKeyPem = key.toPem();
BIO_write(publicKeyBio, publicKeyPem.constData(), publicKeyPem.size());
const auto publicKey = ClientSideEncryption::PKey::readPublicKey(publicKeyBio);
const auto publicKey = PKey::readPublicKey(publicKeyBio);
return EncryptionHelper::encryptStringAsymmetric(publicKey, data);
}

Expand All @@ -541,9 +537,9 @@ QByteArray decryptStringAsymmetric(const QByteArray &privateKeyPem, const QByteA
return {};
}

ClientSideEncryption::Bio privateKeyBio;
Bio privateKeyBio;
BIO_write(privateKeyBio, privateKeyPem.constData(), privateKeyPem.size());
const auto key = ClientSideEncryption::PKey::readPrivateKey(privateKeyBio);
const auto key = PKey::readPrivateKey(privateKeyBio);

// Also base64 decode the result
const auto decryptResult = EncryptionHelper::decryptStringAsymmetric(key, data);
Expand Down Expand Up @@ -641,7 +637,7 @@ QByteArray decryptStringAsymmetric(EVP_PKEY *privateKey, const QByteArray& data)
int err = -1;

qCInfo(lcCseDecryption()) << "Start to work the decryption.";
auto ctx = ClientSideEncryption::PKeyCtx::forKey(privateKey, ENGINE_get_default_RSA());
auto ctx = PKeyCtx::forKey(privateKey, ENGINE_get_default_RSA());
if (!ctx) {
qCInfo(lcCseDecryption()) << "Could not create the PKEY context.";
handleErrors();
Expand Down Expand Up @@ -704,7 +700,7 @@ QByteArray decryptStringAsymmetric(EVP_PKEY *privateKey, const QByteArray& data)
QByteArray encryptStringAsymmetric(EVP_PKEY *publicKey, const QByteArray& data) {
int err = -1;

auto ctx = ClientSideEncryption::PKeyCtx::forKey(publicKey, ENGINE_get_default_RSA());
auto ctx = PKeyCtx::forKey(publicKey, ENGINE_get_default_RSA());
if (!ctx) {
qCInfo(lcCse()) << "Could not initialize the pkey context.";
exit(1);
Expand Down Expand Up @@ -750,40 +746,6 @@ QByteArray encryptStringAsymmetric(EVP_PKEY *publicKey, const QByteArray& data)

}

ClientSideEncryption::PKey::~PKey()
{
EVP_PKEY_free(_pkey);
}

ClientSideEncryption::PKey::PKey(PKey &&other)
{
std::swap(_pkey, other._pkey);
}

ClientSideEncryption::PKey ClientSideEncryption::PKey::readPublicKey(Bio &bio)
{
PKey result;
result._pkey = PEM_read_bio_PUBKEY(bio, nullptr, nullptr, nullptr);
return result;
}

ClientSideEncryption::PKey ClientSideEncryption::PKey::readPrivateKey(Bio &bio)
{
PKey result;
result._pkey = PEM_read_bio_PrivateKey(bio, nullptr, nullptr, nullptr);
return result;
}

ClientSideEncryption::PKey ClientSideEncryption::PKey::generate(PKeyCtx &ctx)
{
PKey result;
if (EVP_PKEY_keygen(ctx, &result._pkey) <= 0) {
result._pkey = nullptr;
}
return result;
}


ClientSideEncryption::ClientSideEncryption() = default;

void ClientSideEncryption::initialize(const AccountPtr &account)
Expand Down Expand Up @@ -931,7 +893,7 @@ void ClientSideEncryption::publicCertificateFetched(Job *incoming)
job->start();
}

QByteArray ClientSideEncryption::generateSignatureCMS(const QByteArray &data) const
QByteArray ClientSideEncryption::generateSignatureCryptographicMessageSyntax(const QByteArray &data) const
{
Bio certificateBio;
const auto certificatePem = _certificate.toPem();
Expand All @@ -946,7 +908,7 @@ QByteArray ClientSideEncryption::generateSignatureCMS(const QByteArray &data) co
BIO_write(privateKeyBio, _privateKey.constData(), _privateKey.size());
const auto privateKey = PKey::readPrivateKey(privateKeyBio);

ClientSideEncryption::Bio dataBio;
Bio dataBio;
BIO_write(dataBio, data.constData(), data.size());

const auto contentInfo = CMS_sign(x509Certificate, privateKey, nullptr, dataBio, CMS_DETACHED);
Expand All @@ -955,7 +917,7 @@ QByteArray ClientSideEncryption::generateSignatureCMS(const QByteArray &data) co
return {};
}

ClientSideEncryption::Bio i2dCmsBioOut;
Bio i2dCmsBioOut;
[[maybe_unused]] auto resultI2dCms = i2d_CMS_bio(i2dCmsBioOut, contentInfo);
const auto i2dCmsBio = BIO2ByteArray(i2dCmsBioOut);

Expand All @@ -964,16 +926,16 @@ QByteArray ClientSideEncryption::generateSignatureCMS(const QByteArray &data) co
return i2dCmsBio;
}

bool ClientSideEncryption::verifySignatureCMS(const QByteArray &cmsContent, const QByteArray &data, const QVector<QByteArray> &certificatePems) const
bool ClientSideEncryption::verifySignatureCryptographicMessageSyntax(const QByteArray &cmsContent, const QByteArray &data, const QVector<QByteArray> &certificatePems) const
{
ClientSideEncryption::Bio cmsContentBio;
Bio cmsContentBio;
BIO_write(cmsContentBio, cmsContent.constData(), cmsContent.size());
const auto cmsDataFromBio = d2i_CMS_bio(cmsContentBio, nullptr);
if (!cmsDataFromBio) {
return false;
}

ClientSideEncryption::Bio detachedData;
Bio detachedData;
BIO_write(detachedData, data.constData(), data.size());

if (CMS_verify(cmsDataFromBio, nullptr, nullptr, detachedData, nullptr, CMS_DETACHED | CMS_NO_SIGNER_CERT_VERIFY) != 1) {
Expand Down Expand Up @@ -1391,7 +1353,7 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account)
});
}

std::pair<QByteArray, ClientSideEncryption::PKey> ClientSideEncryption::generateCSR(const AccountPtr &account,
std::pair<QByteArray, PKey> ClientSideEncryption::generateCSR(const AccountPtr &account,
PKey keyPair,
PKey privateKey)
{
Expand Down
Loading

0 comments on commit 170f0be

Please sign in to comment.