From 05d6a294165c03dab2ed8ec94cb501cd61c4ec34 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 21 Nov 2018 15:15:51 +0800 Subject: [PATCH] Apply PBKDF2 when storing passwords --- src/app/application.cpp | 4 +- src/base/CMakeLists.txt | 2 + src/base/base.pri | 2 + src/base/preferences.cpp | 24 +--- src/base/preferences.h | 4 +- src/base/utils/password.cpp | 120 ++++++++++++++++++ src/base/utils/password.h | 51 ++++++++ src/base/utils/string.cpp | 15 --- src/base/utils/string.h | 7 +- src/gui/optionsdialog.cpp | 7 +- src/gui/optionsdialog.ui | 9 +- src/webui/api/appcontroller.cpp | 4 +- src/webui/api/authcontroller.cpp | 19 +-- .../www/private/preferences_content.html | 10 +- 14 files changed, 208 insertions(+), 70 deletions(-) create mode 100644 src/base/utils/password.cpp create mode 100644 src/base/utils/password.h diff --git a/src/app/application.cpp b/src/app/application.cpp index 3b1df76af..38470b22d 100644 --- a/src/app/application.cpp +++ b/src/app/application.cpp @@ -525,8 +525,8 @@ int Application::exec(const QStringList ¶ms) .arg(QString("http://localhost:") + QString::number(pref->getWebUiPort())) + '\n' + tr("The Web UI administrator user name is: %1").arg(pref->getWebUiUsername()) + '\n'; printf("%s", qUtf8Printable(mesg)); - qDebug() << "Password:" << pref->getWebUiPassword(); - if (pref->getWebUiPassword() == "f6fdffe48c908deb0f4c3bd36c032e72") { + + if (pref->getWebUIPassword() == "ARQ77eY1NUZaQsuDHbIMCA==:0WMRkYTUWVT9wVvdDtHAjU9b3b7uB8NR1Gur2hmQCvCDpm39Q+PsJRJPaCU51dEiz+dTzh8qbPsL8WkFljQYFQ==") { const QString warning = tr("The Web UI administrator password is still the default one: %1").arg("adminadmin") + '\n' + tr("This is a security risk, please consider changing your password from program preferences.") + '\n'; printf("%s", qUtf8Printable(warning)); diff --git a/src/base/CMakeLists.txt b/src/base/CMakeLists.txt index 4049d0cdf..02548b1b5 100644 --- a/src/base/CMakeLists.txt +++ b/src/base/CMakeLists.txt @@ -54,6 +54,7 @@ utils/fs.h utils/gzip.h utils/misc.h utils/net.h +utils/password.h utils/random.h utils/string.h utils/version.h @@ -123,6 +124,7 @@ utils/fs.cpp utils/gzip.cpp utils/misc.cpp utils/net.cpp +utils/password.cpp utils/random.cpp utils/string.cpp asyncfilestorage.cpp diff --git a/src/base/base.pri b/src/base/base.pri index 1357fc754..7aa681937 100644 --- a/src/base/base.pri +++ b/src/base/base.pri @@ -69,6 +69,7 @@ HEADERS += \ $$PWD/utils/gzip.h \ $$PWD/utils/misc.h \ $$PWD/utils/net.h \ + $$PWD/utils/password.h \ $$PWD/utils/random.h \ $$PWD/utils/string.h \ $$PWD/utils/version.h @@ -133,5 +134,6 @@ SOURCES += \ $$PWD/utils/gzip.cpp \ $$PWD/utils/misc.cpp \ $$PWD/utils/net.cpp \ + $$PWD/utils/password.cpp \ $$PWD/utils/random.cpp \ $$PWD/utils/string.cpp diff --git a/src/base/preferences.cpp b/src/base/preferences.cpp index 6bebb22c1..1e86caaca 100644 --- a/src/base/preferences.cpp +++ b/src/base/preferences.cpp @@ -582,28 +582,16 @@ void Preferences::setWebUiUsername(const QString &username) setValue("Preferences/WebUI/Username", username); } -QString Preferences::getWebUiPassword() const +QByteArray Preferences::getWebUIPassword() const { - QString passHa1 = value("Preferences/WebUI/Password_ha1").toString(); - if (passHa1.isEmpty()) { - QCryptographicHash md5(QCryptographicHash::Md5); - md5.addData("adminadmin"); - passHa1 = md5.result().toHex(); - } - return passHa1; + // default: adminadmin + const QByteArray defaultValue = "ARQ77eY1NUZaQsuDHbIMCA==:0WMRkYTUWVT9wVvdDtHAjU9b3b7uB8NR1Gur2hmQCvCDpm39Q+PsJRJPaCU51dEiz+dTzh8qbPsL8WkFljQYFQ=="; + return value("Preferences/WebUI/Password_PBKDF2", defaultValue).toByteArray(); } -void Preferences::setWebUiPassword(const QString &newPassword) +void Preferences::setWebUIPassword(const QByteArray &password) { - // Do not overwrite current password with its hash - if (newPassword == getWebUiPassword()) - return; - - // Encode to md5 and save - QCryptographicHash md5(QCryptographicHash::Md5); - md5.addData(newPassword.toLocal8Bit()); - - setValue("Preferences/WebUI/Password_ha1", md5.result().toHex()); + setValue("Preferences/WebUI/Password_PBKDF2", password); } bool Preferences::isWebUiClickjackingProtectionEnabled() const diff --git a/src/base/preferences.h b/src/base/preferences.h index 26e21f60b..f137aa24c 100644 --- a/src/base/preferences.h +++ b/src/base/preferences.h @@ -193,8 +193,8 @@ public: void setWebUiAuthSubnetWhitelist(QStringList subnets); QString getWebUiUsername() const; void setWebUiUsername(const QString &username); - QString getWebUiPassword() const; - void setWebUiPassword(const QString &newPassword); + QByteArray getWebUIPassword() const; + void setWebUIPassword(const QByteArray &password); // WebUI security bool isWebUiClickjackingProtectionEnabled() const; diff --git a/src/base/utils/password.cpp b/src/base/utils/password.cpp new file mode 100644 index 000000000..bced8359e --- /dev/null +++ b/src/base/utils/password.cpp @@ -0,0 +1,120 @@ +/* + * Bittorrent Client using Qt and libtorrent. + * Copyright (C) 2018 Mike Tzou (Chocobo1) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * In addition, as a special exception, the copyright holders give permission to + * link this program with the OpenSSL project's "OpenSSL" library (or with + * modified versions of it that use the same license as the "OpenSSL" library), + * and distribute the linked executables. You must obey the GNU General Public + * License in all respects for all of the code used other than "OpenSSL". If you + * modify file(s), you may extend this exception to your version of the file(s), + * but you are not obligated to do so. If you do not wish to do so, delete this + * exception statement from your version. + */ + +#include "password.h" + +#include + +#include + +#include +#include +#include + +#include "bytearray.h" +#include "random.h" +#include "string.h" + +namespace Utils +{ + namespace Password + { + namespace PBKDF2 + { + const int hashIterations = 100000; + const auto hashMethod = EVP_sha512(); + } + } +} + +// Implements constant-time comparison to protect against timing attacks +// Taken from https://crackstation.net/hashing-security.htm +bool Utils::Password::slowEquals(const QByteArray &a, const QByteArray &b) +{ + const int lengthA = a.length(); + const int lengthB = b.length(); + + int diff = lengthA ^ lengthB; + for (int i = 0; (i < lengthA) && (i < lengthB); ++i) + diff |= a[i] ^ b[i]; + + return (diff == 0); +} + +QByteArray Utils::Password::PBKDF2::generate(const QString &password) +{ + return generate(password.toUtf8()); +} + +QByteArray Utils::Password::PBKDF2::generate(const QByteArray &password) +{ + const std::array salt {{Random::rand(), Random::rand() + , Random::rand(), Random::rand()}}; + + std::array outBuf {}; + const int hmacResult = PKCS5_PBKDF2_HMAC(password.constData(), password.size() + , reinterpret_cast(salt.data()), (sizeof(salt[0]) * salt.size()) + , hashIterations, hashMethod + , outBuf.size(), outBuf.data()); + if (hmacResult != 1) + return {}; + + const QByteArray saltView = QByteArray::fromRawData( + reinterpret_cast(salt.data()), (sizeof(salt[0]) * salt.size())); + const QByteArray outBufView = QByteArray::fromRawData( + reinterpret_cast(outBuf.data()), outBuf.size()); + + return (saltView.toBase64() + ':' + outBufView.toBase64()); +} + +bool Utils::Password::PBKDF2::verify(const QByteArray &secret, const QString &password) +{ + return verify(secret, password.toUtf8()); +} + +bool Utils::Password::PBKDF2::verify(const QByteArray &secret, const QByteArray &password) +{ + const QList list = ByteArray::splitToViews(secret, ":", QString::SkipEmptyParts); + if (list.size() != 2) + return false; + + const QByteArray salt = QByteArray::fromBase64(list[0]); + const QByteArray key = QByteArray::fromBase64(list[1]); + + std::array outBuf {}; + const int hmacResult = PKCS5_PBKDF2_HMAC(password.constData(), password.size() + , reinterpret_cast(salt.constData()), salt.size() + , hashIterations, hashMethod + , outBuf.size(), outBuf.data()); + if (hmacResult != 1) + return false; + + const QByteArray outBufView = QByteArray::fromRawData( + reinterpret_cast(outBuf.data()), outBuf.size()); + return slowEquals(key, outBufView); +} diff --git a/src/base/utils/password.h b/src/base/utils/password.h new file mode 100644 index 000000000..b7b2320f2 --- /dev/null +++ b/src/base/utils/password.h @@ -0,0 +1,51 @@ +/* + * Bittorrent Client using Qt and libtorrent. + * Copyright (C) 2018 Mike Tzou (Chocobo1) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * In addition, as a special exception, the copyright holders give permission to + * link this program with the OpenSSL project's "OpenSSL" library (or with + * modified versions of it that use the same license as the "OpenSSL" library), + * and distribute the linked executables. You must obey the GNU General Public + * License in all respects for all of the code used other than "OpenSSL". If you + * modify file(s), you may extend this exception to your version of the file(s), + * but you are not obligated to do so. If you do not wish to do so, delete this + * exception statement from your version. + */ + +#pragma once + +class QByteArray; +class QString; + +namespace Utils +{ + namespace Password + { + // Implements constant-time comparison to protect against timing attacks + // Taken from https://crackstation.net/hashing-security.htm + bool slowEquals(const QByteArray &a, const QByteArray &b); + + namespace PBKDF2 + { + QByteArray generate(const QString &password); + QByteArray generate(const QByteArray &password); + + bool verify(const QByteArray &secret, const QString &password); + bool verify(const QByteArray &secret, const QByteArray &password); + } + } +} diff --git a/src/base/utils/string.cpp b/src/base/utils/string.cpp index e8ddcffcb..6ed656c3e 100644 --- a/src/base/utils/string.cpp +++ b/src/base/utils/string.cpp @@ -31,7 +31,6 @@ #include -#include #include #include #include @@ -164,20 +163,6 @@ QString Utils::String::fromDouble(double n, int precision) return QLocale::system().toString(std::floor(n * prec) / prec, 'f', precision); } -// Implements constant-time comparison to protect against timing attacks -// Taken from https://crackstation.net/hashing-security.htm -bool Utils::String::slowEquals(const QByteArray &a, const QByteArray &b) -{ - int lengthA = a.length(); - int lengthB = b.length(); - - int diff = lengthA ^ lengthB; - for (int i = 0; (i < lengthA) && (i < lengthB); ++i) - diff |= a[i] ^ b[i]; - - return (diff == 0); -} - // This is marked as internal in QRegExp.cpp, but is exported. The alternative would be to // copy the code from QRegExp::wc2rx(). QString qt_regexp_toCanonical(const QString &pattern, QRegExp::PatternSyntax patternSyntax); diff --git a/src/base/utils/string.h b/src/base/utils/string.h index 69eb7c020..e33ca7bac 100644 --- a/src/base/utils/string.h +++ b/src/base/utils/string.h @@ -30,10 +30,9 @@ #ifndef UTILS_STRING_H #define UTILS_STRING_H +#include #include -class QByteArray; -class QLatin1String; class TriStateBool; namespace Utils @@ -42,10 +41,6 @@ namespace Utils { QString fromDouble(double n, int precision); - // Implements constant-time comparison to protect against timing attacks - // Taken from https://crackstation.net/hashing-security.htm - bool slowEquals(const QByteArray &a, const QByteArray &b); - int naturalCompare(const QString &left, const QString &right, const Qt::CaseSensitivity caseSensitivity); template bool naturalLessThan(const QString &left, const QString &right) diff --git a/src/gui/optionsdialog.cpp b/src/gui/optionsdialog.cpp index 94a93cee7..a1737ad71 100644 --- a/src/gui/optionsdialog.cpp +++ b/src/gui/optionsdialog.cpp @@ -59,6 +59,7 @@ #include "base/torrentfileguard.h" #include "base/unicodestrings.h" #include "base/utils/fs.h" +#include "base/utils/password.h" #include "base/utils/random.h" #include "addnewtorrentdialog.h" #include "advancedsettings.h" @@ -728,7 +729,8 @@ void OptionsDialog::saveOptions() } // Authentication pref->setWebUiUsername(webUiUsername()); - pref->setWebUiPassword(webUiPassword()); + if (!webUiPassword().isEmpty()) + pref->setWebUIPassword(Utils::Password::PBKDF2::generate(webUiPassword())); pref->setWebUiLocalAuthEnabled(!m_ui->checkBypassLocalAuth->isChecked()); pref->setWebUiAuthSubnetWhitelistEnabled(m_ui->checkBypassAuthSubnetWhitelist->isChecked()); // Security @@ -1090,7 +1092,6 @@ void OptionsDialog::loadOptions() setSslCertificate(pref->getWebUiHttpsCertificate()); setSslKey(pref->getWebUiHttpsKey()); m_ui->textWebUiUsername->setText(pref->getWebUiUsername()); - m_ui->textWebUiPassword->setText(pref->getWebUiPassword()); m_ui->checkBypassLocalAuth->setChecked(!pref->isWebUiLocalAuthEnabled()); m_ui->checkBypassAuthSubnetWhitelist->setChecked(pref->isWebUiAuthSubnetWhitelistEnabled()); m_ui->IPSubnetWhitelistButton->setEnabled(m_ui->checkBypassAuthSubnetWhitelist->isChecked()); @@ -1743,7 +1744,7 @@ bool OptionsDialog::webUIAuthenticationOk() QMessageBox::warning(this, tr("Length Error"), tr("The Web UI username must be at least 3 characters long.")); return false; } - if (webUiPassword().length() < 6) { + if (!webUiPassword().isEmpty() && (webUiPassword().length() < 6)) { QMessageBox::warning(this, tr("Length Error"), tr("The Web UI password must be at least 6 characters long.")); return false; } diff --git a/src/gui/optionsdialog.ui b/src/gui/optionsdialog.ui index 461093d91..7d4f83231 100644 --- a/src/gui/optionsdialog.ui +++ b/src/gui/optionsdialog.ui @@ -3074,15 +3074,12 @@ Specify an IPv4 or IPv6 address. You can specify "0.0.0.0" for any IPv - - - - - 1000 - QLineEdit::Password + + Change current password + diff --git a/src/webui/api/appcontroller.cpp b/src/webui/api/appcontroller.cpp index 82eb7d2e1..9d4432bf4 100644 --- a/src/webui/api/appcontroller.cpp +++ b/src/webui/api/appcontroller.cpp @@ -54,6 +54,7 @@ #include "base/scanfoldersmodel.h" #include "base/utils/fs.h" #include "base/utils/net.h" +#include "base/utils/password.h" #include "../webapplication.h" void AppController::webapiVersionAction() @@ -198,7 +199,6 @@ void AppController::preferencesAction() data["ssl_cert"] = QString::fromLatin1(pref->getWebUiHttpsCertificate()); // Authentication data["web_ui_username"] = pref->getWebUiUsername(); - data["web_ui_password"] = pref->getWebUiPassword(); data["bypass_local_auth"] = !pref->isWebUiLocalAuthEnabled(); data["bypass_auth_subnet_whitelist_enabled"] = pref->isWebUiAuthSubnetWhitelistEnabled(); QStringList authSubnetWhitelistStringList; @@ -474,7 +474,7 @@ void AppController::setPreferencesAction() if (m.contains("web_ui_username")) pref->setWebUiUsername(m["web_ui_username"].toString()); if (m.contains("web_ui_password")) - pref->setWebUiPassword(m["web_ui_password"].toString()); + pref->setWebUIPassword(Utils::Password::PBKDF2::generate(m["web_ui_password"].toByteArray())); if (m.contains("bypass_local_auth")) pref->setWebUiLocalAuthEnabled(!m["bypass_local_auth"].toBool()); if (m.contains("bypass_auth_subnet_whitelist_enabled")) diff --git a/src/webui/api/authcontroller.cpp b/src/webui/api/authcontroller.cpp index 1291f98a5..5798f77ec 100644 --- a/src/webui/api/authcontroller.cpp +++ b/src/webui/api/authcontroller.cpp @@ -28,11 +28,9 @@ #include "authcontroller.h" -#include - #include "base/logger.h" #include "base/preferences.h" -#include "base/utils/string.h" +#include "base/utils/password.h" #include "apierror.h" #include "isessionmanager.h" @@ -58,17 +56,14 @@ void AuthController::loginAction() , tr("Your IP address has been banned after too many failed authentication attempts.")); } - const QString username {Preferences::instance()->getWebUiUsername()}; - const QString password {Preferences::instance()->getWebUiPassword()}; - - QCryptographicHash md5(QCryptographicHash::Md5); - md5.addData(passwordFromWeb.toLocal8Bit()); - const QString passwordFromWebHashed = md5.result().toHex(); + const Preferences *pref = Preferences::instance(); - const bool equalUser = Utils::String::slowEquals(usernameFromWeb.toUtf8(), username.toUtf8()); - const bool equalPass = Utils::String::slowEquals(passwordFromWebHashed.toUtf8(), password.toUtf8()); + const QString username {pref->getWebUiUsername()}; + const QByteArray secret {pref->getWebUIPassword()}; + const bool usernameEqual = Utils::Password::slowEquals(usernameFromWeb.toUtf8(), username.toUtf8()); + const bool passwordEqual = Utils::Password::PBKDF2::verify(secret, passwordFromWeb); - if (equalUser && equalPass) { + if (usernameEqual && passwordEqual) { m_clientFailedLogins.remove(clientAddr); sessionManager()->sessionStart(); diff --git a/src/webui/www/private/preferences_content.html b/src/webui/www/private/preferences_content.html index b9ff22c1b..fd66f593a 100644 --- a/src/webui/www/private/preferences_content.html +++ b/src/webui/www/private/preferences_content.html @@ -433,7 +433,8 @@
- + +
@@ -980,7 +981,6 @@ // Authentication $('webui_username_text').setProperty('value', pref.web_ui_username); - $('webui_password_text').setProperty('value', pref.web_ui_password); $('bypass_local_auth_checkbox').setProperty('checked', pref.bypass_local_auth); $('bypass_auth_subnet_whitelist_checkbox').setProperty('checked', pref.bypass_auth_subnet_whitelist_enabled); $('bypass_auth_subnet_whitelist_textarea').setProperty('value', pref.bypass_auth_subnet_whitelist); @@ -1264,12 +1264,14 @@ return; } var web_ui_password = $('webui_password_text').getProperty('value'); - if (web_ui_password.length < 6) { + if ((0 < web_ui_password.length) && (web_ui_password.length < 6)) { alert("QBT_TR(The Web UI password must be at least 6 characters long.)QBT_TR[CONTEXT=OptionsDialog]"); return; } + settings.set('web_ui_username', web_ui_username); - settings.set('web_ui_password', web_ui_password); + if (web_ui_password.length > 0) + settings.set('web_ui_password', web_ui_password); settings.set('bypass_local_auth', $('bypass_local_auth_checkbox').getProperty('checked')); settings.set('bypass_auth_subnet_whitelist_enabled', $('bypass_auth_subnet_whitelist_checkbox').getProperty('checked')); settings.set('bypass_auth_subnet_whitelist', $('bypass_auth_subnet_whitelist_textarea').getProperty('value'));