better memory management and better logs for end-to-end encryption

Signed-off-by: Matthieu Gallien <[email protected]>
nextcloud · Jun 14, 2024 · 5c7042c · 5c7042c
1 parent 20421ca
commit 5c7042c
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 35 deletions.
diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp
@@ -1693,6 +1693,8 @@ void ClientSideEncryption::forgetSensitiveData(const AccountPtr &account)
     _usbTokenInformation.setSha256Fingerprint({});
     account->setEncryptionCertificateFingerprint({});
     _encryptionCertificate.clear();
+    //_tokenSlots.reset();
+    _context = Pkcs11Context{Pkcs11Context::State::EmptyContext};
     Q_EMIT canDecryptChanged();
     Q_EMIT canEncryptChanged();
     Q_EMIT userCertificateNeedsMigrationChanged();
@@ -3104,26 +3106,4 @@ void CertificateInformation::checkEncryptionCertificate()
     }
 }
 
-Pkcs11Context::Pkcs11Context(State initState)
-    : _pkcsS11Ctx(initState == State::CreateContext ? PKCS11_CTX_new() : nullptr)
-{
-}
-
-Pkcs11Context::Pkcs11Context(Pkcs11Context &&otherContext)
-    : _pkcsS11Ctx(otherContext._pkcsS11Ctx)
-{
-    otherContext._pkcsS11Ctx = nullptr;
-}
-
-Pkcs11Context::~Pkcs11Context()
-{
-    qCWarning(lcCse()) << "destructor" << this;
-    if (_pkcsS11Ctx) {
-        PKCS11_CTX_free(_pkcsS11Ctx);
-        _pkcsS11Ctx = nullptr;
-    } else {
-        qCWarning(lcCse()) << "destructor" << this << "nullptr";
-    }
-}
-
 }
diff --git a/src/libsync/clientsideencryptionprimitives.cpp b/src/libsync/clientsideencryptionprimitives.cpp
@@ -11,11 +11,18 @@
  * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
  * for more details.
  */
+
 #include "clientsideencryptionprimitives.h"
+
+#include <QLoggingCategory>
+
 #include <openssl/pem.h>
 
 namespace OCC
 {
+
+Q_LOGGING_CATEGORY(lcCseUtility, "nextcloud.sync.clientsideencryption.utility", QtInfoMsg)
+
 Bio::operator const BIO *() const
 {
     return _bio;
@@ -111,4 +118,45 @@ PKey::operator EVP_PKEY *() const
     return _pkey;
 }
 
+Pkcs11Context::Pkcs11Context(State initState)
+    : _pkcsS11Ctx(initState == State::CreateContext ? PKCS11_CTX_new() : nullptr)
+{
+    qCDebug(lcCseUtility()) << "[[Pkcs11Context]] constructor" << this << _pkcsS11Ctx;
+}
+
+Pkcs11Context::Pkcs11Context(Pkcs11Context &&otherContext)
+    : _pkcsS11Ctx(otherContext._pkcsS11Ctx)
+{
+    otherContext._pkcsS11Ctx = nullptr;
+    qCDebug(lcCseUtility()) << "[[Pkcs11Context]] constructor" << this << _pkcsS11Ctx;
+}
+
+Pkcs11Context::~Pkcs11Context()
+{
+    qCDebug(lcCseUtility()) << "[[Pkcs11Context]] destructor" << this << _pkcsS11Ctx;
+    if (_pkcsS11Ctx) {
+        qCDebug(lcCseUtility()) << "[[Pkcs11Context]] PKCS11_CTX_free" << _pkcsS11Ctx;
+        PKCS11_CTX_free(_pkcsS11Ctx);
+        _pkcsS11Ctx = nullptr;
+    } else {
+        qCDebug(lcCseUtility()) << "destructor" << this << "nullptr";
+    }
+}
+
+Pkcs11Context &Pkcs11Context::operator=(Pkcs11Context &&otherContext)
+{
+    qCDebug(lcCseUtility()) << "[[Pkcs11Context]] operator=" << this << _pkcsS11Ctx;
+    qCDebug(lcCseUtility()) << "[[Pkcs11Context]] operator=" << &otherContext << otherContext._pkcsS11Ctx;
+    if (&otherContext != this) {
+        if (_pkcsS11Ctx) {
+            qCDebug(lcCseUtility()) << "[[Pkcs11Context]] PKCS11_CTX_free" << _pkcsS11Ctx;
+            PKCS11_CTX_free(_pkcsS11Ctx);
+            _pkcsS11Ctx = nullptr;
+        }
+        std::swap(_pkcsS11Ctx, otherContext._pkcsS11Ctx);
+    }
+
+    return *this;
+}
+
 }
diff --git a/src/libsync/clientsideencryptionprimitives.h b/src/libsync/clientsideencryptionprimitives.h
@@ -120,18 +120,7 @@ class Pkcs11Context {
 
     ~Pkcs11Context();
 
-    Pkcs11Context& operator=(Pkcs11Context &&otherContext)
-    {
-        if (&otherContext != this) {
-            if (_pkcsS11Ctx) {
-                PKCS11_CTX_free(_pkcsS11Ctx);
-                _pkcsS11Ctx = nullptr;
-            }
-            std::swap(_pkcsS11Ctx, otherContext._pkcsS11Ctx);
-        }
-
-        return *this;
-    }
+    Pkcs11Context& operator=(Pkcs11Context &&otherContext);
 
     Pkcs11Context& operator=(const Pkcs11Context&) = delete;
 

diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp
@@ -124,8 +124,10 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString
         QString prefix;
         switch (type) {
         case QtDebugMsg:
+            prefix = QStringLiteral("[DEBUG] ");
             break;
         case QtInfoMsg:
+            prefix = QStringLiteral("[INFO] ");
             break;
         case QtWarningMsg:
             prefix = QStringLiteral("[WARNING] ");
@@ -137,7 +139,7 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString
             prefix = QStringLiteral("[FATAL ERROR] ");
             break;
         }
-        auto msgW = QString(prefix + message).toStdWString();
+        auto msgW = QString(prefix + msg).toStdWString();
         msgW.append(L"\n");
         OutputDebugString(msgW.c_str());
     }