diff --git a/libs/host/Configuration/Options.cs b/libs/host/Configuration/Options.cs index c4b86b48694..8e01c771612 100644 --- a/libs/host/Configuration/Options.cs +++ b/libs/host/Configuration/Options.cs @@ -177,6 +177,9 @@ internal sealed class Options : ICloneable [Option("acl-strict-custom-commands", Required = false, HelpText = "If true (default), the server refuses to start when an ACL rule references a custom (extension) command name that no loaded module has registered. Set to false to load unresolved names as-is and log warnings.")] public bool? AclStrictCustomCommands { get; set; } + [Option("allowed-protocols", Required = false, HelpText = "RESP protocol versions accepted by client sessions. Value options: Both, Resp2, Resp3")] + public RespProtocolMode AllowedProtocols { get; set; } + [Option("aad-authority", Required = false, HelpText = "The authority of AAD authentication.")] public string AadAuthority { get; set; } @@ -860,6 +863,7 @@ endpoint is IPEndPoint listenEp && clusterAnnounceEndpoint[0] is IPEndPoint anno FastMigrate = FastMigrate.GetValueOrDefault(), AuthSettings = GetAuthenticationSettings(logger), AclStrictCustomCommands = AclStrictCustomCommands.GetValueOrDefault(true), + AllowedProtocols = AllowedProtocols, EnableAOF = EnableAOF.GetValueOrDefault(), EnableLua = EnableLua.GetValueOrDefault(), LuaTransactionMode = LuaTransactionMode.GetValueOrDefault(), @@ -1085,4 +1089,4 @@ public class HiddenOptionAttribute : Attribute { } -} \ No newline at end of file +} diff --git a/libs/host/defaults.conf b/libs/host/defaults.conf index 3d14e4e622c..89cef1e7008 100644 --- a/libs/host/defaults.conf +++ b/libs/host/defaults.conf @@ -111,6 +111,9 @@ /* Refuse to start when an ACL rule references a custom (extension) command name that no loaded module has registered. Default true (strict). Set to false to load unresolved names as-is and log warnings. */ "AclStrictCustomCommands" : true, + /* RESP protocol versions accepted by client sessions. Value options: Both, Resp2, Resp3 */ + "AllowedProtocols" : "Both", + /* The authority of AAD authentication. */ "AadAuthority" : "https://login.microsoftonline.com", @@ -466,4 +469,4 @@ /* Enable Range Index (preview) - this feature (and associated RI.* commands) are incomplete, unstable, and subject to change while still in preview */ "EnableRangeIndexPreview": false -} \ No newline at end of file +} diff --git a/libs/server/Resp/BasicCommands.cs b/libs/server/Resp/BasicCommands.cs index a0c2500578e..e729dee12dc 100644 --- a/libs/server/Resp/BasicCommands.cs +++ b/libs/server/Resp/BasicCommands.cs @@ -1364,6 +1364,11 @@ private bool NetworkHELLO() return true; } + if (!storeWrapper.serverOptions.IsRespProtocolVersionAllowed(tmpRespProtocolVersion ?? respProtocolVersion)) + { + return AbortWithErrorMessage(CmdStrings.RESP_ERR_PROTOCOL_VERSION_NOT_ALLOWED); + } + ProcessHelloCommand(tmpRespProtocolVersion, authUsername, authPassword, tmpClientName); return true; } @@ -1870,4 +1875,4 @@ static void SetResult(int c, ref int firstPending, ref (GarnetStatus, StringOutp outputArr[c - firstPending] = (status, output); } } -} \ No newline at end of file +} diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index 5992adacddb..eebcfb51591 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -258,6 +258,7 @@ static partial class CmdStrings public static ReadOnlySpan RESP_ERR_NO_TRANSACTION_PROCEDURE => "ERR Could not get transaction procedure"u8; public static ReadOnlySpan RESP_ERR_WRONG_NUMBER_OF_ARGUMENTS => "ERR wrong number of arguments for command"u8; public static ReadOnlySpan RESP_ERR_UNSUPPORTED_PROTOCOL_VERSION => "ERR Unsupported protocol version"u8; + public static ReadOnlySpan RESP_ERR_PROTOCOL_VERSION_NOT_ALLOWED => "ERR Protocol version not allowed by server configuration"u8; public static ReadOnlySpan RESP_ERR_ASYNC_PROTOCOL_CHANGE => "ERR protocol change is not allowed with pending async operations"u8; public static ReadOnlySpan RESP_ERR_NOT_VALID_FLOAT => "ERR value is not a valid float"u8; public static ReadOnlySpan RESP_ERR_MIN_MAX_NOT_VALID_FLOAT => "ERR min or max is not a float"u8; @@ -543,4 +544,4 @@ static partial class CmdStrings public static ReadOnlySpan LUA_ARGV => "ARGV"u8; public static ReadOnlySpan EXPDELSCAN => "EXPDELSCAN"u8; } -} \ No newline at end of file +} diff --git a/libs/server/Resp/RespServerSession.cs b/libs/server/Resp/RespServerSession.cs index e2e0f20fc6e..d7fd7e5a901 100644 --- a/libs/server/Resp/RespServerSession.cs +++ b/libs/server/Resp/RespServerSession.cs @@ -646,7 +646,11 @@ private void ProcessMessages(ref TBasicApi basicApi, ref TTx if (CheckACLPermissions(cmd) && (noScriptPassed = CheckScriptPermissions(cmd))) { - if (txnManager.state != TxnState.None) + if (!IsCommandAllowedByRespProtocolPolicy(cmd)) + { + WriteError(CmdStrings.RESP_ERR_PROTOCOL_VERSION_NOT_ALLOWED); + } + else if (txnManager.state != TxnState.None) { if (txnManager.state == TxnState.Running) { @@ -726,6 +730,15 @@ private void ProcessMessages(ref TBasicApi basicApi, ref TTx } } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private bool IsCommandAllowedByRespProtocolPolicy(RespCommand cmd) + { + if (storeWrapper.serverOptions.AllowedProtocols != RespProtocolMode.Resp3) + return true; + + return respProtocolVersion == 3 || cmd == RespCommand.HELLO; + } + // Make first command in string as uppercase private bool MakeUpperCase(byte* ptr, int len) { @@ -1709,4 +1722,4 @@ private ObjectOutput GetObjectOutput() private UnifiedOutput GetUnifiedOutput() => UnifiedOutput.FromPinnedPointer(dcurr, (int)(dend - dcurr)); } -} \ No newline at end of file +} diff --git a/libs/server/Servers/GarnetServerOptions.cs b/libs/server/Servers/GarnetServerOptions.cs index c78f95d836f..1d76543492b 100644 --- a/libs/server/Servers/GarnetServerOptions.cs +++ b/libs/server/Servers/GarnetServerOptions.cs @@ -53,6 +53,11 @@ public class GarnetServerOptions : ServerOptions /// public bool AclStrictCustomCommands = true; + /// + /// RESP protocol versions accepted by client sessions. + /// + public RespProtocolMode AllowedProtocols = RespProtocolMode.Both; + /// /// Enable append-only file (write ahead log) /// @@ -602,6 +607,16 @@ public GarnetServerOptions(ILogger logger = null) : base(logger) this.logger = logger; } + /// + /// Check whether a RESP protocol version is allowed by server configuration. + /// + public bool IsRespProtocolVersionAllowed(byte version) => AllowedProtocols switch + { + RespProtocolMode.Resp2 => version == 2, + RespProtocolMode.Resp3 => version == 3, + _ => version is 2 or 3, + }; + /// /// Initialize Garnet server options /// @@ -1099,4 +1114,4 @@ public bool MultiLogEnabled public int AofVirtualSublogCount => AofPhysicalSublogCount * AofReplayTaskCount; } -} \ No newline at end of file +} diff --git a/libs/server/Servers/RespProtocolMode.cs b/libs/server/Servers/RespProtocolMode.cs new file mode 100644 index 00000000000..69414fc2f85 --- /dev/null +++ b/libs/server/Servers/RespProtocolMode.cs @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +namespace Garnet.server +{ + /// + /// RESP protocol versions accepted by the server. + /// + public enum RespProtocolMode + { + /// + /// Accept both RESP2 and RESP3 clients. + /// + Both, + + /// + /// Accept RESP2 clients only. + /// + Resp2, + + /// + /// Accept RESP3 clients only. + /// + Resp3, + } +} diff --git a/test/standalone/Garnet.test/GarnetServerConfigTests.cs b/test/standalone/Garnet.test/GarnetServerConfigTests.cs index b41eb24e358..b1262273c74 100644 --- a/test/standalone/Garnet.test/GarnetServerConfigTests.cs +++ b/test/standalone/Garnet.test/GarnetServerConfigTests.cs @@ -1258,6 +1258,41 @@ public void AclStrictCustomCommands() } } + [Test] + public void AllowedProtocols() + { + { + var args = Array.Empty(); + var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out _, out _, out _); + ClassicAssert.IsTrue(parseSuccessful); + ClassicAssert.AreEqual(RespProtocolMode.Both, options.GetServerOptions().AllowedProtocols); + } + + { + var args = new[] { "--allowed-protocols", "RESP3" }; + var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out _, out _, silentMode: true); + ClassicAssert.IsTrue(parseSuccessful); + ClassicAssert.AreEqual(0, invalidOptions.Count); + ClassicAssert.AreEqual(RespProtocolMode.Resp3, options.GetServerOptions().AllowedProtocols); + } + + { + var args = new[] { "--allowed-protocols", "resp2" }; + var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out _, out _, silentMode: true); + ClassicAssert.IsTrue(parseSuccessful); + ClassicAssert.AreEqual(0, invalidOptions.Count); + ClassicAssert.AreEqual(RespProtocolMode.Resp2, options.GetServerOptions().AllowedProtocols); + } + + { + const string JSON = @"{ ""AllowedProtocols"": ""Resp3"" }"; + var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out _); + ClassicAssert.IsTrue(parseSuccessful); + ClassicAssert.AreEqual(0, invalidOptions.Count); + ClassicAssert.AreEqual(RespProtocolMode.Resp3, options.GetServerOptions().AllowedProtocols); + } + } + [Test] public void EnableVectorSetPreview() { @@ -1829,4 +1864,4 @@ public void InitialIORecordSizeParsing() } } } -} \ No newline at end of file +} diff --git a/test/standalone/Garnet.test/RespTests.cs b/test/standalone/Garnet.test/RespTests.cs index b4cbb681b1c..78f0fb452a5 100644 --- a/test/standalone/Garnet.test/RespTests.cs +++ b/test/standalone/Garnet.test/RespTests.cs @@ -3923,6 +3923,54 @@ public void HelloAuthErrorTest() ClassicAssert.AreEqual(2, (int)resultDict["proto"]); } + [Test] + public async Task HelloRespectsResp2OnlyPolicy() + { + TearDown(); + TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true); + server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, disablePubSub: false, allowedProtocols: RespProtocolMode.Resp2); + server.Start(); + + using var c = TestUtils.GetGarnetClientSession(raw: true); + c.Connect(); + + var response = await c.ExecuteAsync("PING").ConfigureAwait(false); + ClassicAssert.AreEqual("+PONG\r\n", response); + + response = await c.ExecuteAsync("HELLO", "3").ConfigureAwait(false); + ClassicAssert.AreEqual($"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_PROTOCOL_VERSION_NOT_ALLOWED)}\r\n", response); + + response = await c.ExecuteAsync("HELLO", "2").ConfigureAwait(false); + ClassicAssert.IsTrue(response.Contains("$5\r\nproto\r\n:2\r\n"), response); + } + + [Test] + public async Task HelloRespectsResp3OnlyPolicy() + { + TearDown(); + TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true); + server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, disablePubSub: false, allowedProtocols: RespProtocolMode.Resp3); + server.Start(); + + using var c = TestUtils.GetGarnetClientSession(raw: true); + c.Connect(); + + var response = await c.ExecuteAsync("PING").ConfigureAwait(false); + ClassicAssert.AreEqual($"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_PROTOCOL_VERSION_NOT_ALLOWED)}\r\n", response); + + response = await c.ExecuteAsync("HELLO").ConfigureAwait(false); + ClassicAssert.AreEqual($"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_PROTOCOL_VERSION_NOT_ALLOWED)}\r\n", response); + + response = await c.ExecuteAsync("HELLO", "2").ConfigureAwait(false); + ClassicAssert.AreEqual($"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_PROTOCOL_VERSION_NOT_ALLOWED)}\r\n", response); + + response = await c.ExecuteAsync("HELLO", "3").ConfigureAwait(false); + ClassicAssert.IsTrue(response.Contains("$5\r\nproto\r\n:3\r\n"), response); + + response = await c.ExecuteAsync("PING").ConfigureAwait(false); + ClassicAssert.AreEqual("+PONG\r\n", response); + } + [Test] [TestCase([2, "$-1\r\n", "$1\r\n", "*4", '*'], Description = "RESP2 output")] [TestCase([3, "_\r\n", ",", "%2", '~'], Description = "RESP3 output")] @@ -5316,4 +5364,4 @@ public void ExcessiveArgumentCountRejected() ClassicAssert.AreEqual("testvalue", db.StringGet("testkey").ToString()); } } -} \ No newline at end of file +} diff --git a/test/standalone/Garnet.test/TestUtils.cs b/test/standalone/Garnet.test/TestUtils.cs index 0a920ab7842..724549294ab 100644 --- a/test/standalone/Garnet.test/TestUtils.cs +++ b/test/standalone/Garnet.test/TestUtils.cs @@ -293,6 +293,7 @@ public static GarnetServer CreateGarnetServer( bool useAcl = false, // NOTE: Temporary until ACL is enforced as default string aclFile = null, bool aclStrictCustomCommands = true, + RespProtocolMode allowedProtocols = RespProtocolMode.Both, string indexSize = "1m", string indexMaxSize = default, string[] extensionBinPaths = null, @@ -383,6 +384,7 @@ public static GarnetServer CreateGarnetServer( CommitFrequencyMs = commitFrequencyMs, WaitForCommit = commitWait, AclStrictCustomCommands = aclStrictCustomCommands, + AllowedProtocols = allowedProtocols, TlsOptions = enableTLS ? new GarnetTlsOptions( certFileName: certFile, certPassword: certPassword, @@ -1354,4 +1356,4 @@ internal static void OnTearDown(bool waitForDelete = false, ILogger logger = nul } } } -} \ No newline at end of file +}