From cc66903678b797cc352d4fcdf7c6a0e3a7388a1e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 2 Nov 2019 19:23:04 +0100 Subject: [PATCH 1/6] [users] Allow an explicit check for non-emptiness of passwords - move the explicit checking for non-empty into a specific (normal) password check - leave only the-two-fields-are-equal outside of the password- requirements framework - having non-empty is the same as minLength 1, but gives a different error message --- src/modules/users/UsersPage.cpp | 17 +++++++++-------- src/modules/users/users.conf | 11 +++++++++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index c0965a7ed..92070d1da 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -407,14 +407,7 @@ UsersPage::validateHostnameText( const QString& textRef ) bool UsersPage::checkPasswordAcceptance( const QString& pw1, const QString& pw2, QLabel* badge, QLabel* message ) { - if ( pw1.isEmpty() && pw2.isEmpty() ) - { - // Not exactly labelOk() because we also don't want a checkmark OK - badge->clear(); - message->clear(); - return false; - } - else if ( pw1 != pw2 ) + if ( pw1 != pw2 ) { labelError( badge, message, tr( "Your passwords do not match!" ) ); return false; @@ -510,6 +503,14 @@ UsersPage::addPasswordCheck( const QString& key, const QVariant& value ) { add_check_maxLength( m_passwordChecks, value ); } + else if ( key == "nonempty" ) + { + if ( value.toBool() ) + { + m_passwordChecks.push_back( PasswordCheck( []() { return QCoreApplication::translate( "EMP", "Password is empty" ); }, + []( const QString& s ) { return ((cDebug() << "Checking pwd" << s << "for empty"), !s.isEmpty()); } ) ); + } + } #ifdef CHECK_PWQUALITY else if ( key == "libpwquality" ) { diff --git a/src/modules/users/users.conf b/src/modules/users/users.conf index cae9bef0d..00747195c 100644 --- a/src/modules/users/users.conf +++ b/src/modules/users/users.conf @@ -58,8 +58,14 @@ setRootPassword: true doReusePassword: true # These are optional password-requirements that a distro can enforce -# on the user. The values given in this sample file disable each check, -# as if the check was not listed at all. +# on the user. The values given in this sample file set only very weak +# validation settings. +# +# - nonempty rejects empty passwords +# - there are no length validations +# - libpwquality (if it is enabled at all) has no length of class +# restrictions, although it will still reject palindromes and +# dictionary words with these settings. # # Checks may be listed multiple times; each is checked separately, # and no effort is done to ensure that the checks are consistent @@ -84,6 +90,7 @@ doReusePassword: true # (That will show the box *Allow weak passwords* in the user- # interface, and check it by default). passwordRequirements: + nonempty: true minLength: -1 # Password at least this many characters maxLength: -1 # Password at most this many characters libpwquality: From b3e7c3f29490b69524a048ebd4849e6eb9d7f52e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 2 Nov 2019 19:26:40 +0100 Subject: [PATCH 2/6] [users] Run checks more often - check password warnings when the page is entered - re-check (and translate) on language change --- src/modules/users/UsersPage.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index 92070d1da..8a7bca74c 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -144,6 +144,11 @@ UsersPage::retranslate() "use this computer, you can create multiple " "accounts after installation." ) ); } + // Re-do password checks (with output messages) as well. + // .. the password-checking methods get their values from the text boxes, + // not from their parameters. + onPasswordTextChanged(QString()); + onRootPasswordTextChanged(QString()); } @@ -222,6 +227,8 @@ void UsersPage::onActivate() { ui->textBoxFullName->setFocus(); + onPasswordTextChanged(QString()); + onRootPasswordTextChanged(QString()); } From dec0cfb7d300158bf5ad0e35467882990ac5b648 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 8 Nov 2019 13:16:19 +0100 Subject: [PATCH 3/6] [users] Give password-checks a weight, to sort them later --- src/modules/users/CheckPWQuality.cpp | 23 +++++++++++------------ src/modules/users/CheckPWQuality.h | 20 +++++++++++++++----- src/modules/users/UsersPage.cpp | 14 ++++++++------ 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/modules/users/CheckPWQuality.cpp b/src/modules/users/CheckPWQuality.cpp index 3728c5a92..39c1f79de 100644 --- a/src/modules/users/CheckPWQuality.cpp +++ b/src/modules/users/CheckPWQuality.cpp @@ -31,19 +31,15 @@ #include PasswordCheck::PasswordCheck() - : m_message() + : m_weight( 0 ) + , m_message() , m_accept( []( const QString& ) { return true; } ) { } -PasswordCheck::PasswordCheck( const QString& m, AcceptFunc a ) - : m_message( [m]() { return m; } ) - , m_accept( a ) -{ -} - -PasswordCheck::PasswordCheck( MessageFunc m, AcceptFunc a ) - : m_message( m ) +PasswordCheck::PasswordCheck( MessageFunc m, AcceptFunc a, Weight weight ) + : m_weight( weight ) + , m_message( m ) , m_accept( a ) { } @@ -59,7 +55,8 @@ DEFINE_CHECK_FUNC( minLength ) { cDebug() << Logger::SubEntry << "minLength set to" << minLength; checks.push_back( PasswordCheck( []() { return QCoreApplication::translate( "PWQ", "Password is too short" ); }, - [minLength]( const QString& s ) { return s.length() >= minLength; } ) ); + [minLength]( const QString& s ) { return s.length() >= minLength; }, + PasswordCheck::Weight( 10 ) ) ); } } @@ -74,7 +71,8 @@ DEFINE_CHECK_FUNC( maxLength ) { cDebug() << Logger::SubEntry << "maxLength set to" << maxLength; checks.push_back( PasswordCheck( []() { return QCoreApplication::translate( "PWQ", "Password is too long" ); }, - [maxLength]( const QString& s ) { return s.length() <= maxLength; } ) ); + [maxLength]( const QString& s ) { return s.length() <= maxLength; }, + PasswordCheck::Weight( 10 ) ) ); } } @@ -363,7 +361,8 @@ DEFINE_CHECK_FUNC( libpwquality ) cDebug() << "Password strength" << r << "too low"; } return r >= settings->arbitrary_minimum_strength; - } ) ); + }, + PasswordCheck::Weight( 100 ) ) ); } } #endif diff --git a/src/modules/users/CheckPWQuality.h b/src/modules/users/CheckPWQuality.h index 046f31496..5d4b8108a 100644 --- a/src/modules/users/CheckPWQuality.h +++ b/src/modules/users/CheckPWQuality.h @@ -38,11 +38,18 @@ public: using AcceptFunc = std::function< bool( const QString& ) >; using MessageFunc = std::function< QString() >; - /** Generate a @p message if @p filter returns true */ - PasswordCheck( MessageFunc message, AcceptFunc filter ); - /** Yields @p message if @p filter returns true */ - PasswordCheck( const QString& message, AcceptFunc filter ); - /** Null check, always returns empty */ + using Weight = size_t; + + /** @brief Generate a @p message if @p filter returns true + * + * When @p filter returns true on the proposed password, the + * password is accepted (by this check). If false, then the + * @p message will be shown to the user. + * + * @p weight is used to order the checks (low-weight goes first). + */ + PasswordCheck( MessageFunc message, AcceptFunc filter, Weight weight = 1000 ); + /** @brief Null check, always accepts, no message */ PasswordCheck(); /** Applies this check to the given password string @p s @@ -52,7 +59,10 @@ public: */ QString filter( const QString& s ) const { return m_accept( s ) ? QString() : m_message(); } + Weight weight() const { return m_weight; } + private: + Weight m_weight; MessageFunc m_message; AcceptFunc m_accept; }; diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index 8a7bca74c..9a3043a34 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -147,8 +147,8 @@ UsersPage::retranslate() // Re-do password checks (with output messages) as well. // .. the password-checking methods get their values from the text boxes, // not from their parameters. - onPasswordTextChanged(QString()); - onRootPasswordTextChanged(QString()); + onPasswordTextChanged( QString() ); + onRootPasswordTextChanged( QString() ); } @@ -227,8 +227,8 @@ void UsersPage::onActivate() { ui->textBoxFullName->setFocus(); - onPasswordTextChanged(QString()); - onRootPasswordTextChanged(QString()); + onPasswordTextChanged( QString() ); + onRootPasswordTextChanged( QString() ); } @@ -514,8 +514,10 @@ UsersPage::addPasswordCheck( const QString& key, const QVariant& value ) { if ( value.toBool() ) { - m_passwordChecks.push_back( PasswordCheck( []() { return QCoreApplication::translate( "EMP", "Password is empty" ); }, - []( const QString& s ) { return ((cDebug() << "Checking pwd" << s << "for empty"), !s.isEmpty()); } ) ); + m_passwordChecks.push_back( PasswordCheck( + []() { return QCoreApplication::translate( "EMP", "Password is empty" ); }, + []( const QString& s ) { return ( ( cDebug() << "Checking pwd" << s << "for empty" ), !s.isEmpty() ); }, + PasswordCheck::Weight( 1 ) ) ); } } #ifdef CHECK_PWQUALITY From e11c9a049f448024372d47b0bee40d75a08e2294 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 8 Nov 2019 13:21:37 +0100 Subject: [PATCH 4/6] [users] Sort the password checks before applying them --- src/modules/users/CheckPWQuality.h | 1 + src/modules/users/UsersPage.cpp | 8 ++++++++ src/modules/users/UsersPage.h | 1 + 3 files changed, 10 insertions(+) diff --git a/src/modules/users/CheckPWQuality.h b/src/modules/users/CheckPWQuality.h index 5d4b8108a..1aeb34ba8 100644 --- a/src/modules/users/CheckPWQuality.h +++ b/src/modules/users/CheckPWQuality.h @@ -60,6 +60,7 @@ public: QString filter( const QString& s ) const { return m_accept( s ) ? QString() : m_message(); } Weight weight() const { return m_weight; } + bool operator<( const PasswordCheck& other ) const { return weight() < other.weight(); } private: Weight m_weight; diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index 9a3043a34..74bf67655 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -424,6 +424,12 @@ UsersPage::checkPasswordAcceptance( const QString& pw1, const QString& pw2, QLab bool failureIsFatal = ui->checkBoxValidatePassword->isChecked(); bool failureFound = false; + if ( m_passwordChecksChanged ) + { + std::sort( m_passwordChecks.begin(), m_passwordChecks.end() ); + m_passwordChecksChanged = false; + } + for ( auto pc : m_passwordChecks ) { QString s = pc.filter( pw1 ); @@ -502,6 +508,8 @@ UsersPage::setReusePasswordDefault( bool checked ) void UsersPage::addPasswordCheck( const QString& key, const QVariant& value ) { + m_passwordChecksChanged = true; + if ( key == "minLength" ) { add_check_minLength( m_passwordChecks, value ); diff --git a/src/modules/users/UsersPage.h b/src/modules/users/UsersPage.h index a2befbd26..c6bf87ecf 100644 --- a/src/modules/users/UsersPage.h +++ b/src/modules/users/UsersPage.h @@ -90,6 +90,7 @@ private: Ui::Page_UserSetup* ui; PasswordCheckList m_passwordChecks; + bool m_passwordChecksChanged = false; bool m_readyFullName; bool m_readyUsername; From 445d8501a7fe4b19f95759f25047de82f657873c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 8 Nov 2019 13:22:37 +0100 Subject: [PATCH 5/6] [users] Different disambiguation for pwd-empty check - all the other checks use "PWQ" as a tag, so use that here too --- src/modules/users/UsersPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index 74bf67655..b8e2d057c 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -523,7 +523,7 @@ UsersPage::addPasswordCheck( const QString& key, const QVariant& value ) if ( value.toBool() ) { m_passwordChecks.push_back( PasswordCheck( - []() { return QCoreApplication::translate( "EMP", "Password is empty" ); }, + []() { return QCoreApplication::translate( "PWQ", "Password is empty" ); }, []( const QString& s ) { return ( ( cDebug() << "Checking pwd" << s << "for empty" ), !s.isEmpty() ); }, PasswordCheck::Weight( 1 ) ) ); } From 0d7e19d5e9cc5c6fa5542d39882530c02790de4a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 8 Nov 2019 13:23:24 +0100 Subject: [PATCH 6/6] [users] Do not log the password in plain text --- src/modules/users/UsersPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index b8e2d057c..05095da70 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -524,7 +524,7 @@ UsersPage::addPasswordCheck( const QString& key, const QVariant& value ) { m_passwordChecks.push_back( PasswordCheck( []() { return QCoreApplication::translate( "PWQ", "Password is empty" ); }, - []( const QString& s ) { return ( ( cDebug() << "Checking pwd" << s << "for empty" ), !s.isEmpty() ); }, + []( const QString& s ) { return !s.isEmpty(); }, PasswordCheck::Weight( 1 ) ) ); } }