Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for x11 forwarding #231

Merged
merged 4 commits into from
Sep 4, 2024
Merged

Adding support for x11 forwarding #231

merged 4 commits into from
Sep 4, 2024

Conversation

Xenorf
Copy link

@Xenorf Xenorf commented Aug 30, 2024

Description

This PR aims to bring support for X11 forwarding when the host is accessed by SSH from a remote location running a X server.

On top of xhost ACLs I created a secret sharing mechanism between the host and the Exegol container.

Related issues

Some people on the discord server were asking about forwarding the GUI of Exegol containers to a remote location.

Point of attention

Tested setups

I tried this solution with only 2 setups:

  • Linux ==SSH==> Linux ==WRAPPER==> Exegol container
  • Windows ==SSH==> Linux ==WRAPPER==> Exegol container

The following setups are yet to be tested:

  • ? ==SSH==> Mac ==WRAPPER==> Exegol container
  • ? ==SSH==> Windows ==WRAPPER==> Exegol container

Library added

I imported the subprocess library to get the output of a command. An alternative would be to create a file in the container workspace to pass the xauth cookie with os.system().

X11UseLocalhost limitation

The X11UseLocalhost directive has its importance. When it is set to "yes", the X11 server will be listening on the loopback interface only. This causes a limitation, when the network interface of an Exegol container is bridged, it won't be able to forward its GUI to the remote server.

xhost ACLs

xhost ACLs need to be applied on the machine running the X server. Because it is no longer the host when accessing remotely, it is not needed. Moreover, the xhost commands cannot be executed from a SSH shell.

Copy link
Member

@Dramelac Dramelac left a comment

Choose a reason for hiding this comment

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

This has been a much-requested feature for a long time, thanks for finding the time to PoC a way to make it all work!
I've some questions / comments to improve the code before merging it, don't hesitate if you have anyquestions about them!

exegol/model/ExegolContainer.py Outdated Show resolved Hide resolved
exegol/model/ExegolContainer.py Show resolved Hide resolved
exegol/model/ExegolContainer.py Outdated Show resolved Hide resolved
exegol/model/ExegolContainer.py Show resolved Hide resolved
exegol/model/ExegolContainer.py Outdated Show resolved Hide resolved
exegol/model/ExegolContainer.py Show resolved Hide resolved
exegol/model/ExegolContainer.py Show resolved Hide resolved
@Xenorf Xenorf requested a review from Dramelac August 31, 2024 19:46
Copy link
Member

@Dramelac Dramelac left a comment

Choose a reason for hiding this comment

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

Thank you for the first update. Some more comments and it should be fine thank you!
If you have time to add more comments on your major code blocks for later code review it would be nice too !

exegol/model/ExegolContainer.py Outdated Show resolved Hide resolved
exegol/model/ExegolContainer.py Outdated Show resolved Hide resolved
exegol/model/ExegolContainer.py Outdated Show resolved Hide resolved
exegol/model/ExegolContainer.py Outdated Show resolved Hide resolved
@Xenorf
Copy link
Author

Xenorf commented Sep 1, 2024

Hey there, thank you for your reviews. I tried to implement your remarks. Don't hesitate to make more comments if you wish.

@Xenorf Xenorf requested a review from Dramelac September 1, 2024 09:47
Copy link
Member

@ShutdownRepo ShutdownRepo left a comment

Choose a reason for hiding this comment

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

Fyi, when using macOS with XQuartz, the DISPLAY env var looks like this /private/tmp/com.apple.launchd.FU1rU095fG/org.xquartz:0

@Xenorf
Copy link
Author

Xenorf commented Sep 4, 2024

Fyi, when using macOS with XQuartz, the DISPLAY env var looks like this /private/tmp/com.apple.launchd.FU1rU095fG/org.xquartz:0

Thank you for this information @ShutdownRepo.
Is it also like this when you SSH to the macOS environment with a command like ssh -X?

@Xenorf
Copy link
Author

Xenorf commented Sep 4, 2024

Because the macOS DISPLAY variable is so specific and I cannot test it, I made a change. If the host is a macOS it does not support x11 forwarding. But at least it doesn't break the previous implementation with the xhost ACLs.

I still think this PR is valuable for the project because in your documentation you recommend to use Linux as a host to avoid Docker limitations. Here Linux distributions are fully supported and Windows might be with very few tweaks.

Copy link
Member

@Dramelac Dramelac left a comment

Choose a reason for hiding this comment

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

All good to me, thanks for the changes!

@Dramelac
Copy link
Member

Dramelac commented Sep 4, 2024

Thank you @Xenorf for this update ! I'll merge it in dev branch for user to test etc.

@Dramelac Dramelac merged commit 943180a into ThePorgs:dev Sep 4, 2024
17 checks passed
@Xenorf Xenorf deleted the dev branch September 4, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants