Validate invalid HTTP headers (#703)

Creating new function to insert headers by validating characters on it are accepted chars for header name
fixes: https://github.com/Azure/azure-sdk-for-cpp/issues/191
This commit is contained in:
Victor Vazquez 2020-10-09 16:35:58 -07:00 committed by GitHub
parent dc9d3b0595
commit 2cc4ecdea8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 366 additions and 50 deletions

View File

@ -21,6 +21,7 @@ add_library (
src/credentials/policy/policies.cpp
src/http/body_stream.cpp
src/http/curl/curl.cpp
src/http/http.cpp
src/http/logging_policy.cpp
src/http/policy.cpp
src/http/request.cpp

View File

@ -116,7 +116,7 @@ namespace Azure { namespace Core { namespace Http {
/**
* @brief Keeps an unique key for each host and creates a connection pool for each key.
*
* @detail This way getting a connection for an specific host can be done in O(1) instead of
* @detail This way getting a connection for a specific host can be done in O(1) instead of
* looping a single connection list to find the first connection for the required host.
*
* @remark There might be multiple connections for each host.
@ -418,7 +418,7 @@ namespace Azure { namespace Core { namespace Http {
bool isUploadRequest();
/**
* @brief Set up libcurl handle to behave as an specific HTTP Method.
* @brief Set up libcurl handle to behave as a specific HTTP Method.
*
* @return returns the libcurl result after setting up.
*/

View File

@ -40,6 +40,21 @@ namespace Azure { namespace Core { namespace Http {
left.insert(right.begin(), right.end());
return left;
}
/**
* @brief Insert a header into \p headers checking that \p headerName does not contain invalid
* characters.
*
* @param headers The headers map where to insert header.
* @param headerName The header name for the header to be inserted.
* @param headerValue The header value for the header to be inserted.
*
* @throw if \p headerName is invalid.
*/
void InsertHeaderWithValidation(
std::map<std::string, std::string>& headers,
std::string const& headerName,
std::string const& headerValue);
} // namespace Details
/**
@ -309,10 +324,10 @@ namespace Azure { namespace Core { namespace Http {
* @brief Append an HTTP query parameter.
*
* @note Overrides previous query parameters.
* @remark A query key can't contain any characters that need to be URL-encoded (per RFC).
* @remark A query key can't contain any characters that need to be URL-encoded (per RFC 7230).
*
* @param key HTTP query parameter.
* @param value HTTP quary parameter value.
* @param value HTTP query parameter value.
* @param isValueEncoded `true` if \p value is URL-encoded, `false` otherwise.
*/
void AppendQuery(const std::string& key, const std::string& value, bool isValueEncoded = false)
@ -360,7 +375,7 @@ namespace Azure { namespace Core { namespace Http {
const std::string& GetPath() const { return m_encodedPath; }
/**
* @brief Get all the query paramters in the URL.
* @brief Get all the query parameters in the URL.
*
* @note Retry parameters have preference and will override any value from the initial query
* parameters.
@ -418,7 +433,7 @@ namespace Azure { namespace Core { namespace Http {
public:
/**
* @brief Construct an HTTP request.
* @brief Construct an HTTP @request.
*
* @param httpMethod HTTP method.
* @param url URL.
@ -432,7 +447,7 @@ namespace Azure { namespace Core { namespace Http {
}
/**
* @brief Construct an HTTP request.
* @brief Construct an HTTP @request.
*
* @param httpMethod HTTP method.
* @param url URL.
@ -444,13 +459,12 @@ namespace Azure { namespace Core { namespace Http {
}
/**
* @brief Construct an HTTP request.
* @brief Construct an HTTP @request.
*
* @param httpMethod HTTP method.
* @param url URL.
* @param downloadViaStream
*/
// Typically used for GET with no request body that can return bodyStream
explicit Request(HttpMethod httpMethod, Url url, bool downloadViaStream)
: Request(
httpMethod,
@ -461,22 +475,23 @@ namespace Azure { namespace Core { namespace Http {
}
/**
* @brief Construct an HTTP request.
* @brief Construct an HTTP @request.
*
* @param httpMethod HTTP method.
* @param url URL.
*/
// Typically used for GET with no request body.
explicit Request(HttpMethod httpMethod, Url url)
: Request(httpMethod, std::move(url), NullBodyStream::GetNullBodyStream(), false)
{
}
/**
* @brief Add an HTTP header.
* @brief Add HTTP header to the @Request.
*
* @param name HTTP header name.
* @param value HTTP header value.
* @param name The name for the header to be added.
* @param value The value for the header to be added.
*
* @throw if \p name is an invalid header key.
*/
void AddHeader(std::string const& name, std::string const& value);
@ -547,7 +562,7 @@ namespace Azure { namespace Core { namespace Http {
explicit CouldNotResolveHostException(std::string const& msg) : std::runtime_error(msg) {}
};
// Any other exception from transport layer without an specific exception defined above
// Any other exception from transport layer without a specific exception defined above
/**
* @brief HTTP transport layer error.
*/
@ -556,6 +571,15 @@ namespace Azure { namespace Core { namespace Http {
explicit TransportException(std::string const& msg) : std::runtime_error(msg) {}
};
/**
* @brief An invalid header key name in @Request or @RawResponse
*
*/
struct InvalidHeaderException : public std::runtime_error
{
explicit InvalidHeaderException(std::string const& msg) : std::runtime_error(msg) {}
};
/**
* @brief Raw HTTP response.
*/
@ -603,27 +627,42 @@ namespace Azure { namespace Core { namespace Http {
// ===== Methods used to build HTTP response =====
/**
* @brief Add header to the HTTP response.
* @brief Add HTTP header to the @RawResponse.
*
* @param name HTTP response header name.
* @param value HTTP response header value.
* @remark The \p name must contain valid header name characters (RFC 7230).
*
* @param name The name for the header to be added.
* @param value The value for the header to be added.
*
* @throw if \p name contains invalid characters.
*/
void AddHeader(std::string const& name, std::string const& value);
/**
* @brief Add header to the HTTP response.
* @brief Add HTTP header to the @RawResponse.
*
* @param header HTTP response header in RFCnnnn format (`OWS header-value OWS`).
* @remark The \p header must contain valid header name characters (RFC 7230).
* @remark Header name, value and delimiter are expected to be in \p header.
*
* @param header The complete header to be added, in the form "name:value".
*
* @throw if \p header has an invalid header name or if the delimiter is missing.
*/
// rfc form header-name: OWS header-value OWS
void AddHeader(std::string const& header);
/**
* @brief Add header to the HTTP response.
* @detail HTTP response header should be in RFCnnnn format (`OWS header-value OWS`).
* @brief Add HTTP header to the @RawResponse.
*
* @param begin Pointer to the first byte of the header string in RFCnnnn format.
* @param last Pointer to the last byte of the header string in RFCnnnn format.
* @remark The string referenced by \p begin and \p end must contain valid header name
* characters (RFC 7230).
* @remark Header name, value and delimiter are expected to be in the string referenced by \p
* begin and \p end, in the form "name:value".
*
* @param begin Reference to the start of an std::string.
* @param last Reference to the end of an std::string.
*
* @throw if the string referenced by \p begin and \p end contains an invalid header name or if
* the delimiter is missing.
*/
void AddHeader(uint8_t const* const begin, uint8_t const* const last);
@ -674,7 +713,7 @@ namespace Azure { namespace Core { namespace Http {
*/
std::unique_ptr<BodyStream> GetBodyStream()
{
// If m_bodyStream was moved before. nullpr is returned
// If m_bodyStream was moved before. nullptr is returned
return std::move(this->m_bodyStream);
}

View File

@ -79,7 +79,7 @@ int pollSocketUntilEventOrTimeout(
return poll(&poller, 1, timeout);
#endif
#ifdef WINDOWS
// Windows Vista and greater. TODO: detect legacy Windows and use select()
// Windows Vista and greater.
return WSAPoll(&poller, 1, timeout);
#endif
}
@ -757,7 +757,7 @@ int64_t CurlSession::ResponseBufferParser::Parse(
}
else if (this->state == ResponseParserState::Headers)
{
// Add header. TODO: Do toLower so all headers are lowerCase
// will throw if header is invalid
this->m_response->AddHeader(this->m_internalBuffer);
this->m_delimiterStartInPrevPosition = false;
start = index + 1; // jump \n
@ -794,7 +794,7 @@ int64_t CurlSession::ResponseBufferParser::Parse(
return index + 1; // plus 1 to advance the \n. If we were at buffer end.
}
// Add header. TODO: Do toLower so all headers are lowerCase
// will throw if header is invalid
this->m_response->AddHeader(buffer + start, buffer + index - 1);
this->m_delimiterStartInPrevPosition = false;
start = index + 1; // jump \n
@ -940,12 +940,15 @@ int64_t CurlSession::ResponseBufferParser::BuildHeader(
// Append and build response minus the delimiter
this->m_internalBuffer.append(start, indexOfEndOfStatusLine);
}
this->m_response->AddHeader(this->m_internalBuffer);
// will throw if header is invalid
m_response->AddHeader(this->m_internalBuffer);
}
else
{
// Internal Buffer was not required, create response directly from buffer
this->m_response->AddHeader(std::string(start, indexOfEndOfStatusLine));
std::string header(std::string(start, indexOfEndOfStatusLine));
// will throw if header is invalid
this->m_response->AddHeader(header);
}
// reuse buffer

View File

@ -0,0 +1,155 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// SPDX-License-Identifier: MIT
#include "azure/core/http/http.hpp"
void Azure::Core::Http::Details::InsertHeaderWithValidation(
std::map<std::string, std::string>& headers,
std::string const& headerName,
std::string const& headerValue)
{
// Static table for validating header names. It is created just once for the program and reused
// each time AddHeader is called
static const uint8_t validChars[256] = {
0, /* 0 - null */
0, /* 1 - start of heading */
0, /* 2 - start of text */
0, /* 3 - end of text */
0, /* 4 - end of transmission */
0, /* 5 - enquiry */
0, /* 6 - acknowledge */
0, /* 7 - bell */
0, /* 8 - backspace */
0, /* 9 - horizontal tab */
0, /* 10 - new line */
0, /* 11 - vertical tab */
0, /* 12 - new page */
0, /* 13 - carriage return */
0, /* 14 - shift out */
0, /* 15 - shift in */
0, /* 16 - data link escape */
0, /* 17 - device control 1 */
0, /* 18 - device control 2 */
0, /* 19 - device control 3 */
0, /* 20 - device control 4 */
0, /* 21 - negative acknowledge */
0, /* 22 - synchronous idle */
0, /* 23 - end of trans. block */
0, /* 24 - cancel */
0, /* 25 - end of medium */
0, /* 26 - substitute */
0, /* 27 - escape */
0, /* 28 - file separator */
0, /* 29 - group separator */
0, /* 30 - record separator */
0, /* 31 - unit separator */
' ', /* 32 - space */
'!', /* 33 - ! */
0, /* 34 - " */
'#', /* 35 - # */
'$', /* 36 - $ */
'%', /* 37 - % */
'&', /* 38 - & */
'\'', /* 39 - ' */
0, /* 40 - ( */
0, /* 41 - ) */
'*', /* 42 - * */
'+', /* 43 - + */
0, /* 44 - , */
'-', /* 45 - - */
'.', /* 46 - . */
0, /* 47 - / */
'0', /* 48 - 0 */
'1', /* 49 - 1 */
'2', /* 50 - 2 */
'3', /* 51 - 3 */
'4', /* 52 - 4 */
'5', /* 53 - 5 */
'6', /* 54 - 6 */
'7', /* 55 - 7 */
'8', /* 56 - 8 */
'9', /* 57 - 9 */
0, /* 58 - : */
0, /* 59 - ; */
0, /* 60 - < */
0, /* 61 - = */
0, /* 62 - > */
0, /* 63 - ? */
0, /* 64 - @ */
'a', /* 65 - A */
'b', /* 66 - B */
'c', /* 67 - C */
'd', /* 68 - D */
'e', /* 69 - E */
'f', /* 70 - F */
'g', /* 71 - G */
'h', /* 72 - H */
'i', /* 73 - I */
'j', /* 74 - J */
'k', /* 75 - K */
'l', /* 76 - L */
'm', /* 77 - M */
'n', /* 78 - N */
'o', /* 79 - O */
'p', /* 80 - P */
'q', /* 81 - Q */
'r', /* 82 - R */
's', /* 83 - S */
't', /* 84 - T */
'u', /* 85 - U */
'v', /* 86 - V */
'w', /* 87 - W */
'x', /* 88 - X */
'y', /* 89 - Y */
'z', /* 90 - Z */
0, /* 91 - [ */
0, /* 92 - comment */
0, /* 93 - ] */
'^', /* 94 - ^ */
'_', /* 95 - _ */
'`', /* 96 - ` */
'a', /* 97 - a */
'b', /* 98 - b */
'c', /* 99 - c */
'd', /* 100 - d */
'e', /* 101 - e */
'f', /* 102 - f */
'g', /* 103 - g */
'h', /* 104 - h */
'i', /* 105 - i */
'j', /* 106 - j */
'k', /* 107 - k */
'l', /* 108 - l */
'm', /* 109 - m */
'n', /* 110 - n */
'o', /* 111 - o */
'p', /* 112 - p */
'q', /* 113 - q */
'r', /* 114 - r */
's', /* 115 - s */
't', /* 116 - t */
'u', /* 117 - u */
'v', /* 118 - v */
'w', /* 119 - w */
'x', /* 120 - x */
'y', /* 121 - y */
'z', /* 122 - z */
0, /* 123 - { */
'|', /* 124 - | */
0, /* 125 - } */
'~', /* 126 - ~ */
0 /* 127 - DEL */
// ...128-255 is all zeros (not valid) characters}
};
// Check all chars in name are valid
for (size_t index = 0; index < headerName.size(); index++)
{
if (validChars[static_cast<int>(headerName[index])] == 0)
{
throw InvalidHeaderException("Invalid header: " + headerName);
}
}
// insert (override if duplicated)
headers[headerName] = headerValue;
}

View File

@ -15,7 +15,10 @@ HttpStatusCode RawResponse::GetStatusCode() const { return m_statusCode; }
std::string const& RawResponse::GetReasonPhrase() const { return m_reasonPhrase; }
std::map<std::string, std::string> const& RawResponse::GetHeaders() const { return this->m_headers; }
std::map<std::string, std::string> const& RawResponse::GetHeaders() const
{
return this->m_headers;
}
void RawResponse::AddHeader(uint8_t const* const begin, uint8_t const* const last)
{
@ -25,7 +28,7 @@ void RawResponse::AddHeader(uint8_t const* const begin, uint8_t const* const las
if (end == last)
{
return; // not a valid header or end of headers symbol reached
throw InvalidHeaderException("invalid header. No delimiter :");
}
// Always toLower() headers
@ -51,8 +54,7 @@ void RawResponse::AddHeader(std::string const& header)
void RawResponse::AddHeader(std::string const& name, std::string const& value)
{
this->m_headers.insert(std::pair<std::string, std::string>(name, value));
return Details::InsertHeaderWithValidation(this->m_headers, name, value);
}
void RawResponse::SetBodyStream(std::unique_ptr<BodyStream> stream)

View File

@ -13,15 +13,11 @@ using namespace Azure::Core::Http;
void Request::AddHeader(std::string const& name, std::string const& value)
{
auto headerNameLowerCase = Azure::Core::Details::ToLower(name);
if (this->m_retryModeEnabled)
{
// When retry mode is ON, any new value must override previous
this->m_retryHeaders[headerNameLowerCase] = value;
}
else
{
this->m_headers[headerNameLowerCase] = value;
}
return this->m_retryModeEnabled
? Details::InsertHeaderWithValidation(
this->m_retryHeaders, headerNameLowerCase, value)
: Details::InsertHeaderWithValidation(
this->m_headers, headerNameLowerCase, value);
}
void Request::RemoveHeader(std::string const& name)
@ -53,12 +49,11 @@ std::map<std::string, std::string> Request::GetHeaders() const
return Details::MergeMaps(this->m_retryHeaders, this->m_headers);
}
// Writes an HTTP request with RFC2730 without the body (head line and headers)
// Writes an HTTP request with RFC 7230 without the body (head line and headers)
// https://tools.ietf.org/html/rfc7230#section-3.1.1
std::string Request::GetHTTPMessagePreBody() const
{
std::string httpRequest(HttpMethodToString(this->m_method));
// origin-form. TODO: parse Url to split host from path and use it here instead of empty
// HTTP version harcoded to 1.0
auto const url = this->m_url.GetRelativeUrl();
httpRequest += " /" + url + " HTTP/1.1\r\n";

View File

@ -19,6 +19,7 @@ project (${TARGET_NAME} LANGUAGES CXX)
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED True)
include(GoogleTest)
add_executable (
${TARGET_NAME}
http.cpp
@ -34,6 +35,9 @@ add_executable (
uuid.cpp
context.cpp)
target_link_libraries(${TARGET_NAME} PRIVATE azure-core)
add_gtest(${TARGET_NAME})
target_link_libraries(${TARGET_NAME} PRIVATE azure-core gtest)
# gtest_add_tests will scan the test from azure-core-test and call add_test
# for each test to ctest. This enables `ctest -r` to run specific tests directly.
gtest_add_tests(TARGET ${TARGET_NAME})

View File

@ -3,8 +3,8 @@
#include "gtest/gtest.h"
#include <azure/core/http/http.hpp>
#include "http.hpp"
#include <azure/core/http/http.hpp>
#include <string>
#include <vector>
@ -158,4 +158,121 @@ namespace Azure { namespace Core { namespace Test {
req.GetUrl().GetAbsoluteUrl(),
"http://test.com/path/path2/path3?query=value");
}
// Request - Add header
TEST(TestHttp, add_headers)
{
Http::HttpMethod httpMethod = Http::HttpMethod::Post;
Http::Url url("http://test.com");
Http::Request req(httpMethod, url);
std::pair<std::string, std::string> expected("valid", "header");
EXPECT_NO_THROW(req.AddHeader(expected.first, expected.second));
EXPECT_PRED2(
[](std::map<std::string, std::string> headers,
std::pair<std::string, std::string> expected) {
auto firstHeader = headers.begin();
return firstHeader->first == expected.first && firstHeader->second == expected.second
&& headers.size() == 1;
},
req.GetHeaders(),
expected);
EXPECT_THROW(req.AddHeader("invalid()", "header"), std::runtime_error);
// same header will just override
std::pair<std::string, std::string> expectedOverride("valid", "override");
EXPECT_NO_THROW(req.AddHeader(expectedOverride.first, expectedOverride.second));
EXPECT_PRED2(
[](std::map<std::string, std::string> headers,
std::pair<std::string, std::string> expected) {
auto firstHeader = headers.begin();
return firstHeader->first == expected.first && firstHeader->second == expected.second
&& headers.size() == 1;
},
req.GetHeaders(),
expectedOverride);
// adding header after one error happened before
std::pair<std::string, std::string> expected2("valid2", "header2");
EXPECT_NO_THROW(req.AddHeader(expected2.first, expected2.second));
EXPECT_PRED2(
[](std::map<std::string, std::string> headers,
std::pair<std::string, std::string> expected) {
auto secondHeader = headers.begin();
secondHeader++;
return secondHeader->first == expected.first && secondHeader->second == expected.second
&& headers.size() == 2;
},
req.GetHeaders(),
expected2);
}
// Response - Add header
TEST(TestHttp, response_add_headers)
{
Http::RawResponse response(1, 1, Http::HttpStatusCode::Accepted, "Test");
std::pair<std::string, std::string> expected("valid", "header");
EXPECT_NO_THROW(response.AddHeader(expected.first, expected.second));
EXPECT_PRED2(
[](std::map<std::string, std::string> headers,
std::pair<std::string, std::string> expected) {
auto firstHeader = headers.begin();
return firstHeader->first == expected.first && firstHeader->second == expected.second
&& headers.size() == 1;
},
response.GetHeaders(),
expected);
EXPECT_THROW(
response.AddHeader("invalid()", "header"), Azure::Core::Http::InvalidHeaderException);
// same header will just override
std::pair<std::string, std::string> expectedOverride("valid", "override");
EXPECT_NO_THROW(response.AddHeader(expectedOverride.first, expectedOverride.second));
EXPECT_PRED2(
[](std::map<std::string, std::string> headers,
std::pair<std::string, std::string> expected) {
auto firstHeader = headers.begin();
return firstHeader->first == expected.first && firstHeader->second == expected.second
&& headers.size() == 1;
},
response.GetHeaders(),
expectedOverride);
// adding header after on error happened
std::pair<std::string, std::string> expected2("valid2", "header2");
EXPECT_NO_THROW(response.AddHeader(expected2.first, expected2.second));
EXPECT_PRED2(
[](std::map<std::string, std::string> headers,
std::pair<std::string, std::string> expected) {
auto secondtHeader = headers.begin();
secondtHeader++;
return secondtHeader->first == expected.first && secondtHeader->second == expected.second
&& headers.size() == 2;
},
response.GetHeaders(),
expected2);
// Response addHeader overload method to add from string
EXPECT_THROW(response.AddHeader("inv(): header"), Azure::Core::Http::InvalidHeaderException);
EXPECT_THROW(
response.AddHeader("no delimiter header"), Azure::Core::Http::InvalidHeaderException);
// adding header after previous error just happened on add from string
EXPECT_NO_THROW(response.AddHeader("valid3: header3"));
EXPECT_PRED2(
[](std::map<std::string, std::string> headers,
std::pair<std::string, std::string> expected) {
auto secondtHeader = headers.begin();
secondtHeader++;
secondtHeader++;
return secondtHeader->first == expected.first && secondtHeader->second == expected.second
&& headers.size() == 3;
},
response.GetHeaders(),
(std::pair<std::string, std::string>("valid3", "header3")));
}
}}} // namespace Azure::Core::Test

View File

@ -15,7 +15,7 @@ namespace Azure { namespace Storage {
int64_t Offset = 0;
};
// Defines a fn signature to be use to get a bodyStream from an specific offset.
// Defines a fn signature to be use to get a bodyStream from a specific offset.
typedef std::function<std::unique_ptr<Azure::Core::Http::BodyStream>(
Azure::Core::Context const&,
HttpGetterInfo const&)>