Merge pull request #4949 from nextcloud/bugfix/checkCertAgainstPrivateKey

validate certificate for E2EE against private key

2 files changed, 36 insertions, 10 deletions

diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp
index a9a81b1a2..2408b6ade 100644
--- a/src/libsync/clientsideencryption.cpp
+++ b/src/libsync/clientsideencryption.cpp
@@ -196,7 +196,9 @@ namespace {
         EVP_PKEY_CTX* _ctx = nullptr;
     };
 
-    class PKey {
+    }
+
+    class ClientSideEncryption::PKey {
     public:
         ~PKey()
         {
@@ -255,6 +257,8 @@ namespace {
         EVP_PKEY* _pkey = nullptr;
     };
 
+    namespace
+    {
     class X509Certificate {
     public:
         ~X509Certificate()
@@ -619,7 +623,7 @@ QByteArray decryptStringSymmetric(const QByteArray& key, const QByteArray& data)
 QByteArray privateKeyToPem(const QByteArray key) {
     Bio privateKeyBio;
     BIO_write(privateKeyBio, key.constData(), key.size());
-    auto pkey = PKey::readPrivateKey(privateKeyBio);
+    auto pkey = ClientSideEncryption::PKey::readPrivateKey(privateKeyBio);
 
     Bio pemBio;
     PEM_write_bio_PKCS8PrivateKey(pemBio, pkey, nullptr, nullptr, 0, nullptr, nullptr);
@@ -1123,10 +1127,10 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account)
     _privateKey = key;
 
     qCInfo(lcCse()) << "Keys generated correctly, sending to server.";
-    generateCSR(account, localKeyPair);
+    generateCSR(account, std::move(localKeyPair));
 }
 
-void ClientSideEncryption::generateCSR(const AccountPtr &account, EVP_PKEY *keyPair)
+void ClientSideEncryption::generateCSR(const AccountPtr &account, PKey keyPair)
 {
     // OpenSSL expects const char.
     auto cnArray = account->davUser().toLocal8Bit();
@@ -1181,14 +1185,33 @@ void ClientSideEncryption::generateCSR(const AccountPtr &account, EVP_PKEY *keyP
     qCInfo(lcCse()) << "Returning the certificate";
     qCInfo(lcCse()) << output;
 
+    sendSignRequestCSR(account, std::move(keyPair), output);
+}
+
+void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, PKey keyPair, const QByteArray &csrContent)
+{
     auto job = new SignPublicKeyApiJob(account, e2eeBaseUrl() + "public-key", this);
-    job->setCsr(output);
+    job->setCsr(csrContent);
 
-    connect(job, &SignPublicKeyApiJob::jsonReceived, [this, account](const QJsonDocument& json, int retCode) {
+    connect(job, &SignPublicKeyApiJob::jsonReceived, [this, account, keyPair = std::move(keyPair)](const QJsonDocument& json, const int retCode) {
         if (retCode == 200) {
-            QString cert = json.object().value("ocs").toObject().value("data").toObject().value("public-key").toString();
+            const auto cert = json.object().value("ocs").toObject().value("data").toObject().value("public-key").toString();
             _certificate = QSslCertificate(cert.toLocal8Bit(), QSsl::Pem);
             _publicKey = _certificate.publicKey();
+            Bio certificateBio;
+            const auto certificatePem = _certificate.toPem();
+            BIO_write(certificateBio, certificatePem.constData(), certificatePem.size());
+            const auto x509Certificate = X509Certificate::readCertificate(certificateBio);
+            if (!X509_check_private_key(x509Certificate, keyPair)) {
+                auto lastError = ERR_get_error();
+                while (lastError) {
+                    qCInfo(lcCse()) << ERR_lib_error_string(lastError);
+                    lastError = ERR_get_error();
+                }
+                forgetSensitiveData(account);
+                return;
+            }
+            qCInfo(lcCse()) << "received a valid certificate";
             fetchAndValidatePublicKeyFromServer(account);
         }
         qCInfo(lcCse()) << retCode;
@@ -1475,7 +1498,7 @@ QByteArray FolderMetadata::encryptMetadataKey(const QByteArray& data) const
     Bio publicKeyBio;
     QByteArray publicKeyPem = _account->e2e()->_publicKey.toPem();
     BIO_write(publicKeyBio, publicKeyPem.constData(), publicKeyPem.size());
-    auto publicKey = PKey::readPublicKey(publicKeyBio);
+    auto publicKey = ClientSideEncryption::PKey::readPublicKey(publicKeyBio);
 
     // The metadata key is binary so base64 encode it first
     return EncryptionHelper::encryptStringAsymmetric(publicKey, data.toBase64());
@@ -1486,7 +1509,7 @@ QByteArray FolderMetadata::decryptMetadataKey(const QByteArray& encryptedMetadat
     Bio privateKeyBio;
     QByteArray privateKeyPem = _account->e2e()->_privateKey;
     BIO_write(privateKeyBio, privateKeyPem.constData(), privateKeyPem.size());
-    auto key = PKey::readPrivateKey(privateKeyBio);
+    auto key = ClientSideEncryption::PKey::readPrivateKey(privateKeyBio);
 
     // Also base64 decode the result
     QByteArray decryptResult = EncryptionHelper::decryptStringAsymmetric(
diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h
index b628492b8..ec51de61b 100644
--- a/src/libsync/clientsideencryption.h
+++ b/src/libsync/clientsideencryption.h
@@ -116,12 +116,15 @@ private:
 class OWNCLOUDSYNC_EXPORT ClientSideEncryption : public QObject {
     Q_OBJECT
 public:
+    class PKey;
+
     ClientSideEncryption();
     void initialize(const AccountPtr &account);
 
 private:
     void generateKeyPair(const AccountPtr &account);
-    void generateCSR(const AccountPtr &account, EVP_PKEY *keyPair);
+    void generateCSR(const AccountPtr &account, PKey keyPair);
+    void sendSignRequestCSR(const AccountPtr &account, PKey keyPair, const QByteArray &csrContent);
     void encryptPrivateKey(const AccountPtr &account);
 
 public: