feat(host): devolutions-session bootstrap #997

pacmancoder · 2024-08-27T12:51:41Z

This PR adds the following:

Bootstraps devolution-session executable, which will be hosting RDP dynamic virtual channel server for remote execution / other tasks specified in the NowProto protocol
- Basic config/logging
- MSRDPEX test code for further testing
Adds Logic to track sessions state and start DevolutionSession.exe in RDP sessions and terminating it after session termination
Add new WinAPI wrappers for process/session/permissions management which were required for new agent logic

Blocked by #1005

pacmancoder · 2024-08-27T15:03:22Z

ci/tlk.ps1

@@ -345,6 +345,7 @@ class TlkRecipe
                if ($this.Target.IsWindows()) {
                    $agentPackages += [TlkPackage]::new("devolutions-pedm-hook", "crates/devolutions-pedm-hook", $true)
                    $agentPackages += [TlkPackage]::new("devolutions-pedm-contextmenu", "crates/devolutions-pedm-contextmenu", $true)
+                    $agentPackages += [TlkPackage]::new("devolutions-host", "crates/devolutions-host", $false)


Minor changes just to build binary; Packaging + Installer changes will be in the follow-up PR soon

pacmancoder · 2024-08-27T15:04:57Z

crates/win-api-wrappers/src/process.rs

@@ -556,7 +673,18 @@ pub fn create_process_as_user(
    let application_name = application_name.map(WideString::from).unwrap_or_default();
    let current_directory = current_directory.map(WideString::from).unwrap_or_default();

-    let environment = environment.map(serialize_environment).transpose()?;
+    let environment = if let Some(env) = environment {


Now we correctly spawn the user's environment variables for the process if environment is not explicitly specified

pacmancoder · 2024-08-27T15:06:50Z

devolutions-agent/src/config.rs

        /// Devolutions PEDM configuration
        #[serde(default, skip_serializing_if = "Option::is_none")]
        pub pedm: Option<PedmConf>,

+        /// Devolutions Session Host configuration
+        #[serde(default, skip_serializing_if = "Option::is_none")]
+        pub session_host: Option<SessionHostConf>,


Naming is debatable - maybe someone has a better idea instead of "host"?

Some suggestions:

now_host: NowHostConf

now_dvc: NowDvcConf

remote_desktop_host: RemoteDesktopHostConf

remote_desktop_dvc: RemoteDesktopDvcConf

By the way, is the name "Devolutions Host" already agreed upon?
Should it be "Devolutions Now" or "Devolutions Now Host" or "Devolutions Now Dvc", or something else?

pacmancoder · 2024-08-27T15:09:24Z

devolutions-host/src/main.rs

@@ -0,0 +1,46 @@
+// Start the program without a console window.


Nothing fancy here yet; Just logic to initialize log and config and copy-pasted test logic for DVC from MSRDPEX

devolutions-host/Cargo.toml

CBenoit

Well documented, easy to read code. Thank you!

General style notes:

Cargo.toml

crates/win-api-wrappers/src/process.rs

crates/win-api-wrappers/src/token.rs

devolutions-agent/src/main.rs

devolutions-agent/src/service.rs

devolutions-agent/src/session_manager/mod.rs

CBenoit

I’ll leave you squash and merge when you are satisfied!

devolutions-agent/src/session_manager/mod.rs

crates/win-api-wrappers/src/process.rs

CBenoit · 2024-09-03T10:54:43Z

There is also this comment that you may want to check again: #997 (comment)

pacmancoder · 2024-09-03T13:17:40Z

There is also this comment that you may want to check again: #997 (comment)

Yep, I'll clarify the naming with Marc-Andre 👍

pacmancoder · 2024-09-03T14:51:27Z

NOTE: #1005 should be merged first; do not enable auto-merge

CBenoit · 2024-09-04T14:47:49Z

.github/workflows/ci.yml

+
+

nit: You may have added unintentional blank lines

pacmancoder · 2024-09-09T10:11:31Z

crates/win-api-wrappers/src/handle.rs

+    /// # Safety
+    ///
+    /// - The caller should ensure that `handle` is a pseudo handle, as its validity is not checked.
+    pub unsafe fn new_pseudo_handle(handle: HANDLE) -> Self {


@CBenoit FYI I was pulling my hair while trying to find why the process didn't want to start after my last refactoring, but it turned out that current_process function has been broken after #998 was merged :( I added the separate new_pseudo_handle function to create handle without invalid_handle check (CurrentProcess returns (HANDLE)-1 value)

CBenoit · 2024-09-09T15:34:41Z

package.ps1

@pacmancoder Can you document the purpose of this file, please? Is it an helper or something we use in the CI? Should it go in the ci folder or the tools folder instead of the root?

It was reverted already in the last commit, - I accidentally committed it

Oh, got you!

pacmancoder force-pushed the feat/agent-session-manager branch 5 times, most recently from 3131bbd to e3f5b1f Compare August 27, 2024 14:06

pacmancoder commented Aug 27, 2024

View reviewed changes

pacmancoder requested a review from CBenoit August 27, 2024 15:17

pacmancoder changed the title ~~Devolutions host bootstrap~~ feat(host): devolutions-host bootstrap Aug 27, 2024

pacmancoder force-pushed the feat/agent-session-manager branch from aa24d2b to 5d8a1e9 Compare August 27, 2024 15:22

pacmancoder marked this pull request as ready for review August 27, 2024 15:23

CBenoit reviewed Aug 28, 2024

View reviewed changes

devolutions-host/Cargo.toml Outdated Show resolved Hide resolved

CBenoit reviewed Aug 28, 2024

View reviewed changes

pacmancoder added 4 commits September 2, 2024 10:09

Devolutions host bootstrap

53951fa

Updated tlk to build devolutions-host

fc5c39e

Disable Devolutions host by default

22f9a9f

Fixed a bunch of lints

d0a6d57

pacmancoder force-pushed the feat/agent-session-manager branch 2 times, most recently from 7741b2a to 9bfc56e Compare September 2, 2024 08:03

refactoring: fixed minor comments

e53a0d3

pacmancoder force-pushed the feat/agent-session-manager branch from 9bfc56e to e53a0d3 Compare September 2, 2024 08:36

pacmancoder requested a review from CBenoit September 2, 2024 09:07

CBenoit approved these changes Sep 3, 2024

View reviewed changes

devolutions-agent/src/session_manager/mod.rs Outdated Show resolved Hide resolved

crates/win-api-wrappers/src/process.rs Show resolved Hide resolved

refactoring: more refactoring

a6c8089

pacmancoder mentioned this pull request Sep 3, 2024

feat(host): devolutions-host installer changes #1005

Merged

feat(host): devolutions-host installer changes (#1005)

c2eea77

pacmancoder requested review from a team as code owners September 4, 2024 14:45

CBenoit reviewed Sep 4, 2024

View reviewed changes

.github/workflows/ci.yml Outdated

Comment on lines 546 to 547

Copy link

Member

CBenoit Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You may have added unintentional blank lines

pacmancoder added 3 commits September 9, 2024 11:59

refactor: rename Devolutions Host to Devolutions Session

84b971a

fix: fixed Process::current_process from win-api-wrappers crate

45df947

refactor: minor renaming

2d243fb

pacmancoder commented Sep 9, 2024

View reviewed changes

pacmancoder changed the title ~~feat(host): devolutions-host bootstrap~~ feat(host): devolutions-session bootstrap Sep 9, 2024

pacmancoder merged commit f8b291d into master Sep 9, 2024
25 checks passed

pacmancoder deleted the feat/agent-session-manager branch September 9, 2024 11:17

CBenoit reviewed Sep 9, 2024

View reviewed changes

CBenoit mentioned this pull request Oct 2, 2024

chore(release): prepare for 2024.3.3 #1035

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(host): devolutions-session bootstrap #997

feat(host): devolutions-session bootstrap #997

pacmancoder commented Aug 27, 2024 •

edited

Loading

pacmancoder Aug 27, 2024

pacmancoder Aug 27, 2024

pacmancoder Aug 27, 2024

CBenoit Aug 28, 2024 •

edited

Loading

pacmancoder Aug 27, 2024

CBenoit left a comment

CBenoit left a comment

CBenoit commented Sep 3, 2024

pacmancoder commented Sep 3, 2024

pacmancoder commented Sep 3, 2024

CBenoit Sep 4, 2024

pacmancoder Sep 9, 2024

CBenoit Sep 9, 2024

pacmancoder Sep 9, 2024

CBenoit Sep 9, 2024

		@@ -0,0 +1,46 @@
		// Start the program without a console window.

feat(host): devolutions-session bootstrap #997

feat(host): devolutions-session bootstrap #997

Conversation

pacmancoder commented Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CBenoit Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CBenoit left a comment

Choose a reason for hiding this comment

CBenoit left a comment

Choose a reason for hiding this comment

CBenoit commented Sep 3, 2024

pacmancoder commented Sep 3, 2024

pacmancoder commented Sep 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pacmancoder commented Aug 27, 2024 •

edited

Loading

CBenoit Aug 28, 2024 •

edited

Loading