fix review comments

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
This commit is contained in:
Matthieu Gallien 2021-07-01 14:07:09 +02:00
parent 698d5f19ba
commit 2bc72592a0
2 changed files with 40 additions and 46 deletions

View file

@ -34,7 +34,6 @@
#include <QUuid> #include <QUuid>
#include <QScopeGuard> #include <QScopeGuard>
#include <QRandomGenerator> #include <QRandomGenerator>
#include <QSslCertificateExtension>
#include <qt5keychain/keychain.h> #include <qt5keychain/keychain.h>
#include "common/utility.h" #include "common/utility.h"
@ -239,6 +238,11 @@ namespace {
return _pkey; return _pkey;
} }
operator EVP_PKEY*() const
{
return _pkey;
}
private: private:
Q_DISABLE_COPY(PKey) Q_DISABLE_COPY(PKey)
@ -277,6 +281,11 @@ namespace {
return _certificate; return _certificate;
} }
operator X509*() const
{
return _certificate;
}
private: private:
Q_DISABLE_COPY(X509Certificate) Q_DISABLE_COPY(X509Certificate)
@ -867,19 +876,18 @@ bool ClientSideEncryption::checkPublicKeyValidity(const AccountPtr &account) con
return true; return true;
} }
bool ClientSideEncryption::checkServerPublicKeyValidity(const QString &serverPublicKeyString) const bool ClientSideEncryption::checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const
{ {
Bio serverPublicKeyBio; Bio serverPublicKeyBio;
auto serverPublicKeyPem = serverPublicKeyString.toLatin1(); BIO_write(serverPublicKeyBio, serverPublicKeyString.constData(), serverPublicKeyString.size());
BIO_write(serverPublicKeyBio, serverPublicKeyPem.constData(), serverPublicKeyPem.size()); const auto serverPublicKey = PKey::readPrivateKey(serverPublicKeyBio);
auto serverPublicKey = PKey::readPrivateKey(serverPublicKeyBio);
Bio certificateBio; Bio certificateBio;
auto certificatePem = _certificate.toPem(); const auto certificatePem = _certificate.toPem();
BIO_write(certificateBio, certificatePem.constData(), certificatePem.size()); BIO_write(certificateBio, certificatePem.constData(), certificatePem.size());
auto x509Certificate = X509Certificate::readCertificate(certificateBio); const auto x509Certificate = X509Certificate::readCertificate(certificateBio);
if (!x509Certificate) { if (!x509Certificate) {
qCInfo(lcCse()) << "Client certificate is invalid. Impossible to check it against the server public key"; qCInfo(lcCse()) << "Client certificate is invalid. Could not check it against the server public key";
return false; return false;
} }
@ -1176,20 +1184,8 @@ void ClientSideEncryption::generateCSR(const AccountPtr &account, EVP_PKEY *keyP
if (retCode == 200) { if (retCode == 200) {
QString cert = json.object().value("ocs").toObject().value("data").toObject().value("public-key").toString(); QString cert = json.object().value("ocs").toObject().value("data").toObject().value("public-key").toString();
_certificate = QSslCertificate(cert.toLocal8Bit(), QSsl::Pem); _certificate = QSslCertificate(cert.toLocal8Bit(), QSsl::Pem);
qCDebug(lcCse()) << cert;
qCDebug(lcCse()) << _certificate.issuerDisplayName() << _certificate.issuerInfoAttributes();
for (const auto &attribut : _certificate.issuerInfoAttributes()) {
qCDebug(lcCse()) << attribut << _certificate.issuerInfo(attribut);
}
qCDebug(lcCse()) << _certificate.subjectInfoAttributes();
for (const auto &attribut : _certificate.subjectInfoAttributes()) {
qCDebug(lcCse()) << attribut << _certificate.subjectInfo(attribut);
}
for (const auto &extension : _certificate.extensions()) {
qCDebug(lcCse()) << extension.name() << extension.value();
}
_publicKey = _certificate.publicKey(); _publicKey = _certificate.publicKey();
getServerPublicKeyFromServer(account); fetchAndValidatePublicKeyFromServer(account);
} }
qCInfo(lcCse()) << retCode; qCInfo(lcCse()) << retCode;
}); });
@ -1322,9 +1318,8 @@ void ClientSideEncryption::getPublicKeyFromServer(const AccountPtr &account)
QString publicKey = doc.object()["ocs"].toObject()["data"].toObject()["public-keys"].toObject()[account->davUser()].toString(); QString publicKey = doc.object()["ocs"].toObject()["data"].toObject()["public-keys"].toObject()[account->davUser()].toString();
_certificate = QSslCertificate(publicKey.toLocal8Bit(), QSsl::Pem); _certificate = QSslCertificate(publicKey.toLocal8Bit(), QSsl::Pem);
_publicKey = _certificate.publicKey(); _publicKey = _certificate.publicKey();
qCInfo(lcCse()) << publicKey; qCInfo(lcCse()) << "Found Public key, requesting Server Public Key. Public key:" << publicKey;
qCInfo(lcCse()) << "Found Public key, requesting Server Public Key."; fetchAndValidatePublicKeyFromServer(account);
getServerPublicKeyFromServer(account);
} else if (retCode == 404) { } else if (retCode == 404) {
qCInfo(lcCse()) << "No public key on the server"; qCInfo(lcCse()) << "No public key on the server";
generateKeyPair(account); generateKeyPair(account);
@ -1335,34 +1330,33 @@ void ClientSideEncryption::getPublicKeyFromServer(const AccountPtr &account)
job->start(); job->start();
} }
void ClientSideEncryption::getServerPublicKeyFromServer(const AccountPtr &account) void ClientSideEncryption::fetchAndValidatePublicKeyFromServer(const AccountPtr &account)
{ {
qCInfo(lcCse()) << "Retrieving public key from server"; qCInfo(lcCse()) << "Retrieving public key from server";
auto job = new JsonApiJob(account, baseUrl() + "server-key", this); auto job = new JsonApiJob(account, baseUrl() + "server-key", this);
connect(job, &JsonApiJob::jsonReceived, [this, account](const QJsonDocument& doc, int retCode) { connect(job, &JsonApiJob::jsonReceived, [this, account](const QJsonDocument& doc, int retCode) {
if (retCode == 200) { if (retCode == 200) {
auto serverPublicKey = doc.object()["ocs"].toObject()["data"].toObject()["public-key"].toString(); const auto serverPublicKey = doc.object()["ocs"].toObject()["data"].toObject()["public-key"].toString().toLatin1();
qCInfo(lcCse()) << serverPublicKey; qCInfo(lcCse()) << "Found Server Public key, checking it. Server public key:" << serverPublicKey;
qCInfo(lcCse()) << "Found Server Public key, checking it."; if (checkServerPublicKeyValidity(serverPublicKey)) {
if (checkServerPublicKeyValidity(serverPublicKey)) { if (_privateKey.isEmpty()) {
if (_privateKey.isEmpty()) { qCInfo(lcCse()) << "Valid Server Public key, requesting Private Key.";
qCInfo(lcCse()) << "Valid Server Public key, requesting Private Key."; getPrivateKeyFromServer(account);
getPrivateKeyFromServer(account);
} else {
qCInfo(lcCse()) << "Certificate saved, Encrypting Private Key.";
encryptPrivateKey(account);
}
} else { } else {
qCInfo(lcCse()) << "Error invalid server public key"; qCInfo(lcCse()) << "Certificate saved, Encrypting Private Key.";
_certificate = QSslCertificate(); encryptPrivateKey(account);
_publicKey = QSslKey();
_privateKey = QByteArray();
getPublicKeyFromServer(account);
return;
} }
} else { } else {
qCInfo(lcCse()) << "Error while requesting server public key: " << retCode; qCInfo(lcCse()) << "Error invalid server public key";
_certificate = QSslCertificate();
_publicKey = QSslKey();
_privateKey = QByteArray();
getPublicKeyFromServer(account);
return;
} }
} else {
qCInfo(lcCse()) << "Error while requesting server public key: " << retCode;
}
}); });
job->start(); job->start();
} }

View file

@ -99,13 +99,13 @@ signals:
private: private:
void getPrivateKeyFromServer(const AccountPtr &account); void getPrivateKeyFromServer(const AccountPtr &account);
void getPublicKeyFromServer(const AccountPtr &account); void getPublicKeyFromServer(const AccountPtr &account);
void getServerPublicKeyFromServer(const AccountPtr &account); void fetchAndValidatePublicKeyFromServer(const AccountPtr &account);
void decryptPrivateKey(const AccountPtr &account, const QByteArray &key); void decryptPrivateKey(const AccountPtr &account, const QByteArray &key);
void fetchFromKeyChain(const AccountPtr &account); void fetchFromKeyChain(const AccountPtr &account);
bool checkPublicKeyValidity(const AccountPtr &account) const; bool checkPublicKeyValidity(const AccountPtr &account) const;
bool checkServerPublicKeyValidity(const QString &serverPublicKeyString) const; bool checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const;
void writePrivateKey(const AccountPtr &account); void writePrivateKey(const AccountPtr &account);
void writeCertificate(const AccountPtr &account); void writeCertificate(const AccountPtr &account);
void writeMnemonic(const AccountPtr &account); void writeMnemonic(const AccountPtr &account);