From 15737ad6395e69222815859fac679faa171dd83e Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Tue, 2 Jun 2026 10:35:13 -0500 Subject: [PATCH] Harden remote web server security All changes are localized to RemoteWebServer plus two small helpers; the Phase 0 characterization tests confirm routing, JSON/Prometheus shape, and credential pass/fail semantics are otherwise unchanged. - Password hashing: new PasswordHasher uses PBKDF2-HMAC-SHA256 with a per-credential random salt (self-describing pbkdf2$iters$salt$hash). Verify() still accepts the legacy unsalted SHA-256 hex hash and a successful legacy auth transparently upgrades the stored hash, persisted by the view model on save/shutdown. Property renamed PasswordSHA256 -> PasswordHash. - Constant-time comparison: CredentialComparer.FixedTimeEquals for the user name and password hash; both are evaluated fully (no && short-circuit). - No information disclosure: POST failures return a generic message instead of ex.ToString(); detail is logged server-side only. - Bind intent respected: ResolveListenerIp no longer mutates ListenerIp or silently falls back to all-interfaces for a specific configured address (auto/'?'/wildcards still bind all). A bad address now fails Start(). - CORS: removed the Access-Control-Allow-Origin '*' wildcard; common response headers centralized in WriteCommonHeaders. - Prometheus: label values are escaped (EscapePrometheusLabel). 185 tests pass (was 167). Co-Authored-By: Claude Opus 4.8 --- .../Services/CredentialComparerTests.cs | 27 ++++++ .../Services/PasswordHasherTests.cs | 70 +++++++++++++++ .../Services/RemoteWebServerTests.cs | 36 ++++++++ .../Services/CredentialComparer.cs | 20 +++++ .../Services/PasswordHasher.cs | 87 +++++++++++++++++++ .../Services/RemoteWebServer.cs | 78 +++++++++++------ .../ViewModels/MainWindowViewModel.cs | 4 +- 7 files changed, 294 insertions(+), 28 deletions(-) create mode 100644 LibreHardwareMonitor.Windows.WinUI.Tests/Services/CredentialComparerTests.cs create mode 100644 LibreHardwareMonitor.Windows.WinUI.Tests/Services/PasswordHasherTests.cs create mode 100644 LibreHardwareMonitor.Windows.WinUI/Services/CredentialComparer.cs create mode 100644 LibreHardwareMonitor.Windows.WinUI/Services/PasswordHasher.cs diff --git a/LibreHardwareMonitor.Windows.WinUI.Tests/Services/CredentialComparerTests.cs b/LibreHardwareMonitor.Windows.WinUI.Tests/Services/CredentialComparerTests.cs new file mode 100644 index 0000000..c9ef9f3 --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI.Tests/Services/CredentialComparerTests.cs @@ -0,0 +1,27 @@ +// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Copyright (C) LibreHardwareMonitor and Contributors. + +using LibreHardwareMonitor.Windows.WinUI.Services; +using Xunit; + +namespace LibreHardwareMonitor.Windows.WinUI.Tests.Services; + +public class CredentialComparerTests +{ + [Fact] + public void FixedTimeEquals_EqualStrings_ReturnsTrue() + { + Assert.True(CredentialComparer.FixedTimeEquals("admin", "admin")); + } + + [Theory] + [InlineData("admin", "Admin")] + [InlineData("admin", "root")] + [InlineData("admin", "administrator")] + [InlineData("", "x")] + public void FixedTimeEquals_DifferentStrings_ReturnsFalse(string left, string right) + { + Assert.False(CredentialComparer.FixedTimeEquals(left, right)); + } +} diff --git a/LibreHardwareMonitor.Windows.WinUI.Tests/Services/PasswordHasherTests.cs b/LibreHardwareMonitor.Windows.WinUI.Tests/Services/PasswordHasherTests.cs new file mode 100644 index 0000000..a23ee3d --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI.Tests/Services/PasswordHasherTests.cs @@ -0,0 +1,70 @@ +// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Copyright (C) LibreHardwareMonitor and Contributors. + +using LibreHardwareMonitor.Windows.WinUI.Services; +using Xunit; + +namespace LibreHardwareMonitor.Windows.WinUI.Tests.Services; + +public class PasswordHasherTests +{ + [Fact] + public void Hash_ThenVerify_Succeeds() + { + string hash = PasswordHasher.Hash("correct horse"); + Assert.True(PasswordHasher.Verify("correct horse", hash, out bool isLegacy)); + Assert.False(isLegacy); + } + + [Fact] + public void Verify_WrongPassword_Fails() + { + string hash = PasswordHasher.Hash("secret"); + Assert.False(PasswordHasher.Verify("guess", hash, out _)); + } + + [Fact] + public void Hash_UsesSelfDescribingPbkdf2Format() + { + string hash = PasswordHasher.Hash("secret"); + Assert.StartsWith("pbkdf2$", hash); + Assert.Equal(4, hash.Split('$').Length); + } + + [Fact] + public void Hash_UsesRandomSalt_SoTwoHashesOfSamePasswordDiffer() + { + Assert.NotEqual(PasswordHasher.Hash("secret"), PasswordHasher.Hash("secret")); + } + + [Fact] + public void Verify_AcceptsLegacySha256_AndReportsLegacy() + { + string legacy = PasswordHasher.ComputeLegacySha256("secret"); + Assert.True(PasswordHasher.Verify("secret", legacy, out bool isLegacy)); + Assert.True(isLegacy); + Assert.False(PasswordHasher.Verify("wrong", legacy, out _)); + } + + [Fact] + public void ComputeLegacySha256_KnownVector() + { + Assert.Equal("ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", PasswordHasher.ComputeLegacySha256("abc")); + } + + [Fact] + public void Verify_EmptyStoredHash_Fails() + { + Assert.False(PasswordHasher.Verify("anything", "", out _)); + } + + [Theory] + [InlineData("pbkdf2$notanumber$c2FsdA==$aGFzaA==")] + [InlineData("pbkdf2$100000$not-base64$aGFzaA==")] + [InlineData("pbkdf2$100000")] + public void Verify_MalformedPbkdf2_Fails(string storedHash) + { + Assert.False(PasswordHasher.Verify("secret", storedHash, out _)); + } +} diff --git a/LibreHardwareMonitor.Windows.WinUI.Tests/Services/RemoteWebServerTests.cs b/LibreHardwareMonitor.Windows.WinUI.Tests/Services/RemoteWebServerTests.cs index 918931e..938acfa 100644 --- a/LibreHardwareMonitor.Windows.WinUI.Tests/Services/RemoteWebServerTests.cs +++ b/LibreHardwareMonitor.Windows.WinUI.Tests/Services/RemoteWebServerTests.cs @@ -256,6 +256,18 @@ public class RemoteWebServerTests Assert.Equal("", RemoteWebServer.GeneratePrometheusResponse(null, LastValueSettings)); } + [Fact] + public void GeneratePrometheusResponse_EscapesLabelValues() + { + ISensor sensor = WithValues(CreateSensor("Core \"0\"", SensorType.Temperature, new Identifier("cpu", "0", "temperature", "0"), 50f), new SensorValue(50f, DateTime.UtcNow)); + SensorTreeItemViewModel root = BuildRoot("HOST", CreateHardware("CPU", HardwareType.Cpu, new Identifier("cpu", "0"), sensor)); + + string output = RemoteWebServer.GeneratePrometheusResponse(root, LastValueSettings); + + // The double quote in the sensor name must be backslash-escaped so it can't break or inject labels. + Assert.Contains("Core \\\"0\\\"", output); + } + // ---- ComputeSHA256 (legacy hashing; Phase 2 must keep verifying these) ---- [Fact] @@ -292,6 +304,30 @@ public class RemoteWebServerTests Assert.False(server.VerifyCredentials(userName, password)); } + [Fact] + public void SetPassword_ProducesPbkdf2HashThatVerifies() + { + using RemoteWebServer server = CreateServer(authEnabled: true, "admin", "secret"); + server.SetPassword("newpass"); + + Assert.StartsWith("pbkdf2$", server.PasswordHash); + Assert.True(server.VerifyCredentials("admin", "newpass")); + Assert.False(server.VerifyCredentials("admin", "secret")); + } + + [Fact] + public void VerifyCredentials_LegacyHash_UpgradesToPbkdf2OnSuccess() + { + // CreateServer seeds the stored hash with the legacy unsalted SHA-256 of the password. + using RemoteWebServer server = CreateServer(authEnabled: true, "admin", "secret"); + Assert.False(server.PasswordHash.StartsWith("pbkdf2$")); + + Assert.True(server.VerifyCredentials("admin", "secret")); // verified via the legacy path... + Assert.StartsWith("pbkdf2$", server.PasswordHash); // ...and transparently upgraded + + Assert.True(server.VerifyCredentials("admin", "secret")); // still verifies via the upgraded hash + } + // ---- helpers ------------------------------------------------------------ private static RemoteWebServer CreateServer(bool authEnabled, string userName, string password) diff --git a/LibreHardwareMonitor.Windows.WinUI/Services/CredentialComparer.cs b/LibreHardwareMonitor.Windows.WinUI/Services/CredentialComparer.cs new file mode 100644 index 0000000..8296447 --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI/Services/CredentialComparer.cs @@ -0,0 +1,20 @@ +// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Copyright (C) LibreHardwareMonitor and Contributors. + +using System.Security.Cryptography; +using System.Text; + +namespace LibreHardwareMonitor.Windows.WinUI.Services; + +internal static class CredentialComparer +{ + /// + /// Compares two strings for equality in time that does not depend on where they first differ, so an attacker cannot + /// learn a correct prefix from response timing. Used for the user name and password-hash comparisons. + /// + public static bool FixedTimeEquals(string left, string right) + { + return CryptographicOperations.FixedTimeEquals(Encoding.UTF8.GetBytes(left), Encoding.UTF8.GetBytes(right)); + } +} diff --git a/LibreHardwareMonitor.Windows.WinUI/Services/PasswordHasher.cs b/LibreHardwareMonitor.Windows.WinUI/Services/PasswordHasher.cs new file mode 100644 index 0000000..58e9eb9 --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI/Services/PasswordHasher.cs @@ -0,0 +1,87 @@ +// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Copyright (C) LibreHardwareMonitor and Contributors. + +using System; +using System.Globalization; +using System.Linq; +using System.Security.Cryptography; +using System.Text; + +namespace LibreHardwareMonitor.Windows.WinUI.Services; + +/// +/// Hashes and verifies the remote web server's Basic-auth password. +/// +/// New credentials use PBKDF2-HMAC-SHA256 with a per-credential random salt, stored in the self-describing format +/// pbkdf2$<iterations>$<base64 salt>$<base64 hash>. also accepts the +/// legacy unsalted SHA-256 hex hash that older configurations stored, so existing authenticationPassword +/// settings keep working; callers can then opportunistically re-hash to upgrade them. +/// +/// +internal static class PasswordHasher +{ + private const string Pbkdf2Prefix = "pbkdf2$"; + private const int Iterations = 100_000; + private const int SaltSize = 16; + private const int KeySize = 32; + + /// Produces a salted PBKDF2 hash string for . + public static string Hash(string password) + { + byte[] salt = RandomNumberGenerator.GetBytes(SaltSize); + byte[] key = Rfc2898DeriveBytes.Pbkdf2(password, salt, Iterations, HashAlgorithmName.SHA256, KeySize); + return $"{Pbkdf2Prefix}{Iterations}${Convert.ToBase64String(salt)}${Convert.ToBase64String(key)}"; + } + + /// + /// Verifies against in constant time. + /// reports whether the stored hash used the old unsalted SHA-256 scheme, so the caller + /// can transparently upgrade it. + /// + public static bool Verify(string password, string storedHash, out bool isLegacy) + { + isLegacy = false; + if (string.IsNullOrEmpty(storedHash)) + return false; + + if (storedHash.StartsWith(Pbkdf2Prefix, StringComparison.Ordinal)) + return VerifyPbkdf2(password, storedHash); + + // Legacy unsalted SHA-256 hex hash. + isLegacy = true; + return CredentialComparer.FixedTimeEquals(ComputeLegacySha256(password), storedHash); + } + + /// Lowercase hex SHA-256, matching the hash older versions stored. Kept only to verify/upgrade legacy values. + public static string ComputeLegacySha256(string text) + { + byte[] hash = SHA256.HashData(Encoding.UTF8.GetBytes(text)); + return string.Concat(hash.Select(b => b.ToString("x2", CultureInfo.InvariantCulture))); + } + + private static bool VerifyPbkdf2(string password, string storedHash) + { + string[] parts = storedHash.Split('$'); + if (parts.Length != 4) + return false; + + if (!int.TryParse(parts[1], NumberStyles.Integer, CultureInfo.InvariantCulture, out int iterations) || iterations <= 0) + return false; + + byte[] salt; + byte[] expected; + try + { + salt = Convert.FromBase64String(parts[2]); + expected = Convert.FromBase64String(parts[3]); + } + catch (FormatException) + { + return false; + } + + byte[] actual = Rfc2898DeriveBytes.Pbkdf2(password, salt, iterations, HashAlgorithmName.SHA256, expected.Length); + return CryptographicOperations.FixedTimeEquals(actual, expected); + } +} diff --git a/LibreHardwareMonitor.Windows.WinUI/Services/RemoteWebServer.cs b/LibreHardwareMonitor.Windows.WinUI/Services/RemoteWebServer.cs index b2001fd..a92ac3f 100644 --- a/LibreHardwareMonitor.Windows.WinUI/Services/RemoteWebServer.cs +++ b/LibreHardwareMonitor.Windows.WinUI/Services/RemoteWebServer.cs @@ -4,13 +4,13 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.IO.Compression; using System.Linq; using System.Net; using System.Reflection; -using System.Security.Cryptography; using System.Text; using System.Text.Json; using System.Text.Json.Serialization; @@ -40,7 +40,7 @@ public sealed class RemoteWebServer : IDisposable int listenerPort, bool authEnabled, string userName, - string passwordSha256) + string passwordHash) { _rootProvider = rootProvider; _computer = computer; @@ -49,7 +49,7 @@ public sealed class RemoteWebServer : IDisposable ListenerPort = listenerPort; AuthEnabled = authEnabled; UserName = userName; - PasswordSHA256 = passwordSha256; + PasswordHash = passwordHash; try { @@ -69,7 +69,7 @@ public sealed class RemoteWebServer : IDisposable public int ListenerPort { get; set; } - public string PasswordSHA256 { get; private set; } + public string PasswordHash { get; private set; } public bool PlatformNotSupported => _listener == null; @@ -82,7 +82,7 @@ public sealed class RemoteWebServer : IDisposable public void SetPassword(string plainPassword) { - PasswordSHA256 = ComputeSHA256(plainPassword); + PasswordHash = PasswordHasher.Hash(plainPassword); } public bool Start() @@ -307,7 +307,19 @@ public sealed class RemoteWebServer : IDisposable if (userName == null || password == null) return false; - return userName == UserName && ComputeSHA256(password) == PasswordSHA256; + // Compare the user name and the password hash in constant time, and evaluate both fully (no && short-circuit) + // so neither a wrong user name nor a wrong password can be distinguished from response timing. + bool userMatch = CredentialComparer.FixedTimeEquals(userName, UserName); + bool passwordMatch = PasswordHasher.Verify(password, PasswordHash, out bool isLegacy); + if (!(userMatch & passwordMatch)) + return false; + + // Transparently upgrade a legacy unsalted-SHA-256 credential to PBKDF2 on first successful authentication. The + // upgraded hash is persisted by the view model when settings are next saved (e.g. on shutdown). + if (isLegacy) + PasswordHash = PasswordHasher.Hash(password); + + return true; } private async Task HandlePostRequestAsync(HttpListenerResponse response, HttpListenerRequest request) @@ -322,8 +334,10 @@ public sealed class RemoteWebServer : IDisposable } catch (Exception ex) { + // Return a generic message; the exception detail (which can reveal internal state) stays server-side. result["result"] = "fail"; - result["message"] = ex.ToString(); + result["message"] = "Bad request"; + Debug.WriteLine($"Remote web server POST request failed: {ex}"); } await SendJsonSensorAsync(response, result); @@ -410,8 +424,7 @@ public sealed class RemoteWebServer : IDisposable byte[] buffer = Encoding.UTF8.GetBytes(JsonSerializer.Serialize(json, options)); bool acceptGzip = request.Headers["Accept-Encoding"]?.IndexOf("gzip", StringComparison.OrdinalIgnoreCase) >= 0; - response.AddHeader("Cache-Control", "no-cache"); - response.AddHeader("Access-Control-Allow-Origin", "*"); + WriteCommonHeaders(response); response.ContentType = "application/json"; if (acceptGzip) @@ -475,8 +488,7 @@ public sealed class RemoteWebServer : IDisposable lock (_sensorReadLock) content = GeneratePrometheusResponse(_rootProvider(), settings); - response.AddHeader("Cache-Control", "no-cache"); - response.AddHeader("Access-Control-Allow-Origin", "*"); + WriteCommonHeaders(response); response.AddHeader("X-archivelength", settings["archivelength"].ToString(CultureInfo.InvariantCulture)); response.AddHeader("X-timestamps", settings["timestamps"].ToString(CultureInfo.InvariantCulture)); response.AddHeader("X-lastvalue", settings["lastvalue"].ToString(CultureInfo.InvariantCulture)); @@ -540,7 +552,7 @@ public sealed class RemoteWebServer : IDisposable ? sensor.Identifier.ToString()[hardwareId.Length..] : sensor.Identifier.ToString(); string sensorAlias = $"{sensorName} ({sensorId})"; - string tagLine = $$"""{{tagName}} {"sensorName"="{{sensorName}}", "sensorAlias"="{{sensorAlias}}", "hardwareName"="{{hardwareName}}", "hardwareAlias"="{{hardwareAlias}}", "sensorId"="{{sensorId}}", "hardwareId"="{{hardwareId}}", "host"="{{host}}"}"""; + string tagLine = $$"""{{tagName}} {"sensorName"="{{EscapePrometheusLabel(sensorName)}}", "sensorAlias"="{{EscapePrometheusLabel(sensorAlias)}}", "hardwareName"="{{EscapePrometheusLabel(hardwareName)}}", "hardwareAlias"="{{EscapePrometheusLabel(hardwareAlias)}}", "sensorId"="{{EscapePrometheusLabel(sensorId)}}", "hardwareId"="{{EscapePrometheusLabel(hardwareId)}}", "host"="{{EscapePrometheusLabel(host)}}"}"""; if (lastTagName != tagName) { @@ -602,8 +614,7 @@ public sealed class RemoteWebServer : IDisposable private async Task SendJsonSensorAsync(HttpListenerResponse response, Dictionary sensorData) { - response.AddHeader("Cache-Control", "no-cache"); - response.AddHeader("Access-Control-Allow-Origin", "*"); + WriteCommonHeaders(response); await SendResponseAsync(response, JsonSerializer.Serialize(sensorData), "application/json"); } @@ -647,20 +658,18 @@ public sealed class RemoteWebServer : IDisposable private string ResolveListenerIp() { + // Explicit wildcards bind every interface; pass them through with their exact prefixes. if (ListenerIp is "+" or "*" or "0.0.0.0") return ListenerIp; - try - { - IPHostEntry host = Dns.GetHostEntry(Dns.GetHostName()); - if (host.AddressList.Any(ip => ip.ToString() == ListenerIp)) - return ListenerIp; - } - catch - { - } + // "?" (and a blank value) is the unconfigured "auto" default, which historically bound all interfaces. + if (string.IsNullOrWhiteSpace(ListenerIp) || ListenerIp == "?") + return "+"; - ListenerIp = "+"; + // A specific address: bind exactly what the user configured. Unlike before, we no longer silently fall back to + // "+" (every interface) when the address is not one of this host's enumerated addresses — that widened the + // user's chosen scope and was even persisted back to settings. If the address cannot be bound, Start() fails and + // the caller surfaces the error, so the user's binding intent is respected. return ListenerIp; } @@ -756,9 +765,26 @@ public sealed class RemoteWebServer : IDisposable return result; } + private static void WriteCommonHeaders(HttpListenerResponse response) + { + response.AddHeader("Cache-Control", "no-cache"); + + // No "Access-Control-Allow-Origin: *": the bundled web UI is served from this same origin (so it needs no CORS + // grant), and omitting the header lets the browser's same-origin policy keep arbitrary third-party sites from + // reading sensor data. Server-to-server scrapers such as Prometheus are unaffected. + } + + // Escapes a Prometheus exposition-format label value. Hardware/sensor names can contain characters that would + // otherwise break the line or inject extra labels, so backslash, double-quote and newline must be escaped. + private static string EscapePrometheusLabel(string value) + { + return value.Replace("\\", "\\\\").Replace("\"", "\\\"").Replace("\n", "\\n"); + } + + // Retained for backward compatibility (and tests). The legacy unsalted SHA-256 scheme is now used only to verify and + // upgrade pre-existing credentials; new passwords are hashed by PasswordHasher (PBKDF2). public static string ComputeSHA256(string text) { - using SHA256 hash = SHA256.Create(); - return string.Concat(hash.ComputeHash(Encoding.UTF8.GetBytes(text)).Select(item => item.ToString("x2", CultureInfo.InvariantCulture))); + return PasswordHasher.ComputeLegacySha256(text); } } diff --git a/LibreHardwareMonitor.Windows.WinUI/ViewModels/MainWindowViewModel.cs b/LibreHardwareMonitor.Windows.WinUI/ViewModels/MainWindowViewModel.cs index ade2d8c..559998e 100644 --- a/LibreHardwareMonitor.Windows.WinUI/ViewModels/MainWindowViewModel.cs +++ b/LibreHardwareMonitor.Windows.WinUI/ViewModels/MainWindowViewModel.cs @@ -740,7 +740,7 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable Settings.SetValue("listenerPort", ListenerPort); Settings.SetValue("authenticationEnabled", AuthWebServer); Settings.SetValue("authenticationUserName", AuthWebServerUserName); - Settings.SetValue("authenticationPassword", _remoteWebServer.PasswordSHA256); + Settings.SetValue("authenticationPassword", _remoteWebServer.PasswordHash); _remoteWebServer.Quit(); _hardwareMonitor.Dispose(); Save(); @@ -798,7 +798,7 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable return; _remoteWebServer.SetPassword(plainPassword); - Settings.SetValue("authenticationPassword", _remoteWebServer.PasswordSHA256); + Settings.SetValue("authenticationPassword", _remoteWebServer.PasswordHash); RestartWebServerIfRunning(); }