-
Notifications
You must be signed in to change notification settings - Fork 18
feat: integrate GNOME Remote Desktop setup script #131
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
Conversation
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: bluefin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a setup script for GNOME Remote Desktop and a just recipe to execute it. The script automates the configuration of SELinux, firewall, and user settings for RDP. My review has identified a critical security vulnerability related to how RDP credentials are handled, and a high-severity issue concerning automatic package installation without user confirmation. I have provided suggestions to address both issues.
| while true; do | ||
| read -s -p "Enter RDP Password: " RDP_PASS | ||
| echo | ||
| read -s -p "Confirm RDP Password: " RDP_PASS_CONFIRM | ||
| echo | ||
| [ "$RDP_PASS" = "$RDP_PASS_CONFIRM" ] && break | ||
| echo "Passwords do not match. Please try again." | ||
| done | ||
|
|
||
| # Set credentials using grdctl | ||
| echo "Setting credentials..." | ||
| grdctl rdp set-credentials "$RDP_USER" "$RDP_PASS" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing the password as a command-line argument is a critical security vulnerability. It exposes the password in the system's process list (e.g., via ps aux), making it readable by other users on the system. You should modify the script to let grdctl handle password input securely. When invoked with only a username, grdctl will prompt for the password interactively without exposing it.
# Set credentials using grdctl
echo "Setting credentials..."
grdctl rdp set-credentials "$RDP_USER"
| # Check for checkmodule availability | ||
| if ! command -v checkmodule &> /dev/null; then | ||
| echo "Installing policycoreutils-python-utils (required for SELinux compilation)..." | ||
| sudo dnf install -y policycoreutils-python-utils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Automatically installing packages with -y bypasses user confirmation. This can be unexpected for users running the script and is a potential security risk. It's safer to remove the -y flag and let dnf handle the confirmation prompt, giving the user control over system changes.
sudo dnf install policycoreutils-python-utils
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR integrates a GNOME Remote Desktop setup script into the Bluefin system by adding an automated configuration script and making it accessible through a ujust recipe.
Key Changes:
- Adds a comprehensive shell script that automates GNOME Remote Desktop (RDP) setup including SELinux configuration, firewall rules, user credentials, and service management
- Integrates the script into the just command system with a new
setup-remote-desktoprecipe
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| system_files/bluefin/usr/share/ublue-os/just/system.just | Adds new setup-remote-desktop recipe to expose the RDP setup script through ujust |
| system_files/bluefin/usr/bin/ublue-setup-gnome-remote-desktop | New bash script that automates the complete setup of GNOME Remote Desktop including SELinux policies, firewall configuration, credential management, and service enablement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I don't use remote desktop? What's this fix? Seems like a lot for something that should just work ootb? Am I missing something? |
|
At least on LTS the firewall and SELinux need to be fixed. The SELinux issue should be fixed upstream. But opening the port on the firewall should be done manually |
|
We shouldn't redistribute this as this will edit SELinux stuff that should be fixed upstream instead. Maybe this could be on its own repository or something |
Fixes ublue-os/bluefin-lts#772
This PR integrates a GNOME Remote Desktop setup script and adds a corresponding ujust recipe to system.just.