Compare commits

...
Author SHA1 Message Date
ksmithandClaude Sonnet 5 b3cedcfa48 Fix connect-time username prompt never reaching SSH/RDP authentication
The username entered at the connect-time prompt (added for issue #21)
was updating SessionTab's own in-memory Profile copy, but
SshSessionBackend/RdpSessionBackend are constructed with -- and only
ever read from -- their own separate Profile copy on a worker thread,
which never saw that edit. Authentication was still built from the
original (blank) username regardless of what was typed into the
prompt.

SessionConnectOptions gains a username field, populated by SessionTab
on every connect attempt and threaded through the same way password
already is; both backends now prefer options.username over
profile().username. Covered by a new SSH regression test using an
exact-match fixture host that only succeeds for a specific
user@host target.

Bump version to v2026.9.16.5.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-16 08:40:34 -06:00
ksmithandClaude Sonnet 5 0cdf930303 Fix connect-time username prompt being unreachable for SSH/RDP
validateProfileForConnect() still hard-failed with a blocking
QMessageBox for a blank SSH/RDP username, running before
requestConnectOptions() ever got a chance to prompt for it inline --
so the connect-time username prompt added for issue #21 was dead code
in practice; users just got told to go edit the profile instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-16 08:28:41 -06:00
ksmithandClaude Sonnet 5 7559488ceb Bump version to v2026.9.16.3
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-16 08:18:22 -06:00
ksmithandClaude Sonnet 5 ab4a34fc01 Fix mRemoteNG import failing on entries without a username
The previous username relaxation only touched ProfileDialog's own
save-time validation. ProfileRepository::isProfileValid() had the
identical "username required for SSH/RDP" check independently, called
directly by insertProfile()/updateProfile() -- exactly the path
mRemoteNG import uses, since it builds Profile objects and inserts
them directly rather than going through the dialog. Every imported
SSH/RDP entry without a recorded username was rejected outright.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-16 08:17:05 -06:00
ksmithandClaude Sonnet 5 7aa8849f8e Bump version to v2026.9.16.2
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-16 08:07:29 -06:00
ksmithandClaude Sonnet 5 776db5ec04 Allow blank usernames, asking for one at connect time instead
Closes #21. SSH and RDP profiles previously hard-required a username
to even save the profile; that validation is dropped, and
SessionTab::requestConnectOptions() now prompts for it at connect
time when blank, reusing the existing password-prompt bar in
unmasked mode -- the same pattern already used for a blank password.

VNC's username is trickier: most VNC servers never use one (plain VNC
Authentication and no-auth don't), only the two Apple auth schemes
(security types 30/33) do, and which auth method gets used isn't known
until mid-connection, after the server's security-type list has been
negotiated -- too late for the pre-connect prompt SSH/RDP uses. Adds a
new async request/response pair to SessionBackend, usernameRequested()
signal / provideUsername() slot, mirroring the existing SSH host-key-
confirmation pattern. VncSessionBackend pauses its state machine right
before computing an Apple-auth response if no username is available --
without consuming the already-buffered prime/host-key bytes, so
resuming re-parses them identically -- emits the request, and resumes
via provideUsername(). Cancelling (or submitting blank) fails the
connection cleanly instead of sending Apple auth an empty username.

The username is kept on the tab's in-memory profile copy for its
lifetime, not written back to the saved profile, matching how
passwords are already handled.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-16 08:01:57 -06:00
15 changed files with 345 additions and 40 deletions
+1 -1
View File
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.21) cmake_minimum_required(VERSION 3.21)
project(OrbitHub VERSION 2026.9.16 LANGUAGES CXX) project(OrbitHub VERSION 2026.9.16.5 LANGUAGES CXX)
set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_STANDARD_REQUIRED ON)
+52
View File
@@ -157,6 +157,58 @@ Delivered:
gets its own prompt, with an empty password allowed through (unlike 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 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 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
- The blank-username relaxation above initially only covered
`ProfileDialog`'s own save-time validation; `ProfileRepository::
isProfileValid()` had the identical "username required for SSH/RDP"
check independently, called directly by `insertProfile()`/
`updateProfile()` -- which is exactly the path mRemoteNG import uses
(it builds `Profile` objects and inserts them directly, never going
through the dialog), so importing any SSH/RDP entry without a
username still failed outright until this second check was found and
removed too
- A third, independent username check was still live even after the two
above were removed: `SessionTab::validateProfileForConnect()` (run at
the very top of `connectSession()`/`reconnectSession()`, before
`requestConnectOptions()` ever gets a chance to run its async prompt)
had its own hard-fail "SSH/RDP username is required" `QMessageBox`,
so a blank-username profile still couldn't connect at all -- it just
told the user to go edit the profile instead of ever prompting inline.
Removed; connect-time prompting is now the only username gate for
SSH/RDP
- Even with the three checks above gone, a username entered at the
connect-time prompt still never actually reached SSH or RDP
authentication: `SshSessionBackend`/`RdpSessionBackend` are constructed
with their own `Profile` copy up front (moved to a worker thread) and
read `profile().username` directly, which never sees `SessionTab`'s
later edit to its own in-memory profile once the user answers the
prompt. `SessionConnectOptions` (which already carries `password` the
same way) gained a `username` field, populated by `SessionTab` from
its profile copy on every connect attempt; both backends now prefer
`options.username` over `profile().username` when building the actual
connect target/auth call. Covered by a new SSH regression test
(`tests/fixtures/fake_ssh.sh`'s `requireuser` host only accepts an
exact `prompted-user@requireuser` target, so the test fails unless the
option, not the stale profile copy, is actually used) -- RDP has no
equivalent fake-server test harness, so that side relies on mirroring
the already-tested `m_activeOptions.password` pattern exactly
- Robustness fix: an unrecognized `FramebufferUpdate` rectangle encoding - Robustness fix: an unrecognized `FramebufferUpdate` rectangle encoding
used to abort the connection generically; `kAnnouncedEncodings` is now used to abort the connection generically; `kAnnouncedEncodings` is now
the single source of truth for what `SetEncodings` announces and what the single source of truth for what `SetEncodings` announces and what
+6 -11
View File
@@ -257,13 +257,6 @@ void ProfileDialog::accept()
} }
const QString protocol = m_protocolInput->currentText(); 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") if (protocol == QStringLiteral("SSH")
&& m_authModeInput->currentText() == QStringLiteral("Private Key")) { && m_authModeInput->currentText() == QStringLiteral("Private Key")) {
@@ -310,12 +303,14 @@ void ProfileDialog::refreshAuthFields()
if (isSsh) { if (isSsh) {
m_usernameInput->setPlaceholderText(QStringLiteral("deploy")); m_usernameInput->setPlaceholderText(QStringLiteral("deploy"));
m_protocolHint->setText( m_protocolHint->setText(QStringLiteral(
QStringLiteral("SSH: username is required. Choose Password or Private Key auth.")); "SSH: you'll be asked for a username at connect time if left blank here. "
"Choose Password or Private Key auth."));
} else if (isRdp) { } else if (isRdp) {
m_usernameInput->setPlaceholderText(QStringLiteral("Administrator")); m_usernameInput->setPlaceholderText(QStringLiteral("Administrator"));
m_protocolHint->setText( m_protocolHint->setText(QStringLiteral(
QStringLiteral("RDP: username and password are required. Domain is optional.")); "RDP: you'll be asked for a username at connect time if left blank here. "
"Domain is optional."));
} else if (isVnc) { } else if (isVnc) {
m_usernameInput->setPlaceholderText(QStringLiteral("optional")); m_usernameInput->setPlaceholderText(QStringLiteral("optional"));
m_protocolHint->setText(QStringLiteral( m_protocolHint->setText(QStringLiteral(
-7
View File
@@ -235,13 +235,6 @@ bool isProfileValid(const Profile& profile, QString* error)
} }
const QString protocol = normalizedProtocol(profile.protocol); const QString protocol = normalizedProtocol(profile.protocol);
if ((protocol == QStringLiteral("SSH") || protocol == QStringLiteral("RDP"))
&& profile.username.trimmed().isEmpty()) {
if (error != nullptr) {
*error = QStringLiteral("Username is required for %1 profiles.").arg(protocol);
}
return false;
}
const QString authMode = normalizedAuthMode(protocol, profile.authMode); const QString authMode = normalizedAuthMode(protocol, profile.authMode);
if (protocol == QStringLiteral("SSH") && authMode == QStringLiteral("Private Key") if (protocol == QStringLiteral("SSH") && authMode == QStringLiteral("Private Key")
+7 -1
View File
@@ -1640,7 +1640,13 @@ void RdpSessionBackend::workerMain()
const Profile& p = profile(); const Profile& p = profile();
const QString host = p.host.trimmed(); const QString host = p.host.trimmed();
QString username = p.username.trimmed(); // m_activeOptions.username carries a value prompted for at connect time
// (see SessionTab::requestConnectOptions()) when the saved profile's
// own username was blank; profile().username never sees that edit
// since this backend's Profile copy was captured at construction time.
QString username = m_activeOptions.username.trimmed().isEmpty()
? p.username.trimmed()
: m_activeOptions.username.trimmed();
QString domain = p.domain.trimmed(); QString domain = p.domain.trimmed();
if (domain.isEmpty()) { if (domain.isEmpty()) {
const int domainSeparator = username.indexOf(QLatin1Char('\\')); const int domainSeparator = username.indexOf(QLatin1Char('\\'));
+20
View File
@@ -12,6 +12,12 @@
class SessionConnectOptions class SessionConnectOptions
{ {
public: public:
// Only set when the profile's own username was blank and SessionTab
// prompted for one inline at connect time (see issue #21); empty means
// "use the backend's own profile().username" as before. SSH/RDP need
// this up front, unlike VNC's Apple auth which discovers the need for
// one mid-connection via usernameRequested()/provideUsername() instead.
QString username;
QString password; QString password;
QString privateKeyPath; QString privateKeyPath;
QString knownHostsPolicy; QString knownHostsPolicy;
@@ -55,6 +61,16 @@ public slots:
{ {
Q_UNUSED(text); 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, virtual void sendKeyEvent(int key,
quint32 nativeScanCode, quint32 nativeScanCode,
const QString& text, const QString& text,
@@ -93,6 +109,10 @@ signals:
void connectionError(const QString& displayMessage, const QString& rawMessage); void connectionError(const QString& displayMessage, const QString& rawMessage);
void outputReceived(const QString& text); void outputReceived(const QString& text);
void hostKeyConfirmationRequested(const QString& prompt); 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 frameUpdated(const QImage& frame);
void remoteDesktopSizeChanged(int width, int height); void remoteDesktopSizeChanged(int width, int height);
void remoteClipboardTextChanged(const QString& text); void remoteClipboardTextChanged(const QString& text);
+51 -9
View File
@@ -215,6 +215,11 @@ SessionTab::SessionTab(const Profile& profile,
m_backend, m_backend,
&SessionBackend::setClipboardText, &SessionBackend::setClipboardText,
Qt::QueuedConnection); Qt::QueuedConnection);
connect(this,
&SessionTab::requestProvideUsername,
m_backend,
&SessionBackend::provideUsername,
Qt::QueuedConnection);
connect(m_backend, connect(m_backend,
&SessionBackend::stateChanged, &SessionBackend::stateChanged,
@@ -241,6 +246,11 @@ SessionTab::SessionTab(const Profile& profile,
this, this,
&SessionTab::onBackendHostKeyConfirmationRequested, &SessionTab::onBackendHostKeyConfirmationRequested,
Qt::QueuedConnection); Qt::QueuedConnection);
connect(m_backend,
&SessionBackend::usernameRequested,
this,
&SessionTab::onBackendUsernameRequested,
Qt::QueuedConnection);
connect(m_backend, connect(m_backend,
&SessionBackend::frameUpdated, &SessionBackend::frameUpdated,
this, this,
@@ -714,6 +724,16 @@ void SessionTab::onBackendHostKeyConfirmationRequested(const QString& prompt)
emit requestHostKeyConfirmation(reply == QMessageBox::Yes); 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<QString> username) {
emit requestProvideUsername(username.value_or(QString()).trimmed());
},
false);
}
void SessionTab::onBackendRemoteClipboardTextChanged(const QString& text) void SessionTab::onBackendRemoteClipboardTextChanged(const QString& text)
{ {
if (text == m_lastSyncedClipboardText) { if (text == m_lastSyncedClipboardText) {
@@ -978,11 +998,35 @@ void SessionTab::requestConnectOptions(
{ {
SessionConnectOptions baseOptions; SessionConnectOptions baseOptions;
baseOptions.knownHostsPolicy = m_profile.knownHostsPolicy; baseOptions.knownHostsPolicy = m_profile.knownHostsPolicy;
// The backend's own Profile copy was captured when it was constructed
// and never sees later edits to m_profile (e.g. the username prompt
// below) -- it has to travel through here instead.
baseOptions.username = m_profile.username.trimmed();
const bool isSsh = m_profile.protocol.compare(QStringLiteral("SSH"), Qt::CaseInsensitive) == 0; const bool isSsh = m_profile.protocol.compare(QStringLiteral("SSH"), Qt::CaseInsensitive) == 0;
const bool isRdp = m_profile.protocol.compare(QStringLiteral("RDP"), Qt::CaseInsensitive) == 0; const bool isRdp = m_profile.protocol.compare(QStringLiteral("RDP"), Qt::CaseInsensitive) == 0;
const bool isVnc = m_profile.protocol.compare(QStringLiteral("VNC"), 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<QString> username) {
if (!username.has_value() || username->trimmed().isEmpty()) {
callback(std::nullopt);
return;
}
m_profile.username = username->trimmed();
requestConnectOptions(callback);
},
false);
return;
}
if (isVnc) { if (isVnc) {
// Unlike RDP, an empty password is allowed through: some VNC // Unlike RDP, an empty password is allowed through: some VNC
// servers (no-auth) don't need one at all, and there's no // servers (no-auth) don't need one at all, and there's no
@@ -1102,7 +1146,8 @@ void SessionTab::requestConnectOptions(
} }
void SessionTab::showPasswordPrompt(const QString& labelText, void SessionTab::showPasswordPrompt(const QString& labelText,
std::function<void(std::optional<QString>)> callback) std::function<void(std::optional<QString>)> callback,
bool maskInput)
{ {
if (m_passwordPromptCallback) { if (m_passwordPromptCallback) {
const auto previousCallback = m_passwordPromptCallback; const auto previousCallback = m_passwordPromptCallback;
@@ -1113,6 +1158,7 @@ void SessionTab::showPasswordPrompt(const QString& labelText,
m_passwordPromptCallback = std::move(callback); m_passwordPromptCallback = std::move(callback);
m_passwordPromptLabel->setText(labelText); m_passwordPromptLabel->setText(labelText);
m_passwordPromptInput->clear(); m_passwordPromptInput->clear();
m_passwordPromptInput->setEchoMode(maskInput ? QLineEdit::Password : QLineEdit::Normal);
m_passwordPromptBar->setVisible(true); m_passwordPromptBar->setVisible(true);
m_passwordPromptInput->setFocus(); m_passwordPromptInput->setFocus();
} }
@@ -1139,14 +1185,10 @@ bool SessionTab::validateProfileForConnect()
return false; return false;
} }
if ((m_profile.protocol.compare(QStringLiteral("SSH"), Qt::CaseInsensitive) == 0 // SSH/RDP no longer hard-require a username here -- a blank one is
|| m_profile.protocol.compare(QStringLiteral("RDP"), Qt::CaseInsensitive) == 0) // handled by requestConnectOptions() prompting for it inline at connect
&& m_profile.username.trimmed().isEmpty()) { // time (see issue #21). Do not re-add a check here without also
QMessageBox::warning(this, // updating that flow.
QStringLiteral("Connect"),
QStringLiteral("%1 username is required.").arg(m_profile.protocol));
return false;
}
return true; return true;
} }
+4 -1
View File
@@ -91,6 +91,7 @@ signals:
void requestMouseButtonEvent(int x, int y, int button, bool pressed); void requestMouseButtonEvent(int x, int y, int button, bool pressed);
void requestMouseWheelEvent(int x, int y, int deltaX, int deltaY); void requestMouseWheelEvent(int x, int y, int deltaX, int deltaY);
void requestSetClipboardText(const QString& text); void requestSetClipboardText(const QString& text);
void requestProvideUsername(const QString& username);
private slots: private slots:
void onBackendStateChanged(SessionState state, const QString& message); void onBackendStateChanged(SessionState state, const QString& message);
@@ -98,6 +99,7 @@ private slots:
void onBackendConnectionError(const QString& displayMessage, const QString& rawMessage); void onBackendConnectionError(const QString& displayMessage, const QString& rawMessage);
void onBackendOutputReceived(const QString& text); void onBackendOutputReceived(const QString& text);
void onBackendHostKeyConfirmationRequested(const QString& prompt); void onBackendHostKeyConfirmationRequested(const QString& prompt);
void onBackendUsernameRequested(const QString& prompt);
void onBackendRemoteClipboardTextChanged(const QString& text); void onBackendRemoteClipboardTextChanged(const QString& text);
void onSystemClipboardChanged(); void onSystemClipboardChanged();
@@ -149,7 +151,8 @@ private:
void setupUi(); void setupUi();
void requestConnectOptions(std::function<void(std::optional<SessionConnectOptions>)> callback); void requestConnectOptions(std::function<void(std::optional<SessionConnectOptions>)> callback);
void showPasswordPrompt(const QString& labelText, void showPasswordPrompt(const QString& labelText,
std::function<void(std::optional<QString>)> callback); std::function<void(std::optional<QString>)> callback,
bool maskInput = true);
void hidePasswordPrompt(); void hidePasswordPrompt();
bool validateProfileForConnect(); bool validateProfileForConnect();
void appendEvent(const QString& message); void appendEvent(const QString& message);
+9 -2
View File
@@ -388,9 +388,16 @@ bool SshSessionBackend::startSshProcess(const SessionConnectOptions& options)
<< QStringLiteral("PasswordAuthentication=no"); << QStringLiteral("PasswordAuthentication=no");
} }
const QString target = p.username.trimmed().isEmpty() // options.username carries a value prompted for at connect time (see
// SessionTab::requestConnectOptions()) when the saved profile's own
// username was blank; profile().username never sees that edit since
// the backend's Profile copy was captured at construction time.
const QString username = options.username.trimmed().isEmpty()
? p.username.trimmed()
: options.username.trimmed();
const QString target = username.isEmpty()
? p.host.trimmed() ? p.host.trimmed()
: QStringLiteral("%1@%2").arg(p.username.trimmed(), p.host.trimmed()); : QStringLiteral("%1@%2").arg(username, p.host.trimmed());
args << target; args << target;
m_process->setProcessEnvironment(environment); m_process->setProcessEnvironment(environment);
+58 -3
View File
@@ -196,7 +196,8 @@ VncSessionBackend::VncSessionBackend(const Profile& profile, QObject* parent)
m_tightCompressionMode(0), m_tightCompressionMode(0),
m_tightFilterId(0), m_tightFilterId(0),
m_tightLengthByteIndex(0), m_tightLengthByteIndex(0),
m_appleAuthKeyLength(0) m_appleAuthKeyLength(0),
m_waitingForUsername(false)
{ {
std::memset(m_zrleInflateStream, 0, sizeof(z_stream_s)); std::memset(m_zrleInflateStream, 0, sizeof(z_stream_s));
for (z_stream_s* stream : m_tightInflateStreams) { for (z_stream_s* stream : m_tightInflateStreams) {
@@ -531,6 +532,8 @@ void VncSessionBackend::resetProtocolState()
} }
m_appleAuthGenerator.clear(); m_appleAuthGenerator.clear();
m_appleAuthKeyLength = 0; m_appleAuthKeyLength = 0;
m_waitingForUsername = false;
m_promptedUsername.clear();
} }
bool VncSessionBackend::haveBytes(int count) const bool VncSessionBackend::haveBytes(int count) const
@@ -671,6 +674,47 @@ void VncSessionBackend::sendAppleRsaHostKeyRequest()
m_socket->write(msg); 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() void VncSessionBackend::finishHandshakeIntoRunningState()
{ {
emit remoteDesktopSizeChanged(m_framebuffer.width(), m_framebuffer.height()); emit remoteDesktopSizeChanged(m_framebuffer.width(), m_framebuffer.height());
@@ -963,13 +1007,20 @@ void VncSessionBackend::processReceiveBuffer()
if (!haveBytes(static_cast<int>(m_pendingLength))) { if (!haveBytes(static_cast<int>(m_pendingLength))) {
return; 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<int>(m_appleAuthKeyLength); const int keyLength = static_cast<int>(m_appleAuthKeyLength);
const QByteArray prime = m_recvBuffer.left(keyLength); const QByteArray prime = m_recvBuffer.left(keyLength);
const QByteArray serverPublicKey = m_recvBuffer.mid(keyLength, keyLength); const QByteArray serverPublicKey = m_recvBuffer.mid(keyLength, keyLength);
m_recvBuffer.remove(0, static_cast<int>(m_pendingLength)); m_recvBuffer.remove(0, static_cast<int>(m_pendingLength));
const VncAppleDhAuth::Response response = VncAppleDhAuth::computeResponse( const VncAppleDhAuth::Response response = VncAppleDhAuth::computeResponse(
m_appleAuthGenerator, prime, serverPublicKey, profile().username, m_appleAuthGenerator, prime, serverPublicKey, effectiveUsername(),
m_activeOptions.password); m_activeOptions.password);
if (response.clientPublicKey.isEmpty()) { if (response.clientPublicKey.isEmpty()) {
failConnection( failConnection(
@@ -1020,11 +1071,15 @@ void VncSessionBackend::processReceiveBuffer()
if (!haveBytes(totalBytes)) { if (!haveBytes(totalBytes)) {
return; return;
} }
// See the matching comment in WaitingAppleAuthPrimeAndServerKey.
if (!ensureUsernameAvailable()) {
return;
}
const QByteArray hostKeyDer = m_recvBuffer.left(static_cast<int>(m_pendingLength)); const QByteArray hostKeyDer = m_recvBuffer.left(static_cast<int>(m_pendingLength));
m_recvBuffer.remove(0, totalBytes); m_recvBuffer.remove(0, totalBytes);
const VncAppleRsaAuth::Response response = VncAppleRsaAuth::computeResponse( const VncAppleRsaAuth::Response response = VncAppleRsaAuth::computeResponse(
hostKeyDer, profile().username, m_activeOptions.password); hostKeyDer, effectiveUsername(), m_activeOptions.password);
if (response.encryptedCredentials.isEmpty() || response.encryptedAesKey.isEmpty()) { if (response.encryptedCredentials.isEmpty() || response.encryptedAesKey.isEmpty()) {
failConnection( failConnection(
QStringLiteral( QStringLiteral(
+17
View File
@@ -84,6 +84,7 @@ public slots:
void sendMouseButtonEvent(int x, int y, int button, bool pressed) override; void sendMouseButtonEvent(int x, int y, int button, bool pressed) override;
void sendMouseWheelEvent(int x, int y, int deltaX, int deltaY) override; void sendMouseWheelEvent(int x, int y, int deltaX, int deltaY) override;
void setClipboardText(const QString& text) override; void setClipboardText(const QString& text) override;
void provideUsername(const QString& username) override;
private slots: private slots:
void onSocketConnected(); void onSocketConnected();
@@ -209,6 +210,16 @@ private:
QByteArray m_appleAuthGenerator; QByteArray m_appleAuthGenerator;
quint32 m_appleAuthKeyLength; 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 setState(SessionState state, const QString& message);
void resetProtocolState(); void resetProtocolState();
void processReceiveBuffer(); void processReceiveBuffer();
@@ -227,6 +238,12 @@ private:
QRect currentHextileTileRect() const; QRect currentHextileTileRect() const;
void advanceHextileTile(); void advanceHextileTile();
bool inflateTightStream(int streamIndex, const QByteArray& compressed, QByteArray* decompressed); 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 #endif
+16
View File
@@ -6,6 +6,22 @@
# host, optionally as user@host, as the final argument). # host, optionally as user@host, as the final argument).
for arg in "$@"; do for arg in "$@"; do
case "$arg" in case "$arg" in
prompted-user@requireuser)
# Only the exact user@host below is accepted -- used to prove a
# username supplied via SessionConnectOptions (prompted for at
# connect time because the saved profile's own username was
# blank) actually reaches the ssh command line, not just that
# *some* connection to this host succeeds.
echo "Welcome to the fake host."
while IFS= read -r line; do
echo "$line"
done
exit 0
;;
*@requireuser|requireuser)
echo "Permission denied (publickey,password)." >&2
exit 255
;;
*@succeed|succeed) *@succeed|succeed)
echo "Welcome to the fake host." echo "Welcome to the fake host."
# Stay alive echoing stdin back (simulates an interactive # Stay alive echoing stdin back (simulates an interactive
+9 -3
View File
@@ -48,7 +48,7 @@ private slots:
void createProfileRejectsMissingName(); void createProfileRejectsMissingName();
void createProfileRejectsMissingHost(); void createProfileRejectsMissingHost();
void createProfileRejectsInvalidPort(); void createProfileRejectsInvalidPort();
void createProfileRejectsMissingUsernameForSsh(); void createProfileAllowsMissingUsernameForSsh();
void createProfileRejectsMissingPrivateKeyForKeyAuth(); void createProfileRejectsMissingPrivateKeyForKeyAuth();
void createProfileRejectsDuplicateName(); void createProfileRejectsDuplicateName();
void updateProfilePersistsChanges(); void updateProfilePersistsChanges();
@@ -156,11 +156,17 @@ void TestProfileRepository::createProfileRejectsInvalidPort()
QVERIFY(!m_repo->createProfile(profile).has_value()); QVERIFY(!m_repo->createProfile(profile).has_value());
} }
void TestProfileRepository::createProfileRejectsMissingUsernameForSsh() void TestProfileRepository::createProfileAllowsMissingUsernameForSsh()
{ {
// SSH/RDP no longer require a username at save time (issue #21) --
// among other things, this unblocks importing mRemoteNG entries that
// don't have one recorded, which used to fail outright. The user is
// asked for it at connect time instead (see SessionTab).
Profile profile = makeSshProfile(); Profile profile = makeSshProfile();
profile.username.clear(); profile.username.clear();
QVERIFY(!m_repo->createProfile(profile).has_value()); const auto created = m_repo->createProfile(profile);
QVERIFY(created.has_value());
QVERIFY(created->username.isEmpty());
} }
void TestProfileRepository::createProfileRejectsMissingPrivateKeyForKeyAuth() void TestProfileRepository::createProfileRejectsMissingPrivateKeyForKeyAuth()
+34 -2
View File
@@ -21,6 +21,13 @@ Profile makeProfile(const QString& fixtureHost)
return profile; return profile;
} }
Profile makeBlankUsernameProfile(const QString& fixtureHost)
{
Profile profile = makeProfile(fixtureHost);
profile.username.clear();
return profile;
}
SessionConnectOptions makeOptions() SessionConnectOptions makeOptions()
{ {
SessionConnectOptions options; SessionConnectOptions options;
@@ -50,10 +57,12 @@ private slots:
void connectionRefusedReachesFailedState(); void connectionRefusedReachesFailedState();
void sendInputEchoesThroughOutputReceived(); void sendInputEchoesThroughOutputReceived();
void reconnectRestartsAndReachesConnectedAgain(); void reconnectRestartsAndReachesConnectedAgain();
void connectOptionsUsernameReachesProcessWhenProfileUsernameIsBlank();
private: private:
QString fixturePath() const; QString fixturePath() const;
void createBackend(const QString& fixtureHost); void createBackend(const QString& fixtureHost);
void createBackend(const Profile& profile);
std::unique_ptr<SshSessionBackend> m_backend; std::unique_ptr<SshSessionBackend> m_backend;
SessionState m_lastState = SessionState::Disconnected; SessionState m_lastState = SessionState::Disconnected;
@@ -69,8 +78,12 @@ QString TestSshSessionBackend::fixturePath() const
void TestSshSessionBackend::createBackend(const QString& fixtureHost) void TestSshSessionBackend::createBackend(const QString& fixtureHost)
{ {
m_backend = createBackend(makeProfile(fixtureHost));
std::make_unique<SshSessionBackend>(makeProfile(fixtureHost), fixturePath(), nullptr); }
void TestSshSessionBackend::createBackend(const Profile& profile)
{
m_backend = std::make_unique<SshSessionBackend>(profile, fixturePath(), nullptr);
connect(m_backend.get(), connect(m_backend.get(),
&SessionBackend::stateChanged, &SessionBackend::stateChanged,
this, this,
@@ -232,5 +245,24 @@ void TestSshSessionBackend::reconnectRestartsAndReachesConnectedAgain()
QTRY_COMPARE(m_lastState, SessionState::Connected); QTRY_COMPARE(m_lastState, SessionState::Connected);
} }
void TestSshSessionBackend::connectOptionsUsernameReachesProcessWhenProfileUsernameIsBlank()
{
// Regression test for a bug where a username entered at the
// connect-time prompt (SessionTab::requestConnectOptions(), for a
// profile with no saved username -- issue #21) never actually reached
// the ssh process: SshSessionBackend built its target purely from
// profile().username, which is a separate copy captured when the
// backend was constructed and never sees SessionTab's later edit.
// fixtures/fake_ssh.sh's "requireuser" host only accepts the exact
// target "prompted-user@requireuser", so this fails unless
// SessionConnectOptions::username is actually used.
createBackend(makeBlankUsernameProfile(QStringLiteral("requireuser")));
SessionConnectOptions options = makeOptions();
options.username = QStringLiteral("prompted-user");
m_backend->connectSession(options);
QTRY_COMPARE(m_lastState, SessionState::Connected);
}
QTEST_GUILESS_MAIN(TestSshSessionBackend) QTEST_GUILESS_MAIN(TestSshSessionBackend)
#include "test_ssh_session_backend.moc" #include "test_ssh_session_backend.moc"
+61
View File
@@ -420,6 +420,7 @@ private slots:
void connectsWithAppleDhAuthenticationRfb38(); void connectsWithAppleDhAuthenticationRfb38();
void appleDhAuthIsPreferredOverVncAuthWhenBothOffered(); void appleDhAuthIsPreferredOverVncAuthWhenBothOffered();
void appleDhAuthAcceptsRealCapturedMacOsServerParameters(); void appleDhAuthAcceptsRealCapturedMacOsServerParameters();
void cancellingUsernamePromptFailsConnectionCleanly();
private: private:
std::unique_ptr<FakeVncServer> m_server; std::unique_ptr<FakeVncServer> 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)); m_backend->connectSession(makeOptions(password));
QTRY_COMPARE(m_lastState, SessionState::Connected); QTRY_COMPARE(m_lastState, SessionState::Connected);
QVERIFY(!requestedPrompt.isEmpty());
// Verify wire order: encrypted credentials (128 bytes) MUST come // Verify wire order: encrypted credentials (128 bytes) MUST come
// before the client's public key, per neatvnc's authoritative // before the client's public key, per neatvnc's authoritative
@@ -2085,7 +2098,9 @@ void TestVncSessionBackend::connectsWithAppleDhAuthenticationRfb38()
EVP_CIPHER_CTX_free(decCtx); EVP_CIPHER_CTX_free(decCtx);
QByteArray expected(128, char(0)); QByteArray expected(128, char(0));
const QByteArray userBytes = QByteArrayLiteral("tester").left(64);
const QByteArray passBytes = password.toLatin1().left(64); const QByteArray passBytes = password.toLatin1().left(64);
std::memcpy(expected.data(), userBytes.constData(), static_cast<size_t>(userBytes.size()));
std::memcpy(expected.data() + 64, passBytes.constData(), static_cast<size_t>(passBytes.size())); std::memcpy(expected.data() + 64, passBytes.constData(), static_cast<size_t>(passBytes.size()));
QCOMPARE(plain, expected); QCOMPARE(plain, expected);
@@ -2172,5 +2187,51 @@ void TestVncSessionBackend::appleDhAuthAcceptsRealCapturedMacOsServerParameters(
QCOMPARE(response.encryptedCredentials.size(), 128); 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) QTEST_GUILESS_MAIN(TestVncSessionBackend)
#include "test_vnc_session_backend.moc" #include "test_vnc_session_backend.moc"