Skip to content

Commit

Permalink
fix: pass compare should use contant time compare to avoid timing attack
Browse files Browse the repository at this point in the history
  • Loading branch information
nan01ab committed Oct 6, 2024
1 parent f0add81 commit 0a5e52c
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 54 deletions.
25 changes: 0 additions & 25 deletions src/Neo.Extensions/StringExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,30 +43,5 @@ public static int GetVarSize(this string value)
var size = Utility.StrictUTF8.GetByteCount(value);
return UnsafeData.GetVarSize(size) + size;
}

/// <summary>
/// Compares two <see cref="string"/>s for equality in constant time.
/// </summary>
/// <param name="left">The left <see cref="string"/>.</param>
/// <param name="right">The right <see cref="string"/>.</param>
/// <returns>True if the two <see cref="string"/>s are equal, false otherwise.</returns>
public static bool ConstantTimeEquals(this string left, string right)
{
if (left == null && right == null)
return true;

if (left == null || right == null)
return false;

if (left.Length != right.Length)
return false;

var lhs = left.AsSpan();
var rhs = right.AsSpan();
var result = 0;
for (var i = 0; i < lhs.Length; i++)
result |= lhs[i] ^ rhs[i];
return result == 0;
}
}
}
22 changes: 17 additions & 5 deletions src/Plugins/RpcServer/RpcServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
using System.Linq.Expressions;
using System.Net.Security;
using System.Reflection;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Text;
using System.Threading.Tasks;
Expand All @@ -45,10 +46,18 @@ public partial class RpcServer : IDisposable
private readonly NeoSystem system;
private readonly LocalNode localNode;

// avoid GetBytes every time
private readonly byte[] _rpcUser;
private readonly byte[] _rpcPass;

public RpcServer(NeoSystem system, RpcServerSettings settings)
{
this.system = system;
this.settings = settings;

_rpcUser = settings.RpcUser is not null ? Encoding.UTF8.GetBytes(settings.RpcUser) : [];
_rpcPass = settings.RpcPass is not null ? Encoding.UTF8.GetBytes(settings.RpcPass) : [];

localNode = system.LocalNode.Ask<LocalNode>(new LocalNode.GetInstance()).Result;
RegisterMethods(this);
Initialize_SmartContract();
Expand All @@ -66,21 +75,24 @@ internal bool CheckAuth(HttpContext context)
return false;
}

string authstring;
byte[] auths;
try
{
authstring = Encoding.UTF8.GetString(Convert.FromBase64String(reqauth.Replace("Basic ", "").Trim()));
auths = Convert.FromBase64String(reqauth.Replace("Basic ", "").Trim());
}
catch
{
return false;
}

string[] authvalues = authstring.Split(new string[] { ":" }, StringSplitOptions.RemoveEmptyEntries);
if (authvalues.Length < 2)
int colonIndex = Array.IndexOf(auths, (byte)':');
if (colonIndex == -1)
return false;

return authvalues[0].ConstantTimeEquals(settings.RpcUser) && authvalues[1].ConstantTimeEquals(settings.RpcPass);
byte[] user = auths[..colonIndex];
byte[] pass = auths[(colonIndex + 1)..];

return CryptographicOperations.FixedTimeEquals(user, _rpcUser) && CryptographicOperations.FixedTimeEquals(pass, _rpcPass);
}

private static JObject CreateErrorResponse(JToken id, RpcError rpcError)
Expand Down
24 changes: 0 additions & 24 deletions tests/Neo.Extensions.Tests/UT_StringExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,5 @@ public void TestGetVarSizeString()
int result = "AA".GetVarSize();
Assert.AreEqual(3, result);
}

[TestMethod]
public void TestConstantTimeEquals()
{
string str1 = "AA";
string str2 = "AA";
str1.ConstantTimeEquals(str2).Should().BeTrue();

str1 = "AB";
str2 = "AA";
str1.ConstantTimeEquals(str2).Should().BeFalse();

str1 = "AB";
str2 = null;
str1.ConstantTimeEquals(str2).Should().BeFalse();

str1 = null;
str2 = "AA";
str1.ConstantTimeEquals(str2).Should().BeFalse();

str1 = null;
str2 = null;
str1.ConstantTimeEquals(str2).Should().BeTrue();
}
}
}
42 changes: 42 additions & 0 deletions tests/Neo.Plugins.RpcServer.Tests/UT_RpcServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,47 @@ public void TestCheckAuth_ValidCredentials_ReturnsTrue()
// Assert
Assert.IsTrue(result);
}

[TestMethod]
public void TestCheckAuth()
{
var memoryStoreProvider = new TestMemoryStoreProvider(new MemoryStore());
var neoSystem = new NeoSystem(TestProtocolSettings.SoleNode, memoryStoreProvider);
var rpcServerSettings = RpcServerSettings.Default with
{
SessionEnabled = true,
SessionExpirationTime = TimeSpan.FromSeconds(0.3),
MaxGasInvoke = 1500_0000_0000,
Network = TestProtocolSettings.SoleNode.Network,
RpcUser = "testuser",
RpcPass = "testpass",
};
var rpcServer = new RpcServer(neoSystem, rpcServerSettings);

var context = new DefaultHttpContext();
context.Request.Headers["Authorization"] = "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes("testuser:testpass"));
var result = rpcServer.CheckAuth(context);
Assert.IsTrue(result);

context.Request.Headers["Authorization"] = "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes("testuser:wrongpass"));
result = rpcServer.CheckAuth(context);
Assert.IsFalse(result);

context.Request.Headers["Authorization"] = "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes("wronguser:testpass"));
result = rpcServer.CheckAuth(context);
Assert.IsFalse(result);

context.Request.Headers["Authorization"] = "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes("testuser:"));
result = rpcServer.CheckAuth(context);
Assert.IsFalse(result);

context.Request.Headers["Authorization"] = "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes(":testpass"));
result = rpcServer.CheckAuth(context);
Assert.IsFalse(result);

context.Request.Headers["Authorization"] = "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes(""));
result = rpcServer.CheckAuth(context);
Assert.IsFalse(result);
}
}
}

0 comments on commit 0a5e52c

Please sign in to comment.