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

fix: RequestValidator thread-safety #598

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions src/Twilio/Security/RequestValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,15 @@ namespace Twilio.Security
/// </summary>
public class RequestValidator
{
private readonly HMACSHA1 _hmac;
private readonly SHA256 _sha;
private readonly string _secret;

/// <summary>
/// Create a new RequestValidator
/// </summary>
/// <param name="secret">Signing secret</param>
public RequestValidator(string secret)
{
_hmac = new HMACSHA1(Encoding.UTF8.GetBytes(secret));
_sha = SHA256.Create();
_secret = secret;
}

/// <summary>
Expand Down Expand Up @@ -68,10 +66,15 @@ public bool Validate(string url, string body, string expected)
return Validate(url, new Dictionary<string, string>(), expected) && ValidateBody(body, bodyHash);
}

public bool ValidateBody(string rawBody, string expected)
public static bool ValidateBody(string rawBody, string expected)
{
var signature = _sha.ComputeHash(Encoding.UTF8.GetBytes(rawBody));
return SecureCompare(BitConverter.ToString(signature).Replace("-","").ToLower(), expected);
// TODO: In future, use net6's one-shot methods.
// See: https://github.com/twilio/twilio-csharp/issues/466#issuecomment-1028091370
using (var sha = SHA256.Create())
{
var signature = sha.ComputeHash(Encoding.UTF8.GetBytes(rawBody));
return SecureCompare(BitConverter.ToString(signature).Replace("-", "").ToLower(), expected);
}
}

private static IDictionary<string, string> ToDictionary(NameValueCollection col)
Expand All @@ -96,10 +99,15 @@ private string GetValidationSignature(string url, IDictionary<string, string> pa
{
b.Append(key).Append(parameters[key] ?? "");
}
}

// TODO: In future, use net6's one-shot methods.
// See: https://github.com/twilio/twilio-csharp/issues/466#issuecomment-1028091370
using (var hmac = new HMACSHA1(Encoding.UTF8.GetBytes(_secret)))
{
var hash = hmac.ComputeHash(Encoding.UTF8.GetBytes(b.ToString()));
return Convert.ToBase64String(hash);
}

var hash = _hmac.ComputeHash(Encoding.UTF8.GetBytes(b.ToString()));
return Convert.ToBase64String(hash);
}

private static bool SecureCompare(string a, string b)
Expand All @@ -124,18 +132,18 @@ private static bool SecureCompare(string a, string b)
return mismatch == 0;
}

private string RemovePort(string url)
private static string RemovePort(string url)
{
return SetPort(url, -1);
}

private string AddPort(string url)
private static string AddPort(string url)
{
var uri = new UriBuilder(url);
return SetPort(url, uri.Port);
}

private string SetPort(string url, int port)
private static string SetPort(string url, int port)
{
var uri = new UriBuilder(url);
uri.Host = PreserveCase(url, uri.Host);
Expand All @@ -155,7 +163,7 @@ private string SetPort(string url, int port)
return uri.Uri.OriginalString.Replace(uri.Scheme, scheme);
}

private string PreserveCase(string url, string replacementString)
private static string PreserveCase(string url, string replacementString)
{
var startIndex = url.IndexOf(replacementString, StringComparison.OrdinalIgnoreCase);
return url.Substring(startIndex, replacementString.Length);
Expand Down
29 changes: 28 additions & 1 deletion test/Twilio.Test/Security/RequestValidatorTest.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Diagnostics;
using System.Threading;
using NUnit.Framework;
using Twilio.Security;

Expand Down Expand Up @@ -121,7 +123,7 @@ public void TestValidateCollection()
[Test]
public void TestValidateBody()
{
Assert.IsTrue(_validator.ValidateBody(Body, BodyHash), "Request body validation failed");
Assert.IsTrue(RequestValidator.ValidateBody(Body, BodyHash), "Request body validation failed");
}

[Test]
Expand Down Expand Up @@ -183,6 +185,31 @@ public void TestValidateAddsCustomPortHttp()
{
const string url = "http://mycompany.com:1234/myapp.php?foo=1&bar=2";
Assert.IsTrue(_validator.Validate(url, _parameters, "Zmvh+3yNM1Phv2jhDCwEM3q5ebU="), "Request does not match provided signature");
}

[Test]
public void TestIsThreadSafe()
{
var validator = new RequestValidator("secret");
var thread1 = new Thread(Validate);
var thread2 = new Thread(Validate);

Assert.DoesNotThrow(() =>
{
thread1.Start(validator);
thread2.Start(validator);
thread1.Join();
thread2.Join();
});
}

private static void Validate(object obj)
{
var sw = Stopwatch.StartNew();
while (sw.ElapsedMilliseconds < 5000)
{
((RequestValidator)obj).Validate("https://foo.com", "123", "foo");
}
}
}
}