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: requestValidation when hostname URL has any uppercase letters #567

Merged
merged 35 commits into from
May 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
81bb010
tryint to recreate using travis
thinkingserious May 15, 2021
3d9a854
add testing instructions, update uri string
thinkingserious May 19, 2021
08ec577
update path
thinkingserious May 19, 2021
9922195
add test
thinkingserious May 20, 2021
f072dbf
Merge branch 'main' into request-validator
thinkingserious May 20, 2021
24cda85
update tests
thinkingserious May 20, 2021
037fae7
Merge branch 'request-validator' of github.com:twilio/twilio-csharp i…
thinkingserious May 20, 2021
a9a0a37
update tests
thinkingserious May 20, 2021
1288e80
update test instructions
thinkingserious May 20, 2021
a4d9599
update tests
thinkingserious May 20, 2021
baf1d7c
preserve casing
thinkingserious May 20, 2021
9b90ba1
preserve casing
thinkingserious May 20, 2021
1ef2149
update tests
thinkingserious May 20, 2021
6d8d79d
remove dev docker
thinkingserious May 21, 2021
fe89116
formatting
thinkingserious May 21, 2021
9ff3172
add more tests
thinkingserious May 21, 2021
1bb51ba
moar tests
thinkingserious May 21, 2021
f331442
fix scheme and credential logic
thinkingserious May 21, 2021
1a0a193
clean up
thinkingserious May 21, 2021
23a97e8
clean up
thinkingserious May 21, 2021
98dcf75
clean up
thinkingserious May 21, 2021
ec3e057
simplify scheme replace
thinkingserious May 24, 2021
05c8ce0
cleanup
thinkingserious May 24, 2021
80381f9
cleanup
thinkingserious May 24, 2021
c6a39bc
refactoring
thinkingserious May 25, 2021
d1a8df6
refactoring
thinkingserious May 25, 2021
f2ed185
update signature to include custom port
thinkingserious May 26, 2021
aff4259
simplify case preservation
thinkingserious May 26, 2021
6e4de34
fix port assignment logic
thinkingserious May 26, 2021
4e32bee
cleanup
thinkingserious May 26, 2021
59c2890
add tests
thinkingserious May 26, 2021
b50acec
fix port assignment logic
thinkingserious May 27, 2021
abe2405
fix tests
thinkingserious May 27, 2021
0ca4fba
comment
thinkingserious May 27, 2021
cbef3d1
style
thinkingserious May 28, 2021
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: 0 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ you are working:
* All features or bug fixes **must be tested** by one or more tests.
* All classes and methods **must be documented**.


[docs-link]: https://www.twilio.com/docs/libraries/csharp
[issue-link]: https://github.com/twilio/twilio-csharp/issues/new
[github]: https://github.com/twilio/twilio-csharp
42 changes: 30 additions & 12 deletions src/Twilio/Security/RequestValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public bool Validate(string url, NameValueCollection parameters, string expected
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(new UriBuilder(url)), parameters);
var signatureWithPort = GetValidationSignature(AddPort(new UriBuilder(url)), parameters);
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);
}
Expand Down Expand Up @@ -124,23 +124,41 @@ private static bool SecureCompare(string a, string b)
return mismatch == 0;
}

private string RemovePort(UriBuilder uri)
private string RemovePort(string url)
{
// UriBuilder.ToString() will not display the port
// if the Port property is set to -1
uri.Port = -1;
return uri.ToString();
return SetPort(url, -1);
}

private string AddPort(UriBuilder uri)
private string AddPort(string url)
{
if (uri.Port != -1)
var uri = new UriBuilder(url);
return SetPort(url, uri.Port);
childish-sambino marked this conversation as resolved.
Show resolved Hide resolved
}

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

private string PreserveCase(string url, string replacementString)
{
var startIndex = url.IndexOf(replacementString, StringComparison.OrdinalIgnoreCase);
return url.Substring(startIndex, replacementString.Length);
}
}
}
88 changes: 87 additions & 1 deletion test/Twilio.Test/Security/RequestValidatorTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using NUnit.Framework;
using Twilio.Security;
Expand Down Expand Up @@ -31,6 +32,78 @@ public void TestValidateDictionary()
Assert.IsTrue(_validator.Validate(Url, _parameters, signature), "Request does not match provided signature");
}

[Test]
public void TestValidateDictionaryMixedCase()
{
const string signature = "g9IN/x4Cft2g517EjYvEvM/W7LU=";
const string url = "https://MyCompany.com/myapp.php?foo=1&bar=2";
Assert.IsTrue(_validator.Validate(url, _parameters, signature), "Request should have passed validation but didn't");
}

[Test]
public void TestValidateDictionaryMixedCaseWithPort()
{
const string signature = "PCqOZm8cnu/L6+u5i5fuEd+Iac4=";
const string url = "https://MyCompany.com:1234/myapp.php?foo=1&bar=2";
Assert.IsTrue(_validator.Validate(url, _parameters, signature), "Request should have passed validation but didn't");
}

[Test]
public void TestValidateDictionaryMixedCaseAddsPortHttp()
{
const string signature = "XY7AlKOKL6im4yWyX84gldUrtis=";
const string url = "http://MyCompany.com/myapp.php?foo=1&bar=2";
Assert.IsTrue(_validator.Validate(url, _parameters, signature), "Request should have passed validation but didn't");
}

[Test]
public void TestValidateCredentialsArePreserved()
{
const string signature = "PJ08CxXr7KLPjEQCTc8LkUSMwSM=";
const string url = "http://username:password@MyCompany.com/myapp.php?foo=1&bar=2";
Assert.IsTrue(_validator.Validate(url, _parameters, signature), $"Request should have passed validation but didn't");
}

[Test]
public void TestValidateCredentialsArePreservedUpperCaseScheme()
{
const string signature = "hy/l8pca+LFms4cvRdv8uiP6NGc=";
const string url = "HTTP://username:password@MyCompany.com/myapp.php?foo=1&bar=2";
Assert.IsTrue(_validator.Validate(url, _parameters, signature), $"Request should have passed validation but didn't");
}

[Test]
public void TestValidateCredentialsArePreservedMixedCaseCreds()
{
const string signature = "SkTSKyDJH9kUlhItI0slbgZ1UsI=";
const string url = "http://Username:Password@MyCompany.com/myapp.php?foo=1&bar=2";
Assert.IsTrue(_validator.Validate(url, _parameters, signature), $"Request should have passed validation but didn't");
}

[Test]
public void TestValidateDictionaryMixedCaseWithIncorrectSignature()
{
const string signature = "RSOYDt4T1cUTdK1PDd93/VVr8B8=";
const string url = "https://MyCompany.com/myapp.php?foo=1&bar=2";
Assert.IsFalse(_validator.Validate(url, _parameters, signature), "Request should have failed validation but didn't");
}

[Test]
public void TestValidateDictionaryMixedCaseWithPortWithIncorrectSignature()
{
const string signature = "RSOYDt4T1cUTdK1PDd93/VVr8B8=";
const string url = "https://MyCompany.com:1234/myapp.php?foo=1&bar=2";
Assert.IsFalse(_validator.Validate(url, _parameters, signature), "Request should have failed validation but didn't");
}

[Test]
public void TestValidateDictionaryMixedCaseAddsPortHttpWithIncorrectSignature()
{
const string signature = "RSOYDt4T1cUTdK1PDd93/VVr8B8=";
const string url = "http://MyCompany.com/myapp.php?foo=1&bar=2";
Assert.IsFalse(_validator.Validate(url, _parameters, signature), "Request should have failed validation but didn't");
thinkingserious marked this conversation as resolved.
Show resolved Hide resolved
}

[Test]
public void TestValidateFailsWhenIncorrect()
{
Expand Down Expand Up @@ -84,7 +157,7 @@ public void TestValidateRemovesPortHttp()
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 TestValidateAddsPortHttps()
{
Expand All @@ -98,5 +171,18 @@ public void TestValidateAddsPortHttp()
Assert.IsTrue(_validator.Validate(url, _parameters, "0ZXoZLH/DfblKGATFgpif+LLRf4="), "Request does not match provided signature");
}

[Test]
public void TestValidateAddsCustomPortHttps()
{
const string url = "https://mycompany.com:1234/myapp.php?foo=1&bar=2";
Assert.IsTrue(_validator.Validate(url, _parameters, "CYn1h0gLBdNxDGkKJg6hYohgeD8="), "Request does not match provided signature");
}

[Test]
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");
}
}
}