Skip to content

Commit 6d7dd5c

Browse files
committed
Updating passwords on Windows desktop to be more secure, minimizing time spent as plaintext.
1 parent 8c0d858 commit 6d7dd5c

File tree

3 files changed

+38
-9
lines changed

3 files changed

+38
-9
lines changed

Release/include/cpprest/web_utilities.h

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,28 @@
3232

3333
namespace web
3434
{
35+
36+
namespace http { namespace client { namespace details {
37+
class winhttp_client;
38+
}}}
39+
3540
namespace details
3641
{
3742
#if defined(_MS_WINDOWS) && !defined(__cplusplus_winrt)
43+
class zero_memory_deleter
44+
{
45+
public:
46+
_ASYNCRTIMP void operator()(::utility::string_t *data) const;
47+
};
48+
typedef std::unique_ptr<std::wstring, zero_memory_deleter> password_string;
49+
3850
class win32_encryption
3951
{
4052
public:
4153
win32_encryption() {}
4254
_ASYNCRTIMP win32_encryption(const std::wstring &data);
4355
_ASYNCRTIMP ~win32_encryption();
44-
_ASYNCRTIMP std::wstring decrypt() const;
56+
_ASYNCRTIMP password_string decrypt() const;
4557
private:
4658
std::vector<char> m_buffer;
4759
size_t m_numCharacters;
@@ -81,10 +93,11 @@ class credentials
8193
/// The password for the user name associated with the credentials.
8294
/// </summary>
8395
/// <returns>A string containing the password.</returns>
84-
utility::string_t password() const
96+
CASABLANCA_DEPRECATED("This API is deprecated for security reasons to avoid unnecessary password copies stored in plaintext.")
97+
utility::string_t password() const
8598
{
8699
#if defined(_MS_WINDOWS) && !defined(__cplusplus_winrt)
87-
return m_password.decrypt();
100+
return utility::string_t(*m_password.decrypt());
88101
#else
89102
return m_password;
90103
#endif
@@ -97,6 +110,15 @@ class credentials
97110
bool is_set() const { return !m_username.empty(); }
98111

99112
private:
113+
friend class http::client::details::winhttp_client;
114+
115+
#if defined(_MS_WINDOWS) && !defined(__cplusplus_winrt)
116+
details::password_string decrypt() const
117+
{
118+
return m_password.decrypt();
119+
}
120+
#endif
121+
100122
::utility::string_t m_username;
101123
#if defined(_MS_WINDOWS) && !defined(__cplusplus_winrt)
102124
::web::details::win32_encryption m_password;

Release/src/http/client/http_win7.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -923,12 +923,13 @@ class winhttp_client : public _http_client_communicator
923923
{
924924
return false;
925925
}
926+
auto password = cred.decrypt();
926927
if (!WinHttpSetCredentials(
927928
hRequestHandle,
928929
dwAuthTarget,
929930
dwSelectedScheme,
930931
cred.username().c_str(),
931-
cred.password().c_str(),
932+
password->c_str(),
932933
nullptr))
933934
{
934935
return false;

Release/src/utilities/web_utilities.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ namespace web
3636
namespace details
3737
{
3838
#if defined(_MS_WINDOWS) && !defined(__cplusplus_winrt)
39+
void zero_memory_deleter::operator()(::utility::string_t *data) const
40+
{
41+
SecureZeroMemory(reinterpret_cast<void *>(const_cast<::utility::string_t::value_type *>(data->data())), data->size());
42+
delete data;
43+
}
44+
3945
win32_encryption::win32_encryption(const std::wstring &data) :
4046
m_numCharacters(data.size())
4147
{
@@ -60,19 +66,19 @@ win32_encryption::~win32_encryption()
6066
SecureZeroMemory(m_buffer.data(), m_buffer.size());
6167
}
6268

63-
std::wstring win32_encryption::decrypt() const
69+
password_string win32_encryption::decrypt() const
6470
{
6571
// Copy the buffer and decrypt to avoid having to re-encrypt.
66-
std::wstring data(reinterpret_cast<const std::wstring::value_type *>(m_buffer.data()), m_buffer.size() / 2);
72+
auto data = password_string(new std::wstring(reinterpret_cast<const std::wstring::value_type *>(m_buffer.data()), m_buffer.size() / 2));
6773
if (!CryptUnprotectMemory(
68-
reinterpret_cast<void *>(const_cast<std::wstring::value_type *>(data.c_str())),
74+
reinterpret_cast<void *>(const_cast<std::wstring::value_type *>(data->c_str())),
6975
m_buffer.size(),
7076
CRYPTPROTECTMEMORY_SAME_PROCESS))
7177
{
7278
throw ::utility::details::create_system_error(GetLastError());
7379
}
74-
data.resize(m_numCharacters);
75-
return data;
80+
data->resize(m_numCharacters);
81+
return std::move(data);
7682
}
7783
#endif
7884
}

0 commit comments

Comments
 (0)