Skip to content

security issues #13

Open
saksham-45 wants to merge 1 commit into
danchitnis:masterfrom
saksham-45:master
Open

security issues #13
saksham-45 wants to merge 1 commit into
danchitnis:masterfrom
saksham-45:master

Conversation

@saksham-45
Copy link
Copy Markdown

Forced TLS, Environment Variable Support, and Inj…ection Protection

  • Added XRDP_USERS env var support for secure credential handling
  • Forced TLS security layer in xrdp.ini
  • Implemented automatic TLS certificate generation in entrypoints.
  • better shell scripts with quoting and -- separators to prevent argument injection
  • updated README with secure usage guidelines
  • Synchronized hardened config across Ubuntu, Fedora, and CentOS variants
  1. Environment Variable Support (XRDP_USERS)
  • Original Weakness: All user credentials were passed as command-line arguments. In Linux,
    ps aux makes these arguments visible to any user on the host. Furthermore, docker ps stores
    them in the container's metadata indefinitely.
  • The Fix: Support for XRDP_USERS="user:pass:yes".
  • Reasoning: Environment variables are more secure as they aren't visible in the process
    list of the host. This prevents "neighbor" users on a shared development server or CI/CD
    runner from sniffing plaintext passwords.
  1. Command Injection Prevention (-- and Quoting)
  • Original Weakness: Using useradd $1 allowed "Flag Injection." An attacker could provide
    a username like -o -u 0 root2. The shell expands this to useradd -o -u 0 root2, which
    creates a second root user with full administrative privileges.
  • The Fix: Implemented useradd -- "$username" and strict quoting of all shell variables.
  • Reasoning: The -- separator tells the system that everything following it is a
    positional argument (a name), not a command flag. This eliminates a critical "Zero-to-Root"
    privilege escalation vulnerability in the entrypoint logic.
  1. Forced TLS Security (security_layer=tls)
  • Original Weakness: The default negotiate mode allowed for "Downgrade Attacks."
    An active adversary on the network could intercept the RDP handshake and force it into
    an unencrypted RDP security layer, allowing them to sniff the session content and credentials.
  • The Fix: Hardcoded security_layer=tls in xrdp.ini.
  • Reasoning: This enforces mandatory encryption for all sessions. By removing negotiate
    and rdp modes, we ensure that an insecure connection will be rejected rather than
    allowed, protecting the user's data confidentiality.
  1. Automated Cryptographic Identity
  • Original Weakness: Xrdp requires SSL certificates to enable TLS. Without them, the service
    would either fail or fallback to insecure modes. Managing these certs manually is a friction
    point that often leads users to disable security entirely.
  • The Fix: Added an automatic certificate generation routine using openssl in the
    startup scripts.
  • Reasoning: By generating a unique 2048-bit RSA key pair on the fly if one isn't
    mounted, we provide "Security without Friction." The user gets an encrypted session
    immediately without needing to understand certificate management.
  1. Standardized Configuration Across Variants
  • Original Weakness: Ubuntu, Fedora, and CentOS variants had inconsistent configurations.
    The Ubuntu image was relying on system defaults, which were less hardened than our custom
    xrdp.ini.
  • The Fix: Synchronized the hardened xrdp.ini and supporting dependencies (openssl)
    across all Dockerfiles.
  • Reasoning: This ensures a consistent security posture. No matter which image tag
    a user pulls, the same audited security controls are in place, minimizing the "attack
    surface" across the entire repository.
    Every software shuld be safe software :)

…ection Protection

- Added XRDP_USERS env var support for secure credential handling
- Forced TLS security layer in xrdp.ini
- Implemented automatic TLS certificate generation in entrypoints
- Hardened shell scripts with quoting and -- separators to prevent argument injection
- Updated README with secure usage guidelines
- Synchronized hardened config across Ubuntu, Fedora, and CentOS variants
@danchitnis
Copy link
Copy Markdown
Owner

@saksham-45 thanks for this. The original insecure command-line for user and pass is intentional, as the main purpose of this library is to test a multi-user setup and is not intended for actual users. So, I don't want to make testing more complicated with added security layers. However, I see that some users like to use it in a production environment. In that case, it is better to add the fixes above as additional features, as long as they don't compromise the original simplicity of adding users.

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.

2 participants