Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle custom trusted CA certificates in our access manager #9642

Merged
merged 4 commits into from
May 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/gui/accountstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ void AccountState::checkConnectivity(bool blockJobs)
// and #2973.
// As an attempted workaround, reset the QNAM regularly if the account is
// disconnected.
account()->resetNetworkAccessManager();
account()->resetAccessManager();

// If we don't reset the ssl config a second CheckServerJob can produce a
// ssl config that does not have a sensible certificate chain.
Expand Down
4 changes: 2 additions & 2 deletions src/gui/connectionvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ void ConnectionValidator::checkServerAndUpdate()
this, SLOT(systemProxyLookupDone(QNetworkProxy)));
} else {
// We want to reset the QNAM proxy so that the global proxy settings are used (via ClientProxy settings)
_account->networkAccessManager()->setProxy(QNetworkProxy(QNetworkProxy::DefaultProxy));
_account->accessManager()->setProxy(QNetworkProxy(QNetworkProxy::DefaultProxy));
// use a queued invocation so we're as asynchronous as with the other code path
QMetaObject::invokeMethod(this, "slotCheckServerAndAuth", Qt::QueuedConnection);
}
Expand All @@ -90,7 +90,7 @@ void ConnectionValidator::systemProxyLookupDone(const QNetworkProxy &proxy)
} else {
qCInfo(lcConnectionValidator) << "No system proxy set by OS";
}
_account->networkAccessManager()->setProxy(proxy);
_account->accessManager()->setProxy(proxy);

slotCheckServerAndAuth();
}
Expand Down
4 changes: 2 additions & 2 deletions src/gui/folderman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1153,9 +1153,9 @@ void FolderMan::setDirtyProxy()
for (auto *f : qAsConst(_folders)) {
if (f) {
if (f->accountState() && f->accountState()->account()
&& f->accountState()->account()->networkAccessManager()) {
&& f->accountState()->account()->accessManager()) {
// Need to do this so we do not use the old determined system proxy
f->accountState()->account()->networkAccessManager()->setProxy(
f->accountState()->account()->accessManager()->setProxy(
QNetworkProxy(QNetworkProxy::DefaultProxy));
}
}
Expand Down
15 changes: 8 additions & 7 deletions src/gui/newwizard/setupwizardcontroller.cpp
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
#include "setupwizardcontroller.h"

#include "accessmanager.h"
#include "creds/oauth.h"
#include "determineauthtypejobfactory.h"
#include "gui/application.h"
#include "gui/folderman.h"
#include "jobs/resolveurljobfactory.h"
#include "pages/accountconfiguredwizardpage.h"
#include "pages/basiccredentialssetupwizardpage.h"
#include "pages/oauthcredentialssetupwizardpage.h"
#include "pages/serverurlsetupwizardpage.h"
#include "gui/application.h"
#include "gui/folderman.h"
#include "theme.h"

#include <QClipboard>
Expand All @@ -29,7 +30,7 @@ namespace OCC::Wizard {
SetupWizardController::SetupWizardController(QWidget *parent)
: QObject(parent)
, _wizardWindow(new SetupWizardWindow(parent))
, _networkAccessManager(new QNetworkAccessManager(this))
, _accessManager(new AccessManager(this))
{
// initialize pagination
const QStringList paginationEntries = { tr("Server URL"), tr("Credentials"), tr("Sync Options") };
Expand Down Expand Up @@ -182,7 +183,7 @@ void SetupWizardController::nextStep(std::optional<PageIndex> currentPage, std::

connect(messageBox, &QMessageBox::accepted, this, [this, showFirstPage]() {
// first, we must resolve the actual server URL
auto resolveJob = Jobs::ResolveUrlJobFactory(_networkAccessManager).startJob(_accountBuilder.serverUrl());
auto resolveJob = Jobs::ResolveUrlJobFactory(_accessManager).startJob(_accountBuilder.serverUrl());

connect(resolveJob, &CoreJob::finished, this, [this, resolveJob, showFirstPage]() {
resolveJob->deleteLater();
Expand All @@ -196,7 +197,7 @@ void SetupWizardController::nextStep(std::optional<PageIndex> currentPage, std::
const auto resolvedUrl = qvariant_cast<QUrl>(resolveJob->result());

// next, we need to find out which kind of authentication page we have to present to the user
auto authTypeJob = DetermineAuthTypeJobFactory(_networkAccessManager).startJob(resolvedUrl);
auto authTypeJob = DetermineAuthTypeJobFactory(_accessManager).startJob(resolvedUrl);

connect(authTypeJob, &CoreJob::finished, authTypeJob, [this, authTypeJob, resolvedUrl]() {
authTypeJob->deleteLater();
Expand All @@ -214,7 +215,7 @@ void SetupWizardController::nextStep(std::optional<PageIndex> currentPage, std::
auto newPage = new OAuthCredentialsSetupWizardPage(_accountBuilder.serverUrl());

// username might not be set yet, shouldn't matter, though
auto oAuth = new OAuth(_accountBuilder.serverUrl(), QString(), _networkAccessManager, {}, this);
auto oAuth = new OAuth(_accountBuilder.serverUrl(), QString(), _accessManager, {}, this);

connect(oAuth, &OAuth::result, this, [this, newPage](OAuth::Result result, const QString &user, const QString &token, const QString &refreshToken) {
// the button may not be clicked any more, since the server has been shut down right before this signal was emitted by the OAuth instance
Expand Down Expand Up @@ -316,6 +317,6 @@ void SetupWizardController::nextStep(std::optional<PageIndex> currentPage, std::
SetupWizardController::~SetupWizardController() noexcept
{
delete _wizardWindow;
delete _networkAccessManager;
delete _accessManager;
}
}
2 changes: 1 addition & 1 deletion src/gui/newwizard/setupwizardcontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,6 @@ class SetupWizardController : public QObject

SetupWizardAccountBuilder _accountBuilder;

QNetworkAccessManager *_networkAccessManager;
AccessManager *_accessManager;
};
}
2 changes: 1 addition & 1 deletion src/gui/proxyauthhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void ProxyAuthHandler::handleProxyAuthenticationRequired(
// Since we go into an event loop, it's possible for the account's qnam
// to be destroyed before we get back. We can use this to check for its
// liveness.
sending_qnam = account->sharedNetworkAccessManager().data();
sending_qnam = account->sharedAccessManager().data();
}
if (!sending_qnam) {
qCWarning(lcProxy) << "Could not get the sending QNAM for" << sender();
Expand Down
28 changes: 28 additions & 0 deletions src/libsync/accessmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,37 @@ QNetworkReply *AccessManager::createRequest(QNetworkAccessManager::Operation op,
newRequest.setAttribute(QNetworkRequest::Http2AllowedAttribute, http2EnabledEnv);
}

// for some reason, passing an empty list causes the default chain to be removed
// this behavior does not match the documentation
if (!_customTrustedCaCertificates.isEmpty()) {
auto sslConfiguration = newRequest.sslConfiguration();
sslConfiguration.addCaCertificates({ _customTrustedCaCertificates.begin(), _customTrustedCaCertificates.end() });
newRequest.setSslConfiguration(sslConfiguration);
}

const auto reply = QNetworkAccessManager::createRequest(op, newRequest, outgoingData);
HttpLogger::logRequest(reply, op, outgoingData);
return reply;
}

QSet<QSslCertificate> AccessManager::customTrustedCaCertificates()
{
return _customTrustedCaCertificates;
}

void AccessManager::setCustomTrustedCaCertificates(const QSet<QSslCertificate> &certificates)
{
_customTrustedCaCertificates = certificates;
}

void AccessManager::addCustomTrustedCaCertificates(const QList<QSslCertificate> &certificates)
{
_customTrustedCaCertificates.unite({ certificates.begin(), certificates.end() });
}

void AccessManager::addCustomTrustedCaCertificate(const QSslCertificate &certificate)
{
_customTrustedCaCertificates.insert(certificate);
}

} // namespace OCC
8 changes: 8 additions & 0 deletions src/libsync/accessmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,16 @@ class OWNCLOUDSYNC_EXPORT AccessManager : public QNetworkAccessManager

AccessManager(QObject *parent = nullptr);

QSet<QSslCertificate> customTrustedCaCertificates();
void setCustomTrustedCaCertificates(const QSet<QSslCertificate> &certificates);
void addCustomTrustedCaCertificates(const QList<QSslCertificate> &certificates);
void addCustomTrustedCaCertificate(const QSslCertificate &certificate);

protected:
QNetworkReply *createRequest(QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *outgoingData = nullptr) override;

private:
QSet<QSslCertificate> _customTrustedCaCertificates;
};

} // namespace OCC
Expand Down
21 changes: 11 additions & 10 deletions src/libsync/account.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ void Account::setCredentials(AbstractCredentials *cred)
// Note: This way the QNAM can outlive the Account and Credentials.
// This is necessary to avoid issues with the QNAM being deleted while
// processing slotHandleSslErrors().
_am.reset(_credentials->createQNAM(), &QObject::deleteLater);
_am.reset(_credentials->createAM(), &QObject::deleteLater);

if (jar) {
_am->setCookieJar(jar);
Expand Down Expand Up @@ -218,7 +218,7 @@ QString Account::cookieJarPath()
return QStandardPaths::writableLocation(QStandardPaths::AppConfigLocation) + QStringLiteral("/cookies") + id() + QStringLiteral(".db");
}

void Account::resetNetworkAccessManager()
void Account::resetAccessManager()
{
if (!_credentials || !_am) {
return;
Expand All @@ -227,9 +227,9 @@ void Account::resetNetworkAccessManager()
qCDebug(lcAccount) << "Resetting QNAM";
QNetworkCookieJar *jar = _am->cookieJar();

// Use a QSharedPointer to allow locking the life of the QNAM on the stack.
// Make it call deleteLater to make sure that we can return to any QNAM stack frames safely.
_am = QSharedPointer<QNetworkAccessManager>(_credentials->createQNAM(), &QObject::deleteLater);
// Use a QSharedPointer to allow locking the life of the AM on the stack.
// Make it call deleteLater to make sure that we can return to any AM stack frames safely.
_am = QSharedPointer<AccessManager>(_credentials->createAM(), &QObject::deleteLater);

_am->setCookieJar(jar); // takes ownership of the old cookie jar
connect(_am.data(), &QNetworkAccessManager::sslErrors, this,
Expand All @@ -238,12 +238,12 @@ void Account::resetNetworkAccessManager()
this, &Account::proxyAuthenticationRequired);
}

QNetworkAccessManager *Account::networkAccessManager()
AccessManager *Account::accessManager()
{
return _am.data();
}

QSharedPointer<QNetworkAccessManager> Account::sharedNetworkAccessManager()
QSharedPointer<AccessManager> Account::sharedAccessManager()
{
return _am;
}
Expand Down Expand Up @@ -295,12 +295,13 @@ QSslConfiguration Account::getOrCreateSslConfig()
void Account::setApprovedCerts(const QList<QSslCertificate> certs)
{
_approvedCerts = certs;
QSslSocket::addDefaultCaCertificates(certs);
_am->setCustomTrustedCaCertificates({ certs.begin(), certs.end() });
}

void Account::addApprovedCerts(const QList<QSslCertificate> certs)
{
_approvedCerts += certs;
_am->addCustomTrustedCaCertificates(certs);
}

void Account::resetRejectedCertificates()
Expand Down Expand Up @@ -400,7 +401,6 @@ void Account::slotHandleSslErrors(QPointer<QNetworkReply> reply, const QList<QSs
QList<QSslCertificate> approvedCerts;
if (_sslErrorHandler->handleErrors(filteredErrors, sslConfiguration, &approvedCerts, sharedFromThis())) {
if (!approvedCerts.isEmpty()) {
QSslSocket::addDefaultCaCertificates(approvedCerts);
addApprovedCerts(approvedCerts);
emit wantsAccountSaved(this);

Expand All @@ -419,6 +419,7 @@ void Account::slotHandleSslErrors(QPointer<QNetworkReply> reply, const QList<QSs
return {};
};

// FIXME: outdated comment
// Call `handleErrors` NOW, BEFORE checking if the scoped `reply` pointer. The lambda might take
// a long time to complete: if a dialog is shown, the user could be "slow" to click it away, and
// the reply might have been deleted at that point. So if we'd do this call inside the if below,
Expand Down Expand Up @@ -454,7 +455,7 @@ JobQueue *Account::jobQueue()
return &_jobQueue;
}

void Account::clearQNAMCache()
void Account::clearAMCache()
{
_am->clearAccessCache();
}
Expand Down
23 changes: 12 additions & 11 deletions src/libsync/account.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@
#define SERVERCONNECTION_H

#include <QByteArray>
#include <QUrl>
#include <QUuid>
#include <QNetworkAccessManager>
#include <QNetworkCookie>
#include <QNetworkRequest>
#include <QSslSocket>
#include <QSharedPointer>
#include <QSslCertificate>
#include <QSslConfiguration>
#include <QSslCipher>
#include <QSslConfiguration>
#include <QSslError>
#include <QSharedPointer>
#include <QSslSocket>
#include <QUrl>
#include <QUuid>

#ifndef TOKEN_AUTH_ONLY
#include <QPixmap>
Expand All @@ -40,7 +41,7 @@
class QSettings;
class QNetworkReply;
class QUrl;
class QNetworkAccessManager;
class AccessManager;

namespace OCC {

Expand Down Expand Up @@ -226,9 +227,9 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject
void lendCookieJarTo(QNetworkAccessManager *guest);
QString cookieJarPath();

void resetNetworkAccessManager();
QNetworkAccessManager *networkAccessManager();
QSharedPointer<QNetworkAccessManager> sharedNetworkAccessManager();
void resetAccessManager();
AccessManager *accessManager();
QSharedPointer<AccessManager> sharedAccessManager();

JobQueue *jobQueue();

Expand All @@ -238,7 +239,7 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject

public slots:
/// Used when forgetting credentials
void clearQNAMCache();
void clearAMCache();
void slotHandleSslErrors(QPointer<QNetworkReply>, const QList<QSslError> &);

signals:
Expand Down Expand Up @@ -292,7 +293,7 @@ protected Q_SLOTS:
QString _serverVersion;
QScopedPointer<AbstractSslErrorHandler> _sslErrorHandler;
QuotaInfo *_quotaInfo;
QSharedPointer<QNetworkAccessManager> _am;
QSharedPointer<AccessManager> _am;
QScopedPointer<AbstractCredentials> _credentials;
bool _http2Supported = false;

Expand Down
7 changes: 4 additions & 3 deletions src/libsync/creds/abstractcredentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@

#include <QObject>

#include <csync.h>
#include "owncloudlib.h"
#include "accessmanager.h"
#include "accountfwd.h"
#include "owncloudlib.h"
#include <csync.h>

class QNetworkAccessManager;
class QNetworkReply;
Expand All @@ -45,7 +46,7 @@ class OWNCLOUDSYNC_EXPORT AbstractCredentials : public QObject

virtual QString authType() const = 0;
virtual QString user() const = 0;
virtual QNetworkAccessManager *createQNAM() const = 0;
virtual AccessManager *createAM() const = 0;

/** Whether there are credentials that can be used for a connection attempt. */
virtual bool ready() const = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/creds/dummycredentials.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ QString DummyCredentials::user() const
return _user;
}

QNetworkAccessManager *DummyCredentials::createQNAM() const
AccessManager *DummyCredentials::createAM() const
{
return new AccessManager;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/creds/dummycredentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class OWNCLOUDSYNC_EXPORT DummyCredentials : public AbstractCredentials
QString _password;
QString authType() const override;
QString user() const override;
QNetworkAccessManager *createQNAM() const override;
AccessManager *createAM() const override;
bool ready() const override;
bool stillValid(QNetworkReply *reply) override;
void fetchFromKeychain() override;
Expand Down
10 changes: 5 additions & 5 deletions src/libsync/creds/httpcredentials.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,14 @@ void HttpCredentials::setAccount(Account *account)
}
}

QNetworkAccessManager *HttpCredentials::createQNAM() const
AccessManager *HttpCredentials::createAM() const
{
AccessManager *qnam = new HttpCredentialsAccessManager(this);
AccessManager *am = new HttpCredentialsAccessManager(this);

connect(qnam, &QNetworkAccessManager::authenticationRequired,
connect(am, &QNetworkAccessManager::authenticationRequired,
this, &HttpCredentials::slotAuthentication);

return qnam;
return am;
}

bool HttpCredentials::ready() const
Expand Down Expand Up @@ -345,7 +345,7 @@ void HttpCredentials::invalidateToken()
// indirectly) from QNetworkAccessManagerPrivate::authenticationRequired, which itself
// is a called from a BlockingQueuedConnection from the Qt HTTP thread. And clearing the
// cache needs to synchronize again with the HTTP thread.
QTimer::singleShot(0, _account, &Account::clearQNAMCache);
QTimer::singleShot(0, _account, &Account::clearAMCache);
}

void HttpCredentials::forgetSensitiveData()
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/creds/httpcredentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials
explicit HttpCredentials(DetermineAuthTypeJob::AuthType authType, const QString &user, const QString &password);

QString authType() const override;
QNetworkAccessManager *createQNAM() const override;
AccessManager *createAM() const override;
bool ready() const override;
void fetchFromKeychain() override;
bool stillValid(QNetworkReply *reply) override;
Expand Down
Loading