RDP: enforce TLS certificate verification (was fully disabled)

IgnoreCertificate was hardcoded TRUE, meaning FreeRDP's entire
certificate-verification pipeline was bypassed: every RDP server's
TLS certificate was silently accepted, including certificates that
had changed since a prior trusted connection to the same host. That
is precisely the scenario TLS verification exists to catch — an
active MITM presenting a different certificate was indistinguishable
from a legitimate server.

Switches to FreeRDP's own trust-on-first-use certificate store
(AutoAcceptCertificate) so first-time connections still connect
without a prompt, matching SSH's "accept-new" known-hosts policy.
Certificate changes now correctly refuse the connection by default,
via VerifyChangedCertificateEx, with a clear message (host, port,
old/new SHA256 fingerprints) surfaced through the existing
connection-failure event log rather than adding a new, redundant
logging path.

Also fixes CertificateCallbackPreferPEM, which handed the full PEM
certificate to the verify callbacks instead of a short fingerprint —
harmless while those callbacks were dead code, but would have
flooded the event log with multi-KB certificate dumps once actually
exercised.

Verified end-to-end against real infrastructure: first connection
trusts and stores the certificate silently, a simulated changed
certificate (server key swapped) is correctly refused with a clear
message, and restoring the original certificate reconnects normally.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

src/rdp_session_backend.cpp

@@ -420,9 +420,19 @@ BOOL orbitPreConnect(freerdp* instance)
     }
 
     rdpSettings* settings = instance->context->settings;
-    freerdp_settings_set_bool(settings, FreeRDP_CertificateCallbackPreferPEM, TRUE);
+    // FALSE gives the verify callbacks a short SHA256 fingerprint string
+    // (e.g. "ab:cd:12:...") instead of the full PEM certificate blob, which
+    // is what actually belongs in a user-facing changed-certificate message.
+    freerdp_settings_set_bool(settings, FreeRDP_CertificateCallbackPreferPEM, FALSE);
     freerdp_settings_set_bool(settings, FreeRDP_DesktopResize, TRUE);
-    freerdp_settings_set_bool(settings, FreeRDP_IgnoreCertificate, TRUE);
+    // Trust servers on first connection (matching SSH's "accept-new" known-hosts
+    // policy) and persist that trust via FreeRDP's own certificate store, but
+    // actually verify it on every subsequent connection: IgnoreCertificate
+    // previously bypassed verification entirely, so a changed certificate
+    // (e.g. an active MITM) was silently accepted, indistinguishable from a
+    // legitimate server. See orbitVerifyChangedCertificateEx below.
+    freerdp_settings_set_bool(settings, FreeRDP_IgnoreCertificate, FALSE);
+    freerdp_settings_set_bool(settings, FreeRDP_AutoAcceptCertificate, TRUE);
     return TRUE;
 }
 
@@ -708,10 +718,13 @@ DWORD orbitVerifyCertificateEx(freerdp* instance,
                                const char*,
                                DWORD)
 {
+    // With FreeRDP_AutoAcceptCertificate set, FreeRDP accepts and stores a
+    // first-seen certificate itself without calling this callback at all;
+    // it is kept as a defensive fallback for any code path that reaches it.
     if (instance != nullptr && instance->context != nullptr) {
         if (RdpSessionBackend* backend = backendFromContext(instance->context)) {
             emit backend->eventLogged(
-                QStringLiteral("Accepting server certificate for %1:%2.")
+                QStringLiteral("Trusting new server certificate for %1:%2.")
                     .arg(QString::fromUtf8(host == nullptr ? "" : host))
                     .arg(port));
         }
@@ -725,21 +738,42 @@ DWORD orbitVerifyChangedCertificateEx(freerdp* instance,
                                       const char*,
                                       const char*,
                                       const char*,
+                                      const char* newFingerprint,
                                       const char*,
                                       const char*,
-                                      const char*,
-                                      const char*,
+                                      const char* oldFingerprint,
                                       DWORD)
 {
+    // The server's certificate no longer matches the one trusted on a prior
+    // connection. This is exactly the scenario TLS verification exists to
+    // catch: either the server legitimately rotated its certificate, or an
+    // active man-in-the-middle is presenting a different one. Refuse the
+    // connection by default rather than silently trusting it.
+    const QString hostStr = QString::fromUtf8(host == nullptr ? "" : host);
+    const QString message = QStringLiteral(
+        "Server certificate for %1:%2 has changed since it was last trusted. "
+        "Connection refused for safety — this could mean the server's "
+        "certificate was legitimately renewed, or that a different host is "
+        "impersonating it. Previously trusted fingerprint: %3 — now "
+        "presented: %4. If this change is expected, remove the stored entry "
+        "for this host from FreeRDP's certificate store "
+        "(~/.config/freerdp/server) and reconnect.")
+        .arg(hostStr)
+        .arg(port)
+        .arg(QString::fromUtf8(oldFingerprint == nullptr ? "unknown" : oldFingerprint))
+        .arg(QString::fromUtf8(newFingerprint == nullptr ? "unknown" : newFingerprint));
+
+    // Not logged directly here: recordCertificateRejection() feeds this
+    // message into the generic connect-failure handling in workerMain(),
+    // which already reports it through eventLogged/connectionError/setState
+    // (the same channels every other RDP connection failure uses) — an
+    // extra direct log call here would just duplicate that.
     if (instance != nullptr && instance->context != nullptr) {
         if (RdpSessionBackend* backend = backendFromContext(instance->context)) {
-            emit backend->eventLogged(
-                QStringLiteral("Accepting changed server certificate for %1:%2.")
-                    .arg(QString::fromUtf8(host == nullptr ? "" : host))
-                    .arg(port));
+            backend->recordCertificateRejection(message);
         }
     }
-    return 1;
+    return 0;
 }
 
 const char* authReasonName(rdp_auth_reason reason)
@@ -1451,6 +1485,11 @@ void RdpSessionBackend::setState(SessionState state, const QString& message)
     emit eventLogged(message);
 }
 
+void RdpSessionBackend::recordCertificateRejection(const QString& reason)
+{
+    m_certificateRejectionReason = reason;
+}
+
 bool RdpSessionBackend::validateProfile(QString& message) const
 {
     const Profile& p = profile();
@@ -1510,6 +1549,7 @@ void RdpSessionBackend::workerMain()
     m_workerRunning.store(false);
     return;
 #else
+    m_certificateRejectionReason.clear();
     ensureFreeRdpRuntimeInitialized();
 
     freerdp* instance = freerdp_new();
@@ -1671,7 +1711,9 @@ void RdpSessionBackend::workerMain()
             emit eventLogged(QStringLiteral("RDP connect aborted: %1").arg(raw));
             emitStateAsync(SessionState::Disconnected, disconnectMessageForCode(code));
         } else {
-            const QString mapped = mapRdpError(code);
+            const QString mapped = m_certificateRejectionReason.isEmpty()
+                ? mapRdpError(code)
+                : m_certificateRejectionReason;
             emit eventLogged(QStringLiteral("RDP connect failure detail: %1").arg(raw));
             emitConnectionFailureAsync(mapped, raw);
             emitStateAsync(SessionState::Failed, mapped);

src/rdp_session_backend.h

@@ -90,6 +90,14 @@ private:
     void* m_cliprdrContext;
     QString m_pendingLocalClipboardText;
 
+    // Set synchronously by orbitVerifyChangedCertificateEx (called from this
+    // object's own worker thread during freerdp_connect) when a server's TLS
+    // certificate has changed since a prior trusted connection. Read back by
+    // workerMain() right after freerdp_connect() fails, to show the specific
+    // reason instead of a generic "TLS negotiation failed" message. Cleared
+    // at the start of every connect attempt.
+    QString m_certificateRejectionReason;
+
     void setState(SessionState state, const QString& message);
     bool validateProfile(QString& message) const;
     void startWorker();
@@ -109,6 +117,7 @@ public:
     void onCliprdrServerFormatList(bool hasUnicodeText);
     void onCliprdrServerFormatDataRequest(uint32_t requestedFormatId);
     void onCliprdrServerFormatDataResponse(bool success, const uint8_t* data, uint32_t size);
+    void recordCertificateRejection(const QString& reason);
 private:
     void emitStateAsync(SessionState state, const QString& message);
     void emitConnectionFailureAsync(const QString& displayMessage, const QString& rawMessage);