Skip to content

Commit b6e04fd

Browse files
authored
Update sorting algorithm used in storage shared-key signing (#5547)
1 parent 12ce2ac commit b6e04fd

4 files changed

Lines changed: 221 additions & 2 deletions

File tree

sdk/storage/assets.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
"AssetsRepo": "Azure/azure-sdk-assets",
33
"AssetsRepoPrefixPath": "cpp",
44
"TagPrefix": "cpp/storage",
5-
"Tag": "cpp/storage_3d44001e6b"
5+
"Tag": "cpp/storage_d0e72729b7"
66
}

sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2074,4 +2074,75 @@ namespace Azure { namespace Storage { namespace Test {
20742074
= Blobs::BlockBlobClient(m_blockBlobClient->GetUrl(), credential, clientOptions);
20752075
EXPECT_THROW(blockBlobClient.GetProperties(), StorageException);
20762076
}
2077+
2078+
TEST_F(BlockBlobClientTest, SharedKeySigningHeaderWithSymbols)
2079+
{
2080+
class AdditionalHeaderPolicy final : public Azure::Core::Http::Policies::HttpPolicy {
2081+
public:
2082+
~AdditionalHeaderPolicy() override {}
2083+
2084+
std::unique_ptr<HttpPolicy> Clone() const override
2085+
{
2086+
return std::make_unique<AdditionalHeaderPolicy>(*this);
2087+
}
2088+
2089+
std::unique_ptr<Azure::Core::Http::RawResponse> Send(
2090+
Azure::Core::Http::Request& request,
2091+
Azure::Core::Http::Policies::NextHttpPolicy nextPolicy,
2092+
Azure::Core::Context const& context) const override
2093+
{
2094+
// cSpell:disable
2095+
request.SetHeader("x-ms-test", "val");
2096+
request.SetHeader("x-ms-test-", "val");
2097+
request.SetHeader("x-ms-test-a", "val");
2098+
request.SetHeader("x-ms-test-g", "val");
2099+
request.SetHeader("x-ms-test-Z", "val");
2100+
request.SetHeader("x-ms-testa", "val");
2101+
request.SetHeader("x-ms-testd", "val");
2102+
request.SetHeader("x-ms-testx", "val");
2103+
request.SetHeader("x-ms-test--", "val");
2104+
request.SetHeader("x-ms-test-_", "val");
2105+
request.SetHeader("x-ms-test_-", "val");
2106+
request.SetHeader("x-ms-test__", "val");
2107+
request.SetHeader("x-ms-test-a", "val");
2108+
request.SetHeader("x-ms-test-A", "val");
2109+
request.SetHeader("x-ms-test-_A", "val");
2110+
request.SetHeader("x-ms-test_a", "val");
2111+
request.SetHeader("x-ms-test_Z", "val");
2112+
request.SetHeader("x-ms-test_a_", "val");
2113+
request.SetHeader("x-ms-test_a-", "val");
2114+
request.SetHeader("x-ms-test_a-_", "val");
2115+
request.SetHeader("x-ms-testa--", "val");
2116+
request.SetHeader("x-ms-test-a-", "val");
2117+
request.SetHeader("x-ms-test--a", "val");
2118+
request.SetHeader("x-ms-testaa-", "val");
2119+
request.SetHeader("x-ms-testa-a", "val");
2120+
request.SetHeader("x-ms-test-aa", "val");
2121+
2122+
request.SetHeader("x-ms-test-!", "val");
2123+
request.SetHeader("x-ms-test-#", "val");
2124+
request.SetHeader("x-ms-test-$", "val");
2125+
request.SetHeader("x-ms-test-%", "val");
2126+
request.SetHeader("x-ms-test-&", "val");
2127+
request.SetHeader("x-ms-test-*", "val");
2128+
request.SetHeader("x-ms-test-+", "val");
2129+
request.SetHeader("x-ms-test-.", "val");
2130+
request.SetHeader("x-ms-test-^", "val");
2131+
request.SetHeader("x-ms-test-_", "val");
2132+
request.SetHeader("x-ms-test-`", "val");
2133+
request.SetHeader("x-ms-test-|", "val");
2134+
request.SetHeader("x-ms-test-~", "val");
2135+
// cSpell:enable
2136+
return nextPolicy.Send(request, context);
2137+
}
2138+
};
2139+
2140+
auto clientOptions = InitStorageClientOptions<Blobs::BlobClientOptions>();
2141+
clientOptions.PerOperationPolicies.push_back(std::make_unique<AdditionalHeaderPolicy>());
2142+
auto keyCredential
2143+
= _internal::ParseConnectionString(StandardStorageConnectionString()).KeyCredential;
2144+
auto blockBlobClient
2145+
= Blobs::BlockBlobClient(m_blockBlobClient->GetUrl(), keyCredential, clientOptions);
2146+
EXPECT_NO_THROW(blockBlobClient.GetProperties());
2147+
}
20772148
}}} // namespace Azure::Storage::Test

sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,5 +714,75 @@ namespace Azure { namespace Storage { namespace Test {
714714
}
715715
}
716716
}
717+
TEST_F(PageBlobClientTest, SharedKeySigningHeaderWithSymbols)
718+
{
719+
class AdditionalHeaderPolicy final : public Azure::Core::Http::Policies::HttpPolicy {
720+
public:
721+
~AdditionalHeaderPolicy() override {}
722+
723+
std::unique_ptr<HttpPolicy> Clone() const override
724+
{
725+
return std::make_unique<AdditionalHeaderPolicy>(*this);
726+
}
727+
728+
std::unique_ptr<Azure::Core::Http::RawResponse> Send(
729+
Azure::Core::Http::Request& request,
730+
Azure::Core::Http::Policies::NextHttpPolicy nextPolicy,
731+
Azure::Core::Context const& context) const override
732+
{
733+
// cSpell:disable
734+
request.SetHeader("x-ms-test", "val");
735+
request.SetHeader("x-ms-test-", "val");
736+
request.SetHeader("x-ms-test-a", "val");
737+
request.SetHeader("x-ms-test-g", "val");
738+
request.SetHeader("x-ms-test-Z", "val");
739+
request.SetHeader("x-ms-testa", "val");
740+
request.SetHeader("x-ms-testd", "val");
741+
request.SetHeader("x-ms-testx", "val");
742+
request.SetHeader("x-ms-test--", "val");
743+
request.SetHeader("x-ms-test-_", "val");
744+
request.SetHeader("x-ms-test_-", "val");
745+
request.SetHeader("x-ms-test__", "val");
746+
request.SetHeader("x-ms-test-a", "val");
747+
request.SetHeader("x-ms-test-A", "val");
748+
request.SetHeader("x-ms-test-_A", "val");
749+
request.SetHeader("x-ms-test_a", "val");
750+
request.SetHeader("x-ms-test_Z", "val");
751+
request.SetHeader("x-ms-test_a_", "val");
752+
request.SetHeader("x-ms-test_a-", "val");
753+
request.SetHeader("x-ms-test_a-_", "val");
754+
request.SetHeader("x-ms-testa--", "val");
755+
request.SetHeader("x-ms-test-a-", "val");
756+
request.SetHeader("x-ms-test--a", "val");
757+
request.SetHeader("x-ms-testaa-", "val");
758+
request.SetHeader("x-ms-testa-a", "val");
759+
request.SetHeader("x-ms-test-aa", "val");
760+
761+
request.SetHeader("x-ms-test-!", "val");
762+
request.SetHeader("x-ms-test-#", "val");
763+
request.SetHeader("x-ms-test-$", "val");
764+
request.SetHeader("x-ms-test-%", "val");
765+
request.SetHeader("x-ms-test-&", "val");
766+
request.SetHeader("x-ms-test-*", "val");
767+
request.SetHeader("x-ms-test-+", "val");
768+
request.SetHeader("x-ms-test-.", "val");
769+
request.SetHeader("x-ms-test-^", "val");
770+
request.SetHeader("x-ms-test-_", "val");
771+
request.SetHeader("x-ms-test-`", "val");
772+
request.SetHeader("x-ms-test-|", "val");
773+
request.SetHeader("x-ms-test-~", "val");
774+
// cSpell:enable
775+
return nextPolicy.Send(request, context);
776+
}
777+
};
778+
779+
auto clientOptions = InitStorageClientOptions<Blobs::BlobClientOptions>();
780+
clientOptions.PerOperationPolicies.push_back(std::make_unique<AdditionalHeaderPolicy>());
781+
auto keyCredential
782+
= _internal::ParseConnectionString(StandardStorageConnectionString()).KeyCredential;
783+
auto blockBlobClient
784+
= Blobs::BlockBlobClient(m_pageBlobClient->GetUrl(), keyCredential, clientOptions);
785+
EXPECT_NO_THROW(blockBlobClient.GetProperties());
786+
}
717787

718788
}}} // namespace Azure::Storage::Test

sdk/storage/azure-storage-common/src/shared_key_policy.cpp

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,82 @@
1010

1111
#include <algorithm>
1212

13+
namespace {
14+
const static int table_lv0[128] = {
15+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
16+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
17+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x71c, 0x0, 0x71f, 0x721, 0x723, 0x725,
18+
0x0, 0x0, 0x0, 0x72d, 0x803, 0x0, 0x0, 0x733, 0x0, 0x0, 0xd1a, 0xd1c, 0xd1e,
19+
0xd20, 0xd22, 0xd24, 0xd26, 0xd28, 0xd2a, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
20+
0xe02, 0xe09, 0xe0a, 0xe1a, 0xe21, 0xe23, 0xe25, 0xe2c, 0xe32, 0xe35, 0xe36, 0xe48, 0xe51,
21+
0xe70, 0xe7c, 0xe7e, 0xe89, 0xe8a, 0xe91, 0xe99, 0xe9f, 0xea2, 0xea4, 0xea6, 0xea7, 0xea9,
22+
0x0, 0x0, 0x0, 0x743, 0x744, 0x748, 0xe02, 0xe09, 0xe0a, 0xe1a, 0xe21, 0xe23, 0xe25,
23+
0xe2c, 0xe32, 0xe35, 0xe36, 0xe48, 0xe51, 0xe70, 0xe7c, 0xe7e, 0xe89, 0xe8a, 0xe91, 0xe99,
24+
0xe9f, 0xea2, 0xea4, 0xea6, 0xea7, 0xea9, 0x0, 0x74c, 0x0, 0x750, 0x0,
25+
};
26+
const static int table_lv2[128] = {
27+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
28+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
29+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
30+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
31+
0x0, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12,
32+
0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x12, 0x0, 0x0, 0x0, 0x0, 0x0,
33+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
34+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
35+
};
36+
const static int table_lv4[128] = {
37+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
38+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
39+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x8012, 0x0, 0x0, 0x0, 0x0, 0x0, 0x8212, 0x0, 0x0,
40+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
41+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
42+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
43+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
44+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
45+
};
46+
47+
bool comparator(const std::string& lhs, const std::string& rhs)
48+
{
49+
const static std::array<const int*, 3> tables{table_lv0, table_lv2, table_lv4};
50+
size_t curr_level = 0;
51+
size_t i = 0;
52+
size_t j = 0;
53+
while (curr_level < tables.size())
54+
{
55+
if (curr_level == tables.size() - 1 && i != j)
56+
{
57+
return i > j;
58+
}
59+
const int weight1 = i < lhs.size() ? tables[curr_level][static_cast<uint8_t>(lhs[i])] : 0x1;
60+
const int weight2 = j < rhs.size() ? tables[curr_level][static_cast<uint8_t>(rhs[j])] : 0x1;
61+
if (weight1 == 0x1 && weight2 == 0x1)
62+
{
63+
i = 0;
64+
j = 0;
65+
++curr_level;
66+
}
67+
else if (weight1 == weight2)
68+
{
69+
++i;
70+
++j;
71+
}
72+
else if (weight1 == 0)
73+
{
74+
++i;
75+
}
76+
else if (weight2 == 0)
77+
{
78+
++j;
79+
}
80+
else
81+
{
82+
return weight1 < weight2;
83+
}
84+
}
85+
return false;
86+
}
87+
} // namespace
88+
1389
namespace Azure { namespace Storage { namespace _internal {
1490

1591
std::string SharedKeyPolicy::GetSignature(const Core::Http::Request& request) const
@@ -56,7 +132,9 @@ namespace Azure { namespace Storage { namespace _internal {
56132
std::string key = Azure::Core::_internal::StringExtensions::ToLower(ite->first);
57133
ordered_kv.emplace_back(std::make_pair(std::move(key), ite->second));
58134
}
59-
std::sort(ordered_kv.begin(), ordered_kv.end());
135+
std::sort(ordered_kv.begin(), ordered_kv.end(), [](const auto& lhs, const auto& rhs) {
136+
return comparator(lhs.first, rhs.first);
137+
});
60138
for (const auto& p : ordered_kv)
61139
{
62140
string_to_sign += p.first + ":" + p.second + "\n";

0 commit comments

Comments
 (0)