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

feat: Optimize request validator + make thread safe (backwards compatible) #660

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,4 @@ html/

# sonar cloud stuff
.sonarqube
/test/Twilio.Benchmark/BenchmarkDotNet.Artifacts
19 changes: 17 additions & 2 deletions Twilio.sln
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 15
VisualStudioVersion = 15.0.26206.0
# Visual Studio Version 17
VisualStudioVersion = 17.4.33205.214
MinimumVisualStudioVersion = 10.0.40219.1
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{36585F38-8C30-49A9-BDA1-9A0DC61C288B}"
EndProject
Expand All @@ -11,6 +11,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Twilio", "src\Twilio\Twilio
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Twilio.Test", "test\Twilio.Test\Twilio.Test.csproj", "{DC35107A-F987-47A3-B0BC-7110BA15943C}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Twilio.Benchmark", "test\Twilio.Benchmark\Twilio.Benchmark.csproj", "{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -45,13 +47,26 @@ Global
{DC35107A-F987-47A3-B0BC-7110BA15943C}.Release|x64.Build.0 = Release|Any CPU
{DC35107A-F987-47A3-B0BC-7110BA15943C}.Release|x86.ActiveCfg = Release|Any CPU
{DC35107A-F987-47A3-B0BC-7110BA15943C}.Release|x86.Build.0 = Release|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Debug|Any CPU.Build.0 = Debug|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Debug|x64.ActiveCfg = Debug|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Debug|x64.Build.0 = Debug|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Debug|x86.ActiveCfg = Debug|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Debug|x86.Build.0 = Debug|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Release|Any CPU.ActiveCfg = Release|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Release|Any CPU.Build.0 = Release|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Release|x64.ActiveCfg = Release|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Release|x64.Build.0 = Release|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Release|x86.ActiveCfg = Release|Any CPU
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1}.Release|x86.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
GlobalSection(NestedProjects) = preSolution
{62BB8FE9-99DD-475D-80EB-D2E53C380754} = {36585F38-8C30-49A9-BDA1-9A0DC61C288B}
{DC35107A-F987-47A3-B0BC-7110BA15943C} = {FE04FB2E-73FB-4E45-AAEE-EE04754A5E9C}
{80DEE3E3-5F8E-40C2-A68C-15463C4CFAB1} = {FE04FB2E-73FB-4E45-AAEE-EE04754A5E9C}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {75638FC3-0E0B-4D79-8BEB-8CC499BF98C5}
Expand Down
137 changes: 94 additions & 43 deletions src/Twilio/Security/RequestValidator.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Linq;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Security.Cryptography;
Expand Down Expand Up @@ -45,16 +46,63 @@ public bool Validate(string url, NameValueCollection parameters, string expected
/// <returns>true if the signature matches the result; false otherwise</returns>
public bool Validate(string url, IDictionary<string, string> parameters, string expected)
{
// check signature of url with and without port, since sig generation on back end is inconsistent
var signatureWithoutPort = GetValidationSignature(RemovePort(url), parameters);
var signatureWithPort = GetValidationSignature(AddPort(url), parameters);
// If either url produces a valid signature, we accept the request as valid
return SecureCompare(signatureWithoutPort, expected) || SecureCompare(signatureWithPort, expected);
}

if (string.IsNullOrEmpty(url))
throw new ArgumentException("Parameter 'url' cannot be null or empty.");
if (string.IsNullOrEmpty(expected))
throw new ArgumentException("Parameter 'expected' cannot be null or empty.");

if(parameters == null || parameters.Count == 0)
{
var signature = GetValidationSignature(url);
if (SecureCompare(signature, expected)) return true;

// check signature of url with and without port, since sig generation on back end is inconsistent
// If either url produces a valid signature, we accept the request as valid
url = GetUriVariation(url);
signature = GetValidationSignature(url);
if (SecureCompare(signature, expected)) return true;
return false;
}
else
{
var parameterStringBuilder = GetJoinedParametersStringBuilder(parameters);
parameterStringBuilder.Insert(0, url);
var signature = GetValidationSignature(parameterStringBuilder.ToString());
if (SecureCompare(signature, expected)) return true;
parameterStringBuilder.Remove(0, url.Length);

// check signature of url with and without port, since sig generation on back end is inconsistent
// If either url produces a valid signature, we accept the request as valid
url = GetUriVariation(url);
parameterStringBuilder.Insert(0, url);
signature = GetValidationSignature(parameterStringBuilder.ToString());
if (SecureCompare(signature, expected)) return true;

return false;
}
}

private StringBuilder GetJoinedParametersStringBuilder(IDictionary<string, string> parameters)
{
var keys = parameters.Keys.ToArray();
Array.Sort(keys, StringComparer.Ordinal);

var b = new StringBuilder();
foreach (var key in keys)
{
b.Append(key).Append(parameters[key] ?? "");
}
return b;
}

public bool Validate(string url, string body, string expected)
{
var paramString = new UriBuilder(url).Query.TrimStart('?');
if (string.IsNullOrEmpty(url))
throw new ArgumentException("Parameter 'url' cannot be null or empty.");
if (string.IsNullOrEmpty(expected))
throw new ArgumentException("Parameter 'expected' cannot be null or empty.");

var paramString = new Uri(url, UriKind.Absolute).Query.TrimStart('?');
var bodyHash = "";
foreach (var param in paramString.Split('&'))
{
Expand All @@ -65,13 +113,13 @@ public bool Validate(string url, string body, string expected)
}
}

return Validate(url, new Dictionary<string, string>(), expected) && ValidateBody(body, bodyHash);
return Validate(url, (IDictionary<string, string>) null, expected) && ValidateBody(body, bodyHash);
}

public bool ValidateBody(string rawBody, string expected)
{
var signature = _sha.ComputeHash(Encoding.UTF8.GetBytes(rawBody));
return SecureCompare(BitConverter.ToString(signature).Replace("-","").ToLower(), expected);
return SecureCompare(BitConverter.ToString(signature).Replace("-", "").ToLower(), expected);
Swimburger marked this conversation as resolved.
Show resolved Hide resolved
}

private static IDictionary<string, string> ToDictionary(NameValueCollection col)
Expand All @@ -81,27 +129,16 @@ private static IDictionary<string, string> ToDictionary(NameValueCollection col)
{
dict.Add(k, col[k]);
}

return dict;
}

private string GetValidationSignature(string url, IDictionary<string, string> parameters)
private string GetValidationSignature(string urlWithParameters)
{
var b = new StringBuilder(url);
if (parameters != null)
{
var sortedKeys = new List<string>(parameters.Keys);
sortedKeys.Sort(StringComparer.Ordinal);

foreach (var key in sortedKeys)
{
b.Append(key).Append(parameters[key] ?? "");
}
}

var hash = _hmac.ComputeHash(Encoding.UTF8.GetBytes(b.ToString()));
byte[] hash = _hmac.ComputeHash(Encoding.UTF8.GetBytes(urlWithParameters));
Swimburger marked this conversation as resolved.
Show resolved Hide resolved
return Convert.ToBase64String(hash);
}

}
private static bool SecureCompare(string a, string b)
Swimburger marked this conversation as resolved.
Show resolved Hide resolved
{
if (a == null || b == null)
Expand All @@ -124,41 +161,55 @@ private static bool SecureCompare(string a, string b)
return mismatch == 0;
}

private string RemovePort(string url)
/// <summary>
/// Returns URL without port if given URL has port, returns URL with port if given URL has no port
/// </summary>
/// <param name="url"></param>
/// <returns></returns>
private string GetUriVariation(string url)
{
return SetPort(url, -1);
}
var uri = new Uri(url);
var uriBuilder = new UriBuilder(uri);
var port = uri.GetComponents(UriComponents.Port, UriFormat.UriEscaped);
// if port already removed
if (port == "")
{
return SetPort(url, uriBuilder, uriBuilder.Port);
}

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

private string SetPort(string url, int port)
private string SetPort(string url, UriBuilder uri, int newPort)
{
var uri = new UriBuilder(url);
uri.Host = PreserveCase(url, uri.Host);
if (port == -1)
if (newPort == -1)
{
uri.Port = port;
uri.Port = newPort;
}
else if ((port != 443) && (port != 80))
else if (newPort != 443 && newPort != 80)
{
uri.Port = port;
uri.Port = newPort;
}
else
{
uri.Port = uri.Scheme == "https" ? 443 : 80;
}

var uriStringBuilder = new StringBuilder(uri.ToString());

var host = PreserveCase(url, uri.Host);
uriStringBuilder.Replace(uri.Host, host);

var scheme = PreserveCase(url, uri.Scheme);
return uri.Uri.OriginalString.Replace(uri.Scheme, scheme);
}
uriStringBuilder.Replace(uri.Scheme, scheme);

return uriStringBuilder.ToString();
}

private string PreserveCase(string url, string replacementString)
{
var startIndex = url.IndexOf(replacementString, StringComparison.OrdinalIgnoreCase);
return url.Substring(startIndex, replacementString.Length);
}
}
}
}
92 changes: 92 additions & 0 deletions test/Twilio.Benchmark/Program.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
using System.Collections.Specialized;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using Twilio.Security;

var summary = BenchmarkRunner.Run<RequestValidationBenchmark>();
Console.Write(summary);

[MemoryDiagnoser]
public class RequestValidationBenchmark
{
private const string Secret = "12345";
private const string UnhappyPathUrl = "HTTP://MyCompany.com:8080/myapp.php?foo=1&bar=2";
private const string UnhappyPathSignature = "eYYN9fMlxrQMXOsr7bIzoPTrbxA=";
private const string HappyPathUrl = "https://mycompany.com/myapp.php?foo=1&bar=2";
private const string HappyPathSignature = "3LL3BFKOcn80artVM5inMPFpmtU=";
private static readonly NameValueCollection UnhappyPathParameters = new()
{
{"ToCountry", "US"},
{"ToState", "OH"},
{"SmsMessageSid", "SMcea2a3bd6f50296f8fab60f377db03eb"},
{"NumMedia", "0"},
{"ToCity", "UTICA"},
{"FromZip", "20705"},
{"SmsSid", "SMcea2a3bd6f50296f8fab60f377db03eb"},
{"FromState", "DC"},
{"SmsStatus", "received"},
{"FromCity", "BELTSVILLE"},
{"Body", "Ahoy!"},
{"FromCountry", "US"},
{"To", "+10123456789"},
{"ToZip", "43037"},
{"NumSegments", "1"},
{"ReferralNumMedia", "0"},
{"MessageSid", "SMcea2a3bd6f50296f8fab60f377db03eb"},
{"AccountSid", "ACe718619887aac3ee5b21edafbvsdf6h7fgb"},
{"From", "+10123456789"},
{"ApiVersion", "2010-04-01"}
};
private static readonly Dictionary<string, string> HappyPathParameters = new()
{
{"ToCountry", "US"},
{"ToState", "OH"},
{"SmsMessageSid", "SMcea2a3bd6f50296f8fab60f377db03eb"},
{"NumMedia", "0"},
{"ToCity", "UTICA"},
{"FromZip", "20705"},
{"SmsSid", "SMcea2a3bd6f50296f8fab60f377db03eb"},
{"FromState", "DC"},
{"SmsStatus", "received"},
{"FromCity", "BELTSVILLE"},
{"Body", "Ahoy!"},
{"FromCountry", "US"},
{"To", "+10123456789"},
{"ToZip", "43037"},
{"NumSegments", "1"},
{"ReferralNumMedia", "0"},
{"MessageSid", "SMcea2a3bd6f50296f8fab60f377db03eb"},
{"AccountSid", "ACe718619887aac3ee5b21edafbvsdf6h7fgb"},
{"From", "+10123456789"},
{"ApiVersion", "2010-04-01"}
};


[Benchmark]
public void OriginalUnhappyPath()
{
var requestValidator = new RequestValidatorOriginal(Secret);
requestValidator.Validate(UnhappyPathUrl, UnhappyPathParameters, UnhappyPathSignature);
}

[Benchmark]
public void CurrentUnhappyPath()
{
var requestValidator = new RequestValidator(Secret);
requestValidator.Validate(UnhappyPathUrl, UnhappyPathParameters, UnhappyPathSignature);
}

[Benchmark]
public void OriginalHappyPath()
{
var requestValidator = new RequestValidatorOriginal(Secret);
requestValidator.Validate(HappyPathUrl, HappyPathParameters, HappyPathSignature);
}

[Benchmark]
public void CurrentHappyPath()
{
var requestValidator = new RequestValidator(Secret);
requestValidator.Validate(HappyPathUrl, HappyPathParameters, HappyPathSignature);
}
}
Loading