From f8bffa2ad5675acec237dd845d00e086e4f4d888 Mon Sep 17 00:00:00 2001 From: Shane Synan Date: Fri, 24 Jul 2020 21:49:21 -0400 Subject: [PATCH] client: Group SASL EXTERNAL under SASL, test cap Move the "SASL EXTERNAL" notice into the "Use SASL Authentication" group box. This makes it more clear that "SASL EXTERNAL" is only used when SASL is enabled. Change the SASL IRCv3 capability check to look for SASL PLAIN for username/password authentication, and SASL EXTERNAL for SSL certificate authentication. Update the Details... dialog box info to match. This should hopefully address some confusion about when SASL is actually in use, versus things like NickServ certificate fingerprint sign-in. Change the wording for SASL EXTERNAL from "Supported by network" to "May be supported by network", as SASL v3.1 does not provide in the capability negotiation message which SASL variants are supported. SASL EXTERNAL seems less common than SASL PLAIN. --- .../settingspages/networkssettingspage.cpp | 98 ++++++++++++++----- src/qtui/settingspages/networkssettingspage.h | 15 ++- .../settingspages/networkssettingspage.ui | 84 ++++++++-------- 3 files changed, 127 insertions(+), 70 deletions(-) diff --git a/src/qtui/settingspages/networkssettingspage.cpp b/src/qtui/settingspages/networkssettingspage.cpp index d4fdca1f..de84f373 100644 --- a/src/qtui/settingspages/networkssettingspage.cpp +++ b/src/qtui/settingspages/networkssettingspage.cpp @@ -372,11 +372,15 @@ void NetworksSettingsPage::setNetworkCapStates(NetworkId id) if (net->connectionState() != Network::Disconnected) { // Network exists and is connected, check available capabilities... // [SASL] - if (net->saslMaybeSupports(IrcCap::SaslMech::PLAIN)) { - setSASLStatus(CapSupportStatus::MaybeSupported); + // Quassel switches between SASL PLAIN and SASL EXTERNAL based on the existence of an + // SSL certificate on the identity - check EXTERNAL if CertID exists, PLAIN if not + bool usingSASLExternal = displayedNetworkHasCertId(); + if ((usingSASLExternal && net->saslMaybeSupports(IrcCap::SaslMech::EXTERNAL)) + || (!usingSASLExternal && net->saslMaybeSupports(IrcCap::SaslMech::PLAIN))) { + setCapSASLStatus(CapSupportStatus::MaybeSupported, usingSASLExternal); } else { - setSASLStatus(CapSupportStatus::MaybeUnsupported); + setCapSASLStatus(CapSupportStatus::MaybeUnsupported, usingSASLExternal); } // Add additional capability-dependent interface updates here @@ -384,7 +388,7 @@ void NetworksSettingsPage::setNetworkCapStates(NetworkId id) else { // Network is disconnected // [SASL] - setSASLStatus(CapSupportStatus::Disconnected); + setCapSASLStatus(CapSupportStatus::Disconnected); // Add additional capability-dependent interface updates here } @@ -393,7 +397,7 @@ void NetworksSettingsPage::setNetworkCapStates(NetworkId id) // Capability negotiation is not supported and/or network doesn't exist. // Don't assume anything and reset all capability-dependent interface elements to neutral. // [SASL] - setSASLStatus(CapSupportStatus::Unknown); + setCapSASLStatus(CapSupportStatus::Unknown); // Add additional capability-dependent interface updates here } @@ -730,11 +734,12 @@ void NetworksSettingsPage::clientNetworkCapsUpdated() } } -void NetworksSettingsPage::setSASLStatus(const CapSupportStatus saslStatus) +void NetworksSettingsPage::setCapSASLStatus(const CapSupportStatus saslStatus, bool usingSASLExternal) { - if (_saslStatusSelected != saslStatus) { + if (_capSaslStatusSelected != saslStatus || _capSaslStatusUsingExternal != usingSASLExternal) { // Update the cached copy of SASL status used with the Details dialog - _saslStatusSelected = saslStatus; + _capSaslStatusSelected = saslStatus; + _capSaslStatusUsingExternal = usingSASLExternal; // Update the user interface switch (saslStatus) { @@ -750,14 +755,23 @@ void NetworksSettingsPage::setSASLStatus(const CapSupportStatus saslStatus) ui.saslStatusIcon->setPixmap(questionIcon.pixmap(16)); break; case CapSupportStatus::MaybeUnsupported: - // The network doesn't advertise support for SASL PLAIN. Here be dragons. + // The network doesn't advertise support for SASL PLAIN/EXTERNAL. Here be dragons. ui.saslStatusLabel->setText(QString("%1").arg(tr("Not currently supported by network"))); ui.saslStatusIcon->setPixmap(unavailableIcon.pixmap(16)); break; case CapSupportStatus::MaybeSupported: - // The network advertises support for SASL PLAIN. Encourage using it! + // The network advertises support for SASL PLAIN/EXTERNAL. Encourage using it! // Unfortunately we don't know for sure if it's desired or functional. - ui.saslStatusLabel->setText(QString("%1").arg(tr("Supported by network"))); + if (usingSASLExternal) { + // SASL EXTERNAL is used + // With SASL v3.1, it's not possible to reliably tell if SASL EXTERNAL is supported, + // or just SASL PLAIN. Use less assertive phrasing. + ui.saslStatusLabel->setText(QString("%1").arg(tr("May be supported by network"))); + } + else { + // SASL PLAIN is used + ui.saslStatusLabel->setText(QString("%1").arg(tr("Supported by network"))); + } ui.saslStatusIcon->setPixmap(successIcon.pixmap(16)); break; } @@ -766,25 +780,29 @@ void NetworksSettingsPage::setSASLStatus(const CapSupportStatus saslStatus) void NetworksSettingsPage::sslUpdated() { - if (_cid && !_cid->sslKey().isNull()) { - ui.saslContents->setDisabled(true); + if (displayedNetworkHasCertId()) { + ui.saslPlainContents->setDisabled(true); ui.saslExtInfo->setHidden(false); } else { - ui.saslContents->setDisabled(false); + ui.saslPlainContents->setDisabled(false); // Directly re-enabling causes the widgets to ignore the parent "Use SASL Authentication" // state to indicate whether or not it's disabled. To workaround this, keep track of // whether or not "Use SASL Authentication" is enabled, then quickly uncheck/recheck the // group box. if (!ui.sasl->isChecked()) { - // SASL is not enabled, uncheck/recheck the group box to re-disable saslContents. - // Leaving saslContents disabled doesn't work as that prevents it from re-enabling if + // SASL is not enabled, uncheck/recheck the group box to re-disable saslPlainContents. + // Leaving saslPlainContents disabled doesn't work as that prevents it from re-enabling if // sasl is later checked. ui.sasl->setChecked(true); ui.sasl->setChecked(false); } ui.saslExtInfo->setHidden(true); } + // Update whether SASL PLAIN or SASL EXTERNAL is used to detect SASL status + if (currentId != 0) { + setNetworkCapStates(currentId); + } } /*** Network list ***/ @@ -968,7 +986,7 @@ void NetworksSettingsPage::on_saslStatusDetails_clicked() bool useWarningIcon = false; // Determine which explanation to show - switch (_saslStatusSelected) { + switch (_capSaslStatusSelected) { case CapSupportStatus::Unknown: saslStatusHeader = tr("Could not check if SASL supported by network"); saslStatusExplanation = tr("Quassel could not check if \"%1\" supports SASL. This may " @@ -984,17 +1002,41 @@ void NetworksSettingsPage::on_saslStatusDetails_clicked() .arg(netName); break; case CapSupportStatus::MaybeUnsupported: - saslStatusHeader = tr("SASL not currently supported by network"); - saslStatusExplanation = tr("The network \"%1\" does not currently support SASL. " - "However, support might be added later on.") - .arg(netName); + if (displayedNetworkHasCertId()) { + // SASL EXTERNAL is used + saslStatusHeader = tr("SASL EXTERNAL not currently supported by network"); + saslStatusExplanation = tr("The network \"%1\" does not currently support SASL " + "EXTERNAL for SSL certificate authentication. However, " + "support might be added later on.") + .arg(netName); + } + else { + // SASL PLAIN is used + saslStatusHeader = tr("SASL not currently supported by network"); + saslStatusExplanation = tr("The network \"%1\" does not currently support SASL. " + "However, support might be added later on.") + .arg(netName); + } useWarningIcon = true; break; case CapSupportStatus::MaybeSupported: - saslStatusHeader = tr("SASL supported by network"); - saslStatusExplanation = tr("The network \"%1\" supports SASL. In most cases, you " - "should use SASL instead of NickServ identification.") - .arg(netName); + if (displayedNetworkHasCertId()) { + // SASL EXTERNAL is used + // With SASL v3.1, it's not possible to reliably tell if SASL EXTERNAL is supported, + // or just SASL PLAIN. Caution about this in the details dialog. + saslStatusHeader = tr("SASL EXTERNAL may be supported by network"); + saslStatusExplanation = tr("The network \"%1\" may support SASL EXTERNAL for SSL " + "certificate authentication. In most cases, you should " + "use SASL instead of NickServ identification.") + .arg(netName); + } + else { + // SASL PLAIN is used + saslStatusHeader = tr("SASL supported by network"); + saslStatusExplanation = tr("The network \"%1\" supports SASL. In most cases, you " + "should use SASL instead of NickServ identification.") + .arg(netName); + } break; } @@ -1095,6 +1137,12 @@ IdentityId NetworksSettingsPage::defaultIdentity() const return defaultId; } +bool NetworksSettingsPage::displayedNetworkHasCertId() const +{ + // Check if the CertIdentity exists and that it has a non-null SSL key set + return (_cid && !_cid->sslKey().isNull()); +} + /************************************************************************** * NetworkAddDlg *************************************************************************/ diff --git a/src/qtui/settingspages/networkssettingspage.h b/src/qtui/settingspages/networkssettingspage.h index 73bee52c..d6614d91 100644 --- a/src/qtui/settingspages/networkssettingspage.h +++ b/src/qtui/settingspages/networkssettingspage.h @@ -150,7 +150,8 @@ private: // Status icons QIcon infoIcon, successIcon, unavailableIcon, questionIcon; - CapSupportStatus _saslStatusSelected; /// Status of SASL support for currently-selected network + CapSupportStatus _capSaslStatusSelected; ///< Status of SASL support for selected network + bool _capSaslStatusUsingExternal{false}; ///< Whether SASL support status is for SASL EXTERNAL void reset(); bool testHasChanged(); @@ -160,12 +161,20 @@ private: void saveToNetworkInfo(NetworkInfo&); IdentityId defaultIdentity() const; + /** + * Get whether or not the displayed network's identity has SSL certs associated with it + * + * @return True if the currently displayed network has SSL certs set, otherwise false + */ + bool displayedNetworkHasCertId() const; + /** * Update the SASL settings interface according to the given SASL state * - * @param[in] saslStatus Current status of SASL support. + * @param saslStatus Current status of SASL support. + * @param usingSASLExternal If true, SASL support status is for SASL EXTERNAL, else SASL PLAIN */ - void setSASLStatus(const CapSupportStatus saslStatus); + void setCapSASLStatus(const CapSupportStatus saslStatus, bool usingSASLExternal = false); }; class NetworkAddDlg : public QDialog diff --git a/src/qtui/settingspages/networkssettingspage.ui b/src/qtui/settingspages/networkssettingspage.ui index 03a8321c..1f7983a9 100644 --- a/src/qtui/settingspages/networkssettingspage.ui +++ b/src/qtui/settingspages/networkssettingspage.ui @@ -682,7 +682,7 @@ Note that Quassel IRC automatically rejoins channels, so /join will rarely be ne - + QFrame::NoFrame @@ -755,53 +755,53 @@ Note that Quassel IRC automatically rejoins channels, so /join will rarely be ne - - - - - - [icon] - - - - - - - - 0 - 0 - - - - Could not detect if supported by server - - - - - - - Details... - - - - - + + + + <html><head/><body><p><span style=" font-weight:600;">Note:</span> because the identity has an ssl certificate set, SASL EXTERNAL will be used.</p></body></html> + + + true + + + + + + + + + [icon] + + + + + + + + 0 + 0 + + + + Could not detect if supported by server + + + + + + + Details... + + + + + - - - - <html><head/><body><p><span style=" font-weight:600;">Note:</span> because the identity has an ssl certificate set, SASL EXTERNAL will be used.</p></body></html> - - - true - - - -- 2.20.1