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

Warns when running TGS as root #1882

Merged
merged 20 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ EXPOSE 5000

ENV General__ValidInstancePaths__0 /tgs_instances
ENV FileLogging__Directory /tgs_logs
ENV Internal__UsingDocker true

WORKDIR /app

Expand Down
13 changes: 13 additions & 0 deletions src/Tgstation.Server.Host/Components/InstanceManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ sealed class InstanceManager :
/// </summary>
readonly SwarmConfiguration swarmConfiguration;

/// <summary>
/// The <see cref="InternalConfiguration"/> for the <see cref="InstanceManager"/>.
/// </summary>
readonly InternalConfiguration internalConfiguration;

/// <summary>
/// The <see cref="TaskCompletionSource"/> for <see cref="Ready"/>.
/// </summary>
Expand Down Expand Up @@ -177,6 +182,7 @@ sealed class InstanceManager :
/// <param name="platformIdentifier">The value of <see cref="platformIdentifier"/>.</param>
/// <param name="generalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="generalConfiguration"/>.</param>
/// <param name="swarmConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="swarmConfiguration"/>.</param>
/// <param name="internalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="internalConfiguration"/>.</param>
/// <param name="logger">The value of <see cref="logger"/>.</param>
public InstanceManager(
IInstanceFactory instanceFactory,
Expand All @@ -193,6 +199,7 @@ public InstanceManager(
IPlatformIdentifier platformIdentifier,
IOptions<GeneralConfiguration> generalConfigurationOptions,
IOptions<SwarmConfiguration> swarmConfigurationOptions,
IOptions<InternalConfiguration> internalConfigurationOptions,
ILogger<InstanceManager> logger)
{
this.instanceFactory = instanceFactory ?? throw new ArgumentNullException(nameof(instanceFactory));
Expand All @@ -209,6 +216,7 @@ public InstanceManager(
this.platformIdentifier = platformIdentifier ?? throw new ArgumentNullException(nameof(platformIdentifier));
generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions));
swarmConfiguration = swarmConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(swarmConfigurationOptions));
internalConfiguration = internalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(internalConfigurationOptions));
this.logger = logger ?? throw new ArgumentNullException(nameof(logger));

originalConsoleTitle = console.Title;
Expand Down Expand Up @@ -675,6 +683,11 @@ void CheckSystemCompatibility()
{
if (!systemIdentity.CanCreateSymlinks)
throw new InvalidOperationException($"The user running {Constants.CanonicalPackageName} cannot create symlinks! Please try running as an administrative user!");

if (!platformIdentifier.IsWindows && systemIdentity.IsSuperUser && !internalConfiguration.UsingDocker)
{
logger.LogWarning("TGS is being run as the root account. This is not recommended.");
}
}

// This runs before the real socket is opened, ensures we don't perform reattaches unless we're fairly certain the bind won't fail
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace Tgstation.Server.Host.Configuration
using System;

namespace Tgstation.Server.Host.Configuration
{
/// <summary>
/// Unstable configuration options used internally by TGS.
Expand All @@ -25,6 +27,11 @@ public sealed class InternalConfiguration
/// </summary>
public bool UsingSystemD { get; set; }

/// <summary>
/// If the server is running inside of a Docker container.
/// </summary>
public bool UsingDocker => Environment.GetEnvironmentVariable("Internal__UsingDocker")?.Equals("true", StringComparison.OrdinalIgnoreCase) ?? false;

ZephyrTFA marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// The base path for the app settings configuration files.
/// </summary>
Expand Down
6 changes: 6 additions & 0 deletions src/Tgstation.Server.Host/Security/ISystemIdentity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ public interface ISystemIdentity : IDisposable
/// </summary>
bool CanCreateSymlinks { get; }

/// <summary>
/// Is this identity a SuperUser for the OS.
/// See Administrator on Windows or root on Linux.
/// </summary>
bool IsSuperUser { get; }

/// <summary>
/// Clone the <see cref="ISystemIdentity"/> creating another copy that must have <see cref="IDisposable.Dispose"/> called on it.
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions src/Tgstation.Server.Host/Security/PosixSystemIdentity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,18 @@
using System.Threading;
using System.Threading.Tasks;

using Mono.Unix.Native;

namespace Tgstation.Server.Host.Security
{
/// <summary>
/// <see cref="ISystemIdentity"/> for POSIX systems.
/// </summary>
sealed class PosixSystemIdentity : ISystemIdentity
{
/// <inheritdoc />
public bool IsSuperUser => Syscall.getuid() == 0;

/// <inheritdoc />
public string Uid => throw new NotImplementedException();

Expand Down
11 changes: 7 additions & 4 deletions src/Tgstation.Server.Host/Security/WindowsSystemIdentity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ sealed class WindowsSystemIdentity : ISystemIdentity
public string Username => userPrincipal?.Name ?? identity!.Name;

/// <inheritdoc />
public bool CanCreateSymlinks => canCreateSymlinks ?? throw new NotSupportedException();
public bool CanCreateSymlinks => IsSuperUser;

/// <inheritdoc />
public bool IsSuperUser => isAdmin ?? throw new NotSupportedException();

/// <summary>
/// The <see cref="WindowsIdentity"/> for the <see cref="WindowsSystemIdentity"/>.
Expand All @@ -35,9 +38,9 @@ sealed class WindowsSystemIdentity : ISystemIdentity
readonly UserPrincipal? userPrincipal;

/// <summary>
/// Backing field for <see cref="CanCreateSymlinks"/>.
/// Backing field for <see cref="IsSuperUser"/>.
/// </summary>
readonly bool? canCreateSymlinks;
readonly bool? isAdmin;

/// <summary>
/// Initializes a new instance of the <see cref="WindowsSystemIdentity"/> class.
Expand All @@ -49,7 +52,7 @@ public WindowsSystemIdentity(WindowsIdentity identity)
if (identity.IsAnonymous)
throw new ArgumentException($"Cannot use anonymous {nameof(WindowsIdentity)} as a {nameof(WindowsSystemIdentity)}!", nameof(identity));

canCreateSymlinks = new WindowsPrincipal(identity).IsInRole(WindowsBuiltInRole.Administrator);
isAdmin = new WindowsPrincipal(identity).IsInRole(WindowsBuiltInRole.Administrator);
}

/// <summary>
Expand Down
Loading