ReleaseContext and refactor handle_scard_call
#33434
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 11 commits
October 4, 2023 09:30
updating to the latest RdpdrBackend interface in the process.
Refactor handle_scard_call to validate ioctl code and call combinations. Add handling for ScardIoCtlCode::ReleaseContext. This ensures future compatibility with potential IronRDP changes.
probakowski
approved these changes
Oct 16, 2023
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 20, 2023
* adds ironrdp client and has it connect, alongside rdp-rs * Integrates IronRDP client into the TDP png-response system. Instead of sending back chunks of the screen, its sends the full screen every time, and this is my running hypothesis for the current sticking point: a "Maximum call stack size exceeded" from the tdp client in the react app * Changes arrayBufferToBase64 to use reduce, which fixes the "Maximum call stack size exceeded" bug * removes convert_bgra_to_rgba which isn't needed with the bitmaps being produced by IronRDP * Adds support for updating only a specific image region * removes mistaken read_rdp_output_inner call from connect_rdp_inner * Adds support for mouse input. I believe this code has race conditions for client.write_frame though this requires more research. * Patches things up with the new IronRDP structure Requires some modifications to IronRDP, see the corresponding commit hash for IronRDP in lib/srv/desktop/rdp/rdpclient/Cargo.toml. Started getting call stack overflows when manual testing again, but I'm going to ignore those for now and focus on the version of this architecture that passes the remoteFX encoded messages over TDP. * Adds RemoteFX Frame to the backend * Proof of concept for integrating a wasm module into the frontend Building the wasm module still needs to be done manually, this was just to get to a point where tools are configured and it can be successfully called in both dev and prod modes. Required that I comment out "default-src 'self'" in the CSP, this will need to be figured out later. We're also using a package called vite-plugin-wasm-pack whereas ironrdp uses vite-plugin-wasm. I'm not sure why, I'm going to look into this and swap out to vite-plugin-wasm if possible. * Swaps out vite-plugin-wasm-pack for vite-plugin-wasm and adds build-wasm script to the teleport package.json. vite-plugin-wasm-pack looks relatively unmaintained in comparison to vite-plugin-wasm-pack. build-wasm script is needed to build the wasm binary before calling vite [build] * Adds backend structure for RPC PoC Untested as of yet and it isn't pretty, but this more or less imitates the RPC approach we have for directory sharing. * hack solution which calls everything 'static. THIS IS VERY UNSAFE, WE SHOULD DO SOMETHING ELSE * Gets the full display working end to end with a particular commit of IronRDP. Unfortunately this commit is now well out of date, and IronRDP had some bugs that were preventing RemoteFX frames from being sent back. * Full display working end to end with the updated IronRDP * updates init_wasm_log to enable logging in the console * updates hardcoded io_channel_id and user_channel_id * Uses specific IronRDP sub-crates (i.e. ironrdp-session, ironrdp-tokio, etc.) rather than relying on whatever is re-exported from the ironrdp sub-crate itself * Gets FastPathFrameResponse up to speed and the PoC is now working * deletes all the rpc stuff which it turns out we don't need * more cleanup including consolidating the FastPathFrame message between wasm and js, adding some comments * Updates the documentation string for top level Client in lib.rs * Removes hardcoded rdp server address, screen size, and username * Swaps to online IronRDP repo * Updates the TDP RFD with new messages renaming them to be precise. * remove hardcoded ip * creates asRustBackedSlice * removing some old todos * Add cleanup for wasm-pack build artifacts to make clean * Fix wasm in production builds (#27428) * Converts the default Content-Security-Policy representation to a map which makes it easier to programmatically add-to/overwrite the defaults for special cases. Also adds tests for the various custom CSPs. * Alphabetize CSP directives for ease of testing/debugging in the future * makes maps more easily composable * removes commented code, and topLevelAwait() which was causing production builds to break * Adds 'wasm-unsafe-eval' to the script-src section of the Content-Security-Policy when a desktop session's url is detected. The app already enforces no caching via the Cache-Control, Pragma, and Expires headers (SetNoCacheHeaders), which means that any time the app is requested in a new tab, the browser will always request a new index.html which can be sent back with a new Content-Security-Policy header. Since the only way to get to a desktop session is to either click a link in the app which opens a new tab, or to open a new tab and navigate to the url manually (this differs from most components, which can be navigated to without a page reload using React Router), we can reliably check each index.html request for a characteristic desktop session url path, and only include the 'wasm-unsafe-eval' directive when strictly necessary (in the spirit of the principle of least privilege). * Removes vite-plugin-top-level-await from package.json * moves regex to global so that it only need be compiled once * CR fixes updates Cargo.lock removes commented feature flag removes -framework SystemConfiguration fixes makefile comment * unsafe.SliceData * fixes phrasing * changes to docstring * changes handle_tdp_fast_path_response to handle_tdp_rdp_response_pdu so that its name is in line with its definition in RFD 0037 and adds a more descriptive docstring. * small fixes to comments * preallocate RDPFastPathPDU Encode buffer * updating comments * Adds a detailed discussion of the Go/Rust Interface to the README (#29762) * Aligns the codebase with the Go/Rust interface principles in the README (#29764) * Move client functionality into Client (#29765) * Adds the RDP Channel IDs TDP message (#27514) * Refactoring `rdpclient` (#29766) * Ironrdp recording playback (#28228) * httplib.SetIndexContentSecurityPolicyWithWasm for recordings as well as desktop sessions so that the ironrdp wasm client can be used for recording playback * Adds desktopplayback path to websocket proxy definitions so that it can be used with the dev server * adds tdp.TypeRDPFastPathPDU as a tdp message to be recorded * With this commit, recordings can be played back with the FastPathProcessor Refactors tdp Client and subclass PlayerClient such that now the classes are always constructed the same way, and the internal representation of the screen size (the FastPathProcessor) is either: 1. Created upon the call to Client.connect(spec) when spec is !undefined. This is the case when the Client is being used for a desktop session. 2. Created when a client screen spec is received from the server. This is the case when PlayerClient is being used for playback. * Switches from sending the width/height via ur parameter string to sending it via the client screen spec tdp message. * changes tdpCli* to client* and adds a canvas* to the canvas related functions * Static tokio (#30066) * Beginnings of RdpdrBackend integration (#32814) * Adds handling for the EstablishContextCall (#32971) * Adds support for `ListReadersCall` and `ListReadersReturn` (#33004) * Adds support for `GetStatusChangeCall`/`GetStatusChangeReturn` (#33056) * Adds support for ConnectCall and ConnectReturn (#33136) * Better error handling (#32925) This change removes couple of unwrap that could cause panics on the Rust side and prevents panics from invalid cgo handle on the Go side. It also sends error message from IronRDP to UI and cleans up a little final sequence of closing various goroutines in case of error or sessions end. * Adds support for `HCardAndDispositionCall` with `ScardIoCtlCode::BeginTransaction` (#33192) * `TransmitCall` and `TransmitReturn` (#33282) * `StatusCall` and `StatusReturn` (#33284) * `ReleaseContext` and refactor `handle_scard_call` (#33434) * IronRDP clipboard sharing (#33335) This change adds clipboard sharing to our IronRDP migration using ironrdp_cliprdr crate with out TDP-based backend (reused from rdp-rs times) * Adds EndTransaction, Disconnect, Cancel, and IsValidContext (#33612) * Adds support for `GetDeviceTypeId` (#33613) * Adds support for `ReadCacheW` (#33618) * `WriteCacheW` (#33620) * Smartcard Login (#33712) * `GetReaderIcon` (#33779) * Update to IronRDP latest commit * Removes a bunch of old deprecated code (including `rdp-rs`) (#33786) * Pipes in `SharedDirectoryAnnounce` (#33911) * Use tokio-boring for TLS upgrade (#33900) This change switches TLS upgrade in our new RDP approach to tokio-boring in FIPS-compliant builds. It also adds runtime check for FIPS mode. Tested locally in custom Docker image. * Update sspi (#34558) * Break up rdpdr.rs, add support for the first directory sharing pdus (#34009) * Adds support for `SharedDirectoryCreateRequest` and `SharedDirectoryCreateResponse`. (#34011) * `SharedDirectoryDeleteRequest` and `SharedDirectoryDeleteResponse` (#34051) * remove leftover code * Server drive query information request (#34082) * Rdp client refactor (#34122) * Add handling for `DeviceCloseRequest` (#34139) * Updates to `IronRDP` latest (#34744) * Updates to IronRDP latest with a slightly different API * remove defunct comment * `ServerDriveQueryDirectoryRequest` and `SharedDirectoryListRequest/Response` (#34746) * `ServerDriveQueryVolumeInformationRequest` and `ClientDriveQueryVolumeInformationResponse` (#34871) * `DeviceControlRequest` (#34898) * `Read` (#34914) * `Write` (#34919) * `SetInformation` (#35092) * `LockControl` (#35146) * Stack pr feedback and other cleanup (#35306) * Adds desktop access websocket pattern to vite config * Updates IronRDP to latest, removes manual context strings in favor of automatically generated (by this latest IronRDP commit) (#35392) * Small line fixes, comment removal, changing license to AGPL * Update Go/Rust interface section of README (#35590) * Ironrdp manual docs (#35455) * Remove unnecessary whitespace * Refactor navigation paths in Active Directory documentation for clarity Replaced textual descriptions with formatted paths using `>` symbols for better readability. Having been subject to going through many such tutorials myself, I find this to be a more user-friendly way of presenting the information. It's very common to need to go back and forth between the documentation and the group policy manager, and having these paths in a format that can catch the eye quickly is a big help. This also appears to be common practice e.g. - https://softwarekeep.com/help-center/how-to-enable-remote-desktop-on-windows - https://www.helpwire.app/blog/remote-desktop-group-policy/ * Add instructions for enabling RemoteFX * Add an optional step to the end of each GPO update subsection This lets us point users to subsections individually without worrying about reminding them that they need to run a `gpupdate /force` each time (because its already right there in the subsection). * lowercase "open" * Updates configuration script to enable RemoteFX (#35461) * Restructure Cargo workspace (#35591) * Update `IronRDP` to latest, fix key log leak (#35707) Updates IronRDP to latest with the changes necessary to keep up with the changes in Devolutions/IronRDP#328. We select the `"rustls"` feature explicitly now, this is in line with our implicit choice of it as [the default](https://github.com/Devolutions/IronRDP/pull/325/files#diff-1f64036f3e460819b2b1e954803b7f266feca0e2787ca0614ae1253323944f59L19) before these latest changes. Also creates a custom debug for `ScardBackend` so as to not leak `key_der` in the logs. * Don't leak pin in the logs * Updates CI to fix broken checks and builds caused by WASM introduction (#35401) * removes SystemConfiguration framework (not sure why it was there). * Updates cspell * fixes TestSetRedirectPageContentSecurityPolicy * Updates e ref to a currently-draft-PR ref. * Fix per session mfa (#35902) * Updating e-ref to tip of `master` * dependency-review: Ignore cargo dependencies with invalid license specifications * Updates OSS build-macos.yaml to support wasm-pack --------- Co-authored-by: Przemko Robakowski <przemko.robakowski@goteleport.com> Co-authored-by: Mike Jensen <mike.jensen@goteleport.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 21, 2023
* adds ironrdp client and has it connect, alongside rdp-rs * Integrates IronRDP client into the TDP png-response system. Instead of sending back chunks of the screen, its sends the full screen every time, and this is my running hypothesis for the current sticking point: a "Maximum call stack size exceeded" from the tdp client in the react app * Changes arrayBufferToBase64 to use reduce, which fixes the "Maximum call stack size exceeded" bug * removes convert_bgra_to_rgba which isn't needed with the bitmaps being produced by IronRDP * Adds support for updating only a specific image region * removes mistaken read_rdp_output_inner call from connect_rdp_inner * Adds support for mouse input. I believe this code has race conditions for client.write_frame though this requires more research. * Patches things up with the new IronRDP structure Requires some modifications to IronRDP, see the corresponding commit hash for IronRDP in lib/srv/desktop/rdp/rdpclient/Cargo.toml. Started getting call stack overflows when manual testing again, but I'm going to ignore those for now and focus on the version of this architecture that passes the remoteFX encoded messages over TDP. * Adds RemoteFX Frame to the backend * Proof of concept for integrating a wasm module into the frontend Building the wasm module still needs to be done manually, this was just to get to a point where tools are configured and it can be successfully called in both dev and prod modes. Required that I comment out "default-src 'self'" in the CSP, this will need to be figured out later. We're also using a package called vite-plugin-wasm-pack whereas ironrdp uses vite-plugin-wasm. I'm not sure why, I'm going to look into this and swap out to vite-plugin-wasm if possible. * Swaps out vite-plugin-wasm-pack for vite-plugin-wasm and adds build-wasm script to the teleport package.json. vite-plugin-wasm-pack looks relatively unmaintained in comparison to vite-plugin-wasm-pack. build-wasm script is needed to build the wasm binary before calling vite [build] * Adds backend structure for RPC PoC Untested as of yet and it isn't pretty, but this more or less imitates the RPC approach we have for directory sharing. * hack solution which calls everything 'static. THIS IS VERY UNSAFE, WE SHOULD DO SOMETHING ELSE * Gets the full display working end to end with a particular commit of IronRDP. Unfortunately this commit is now well out of date, and IronRDP had some bugs that were preventing RemoteFX frames from being sent back. * Full display working end to end with the updated IronRDP * updates init_wasm_log to enable logging in the console * updates hardcoded io_channel_id and user_channel_id * Uses specific IronRDP sub-crates (i.e. ironrdp-session, ironrdp-tokio, etc.) rather than relying on whatever is re-exported from the ironrdp sub-crate itself * Gets FastPathFrameResponse up to speed and the PoC is now working * deletes all the rpc stuff which it turns out we don't need * more cleanup including consolidating the FastPathFrame message between wasm and js, adding some comments * Updates the documentation string for top level Client in lib.rs * Removes hardcoded rdp server address, screen size, and username * Swaps to online IronRDP repo * Updates the TDP RFD with new messages renaming them to be precise. * remove hardcoded ip * creates asRustBackedSlice * removing some old todos * Add cleanup for wasm-pack build artifacts to make clean * Fix wasm in production builds (#27428) * Converts the default Content-Security-Policy representation to a map which makes it easier to programmatically add-to/overwrite the defaults for special cases. Also adds tests for the various custom CSPs. * Alphabetize CSP directives for ease of testing/debugging in the future * makes maps more easily composable * removes commented code, and topLevelAwait() which was causing production builds to break * Adds 'wasm-unsafe-eval' to the script-src section of the Content-Security-Policy when a desktop session's url is detected. The app already enforces no caching via the Cache-Control, Pragma, and Expires headers (SetNoCacheHeaders), which means that any time the app is requested in a new tab, the browser will always request a new index.html which can be sent back with a new Content-Security-Policy header. Since the only way to get to a desktop session is to either click a link in the app which opens a new tab, or to open a new tab and navigate to the url manually (this differs from most components, which can be navigated to without a page reload using React Router), we can reliably check each index.html request for a characteristic desktop session url path, and only include the 'wasm-unsafe-eval' directive when strictly necessary (in the spirit of the principle of least privilege). * Removes vite-plugin-top-level-await from package.json * moves regex to global so that it only need be compiled once * CR fixes updates Cargo.lock removes commented feature flag removes -framework SystemConfiguration fixes makefile comment * unsafe.SliceData * fixes phrasing * changes to docstring * changes handle_tdp_fast_path_response to handle_tdp_rdp_response_pdu so that its name is in line with its definition in RFD 0037 and adds a more descriptive docstring. * small fixes to comments * preallocate RDPFastPathPDU Encode buffer * updating comments * Adds a detailed discussion of the Go/Rust Interface to the README (#29762) * Aligns the codebase with the Go/Rust interface principles in the README (#29764) * Move client functionality into Client (#29765) * Adds the RDP Channel IDs TDP message (#27514) * Refactoring `rdpclient` (#29766) * Ironrdp recording playback (#28228) * httplib.SetIndexContentSecurityPolicyWithWasm for recordings as well as desktop sessions so that the ironrdp wasm client can be used for recording playback * Adds desktopplayback path to websocket proxy definitions so that it can be used with the dev server * adds tdp.TypeRDPFastPathPDU as a tdp message to be recorded * With this commit, recordings can be played back with the FastPathProcessor Refactors tdp Client and subclass PlayerClient such that now the classes are always constructed the same way, and the internal representation of the screen size (the FastPathProcessor) is either: 1. Created upon the call to Client.connect(spec) when spec is !undefined. This is the case when the Client is being used for a desktop session. 2. Created when a client screen spec is received from the server. This is the case when PlayerClient is being used for playback. * Switches from sending the width/height via ur parameter string to sending it via the client screen spec tdp message. * changes tdpCli* to client* and adds a canvas* to the canvas related functions * Static tokio (#30066) * Beginnings of RdpdrBackend integration (#32814) * Adds handling for the EstablishContextCall (#32971) * Adds support for `ListReadersCall` and `ListReadersReturn` (#33004) * Adds support for `GetStatusChangeCall`/`GetStatusChangeReturn` (#33056) * Adds support for ConnectCall and ConnectReturn (#33136) * Better error handling (#32925) This change removes couple of unwrap that could cause panics on the Rust side and prevents panics from invalid cgo handle on the Go side. It also sends error message from IronRDP to UI and cleans up a little final sequence of closing various goroutines in case of error or sessions end. * Adds support for `HCardAndDispositionCall` with `ScardIoCtlCode::BeginTransaction` (#33192) * `TransmitCall` and `TransmitReturn` (#33282) * `StatusCall` and `StatusReturn` (#33284) * `ReleaseContext` and refactor `handle_scard_call` (#33434) * IronRDP clipboard sharing (#33335) This change adds clipboard sharing to our IronRDP migration using ironrdp_cliprdr crate with out TDP-based backend (reused from rdp-rs times) * Adds EndTransaction, Disconnect, Cancel, and IsValidContext (#33612) * Adds support for `GetDeviceTypeId` (#33613) * Adds support for `ReadCacheW` (#33618) * `WriteCacheW` (#33620) * Smartcard Login (#33712) * `GetReaderIcon` (#33779) * Update to IronRDP latest commit * Removes a bunch of old deprecated code (including `rdp-rs`) (#33786) * Pipes in `SharedDirectoryAnnounce` (#33911) * Use tokio-boring for TLS upgrade (#33900) This change switches TLS upgrade in our new RDP approach to tokio-boring in FIPS-compliant builds. It also adds runtime check for FIPS mode. Tested locally in custom Docker image. * Update sspi (#34558) * Break up rdpdr.rs, add support for the first directory sharing pdus (#34009) * Adds support for `SharedDirectoryCreateRequest` and `SharedDirectoryCreateResponse`. (#34011) * `SharedDirectoryDeleteRequest` and `SharedDirectoryDeleteResponse` (#34051) * remove leftover code * Server drive query information request (#34082) * Rdp client refactor (#34122) * Add handling for `DeviceCloseRequest` (#34139) * Updates to `IronRDP` latest (#34744) * Updates to IronRDP latest with a slightly different API * remove defunct comment * `ServerDriveQueryDirectoryRequest` and `SharedDirectoryListRequest/Response` (#34746) * `ServerDriveQueryVolumeInformationRequest` and `ClientDriveQueryVolumeInformationResponse` (#34871) * `DeviceControlRequest` (#34898) * `Read` (#34914) * `Write` (#34919) * `SetInformation` (#35092) * `LockControl` (#35146) * Stack pr feedback and other cleanup (#35306) * Adds desktop access websocket pattern to vite config * Updates IronRDP to latest, removes manual context strings in favor of automatically generated (by this latest IronRDP commit) (#35392) * Small line fixes, comment removal, changing license to AGPL * Update Go/Rust interface section of README (#35590) * Ironrdp manual docs (#35455) * Remove unnecessary whitespace * Refactor navigation paths in Active Directory documentation for clarity Replaced textual descriptions with formatted paths using `>` symbols for better readability. Having been subject to going through many such tutorials myself, I find this to be a more user-friendly way of presenting the information. It's very common to need to go back and forth between the documentation and the group policy manager, and having these paths in a format that can catch the eye quickly is a big help. This also appears to be common practice e.g. - https://softwarekeep.com/help-center/how-to-enable-remote-desktop-on-windows - https://www.helpwire.app/blog/remote-desktop-group-policy/ * Add instructions for enabling RemoteFX * Add an optional step to the end of each GPO update subsection This lets us point users to subsections individually without worrying about reminding them that they need to run a `gpupdate /force` each time (because its already right there in the subsection). * lowercase "open" * Updates configuration script to enable RemoteFX (#35461) * Restructure Cargo workspace (#35591) * Update `IronRDP` to latest, fix key log leak (#35707) Updates IronRDP to latest with the changes necessary to keep up with the changes in Devolutions/IronRDP#328. We select the `"rustls"` feature explicitly now, this is in line with our implicit choice of it as [the default](https://github.com/Devolutions/IronRDP/pull/325/files#diff-1f64036f3e460819b2b1e954803b7f266feca0e2787ca0614ae1253323944f59L19) before these latest changes. Also creates a custom debug for `ScardBackend` so as to not leak `key_der` in the logs. * Don't leak pin in the logs * Show clear error when export can't be done * Show url of recording in webui * patching up merge mistake * use EndTime and StartTime to calculate duration --------- Co-authored-by: Isaiah Becker-Mayer <isaiah@goteleport.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor handle_scard_call to validate ioctl code and call combinations. Add handling for ScardIoCtlCode::ReleaseContext. This ensures future compatibility with potential IronRDP changes.