Fix crash when storing secrets

Nested QEventLoops are scary. Ultimately we shouldn't use them, but I
have no better solution right now.

fixes #656
This commit is contained in:
Nicolas Werner 2021-07-25 17:07:10 +02:00
parent 80b9d4528e
commit 570d00b000
No known key found for this signature in database
GPG key ID: C8D75E610773F2D9
2 changed files with 26 additions and 25 deletions

View file

@ -715,32 +715,29 @@ Cache::restoreOlmAccount()
} }
void void
Cache::storeSecret(const std::string &name, const std::string &secret) Cache::storeSecret(const std::string name, const std::string secret)
{ {
auto settings = UserSettings::instance(); auto settings = UserSettings::instance();
QKeychain::WritePasswordJob job(QCoreApplication::applicationName()); auto job = new QKeychain::WritePasswordJob(QCoreApplication::applicationName());
job.setAutoDelete(false); job->setInsecureFallback(true);
job.setInsecureFallback(true); job->setKey("matrix." +
job.setKey("matrix." + QString(QCryptographicHash::hash(settings->profile().toUtf8(),
QString(QCryptographicHash::hash(settings->profile().toUtf8(), QCryptographicHash::Sha256)) +
QCryptographicHash::Sha256)) + "." + name.c_str());
"." + name.c_str()); job->setTextData(QString::fromStdString(secret));
job.setTextData(QString::fromStdString(secret)); QObject::connect(job, &QKeychain::Job::finished, job, [name, this](QKeychain::Job *job) {
QEventLoop loop; if (job->error()) {
job.connect(&job, &QKeychain::Job::finished, &loop, &QEventLoop::quit); nhlog::db()->warn(
job.start(); "Storing secret '{}' failed: {}", name, job->errorString().toStdString());
loop.exec(); } else {
emit secretChanged(name);
if (job.error()) { }
nhlog::db()->warn( });
"Storing secret '{}' failed: {}", name, job.errorString().toStdString()); job->start();
} else {
emit secretChanged(name);
}
} }
void void
Cache::deleteSecret(const std::string &name) Cache::deleteSecret(const std::string name)
{ {
auto settings = UserSettings::instance(); auto settings = UserSettings::instance();
QKeychain::DeletePasswordJob job(QCoreApplication::applicationName()); QKeychain::DeletePasswordJob job(QCoreApplication::applicationName());
@ -750,6 +747,8 @@ Cache::deleteSecret(const std::string &name)
QString(QCryptographicHash::hash(settings->profile().toUtf8(), QString(QCryptographicHash::hash(settings->profile().toUtf8(),
QCryptographicHash::Sha256)) + QCryptographicHash::Sha256)) +
"." + name.c_str()); "." + name.c_str());
// FIXME(Nico): Nested event loops are dangerous. Some other slots may resume in the mean
// time!
QEventLoop loop; QEventLoop loop;
job.connect(&job, &QKeychain::Job::finished, &loop, &QEventLoop::quit); job.connect(&job, &QKeychain::Job::finished, &loop, &QEventLoop::quit);
job.start(); job.start();
@ -759,7 +758,7 @@ Cache::deleteSecret(const std::string &name)
} }
std::optional<std::string> std::optional<std::string>
Cache::secret(const std::string &name) Cache::secret(const std::string name)
{ {
auto settings = UserSettings::instance(); auto settings = UserSettings::instance();
QKeychain::ReadPasswordJob job(QCoreApplication::applicationName()); QKeychain::ReadPasswordJob job(QCoreApplication::applicationName());
@ -769,6 +768,8 @@ Cache::secret(const std::string &name)
QString(QCryptographicHash::hash(settings->profile().toUtf8(), QString(QCryptographicHash::hash(settings->profile().toUtf8(),
QCryptographicHash::Sha256)) + QCryptographicHash::Sha256)) +
"." + name.c_str()); "." + name.c_str());
// FIXME(Nico): Nested event loops are dangerous. Some other slots may resume in the mean
// time!
QEventLoop loop; QEventLoop loop;
job.connect(&job, &QKeychain::Job::finished, &loop, &QEventLoop::quit); job.connect(&job, &QKeychain::Job::finished, &loop, &QEventLoop::quit);
job.start(); job.start();

View file

@ -285,9 +285,9 @@ public:
void saveOlmAccount(const std::string &pickled); void saveOlmAccount(const std::string &pickled);
std::string restoreOlmAccount(); std::string restoreOlmAccount();
void storeSecret(const std::string &name, const std::string &secret); void storeSecret(const std::string name, const std::string secret);
void deleteSecret(const std::string &name); void deleteSecret(const std::string name);
std::optional<std::string> secret(const std::string &name); std::optional<std::string> secret(const std::string name);
template<class T> template<class T>
static constexpr bool isStateEvent(const mtx::events::StateEvent<T> &) static constexpr bool isStateEvent(const mtx::events::StateEvent<T> &)