From be027523ac157265ae9d18e1281971c55347d64b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20G=C3=B6ls?= <6608231+Abrynos@users.noreply.github.com> Date: Wed, 13 Oct 2021 21:44:48 +0200 Subject: [PATCH] Warn about insecure passwords (#2419) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add warnings about password security * Warn about weak steam passwords even if they are encrypted * Apply feedback * Apply feedback * Simplify code * Move return criteria up a bit for increased performance * Choose more fitting strings for localization * Extract const value * Fix incorrect null reference warning * Switch prefix operator for postfix one Co-authored-by: Łukasz Domeradzki * Add tests * Disable CA1724 The type name Utilities conflicts in whole or in part with the namespace name 'Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework.Utilities'. * Tell users why their password is considered weak * Apply feedback * Merge resource comments * Misc. * Use library for password testing and Run testing in background * Clean up * OncSeparate forbidden phrases forfor IPC passwords (once again) * Additionally check encryption key * Add comment about {0} Co-authored-by: Łukasz Domeradzki --- ArchiSteamFarm.Tests/Utilities.cs | 52 +++++++++++++++++++ ArchiSteamFarm/ArchiSteamFarm.csproj | 1 + ArchiSteamFarm/Core/Utilities.cs | 27 ++++++++++ .../Localization/Strings.Designer.cs | 29 ++++++++++- ArchiSteamFarm/Localization/Strings.resx | 12 +++++ ArchiSteamFarm/Program.cs | 13 +++++ ArchiSteamFarm/Steam/Storage/BotConfig.cs | 18 +++++++ ArchiSteamFarm/Storage/GlobalConfig.cs | 14 +++++ Directory.Packages.props | 1 + 9 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 ArchiSteamFarm.Tests/Utilities.cs diff --git a/ArchiSteamFarm.Tests/Utilities.cs b/ArchiSteamFarm.Tests/Utilities.cs new file mode 100644 index 000000000..446bdf4c2 --- /dev/null +++ b/ArchiSteamFarm.Tests/Utilities.cs @@ -0,0 +1,52 @@ +// _ _ _ ____ _ _____ +// / \ _ __ ___ | |__ (_)/ ___| | |_ ___ __ _ _ __ ___ | ___|__ _ _ __ _ __ ___ +// / _ \ | '__|/ __|| '_ \ | |\___ \ | __|/ _ \ / _` || '_ ` _ \ | |_ / _` || '__|| '_ ` _ \ +// / ___ \ | | | (__ | | | || | ___) || |_| __/| (_| || | | | | || _|| (_| || | | | | | | | +// /_/ \_\|_| \___||_| |_||_||____/ \__|\___| \__,_||_| |_| |_||_| \__,_||_| |_| |_| |_| +// | +// Copyright 2015-2021 Łukasz "JustArchi" Domeradzki +// Contact: JustArchi@JustArchi.net +// | +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// | +// http://www.apache.org/licenses/LICENSE-2.0 +// | +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System.Collections.Generic; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using static ArchiSteamFarm.Core.Utilities; + +namespace ArchiSteamFarm.Tests { + [TestClass] +#pragma warning disable CA1724 + public sealed class Utilities { +#pragma warning restore CA1724 + [TestMethod] + public void LongPassphraseIsNotWeak() => Assert.IsFalse(TestPasswordStrength("10charsasdf").IsWeak); + + [TestMethod] + public void ShortPassphraseIsWeak() => Assert.IsTrue(TestPasswordStrength("four").IsWeak); + + [TestMethod] + public void RepetitiveCharactersWeakenPassphrases() => Assert.IsTrue(TestPasswordStrength("testaaaatest").IsWeak); + + [TestMethod] + public void SequentialCharactersWeakenPassphrases() => Assert.IsTrue(TestPasswordStrength("testabcdtest").IsWeak); + + [TestMethod] + public void SequentialDescendingCharactersWeakenPassphrases() => Assert.IsTrue(TestPasswordStrength("testdcbatest").IsWeak); + + [TestMethod] + public void ContextSpecificWordsWeakenPassphrases() => Assert.IsTrue(TestPasswordStrength("archisteamfarmpassword").IsWeak); + + [TestMethod] + public void AdditionallyForbiddenWordsWeakenPassphrases() => Assert.IsTrue(TestPasswordStrength("10charsasdf", new HashSet { "chars" }).IsWeak); + } +} diff --git a/ArchiSteamFarm/ArchiSteamFarm.csproj b/ArchiSteamFarm/ArchiSteamFarm.csproj index 1230ebe2b..261602b46 100644 --- a/ArchiSteamFarm/ArchiSteamFarm.csproj +++ b/ArchiSteamFarm/ArchiSteamFarm.csproj @@ -22,6 +22,7 @@ + diff --git a/ArchiSteamFarm/Core/Utilities.cs b/ArchiSteamFarm/Core/Utilities.cs index 8929e4e59..ebdd743a1 100644 --- a/ArchiSteamFarm/Core/Utilities.cs +++ b/ArchiSteamFarm/Core/Utilities.cs @@ -21,6 +21,8 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; +using System.Globalization; using System.IO; using System.Linq; using System.Net; @@ -29,16 +31,24 @@ using System.Threading; using System.Threading.Tasks; using AngleSharp.Dom; using AngleSharp.XPath; +using ArchiSteamFarm.Localization; using ArchiSteamFarm.Storage; using Humanizer; using Humanizer.Localisation; using JetBrains.Annotations; using SteamKit2; +using Zxcvbn; +#if NETFRAMEWORK +using JustArchiNET.Madness; +#endif namespace ArchiSteamFarm.Core { public static class Utilities { private const byte TimeoutForLongRunningTasksInSeconds = 60; + // normally we'd just use words like "steam" and "farm", but the library we're currently using is a bit iffy about banned words, so we need to also add combinations such as "steamfarm" + private static readonly ImmutableHashSet ForbiddenPasswordPhrases = ImmutableHashSet.Create(StringComparer.InvariantCultureIgnoreCase, "archisteamfarm", "archi", "steam", "farm", "archisteam", "archifarm", "steamfarm", "asf", "asffarm", "password"); + // Normally we wouldn't need to use this singleton, but we want to ensure decent randomness across entire program's lifetime private static readonly Random Random = new(); @@ -319,6 +329,23 @@ namespace ArchiSteamFarm.Core { } } + internal static (bool IsWeak, string? Reason) TestPasswordStrength(string password, ISet? additionallyForbiddenPhrases = null) { + if (string.IsNullOrEmpty(password)) { + throw new ArgumentNullException(nameof(password)); + } + + HashSet forbiddenPhrases = ForbiddenPasswordPhrases.ToHashSet(StringComparer.InvariantCultureIgnoreCase); + + if (additionallyForbiddenPhrases != null) { + forbiddenPhrases.UnionWith(additionallyForbiddenPhrases); + } + + Result result = Zxcvbn.Core.EvaluatePassword(password, forbiddenPhrases); + FeedbackItem feedback = result.Feedback; + + return (result.Score < 4, string.IsNullOrEmpty(feedback.Warning) ? feedback.Suggestions.FirstOrDefault() : feedback.Warning); + } + internal static bool RelativeDirectoryStartsWith(string directory, params string[] prefixes) { if (string.IsNullOrEmpty(directory)) { throw new ArgumentNullException(nameof(directory)); diff --git a/ArchiSteamFarm/Localization/Strings.Designer.cs b/ArchiSteamFarm/Localization/Strings.Designer.cs index 52958e13a..141db9cdd 100644 --- a/ArchiSteamFarm/Localization/Strings.Designer.cs +++ b/ArchiSteamFarm/Localization/Strings.Designer.cs @@ -992,7 +992,7 @@ namespace ArchiSteamFarm.Localization { } /// - /// Looks up a localized string similar to No bots are defined. Did you forget to configure your ASF?. + /// Looks up a localized string similar to No bots are defined. Did you forget to configure your ASF? Follow 'setting up' guide on the wiki if you're confused.. /// public static string ErrorNoBotsDefined { get { @@ -1675,6 +1675,33 @@ namespace ArchiSteamFarm.Localization { } } + /// + /// Looks up a localized string similar to Your encryption key seems to be weak. Consider choosing a stronger one for increased security. Reason: {0}. + /// + public static string WarningWeakCryptKey { + get { + return ResourceManager.GetString("WarningWeakCryptKey", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Your IPC password seems to be weak. Consider choosing a stronger one for increased security. Reason: {0}. + /// + public static string WarningWeakIPCPassword { + get { + return ResourceManager.GetString("WarningWeakIPCPassword", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Your Steam password for '{0}' seems to be weak. Consider choosing a stronger one for increased security. Reason: {1}. + /// + public static string WarningWeakSteamPassword { + get { + return ResourceManager.GetString("WarningWeakSteamPassword", resourceCulture); + } + } + /// /// Looks up a localized string similar to Workaround for {0} bug has been triggered.. /// diff --git a/ArchiSteamFarm/Localization/Strings.resx b/ArchiSteamFarm/Localization/Strings.resx index eead4ab3a..8f04328f8 100644 --- a/ArchiSteamFarm/Localization/Strings.resx +++ b/ArchiSteamFarm/Localization/Strings.resx @@ -699,4 +699,16 @@ Process uptime: {1} {0} config file will be migrated to the latest syntax... {0} will be replaced with the relative path to the affected config file + + Your IPC password seems to be weak. Consider choosing a stronger one for increased security. Reason: {0} + {0} will be replaced by the reason for the password being considered weak + + + Your Steam password for '{0}' seems to be weak. Consider choosing a stronger one for increased security. Reason: {1} + {0} will be replaced by either the affected bot name or the path to the bots configuration file, {1} will be replaced by the reason for the password being considered weak + + + Your encryption key seems to be weak. Consider choosing a stronger one for increased security. Reason: {0} + {0} will be replaced by the reason for the encryption key being considered weak + diff --git a/ArchiSteamFarm/Program.cs b/ArchiSteamFarm/Program.cs index 16819300b..022c398ea 100644 --- a/ArchiSteamFarm/Program.cs +++ b/ArchiSteamFarm/Program.cs @@ -22,6 +22,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Collections.Immutable; using System.Diagnostics; using System.Globalization; using System.IO; @@ -46,6 +47,8 @@ using SteamKit2; namespace ArchiSteamFarm { internal static class Program { + private static readonly ImmutableHashSet ForbiddenCryptKeyPhrases = ImmutableHashSet.Create(StringComparer.InvariantCultureIgnoreCase, "crypt", "key", "cryptkey"); + internal static bool ConfigMigrate { get; private set; } = true; internal static bool ConfigWatch { get; private set; } = true; internal static string? NetworkGroup { get; private set; } @@ -99,6 +102,16 @@ namespace ArchiSteamFarm { throw new ArgumentNullException(nameof(cryptKey)); } + Utilities.InBackground( + () => { + (bool isWeak, string? reason) = Utilities.TestPasswordStrength(cryptKey, ForbiddenCryptKeyPhrases); + + if (isWeak) { + ASF.ArchiLogger.LogGenericWarning(string.Format(CultureInfo.CurrentCulture, Strings.WarningWeakCryptKey, reason)); + } + } + ); + ArchiCryptoHelper.SetEncryptionKey(cryptKey); } diff --git a/ArchiSteamFarm/Steam/Storage/BotConfig.cs b/ArchiSteamFarm/Steam/Storage/BotConfig.cs index 9bb60357d..94126e222 100644 --- a/ArchiSteamFarm/Steam/Storage/BotConfig.cs +++ b/ArchiSteamFarm/Steam/Storage/BotConfig.cs @@ -580,6 +580,24 @@ namespace ArchiSteamFarm.Steam.Storage { return (null, null); } + if (!string.IsNullOrEmpty(botConfig.DecryptedSteamPassword)) { + HashSet disallowedValues = new(StringComparer.InvariantCultureIgnoreCase) { "account" }; + + if (!string.IsNullOrEmpty(botConfig.SteamLogin)) { + disallowedValues.Add(botConfig.SteamLogin!); + } + + Utilities.InBackground( + () => { + (bool isWeak, string? reason) = Utilities.TestPasswordStrength(botConfig.DecryptedSteamPassword!, disallowedValues); + + if (isWeak) { + ASF.ArchiLogger.LogGenericWarning(string.Format(CultureInfo.CurrentCulture, Strings.WarningWeakSteamPassword, !string.IsNullOrEmpty(botConfig.SteamLogin) ? botConfig.SteamLogin! : filePath, reason)); + } + } + ); + } + if (!Program.ConfigMigrate) { return (botConfig, null); } diff --git a/ArchiSteamFarm/Storage/GlobalConfig.cs b/ArchiSteamFarm/Storage/GlobalConfig.cs index aaaea59ce..14bc493d4 100644 --- a/ArchiSteamFarm/Storage/GlobalConfig.cs +++ b/ArchiSteamFarm/Storage/GlobalConfig.cs @@ -45,6 +45,8 @@ using SteamKit2; namespace ArchiSteamFarm.Storage { [SuppressMessage("ReSharper", "ClassCannotBeInstantiated")] public sealed class GlobalConfig { + private static readonly ImmutableHashSet ForbiddenIPCPasswordPhrases = ImmutableHashSet.Create(StringComparer.InvariantCultureIgnoreCase, "ipc", "api", "gui", "asf-ui", "asf-gui"); + [PublicAPI] public const bool DefaultAutoRestart = true; @@ -504,6 +506,18 @@ namespace ArchiSteamFarm.Storage { return (null, null); } + if (globalConfig.IPCPasswordFormat == ArchiCryptoHelper.EHashingMethod.PlainText && !string.IsNullOrEmpty(globalConfig.IPCPassword)) { + Utilities.InBackground( + () => { + (bool isWeak, string? reason) = Utilities.TestPasswordStrength(globalConfig.IPCPassword!, ForbiddenIPCPasswordPhrases); + + if (isWeak) { + ASF.ArchiLogger.LogGenericWarning(string.Format(CultureInfo.CurrentCulture, Strings.WarningWeakIPCPassword, reason)); + } + } + ); + } + if (!Program.ConfigMigrate) { return (globalConfig, null); } diff --git a/Directory.Packages.props b/Directory.Packages.props index 36e0e51a8..b356b7811 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -19,6 +19,7 @@ +