From a9930351c97b6a2ff59df259b96fafaf94f68294 Mon Sep 17 00:00:00 2001 From: Zac Bergquist Date: Sat, 24 Aug 2024 08:26:44 -0600 Subject: [PATCH] desktop access: improve error handling when directory sharing fails If drive redirection is disabled on the Windows side, then directory sharing will fail even if Teleport RBAC allows it. Prior to this change failure to share a directory was considered a fatal error that would terminate the session. This change updates the error message and makes the error a warning so that the session remains alive even if the directory sharing operation fails. --- .../rdp/rdpclient/src/rdpdr/filesystem.rs | 2 ++ web/packages/teleport/src/lib/tdp/client.ts | 16 ++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/srv/desktop/rdp/rdpclient/src/rdpdr/filesystem.rs b/lib/srv/desktop/rdp/rdpclient/src/rdpdr/filesystem.rs index 5210a704252e3..a954af5d0571e 100644 --- a/lib/srv/desktop/rdp/rdpclient/src/rdpdr/filesystem.rs +++ b/lib/srv/desktop/rdp/rdpclient/src/rdpdr/filesystem.rs @@ -79,6 +79,8 @@ impl FilesystemBackend { &mut self, res: efs::ServerDeviceAnnounceResponse, ) -> PduResult<()> { + // TODO(zmb3): send the underlying NTSTATUS code instead + // of converting everything to 0 or 1. let err_code = match res.result_code { NtStatus::SUCCESS => TdpErrCode::Nil, _ => TdpErrCode::Failed, diff --git a/web/packages/teleport/src/lib/tdp/client.ts b/web/packages/teleport/src/lib/tdp/client.ts index dc1496d68ec51..a415932b5e45c 100644 --- a/web/packages/teleport/src/lib/tdp/client.ts +++ b/web/packages/teleport/src/lib/tdp/client.ts @@ -397,10 +397,13 @@ export default class Client extends EventEmitterWebAuthnSender { handleSharedDirectoryAcknowledge(buffer: ArrayBuffer) { const ack = this.codec.decodeSharedDirectoryAcknowledge(buffer); if (ack.errCode !== SharedDirectoryErrCode.Nil) { - // TODO(zmb3): get a better error message here - this.handleError( - new Error(`Encountered shared directory error: ${ack.errCode}`), - TdpClientEvent.CLIENT_ERROR + // A failure in the acknowledge message means the directory + // share operation failed (likely due to server side configuration). + // Since this is not a fatal error, we emit a warning but otherwise + // keep the sesion alive. + this.handleWarning( + `Failed to share directory '${this.sdManager.getName()}', drive redirection may be disabled on the RDP server.`, + TdpClientEvent.TDP_WARNING ); return; } @@ -699,7 +702,8 @@ export default class Client extends EventEmitterWebAuthnSender { this.send(this.codec.encodeRdpResponsePDU(responseFrame)); } - // Emits an errType event, closing the socket if the error was fatal. + // Emits an errType event and closes the websocket connection. + // Should only be used for fatal errors. private handleError( err: Error, errType: TdpClientEvent.TDP_ERROR | TdpClientEvent.CLIENT_ERROR @@ -709,7 +713,7 @@ export default class Client extends EventEmitterWebAuthnSender { this.socket?.close(); } - // Emits an warnType event + // Emits a warning event, but keeps the socket open. private handleWarning( warning: string, warnType: TdpClientEvent.TDP_WARNING | TdpClientEvent.CLIENT_WARNING