From 8a6cac8338111341f1655591ca73e2f2329c6600 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 21 Nov 2018 21:29:10 +0800 Subject: [PATCH 1/4] Make OpenSSL a direct dependency --- .travis.yml | 13 +++--- CMakeLists.txt | 1 + configure | 101 +++++++++++++++++++++++++++++++++++++++++++++ configure.ac | 7 +++- src/CMakeLists.txt | 1 + 5 files changed, 116 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index b5a6c4249..646e02ed1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -80,7 +80,7 @@ before_install: if [ "$TRAVIS_OS_NAME" = "linux" ]; then qbtconf="$qbtconf --prefix="$qbt_path" PKG_CONFIG_PATH=/opt/qt55/lib/pkgconfig:$PKG_CONFIG_PATH" else - qbtconf="$qbtconf --prefix="$qbt_path"" + qbtconf="$qbtconf --prefix="$qbt_path" PKG_CONFIG_PATH=/usr/local/opt/openssl/lib/pkgconfig:$PKG_CONFIG_PATH" fi # options for specific branches @@ -120,7 +120,8 @@ install: # dependencies brew update > /dev/null brew outdated "pkg-config" || brew upgrade "pkg-config" - brew install colormake ccache zlib qt libtorrent-rasterbar + brew install colormake ccache zlib qt openssl libtorrent-rasterbar + PATH="/usr/local/opt/ccache/libexec:$PATH" brew link --force zlib qt @@ -130,9 +131,9 @@ install: sudo ln -s /usr/local/opt/qt/mkspecs /usr/local/mkspecs sudo ln -s /usr/local/opt/qt/plugins /usr/local/plugins - fi - MY_CMAKE_OPENSSL_HINT="-DOPENSSL_ROOT_DIR=/usr/local/opt/openssl/" + MY_CMAKE_OPENSSL_HINT="-DOPENSSL_ROOT_DIR=/usr/local/opt/openssl/" + fi fi - | if [ "$TRAVIS_BRANCH" != "$coverity_branch" ]; then @@ -159,8 +160,8 @@ script: # For some reason for RC_1_1 we need to also specify the OpenSSL compiler/linker flags # Homebrew doesn't symlink OpenSSL for security reasons ./bootstrap.sh - ./configure $qbtconf CXXFLAGS="$CXXFLAGS $(PKG_CONFIG_PATH="/usr/local/opt/openssl/lib/pkgconfig:$PKG_CONFIG_PATH" pkg-config --cflags openssl) -std=c++14" \ - LDFLAGS="$LDFLAGS $(PKG_CONFIG_PATH="/usr/local/opt/openssl/lib/pkgconfig:$PKG_CONFIG_PATH" pkg-config --libs openssl)" + ./configure $qbtconf CXXFLAGS="$CXXFLAGS -std=c++14" + sed -i "" -e "s/^\(CC.*&&\).*$/\1 $CC/" src/Makefile # workaround for Qt & ccache: https://bugreports.qt.io/browse/QTBUG-31034 sed -i "" -e "s/^\(CXX.*&&\).*$/\1 $CXX/" src/Makefile sed -i "" -e 's/^\(CXXFLAGS.*\)$/\1 -Wno-unused-local-typedefs -Wno-inconsistent-missing-override/' src/Makefile diff --git a/CMakeLists.txt b/CMakeLists.txt index fc3fc7f27..012605d47 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,6 +32,7 @@ include(FeatureSummary) # version requirements set(requiredBoostVersion 1.35) set(requiredQtVersion 5.5.1) +set(requiredOpensslVersion 1.0) if(WIN32) include(winconf) diff --git a/configure b/configure index 0cc216f94..47533ffb9 100755 --- a/configure +++ b/configure @@ -601,6 +601,8 @@ EXPAND_BINDIR EXPAND_PREFIX zlib_LIBS zlib_CFLAGS +openssl_LIBS +openssl_CFLAGS libtorrent_LIBS libtorrent_CFLAGS BOOST_SYSTEM_LIB @@ -745,6 +747,8 @@ Qt5Svg_CFLAGS Qt5Svg_LIBS libtorrent_CFLAGS libtorrent_LIBS +openssl_CFLAGS +openssl_LIBS zlib_CFLAGS zlib_LIBS' @@ -1433,6 +1437,10 @@ Some influential environment variables: C compiler flags for libtorrent, overriding pkg-config libtorrent_LIBS linker flags for libtorrent, overriding pkg-config + openssl_CFLAGS + C compiler flags for openssl, overriding pkg-config + openssl_LIBS + linker flags for openssl, overriding pkg-config zlib_CFLAGS C compiler flags for zlib, overriding pkg-config zlib_LIBS linker flags for zlib, overriding pkg-config @@ -5391,6 +5399,99 @@ $as_echo "yes" >&6; } fi +pkg_failed=no +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for openssl" >&5 +$as_echo_n "checking for openssl... " >&6; } + +if test -n "$openssl_CFLAGS"; then + pkg_cv_openssl_CFLAGS="$openssl_CFLAGS" + elif test -n "$PKG_CONFIG"; then + if test -n "$PKG_CONFIG" && \ + { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"openssl >= 1.0\""; } >&5 + ($PKG_CONFIG --exists --print-errors "openssl >= 1.0") 2>&5 + ac_status=$? + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 + test $ac_status = 0; }; then + pkg_cv_openssl_CFLAGS=`$PKG_CONFIG --cflags "openssl >= 1.0" 2>/dev/null` + test "x$?" != "x0" && pkg_failed=yes +else + pkg_failed=yes +fi + else + pkg_failed=untried +fi +if test -n "$openssl_LIBS"; then + pkg_cv_openssl_LIBS="$openssl_LIBS" + elif test -n "$PKG_CONFIG"; then + if test -n "$PKG_CONFIG" && \ + { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"openssl >= 1.0\""; } >&5 + ($PKG_CONFIG --exists --print-errors "openssl >= 1.0") 2>&5 + ac_status=$? + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 + test $ac_status = 0; }; then + pkg_cv_openssl_LIBS=`$PKG_CONFIG --libs "openssl >= 1.0" 2>/dev/null` + test "x$?" != "x0" && pkg_failed=yes +else + pkg_failed=yes +fi + else + pkg_failed=untried +fi + + + +if test $pkg_failed = yes; then + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } + +if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then + _pkg_short_errors_supported=yes +else + _pkg_short_errors_supported=no +fi + if test $_pkg_short_errors_supported = yes; then + openssl_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "openssl >= 1.0" 2>&1` + else + openssl_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "openssl >= 1.0" 2>&1` + fi + # Put the nasty error message in config.log where it belongs + echo "$openssl_PKG_ERRORS" >&5 + + as_fn_error $? "Package requirements (openssl >= 1.0) were not met: + +$openssl_PKG_ERRORS + +Consider adjusting the PKG_CONFIG_PATH environment variable if you +installed software in a non-standard prefix. + +Alternatively, you may set the environment variables openssl_CFLAGS +and openssl_LIBS to avoid the need to call pkg-config. +See the pkg-config man page for more details." "$LINENO" 5 +elif test $pkg_failed = untried; then + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } + { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5 +$as_echo "$as_me: error: in \`$ac_pwd':" >&2;} +as_fn_error $? "The pkg-config script could not be found or is too old. Make sure it +is in your PATH or set the PKG_CONFIG environment variable to the full +path to pkg-config. + +Alternatively, you may set the environment variables openssl_CFLAGS +and openssl_LIBS to avoid the need to call pkg-config. +See the pkg-config man page for more details. + +To get pkg-config, see . +See \`config.log' for more details" "$LINENO" 5; } +else + openssl_CFLAGS=$pkg_cv_openssl_CFLAGS + openssl_LIBS=$pkg_cv_openssl_LIBS + { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +$as_echo "yes" >&6; } + CXXFLAGS="$openssl_CFLAGS $CXXFLAGS" + LIBS="$openssl_LIBS $LIBS" +fi + + pkg_failed=no { $as_echo "$as_me:${as_lineno-$LINENO}: checking for zlib" >&5 $as_echo_n "checking for zlib... " >&6; } diff --git a/configure.ac b/configure.ac index 6fb0ec932..751403ba8 100644 --- a/configure.ac +++ b/configure.ac @@ -195,6 +195,11 @@ PKG_CHECK_MODULES(libtorrent, [CXXFLAGS="$libtorrent_CFLAGS $CXXFLAGS" LIBS="$libtorrent_LIBS $LIBS"]) +PKG_CHECK_MODULES(openssl, + [openssl >= 1.0], + [CXXFLAGS="$openssl_CFLAGS $CXXFLAGS" + LIBS="$openssl_LIBS $LIBS"]) + PKG_CHECK_MODULES(zlib, [zlib >= 1.2.5.2], [CXXFLAGS="$zlib_CFLAGS $CXXFLAGS" @@ -207,7 +212,7 @@ AC_COMPILE_IFELSE([DETECT_CPP11_PROGRAM()], QBT_CXX11_FOUND="yes"], [AC_MSG_RESULT([no]) QBT_CXX11_FOUND="no"]) - + # In case of no, check if the compiler can support at least C++11 # and if yes, enable it leaving a warning to the user AS_IF([test "x$QBT_CXX11_FOUND" = "xno"], diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 364d9fc59..eb746d6c5 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -16,6 +16,7 @@ include(QbtTargetSources) find_package(Boost ${requiredBoostVersion} REQUIRED) find_package(LibtorrentRasterbar REQUIRED) +find_package(OpenSSL ${requiredOpensslVersion} REQUIRED) if (Boost_VERSION VERSION_LESS 106000) add_definitions(-DBOOST_NO_CXX11_RVALUE_REFERENCES) From 05d6a294165c03dab2ed8ec94cb501cd61c4ec34 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 21 Nov 2018 15:15:51 +0800 Subject: [PATCH 2/4] 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()}; + const Preferences *pref = Preferences::instance(); - QCryptographicHash md5(QCryptographicHash::Md5); - md5.addData(passwordFromWeb.toLocal8Bit()); - const QString passwordFromWebHashed = md5.result().toHex(); + 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); - const bool equalUser = Utils::String::slowEquals(usernameFromWeb.toUtf8(), username.toUtf8()); - const bool equalPass = Utils::String::slowEquals(passwordFromWebHashed.toUtf8(), password.toUtf8()); - - 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')); From 2c8890bd0693f23b7a96df7595cfe40152b4e372 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 21 Nov 2018 21:40:31 +0800 Subject: [PATCH 3/4] Apply PBKDF2 to GUI lock --- src/base/preferences.cpp | 17 +++------- src/base/preferences.h | 5 ++- src/gui/mainwindow.cpp | 73 ++++++++++++++++++---------------------- src/gui/mainwindow.h | 2 +- 4 files changed, 40 insertions(+), 57 deletions(-) diff --git a/src/base/preferences.cpp b/src/base/preferences.cpp index 1e86caaca..2f6c8f946 100644 --- a/src/base/preferences.cpp +++ b/src/base/preferences.cpp @@ -29,7 +29,6 @@ #include "preferences.h" -#include #include #include #include @@ -725,22 +724,14 @@ void Preferences::setDynDNSPassword(const QString &password) } // Advanced settings -void Preferences::clearUILockPassword() +QByteArray Preferences::getUILockPassword() const { - setValue("Locking/password", QString()); + return value("Locking/password_PBKDF2").toByteArray(); } -QString Preferences::getUILockPasswordMD5() const +void Preferences::setUILockPassword(const QByteArray &password) { - return value("Locking/password").toString(); -} - -void Preferences::setUILockPassword(const QString &clearPassword) -{ - QCryptographicHash md5(QCryptographicHash::Md5); - md5.addData(clearPassword.toLocal8Bit()); - QString md5Password = md5.result().toHex(); - setValue("Locking/password", md5Password); + setValue("Locking/password_PBKDF2", password); } bool Preferences::isUILocked() const diff --git a/src/base/preferences.h b/src/base/preferences.h index f137aa24c..988917e2b 100644 --- a/src/base/preferences.h +++ b/src/base/preferences.h @@ -229,9 +229,8 @@ public: void setDynDNSPassword(const QString &password); // Advanced settings - void setUILockPassword(const QString &clearPassword); - void clearUILockPassword(); - QString getUILockPasswordMD5() const; + QByteArray getUILockPassword() const; + void setUILockPassword(const QByteArray &password); bool isUILocked() const; void setUILocked(bool locked); bool isAutoRunEnabled() const; diff --git a/src/gui/mainwindow.cpp b/src/gui/mainwindow.cpp index d91e3f29b..e3b4d61d9 100644 --- a/src/gui/mainwindow.cpp +++ b/src/gui/mainwindow.cpp @@ -30,7 +30,6 @@ #include #include -#include #include #include #include @@ -69,6 +68,7 @@ #include "base/utils/foreignapps.h" #include "base/utils/fs.h" #include "base/utils/misc.h" +#include "base/utils/password.h" #include "aboutdialog.h" #include "addnewtorrentdialog.h" #include "application.h" @@ -631,45 +631,41 @@ void MainWindow::toolbarFollowSystem() Preferences::instance()->setToolbarTextPosition(Qt::ToolButtonFollowStyle); } -void MainWindow::defineUILockPassword() +bool MainWindow::defineUILockPassword() { - QString oldPassMd5 = Preferences::instance()->getUILockPasswordMD5(); - if (oldPassMd5.isNull()) - oldPassMd5 = ""; - bool ok = false; - QString newClearPassword = AutoExpandableDialog::getText(this, tr("UI lock password"), tr("Please type the UI lock password:"), QLineEdit::Password, oldPassMd5, &ok); - if (ok) { - newClearPassword = newClearPassword.trimmed(); - if (newClearPassword.size() < 3) { - QMessageBox::warning(this, tr("Invalid password"), tr("The password should contain at least 3 characters")); - } - else { - if (newClearPassword != oldPassMd5) - Preferences::instance()->setUILockPassword(newClearPassword); - QMessageBox::information(this, tr("Password update"), tr("The UI lock password has been successfully updated")); - } + const QString newPassword = AutoExpandableDialog::getText(this, tr("UI lock password") + , tr("Please type the UI lock password:"), QLineEdit::Password, {}, &ok); + if (!ok) + return false; + + if (newPassword.size() < 3) { + QMessageBox::warning(this, tr("Invalid password"), tr("The password should contain at least 3 characters")); + return false; } + + Preferences::instance()->setUILockPassword(Utils::Password::PBKDF2::generate(newPassword)); + return true; } void MainWindow::clearUILockPassword() { - QMessageBox::StandardButton answer = QMessageBox::question(this, tr("Clear the password"), tr("Are you sure you want to clear the password?"), QMessageBox::Yes | QMessageBox::No, QMessageBox::No); + const QMessageBox::StandardButton answer = QMessageBox::question(this, tr("Clear the password") + , tr("Are you sure you want to clear the password?"), (QMessageBox::Yes | QMessageBox::No), QMessageBox::No); if (answer == QMessageBox::Yes) - Preferences::instance()->clearUILockPassword(); + Preferences::instance()->setUILockPassword({}); } void MainWindow::on_actionLock_triggered() { Preferences *const pref = Preferences::instance(); + // Check if there is a password - if (pref->getUILockPasswordMD5().isEmpty()) { - // Ask for a password - bool ok = false; - QString clearPassword = AutoExpandableDialog::getText(this, tr("UI lock password"), tr("Please type the UI lock password:"), QLineEdit::Password, "", &ok); - if (!ok) return; - pref->setUILockPassword(clearPassword); + if (pref->getUILockPassword().isEmpty()) { + if (!defineUILockPassword()) + return; } + // Lock the interface m_uiLocked = true; pref->setUILocked(true); @@ -1049,27 +1045,24 @@ bool MainWindow::unlockUI() { if (m_unlockDlgShowing) return false; - else - m_unlockDlgShowing = true; bool ok = false; - QString clearPassword = AutoExpandableDialog::getText(this, tr("UI lock password"), tr("Please type the UI lock password:"), QLineEdit::Password, "", &ok); - m_unlockDlgShowing = false; + const QString password = AutoExpandableDialog::getText(this, tr("UI lock password") + , tr("Please type the UI lock password:"), QLineEdit::Password, {}, &ok); if (!ok) return false; Preferences *const pref = Preferences::instance(); - QString realPassMd5 = pref->getUILockPasswordMD5(); - QCryptographicHash md5(QCryptographicHash::Md5); - md5.addData(clearPassword.toLocal8Bit()); - QString passwordMd5 = md5.result().toHex(); - if (realPassMd5 == passwordMd5) { - m_uiLocked = false; - pref->setUILocked(false); - m_trayIconMenu->setEnabled(true); - return true; + + const QByteArray secret = pref->getUILockPassword(); + if (!Utils::Password::PBKDF2::verify(secret, password)) { + QMessageBox::warning(this, tr("Invalid password"), tr("The password is invalid")); + return false; } - QMessageBox::warning(this, tr("Invalid password"), tr("The password is invalid")); - return false; + + m_uiLocked = false; + pref->setUILocked(false); + m_trayIconMenu->setEnabled(true); + return true; } void MainWindow::notifyOfUpdate(QString) diff --git a/src/gui/mainwindow.h b/src/gui/mainwindow.h index 3d80066b7..c6d5a80e5 100644 --- a/src/gui/mainwindow.h +++ b/src/gui/mainwindow.h @@ -109,7 +109,7 @@ private slots: void fullDiskError(BitTorrent::TorrentHandle *const torrent, QString msg) const; void handleDownloadFromUrlFailure(QString, QString) const; void tabChanged(int newTab); - void defineUILockPassword(); + bool defineUILockPassword(); void clearUILockPassword(); bool unlockUI(); void notifyOfUpdate(QString); From 593052dd937e7373d4b7775f8abf032af91415fc Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 27 Nov 2018 01:44:47 +0800 Subject: [PATCH 4/4] Revise startup message in nox version Only print the WebUI username when password is still the default. --- src/app/application.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/application.cpp b/src/app/application.cpp index 38470b22d..cf5086373 100644 --- a/src/app/application.cpp +++ b/src/app/application.cpp @@ -522,12 +522,12 @@ int Application::exec(const QStringList ¶ms) // Display some information to the user const QString mesg = QString("\n******** %1 ********\n").arg(tr("Information")) + tr("To control qBittorrent, access the Web UI at %1") - .arg(QString("http://localhost:") + QString::number(pref->getWebUiPort())) + '\n' - + tr("The Web UI administrator user name is: %1").arg(pref->getWebUiUsername()) + '\n'; + .arg(QString("http://localhost:") + QString::number(pref->getWebUiPort())) + '\n'; printf("%s", qUtf8Printable(mesg)); 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' + const QString warning = tr("The Web UI administrator username is: %1").arg(pref->getWebUiUsername()) + '\n' + + 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)); }