diff --git a/docs/PROGRESS.md b/docs/PROGRESS.md index 156eae0..3b6c827 100644 --- a/docs/PROGRESS.md +++ b/docs/PROGRESS.md @@ -157,6 +157,24 @@ Delivered: gets its own prompt, with an empty password allowed through (unlike RDP's hard requirement) since some VNC servers are no-auth and there's no way to know that before the server's security-type negotiation +- Profiles can now leave the username blank for every protocol (issue + #21): SSH/RDP previously hard-required one at profile-save time; that + validation is gone, and `SessionTab` now asks for it at connect time + instead (reusing the existing password-prompt bar in unmasked mode), + same as it already does for a blank password. VNC's username is only + ever asked for if the server's negotiated auth method actually needs + one -- plain VNC Authentication and no-auth never do, only the two + Apple schemes (30/33) do -- which happens *mid-connection*, after the + backend has already picked a security type, not before connecting like + SSH/RDP. This needed a new async request/response pair on + `SessionBackend` (`usernameRequested()` / `provideUsername()`, + mirroring the existing SSH host-key-confirmation pattern): + `VncSessionBackend` pauses its state machine mid-parse (without + consuming the already-buffered response bytes) and emits the request, + resuming once `SessionTab` answers; cancelling fails the connection + cleanly rather than sending Apple auth a blank username. The value is + kept on the tab's in-memory profile copy for its lifetime, not written + back to the saved profile - Robustness fix: an unrecognized `FramebufferUpdate` rectangle encoding used to abort the connection generically; `kAnnouncedEncodings` is now the single source of truth for what `SetEncodings` announces and what diff --git a/src/profile_dialog.cpp b/src/profile_dialog.cpp index eeb8744..3de71ed 100644 --- a/src/profile_dialog.cpp +++ b/src/profile_dialog.cpp @@ -257,13 +257,6 @@ void ProfileDialog::accept() } const QString protocol = m_protocolInput->currentText(); - if ((protocol == QStringLiteral("SSH") || protocol == QStringLiteral("RDP")) - && m_usernameInput->text().trimmed().isEmpty()) { - QMessageBox::warning(this, - QStringLiteral("Validation Error"), - QStringLiteral("Username is required for %1 profiles.").arg(protocol)); - return; - } if (protocol == QStringLiteral("SSH") && m_authModeInput->currentText() == QStringLiteral("Private Key")) { @@ -310,12 +303,14 @@ void ProfileDialog::refreshAuthFields() if (isSsh) { m_usernameInput->setPlaceholderText(QStringLiteral("deploy")); - m_protocolHint->setText( - QStringLiteral("SSH: username is required. Choose Password or Private Key auth.")); + m_protocolHint->setText(QStringLiteral( + "SSH: you'll be asked for a username at connect time if left blank here. " + "Choose Password or Private Key auth.")); } else if (isRdp) { m_usernameInput->setPlaceholderText(QStringLiteral("Administrator")); - m_protocolHint->setText( - QStringLiteral("RDP: username and password are required. Domain is optional.")); + m_protocolHint->setText(QStringLiteral( + "RDP: you'll be asked for a username at connect time if left blank here. " + "Domain is optional.")); } else if (isVnc) { m_usernameInput->setPlaceholderText(QStringLiteral("optional")); m_protocolHint->setText(QStringLiteral( diff --git a/src/session_backend.h b/src/session_backend.h index 8db68be..a5c6b96 100644 --- a/src/session_backend.h +++ b/src/session_backend.h @@ -55,6 +55,16 @@ public slots: { Q_UNUSED(text); } + // Response to usernameRequested(), for backends that discover mid- + // connection (not before connectSession() is even called) that they + // need one -- currently only VNC's Apple authentication schemes, + // which only require a username for security types 30/33, not for + // plain VNC Authentication or no-auth. An empty username is treated + // the same as the prompt being cancelled. + virtual void provideUsername(const QString& username) + { + Q_UNUSED(username); + } virtual void sendKeyEvent(int key, quint32 nativeScanCode, const QString& text, @@ -93,6 +103,10 @@ signals: void connectionError(const QString& displayMessage, const QString& rawMessage); void outputReceived(const QString& text); void hostKeyConfirmationRequested(const QString& prompt); + // Mirrors hostKeyConfirmationRequested()'s request/response shape, for + // a backend that discovers mid-connection it needs a username it + // wasn't given -- see provideUsername(). + void usernameRequested(const QString& prompt); void frameUpdated(const QImage& frame); void remoteDesktopSizeChanged(int width, int height); void remoteClipboardTextChanged(const QString& text); diff --git a/src/session_tab.cpp b/src/session_tab.cpp index aac9131..a96ed2f 100644 --- a/src/session_tab.cpp +++ b/src/session_tab.cpp @@ -215,6 +215,11 @@ SessionTab::SessionTab(const Profile& profile, m_backend, &SessionBackend::setClipboardText, Qt::QueuedConnection); + connect(this, + &SessionTab::requestProvideUsername, + m_backend, + &SessionBackend::provideUsername, + Qt::QueuedConnection); connect(m_backend, &SessionBackend::stateChanged, @@ -241,6 +246,11 @@ SessionTab::SessionTab(const Profile& profile, this, &SessionTab::onBackendHostKeyConfirmationRequested, Qt::QueuedConnection); + connect(m_backend, + &SessionBackend::usernameRequested, + this, + &SessionTab::onBackendUsernameRequested, + Qt::QueuedConnection); connect(m_backend, &SessionBackend::frameUpdated, this, @@ -714,6 +724,16 @@ void SessionTab::onBackendHostKeyConfirmationRequested(const QString& prompt) emit requestHostKeyConfirmation(reply == QMessageBox::Yes); } +void SessionTab::onBackendUsernameRequested(const QString& prompt) +{ + showPasswordPrompt( + prompt.isEmpty() ? QStringLiteral("Username for %1:").arg(m_profile.host) : prompt, + [this](std::optional username) { + emit requestProvideUsername(username.value_or(QString()).trimmed()); + }, + false); +} + void SessionTab::onBackendRemoteClipboardTextChanged(const QString& text) { if (text == m_lastSyncedClipboardText) { @@ -983,6 +1003,26 @@ void SessionTab::requestConnectOptions( const bool isRdp = m_profile.protocol.compare(QStringLiteral("RDP"), Qt::CaseInsensitive) == 0; const bool isVnc = m_profile.protocol.compare(QStringLiteral("VNC"), Qt::CaseInsensitive) == 0; + // SSH and RDP always need a username; a profile is now allowed to + // leave it blank (see profile_dialog.cpp) and get asked here instead, + // the same way a blank password is already handled below. The value + // is kept on this in-memory m_profile copy for the rest of the tab's + // lifetime, not written back to the saved profile. + if ((isSsh || isRdp) && m_profile.username.trimmed().isEmpty()) { + showPasswordPrompt( + QStringLiteral("%1 username for %2:").arg(m_profile.protocol, m_profile.host), + [this, callback](std::optional username) { + if (!username.has_value() || username->trimmed().isEmpty()) { + callback(std::nullopt); + return; + } + m_profile.username = username->trimmed(); + requestConnectOptions(callback); + }, + false); + return; + } + if (isVnc) { // Unlike RDP, an empty password is allowed through: some VNC // servers (no-auth) don't need one at all, and there's no @@ -1102,7 +1142,8 @@ void SessionTab::requestConnectOptions( } void SessionTab::showPasswordPrompt(const QString& labelText, - std::function)> callback) + std::function)> callback, + bool maskInput) { if (m_passwordPromptCallback) { const auto previousCallback = m_passwordPromptCallback; @@ -1113,6 +1154,7 @@ void SessionTab::showPasswordPrompt(const QString& labelText, m_passwordPromptCallback = std::move(callback); m_passwordPromptLabel->setText(labelText); m_passwordPromptInput->clear(); + m_passwordPromptInput->setEchoMode(maskInput ? QLineEdit::Password : QLineEdit::Normal); m_passwordPromptBar->setVisible(true); m_passwordPromptInput->setFocus(); } diff --git a/src/session_tab.h b/src/session_tab.h index d83090a..db4fa35 100644 --- a/src/session_tab.h +++ b/src/session_tab.h @@ -91,6 +91,7 @@ signals: void requestMouseButtonEvent(int x, int y, int button, bool pressed); void requestMouseWheelEvent(int x, int y, int deltaX, int deltaY); void requestSetClipboardText(const QString& text); + void requestProvideUsername(const QString& username); private slots: void onBackendStateChanged(SessionState state, const QString& message); @@ -98,6 +99,7 @@ private slots: void onBackendConnectionError(const QString& displayMessage, const QString& rawMessage); void onBackendOutputReceived(const QString& text); void onBackendHostKeyConfirmationRequested(const QString& prompt); + void onBackendUsernameRequested(const QString& prompt); void onBackendRemoteClipboardTextChanged(const QString& text); void onSystemClipboardChanged(); @@ -149,7 +151,8 @@ private: void setupUi(); void requestConnectOptions(std::function)> callback); void showPasswordPrompt(const QString& labelText, - std::function)> callback); + std::function)> callback, + bool maskInput = true); void hidePasswordPrompt(); bool validateProfileForConnect(); void appendEvent(const QString& message); diff --git a/src/vnc_session_backend.cpp b/src/vnc_session_backend.cpp index b01a74d..63090af 100644 --- a/src/vnc_session_backend.cpp +++ b/src/vnc_session_backend.cpp @@ -196,7 +196,8 @@ VncSessionBackend::VncSessionBackend(const Profile& profile, QObject* parent) m_tightCompressionMode(0), m_tightFilterId(0), m_tightLengthByteIndex(0), - m_appleAuthKeyLength(0) + m_appleAuthKeyLength(0), + m_waitingForUsername(false) { std::memset(m_zrleInflateStream, 0, sizeof(z_stream_s)); for (z_stream_s* stream : m_tightInflateStreams) { @@ -531,6 +532,8 @@ void VncSessionBackend::resetProtocolState() } m_appleAuthGenerator.clear(); m_appleAuthKeyLength = 0; + m_waitingForUsername = false; + m_promptedUsername.clear(); } bool VncSessionBackend::haveBytes(int count) const @@ -671,6 +674,47 @@ void VncSessionBackend::sendAppleRsaHostKeyRequest() m_socket->write(msg); } +QString VncSessionBackend::effectiveUsername() const +{ + const QString profileUsername = profile().username.trimmed(); + return profileUsername.isEmpty() ? m_promptedUsername : profileUsername; +} + +bool VncSessionBackend::ensureUsernameAvailable() +{ + if (!effectiveUsername().isEmpty()) { + return true; + } + if (!m_waitingForUsername) { + m_waitingForUsername = true; + emit usernameRequested( + QStringLiteral("A username is required for %1's authentication method:") + .arg(profile().host)); + } + return false; +} + +void VncSessionBackend::provideUsername(const QString& username) +{ + if (!m_waitingForUsername) { + return; + } + m_waitingForUsername = false; + if (m_state != SessionState::Connecting) { + // The connection already failed or was torn down while the + // prompt was pending -- nothing left to resume. + return; + } + m_promptedUsername = username.trimmed(); + if (m_promptedUsername.isEmpty()) { + failConnection( + QStringLiteral("A username is required for this VNC server's authentication method."), + QStringLiteral("Username prompt was cancelled or left empty")); + return; + } + processReceiveBuffer(); +} + void VncSessionBackend::finishHandshakeIntoRunningState() { emit remoteDesktopSizeChanged(m_framebuffer.width(), m_framebuffer.height()); @@ -963,13 +1007,20 @@ void VncSessionBackend::processReceiveBuffer() if (!haveBytes(static_cast(m_pendingLength))) { return; } + // Checked before consuming any bytes: if a username is needed + // and not yet available, pause here (leaving m_recvBuffer + // untouched) until provideUsername() resumes us and this same + // case re-parses identically. + if (!ensureUsernameAvailable()) { + return; + } const int keyLength = static_cast(m_appleAuthKeyLength); const QByteArray prime = m_recvBuffer.left(keyLength); const QByteArray serverPublicKey = m_recvBuffer.mid(keyLength, keyLength); m_recvBuffer.remove(0, static_cast(m_pendingLength)); const VncAppleDhAuth::Response response = VncAppleDhAuth::computeResponse( - m_appleAuthGenerator, prime, serverPublicKey, profile().username, + m_appleAuthGenerator, prime, serverPublicKey, effectiveUsername(), m_activeOptions.password); if (response.clientPublicKey.isEmpty()) { failConnection( @@ -1020,11 +1071,15 @@ void VncSessionBackend::processReceiveBuffer() if (!haveBytes(totalBytes)) { return; } + // See the matching comment in WaitingAppleAuthPrimeAndServerKey. + if (!ensureUsernameAvailable()) { + return; + } const QByteArray hostKeyDer = m_recvBuffer.left(static_cast(m_pendingLength)); m_recvBuffer.remove(0, totalBytes); const VncAppleRsaAuth::Response response = VncAppleRsaAuth::computeResponse( - hostKeyDer, profile().username, m_activeOptions.password); + hostKeyDer, effectiveUsername(), m_activeOptions.password); if (response.encryptedCredentials.isEmpty() || response.encryptedAesKey.isEmpty()) { failConnection( QStringLiteral( diff --git a/src/vnc_session_backend.h b/src/vnc_session_backend.h index 3d15a8c..5f7cdb5 100644 --- a/src/vnc_session_backend.h +++ b/src/vnc_session_backend.h @@ -84,6 +84,7 @@ public slots: void sendMouseButtonEvent(int x, int y, int button, bool pressed) override; void sendMouseWheelEvent(int x, int y, int deltaX, int deltaY) override; void setClipboardText(const QString& text) override; + void provideUsername(const QString& username) override; private slots: void onSocketConnected(); @@ -209,6 +210,16 @@ private: QByteArray m_appleAuthGenerator; quint32 m_appleAuthKeyLength; + // Apple auth (types 30 and 33) is the only case where VNC ever needs a + // username -- plain VNC Authentication and no-auth never do, so this + // isn't asked for upfront. If profile().username is empty when an + // Apple-auth response is about to be computed, processReceiveBuffer() + // pauses (leaving the already-buffered bytes untouched so re-entry + // re-parses them identically) and emits usernameRequested(); resumed + // by provideUsername() once SessionTab answers. + bool m_waitingForUsername; + QString m_promptedUsername; + void setState(SessionState state, const QString& message); void resetProtocolState(); void processReceiveBuffer(); @@ -227,6 +238,12 @@ private: QRect currentHextileTileRect() const; void advanceHextileTile(); bool inflateTightStream(int streamIndex, const QByteArray& compressed, QByteArray* decompressed); + QString effectiveUsername() const; + // Returns false (and pauses, emitting usernameRequested() at most once + // until provideUsername() resumes processing) if a username is needed + // but not yet available. Callers must return from processReceiveBuffer() + // immediately when this returns false, without consuming any bytes. + bool ensureUsernameAvailable(); }; #endif diff --git a/tests/test_vnc_session_backend.cpp b/tests/test_vnc_session_backend.cpp index b87b223..5cf31a8 100644 --- a/tests/test_vnc_session_backend.cpp +++ b/tests/test_vnc_session_backend.cpp @@ -420,6 +420,7 @@ private slots: void connectsWithAppleDhAuthenticationRfb38(); void appleDhAuthIsPreferredOverVncAuthWhenBothOffered(); void appleDhAuthAcceptsRealCapturedMacOsServerParameters(); + void cancellingUsernamePromptFailsConnectionCleanly(); private: std::unique_ptr m_server; @@ -2036,8 +2037,20 @@ void TestVncSessionBackend::connectsWithAppleDhAuthenticationRfb38() } }); + // makeVncProfile() sets no username, and Apple DH is the one VNC auth + // method that needs one -- expect the backend to pause and ask, + // exactly like a real profile with a blank username would hit at + // connect time (see issue #21 / SessionBackend::usernameRequested). + QString requestedPrompt; + connect(m_backend.get(), &SessionBackend::usernameRequested, this, + [this, &requestedPrompt](const QString& prompt) { + requestedPrompt = prompt; + m_backend->provideUsername(QStringLiteral("tester")); + }); + m_backend->connectSession(makeOptions(password)); QTRY_COMPARE(m_lastState, SessionState::Connected); + QVERIFY(!requestedPrompt.isEmpty()); // Verify wire order: encrypted credentials (128 bytes) MUST come // before the client's public key, per neatvnc's authoritative @@ -2085,7 +2098,9 @@ void TestVncSessionBackend::connectsWithAppleDhAuthenticationRfb38() EVP_CIPHER_CTX_free(decCtx); QByteArray expected(128, char(0)); + const QByteArray userBytes = QByteArrayLiteral("tester").left(64); const QByteArray passBytes = password.toLatin1().left(64); + std::memcpy(expected.data(), userBytes.constData(), static_cast(userBytes.size())); std::memcpy(expected.data() + 64, passBytes.constData(), static_cast(passBytes.size())); QCOMPARE(plain, expected); @@ -2172,5 +2187,51 @@ void TestVncSessionBackend::appleDhAuthAcceptsRealCapturedMacOsServerParameters( QCOMPARE(response.encryptedCredentials.size(), 128); } +void TestVncSessionBackend::cancellingUsernamePromptFailsConnectionCleanly() +{ + ToyDhKeypair serverKeypair = generateToyDhKeypair(); + const QByteArray authMessage = appleDhAuthServerMessage( + serverKeypair.generatorBytes, serverKeypair.primeBytes, serverKeypair.publicKeyBytes); + + bool gotUsernameRequest = false; + connect(m_backend.get(), &SessionBackend::usernameRequested, this, + [this, &gotUsernameRequest](const QString&) { + gotUsernameRequest = true; + // An empty response mirrors the prompt being cancelled + // (see SessionTab::onBackendUsernameRequested). + m_backend->provideUsername(QString()); + }); + + connect(m_server.get(), &FakeVncServer::clientConnected, this, [this]() { + m_server->sendWhenConnected(QByteArray("RFB 003.008\n")); + }); + connect(m_server.get(), &FakeVncServer::dataReceived, this, [this, authMessage]() { + switch (m_server->nextStep()) { + case 0: { + QByteArray securityTypes; + securityTypes.append(char(1)); + securityTypes.append(char(30)); // Apple's scheme -- no other option offered + m_server->sendWhenConnected(securityTypes); + break; + } + case 1: + m_server->sendWhenConnected(authMessage); + break; + default: + break; + } + }); + + // No username in the profile (makeVncProfile() sets none) and no + // other security type to fall back to. + m_backend->connectSession(makeOptions(QStringLiteral("s3cret-pass"))); + QTRY_COMPARE(m_lastState, SessionState::Failed); + QVERIFY(gotUsernameRequest); + QVERIFY(m_lastErrorDisplay.contains(QStringLiteral("username"), Qt::CaseInsensitive)); + + BN_free(serverKeypair.privateExponent); + BN_free(serverKeypair.prime); +} + QTEST_GUILESS_MAIN(TestVncSessionBackend) #include "test_vnc_session_backend.moc"