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

Implement high integrity mode for commands #2741

Merged
merged 13 commits into from
Jun 24, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ public static DefaultOptionsProvider GetProvider(EndPoint endpoint)
/// </summary>
public virtual bool CheckCertificateRevocation => true;

/// <summary>
/// A Boolean value that specifies whether to use per-command validation of strict protocol validity
NickCraver marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <remarks>The regular RESP protocol does not include correlation identifiers between requests and responses; in exceptional
/// scenarios, protocol desynchronization can occur, which may not be noticed immediately; this option adds additional data
/// to ensure that this cannot occur, at the cost of some (small) additional bandwidth usage.</remarks>
public virtual bool HighIntegrity => true;
NickCraver marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// The number of times to repeat the initial connect cycle if no servers respond promptly.
/// </summary>
Expand Down
25 changes: 22 additions & 3 deletions src/StackExchange.Redis/ConfigurationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ internal const string
CheckCertificateRevocation = "checkCertificateRevocation",
Tunnel = "tunnel",
SetClientLibrary = "setlib",
Protocol = "protocol";
Protocol = "protocol",
HighIntegrity = "highIntegrity";

private static readonly Dictionary<string, string> normalizedOptions = new[]
{
Expand Down Expand Up @@ -141,6 +142,7 @@ internal const string
WriteBuffer,
CheckCertificateRevocation,
Protocol,
HighIntegrity,
}.ToDictionary(x => x, StringComparer.OrdinalIgnoreCase);

public static string TryNormalize(string value)
Expand All @@ -156,7 +158,7 @@ public static string TryNormalize(string value)
private DefaultOptionsProvider? defaultOptions;

private bool? allowAdmin, abortOnConnectFail, resolveDns, ssl, checkCertificateRevocation, heartbeatConsistencyChecks,
includeDetailInExceptions, includePerformanceCountersInExceptions, setClientLibrary;
includeDetailInExceptions, includePerformanceCountersInExceptions, setClientLibrary, highIntegrity;

private string? tieBreaker, sslHost, configChannel, user, password;

Expand Down Expand Up @@ -279,6 +281,18 @@ public bool CheckCertificateRevocation
set => checkCertificateRevocation = value;
}

/// <summary>
/// A Boolean value that specifies whether to use per-command validation of strict protocol validity
NickCraver marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <remarks>The regular RESP protocol does not include correlation identifiers between requests and responses; in exceptional
/// scenarios, protocol desynchronization can occur, which may not be noticed immediately; this option adds additional data
/// to ensure that this cannot occur, at the cost of some (small) additional bandwidth usage.</remarks>
public bool HighIntegrity
{
get => highIntegrity ?? Defaults.HighIntegrity;
set => highIntegrity = value;
}

/// <summary>
/// Create a certificate validation check that checks against the supplied issuer even when not known by the machine.
/// </summary>
Expand Down Expand Up @@ -769,6 +783,7 @@ public int ConfigCheckSeconds
Protocol = Protocol,
heartbeatInterval = heartbeatInterval,
heartbeatConsistencyChecks = heartbeatConsistencyChecks,
highIntegrity = highIntegrity,
};

/// <summary>
Expand Down Expand Up @@ -849,6 +864,7 @@ public string ToString(bool includePassword)
Append(sb, OptionKeys.ResponseTimeout, responseTimeout);
Append(sb, OptionKeys.DefaultDatabase, DefaultDatabase);
Append(sb, OptionKeys.SetClientLibrary, setClientLibrary);
Append(sb, OptionKeys.HighIntegrity, highIntegrity);
Append(sb, OptionKeys.Protocol, FormatProtocol(Protocol));
if (Tunnel is { IsInbuilt: true } tunnel)
{
Expand Down Expand Up @@ -894,7 +910,7 @@ private void Clear()
{
ClientName = ServiceName = user = password = tieBreaker = sslHost = configChannel = null;
keepAlive = syncTimeout = asyncTimeout = connectTimeout = connectRetry = configCheckSeconds = DefaultDatabase = null;
allowAdmin = abortOnConnectFail = resolveDns = ssl = setClientLibrary = null;
allowAdmin = abortOnConnectFail = resolveDns = ssl = setClientLibrary = highIntegrity = null;
SslProtocols = null;
defaultVersion = null;
EndPoints.Clear();
Expand Down Expand Up @@ -1013,6 +1029,9 @@ private ConfigurationOptions DoParse(string configuration, bool ignoreUnknown)
case OptionKeys.SetClientLibrary:
SetClientLibrary = OptionKeys.ParseBoolean(key, value);
break;
case OptionKeys.HighIntegrity:
HighIntegrity = OptionKeys.ParseBoolean(key, value);
break;
case OptionKeys.Tunnel:
if (value.IsNullOrWhiteSpace())
{
Expand Down
37 changes: 37 additions & 0 deletions src/StackExchange.Redis/Message.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.Extensions.Logging;
using StackExchange.Redis.Profiling;
using System;
using System.Buffers.Binary;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
Expand Down Expand Up @@ -52,6 +53,8 @@ internal abstract class Message : ICompletable
{
public readonly int Db;

private int _highIntegrityChecksum;

internal const CommandFlags InternalCallFlag = (CommandFlags)128;

protected RedisCommand command;
Expand Down Expand Up @@ -198,6 +201,18 @@ public bool IsAdmin

public bool IsAsking => (Flags & AskingFlag) != 0;

public bool IsHighIntegrity => _highIntegrityChecksum != 0;
public int HighIntegrityChecksum => _highIntegrityChecksum;

internal void WithHighIntegrity(Random rng)
{
// create a required value if needed; avoid sentinel zero
while (_highIntegrityChecksum is 0)
mgravell marked this conversation as resolved.
Show resolved Hide resolved
{
_highIntegrityChecksum = rng.Next();
}
}
mgravell marked this conversation as resolved.
Show resolved Hide resolved

internal bool IsScriptUnavailable => (Flags & ScriptUnavailableFlag) != 0;

internal void SetScriptUnavailable() => Flags |= ScriptUnavailableFlag;
Expand Down Expand Up @@ -710,6 +725,28 @@ internal void WriteTo(PhysicalConnection physical)
}
}

private static ReadOnlySpan<byte> ChecksumTemplate => "$4\r\nXXXX\r\n"u8;

internal void WriteHighIntegrityChecksumRequest(PhysicalConnection physical)
{
Debug.Assert(IsHighIntegrity, "should only be used for high-integrity");
try
{
physical.WriteHeader(RedisCommand.ECHO, 1); // use WriteHeader to allow command-rewrite

Span<byte> chk = stackalloc byte[10];
Debug.Assert(ChecksumTemplate.Length == chk.Length, "checksum template length error");
ChecksumTemplate.CopyTo(chk);
BinaryPrimitives.WriteInt32LittleEndian(chk.Slice(4, 4), _highIntegrityChecksum);
physical.WriteRaw(chk);
}
catch (Exception ex)
{
physical?.OnInternalError(ex);
Fail(ConnectionFailureType.InternalFailure, ex, null, physical?.BridgeCouldBeNull?.Multiplexer);
}
}

internal static Message CreateHello(int protocolVersion, string? username, string? password, string? clientName, CommandFlags flags)
=> new HelloMessage(protocolVersion, username, password, clientName, flags);

Expand Down
23 changes: 23 additions & 0 deletions src/StackExchange.Redis/PhysicalBridge.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ internal sealed class PhysicalBridge : IDisposable

internal string? PhysicalName => physical?.ToString();

private readonly Random? _highIntegrityEntropy = null;

public DateTime? ConnectedAt { get; private set; }

public PhysicalBridge(ServerEndPoint serverEndPoint, ConnectionType type, int timeoutMilliseconds)
Expand All @@ -82,6 +84,10 @@ public PhysicalBridge(ServerEndPoint serverEndPoint, ConnectionType type, int ti
#if !NETCOREAPP
_singleWriterMutex = new MutexSlim(timeoutMilliseconds: timeoutMilliseconds);
#endif
if (type == ConnectionType.Interactive && Multiplexer.RawConfig.HighIntegrity)
{
_highIntegrityEntropy = new Random();
}
}

private readonly int TimeoutMilliseconds;
Expand Down Expand Up @@ -1546,10 +1552,27 @@ private WriteResult WriteMessageToServerInsideWriteLock(PhysicalConnection conne
break;
}

if (_highIntegrityEntropy is not null && !connection.TransactionActive)
{
// make sure this value exists early to avoid a race condition
// if the response comes back super quickly
message.WithHighIntegrity(_highIntegrityEntropy);
Debug.Assert(message.IsHighIntegrity, "message should be high integrity");
}
else
{
Debug.Assert(!message.IsHighIntegrity, "prior high integrity message found during transaction?");
}
connection.EnqueueInsideWriteLock(message);
isQueued = true;
message.WriteTo(connection);

if (message.IsHighIntegrity)
{
message.WriteHighIntegrityChecksumRequest(connection);
IncrementOpCount();
}

message.SetRequestSent();
IncrementOpCount();

Expand Down
Loading
Loading