From a6bdb42ece70686e3a2413d5b3750b970016a8c7 Mon Sep 17 00:00:00 2001 From: Simon Rozman Date: Wed, 13 May 2020 08:26:07 +0200 Subject: [PATCH] Deprecate encrypted BLOB checksum The MD5 checksum was calculated on unencrypted data. This offered a possibility for a dictionary attack. Signed-off-by: Simon Rozman --- lib/EAPBase/include/Credentials.h | 3 ++- lib/EAPBase/include/Module.h | 18 ++++++++++++------ lib/EAPBase/src/Credentials.cpp | 23 +++++++++++++++++++---- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/lib/EAPBase/include/Credentials.h b/lib/EAPBase/include/Credentials.h index f71d6ab..07d58d0 100644 --- a/lib/EAPBase/include/Credentials.h +++ b/lib/EAPBase/include/Credentials.h @@ -327,7 +327,8 @@ namespace eap enum class enc_alg_t { unknown = -1, ///< Unknown encryption none = 0, ///< Unencrypted - native, ///< native module encryption + native, ///< native module encryption (version 2) + native_v1, ///< native module encryption (version 1, deprecated) kph, ///< KPH encryption }; diff --git a/lib/EAPBase/include/Module.h b/lib/EAPBase/include/Module.h index 5add54d..80f5fa8 100644 --- a/lib/EAPBase/include/Module.h +++ b/lib/EAPBase/include/Module.h @@ -364,6 +364,7 @@ namespace eap /// /// \returns Encrypted data with 16B MD5 hash appended /// + [[deprecated("Unencrypted hash allows dictionary attacks")]] std::vector encrypt_md5(_In_ HCRYPTPROV hProv, _In_bytecount_(size) const void *data, _In_ size_t size) const; @@ -376,6 +377,7 @@ namespace eap /// \returns Encrypted data with 16B MD5 hash appended /// template + [[deprecated("Unencrypted hash allows dictionary attacks")]] std::vector encrypt_md5(_In_ HCRYPTPROV hProv, _In_ const std::basic_string<_Elem, _Traits, _Ax> &val) const { return encrypt_md5(hProv, val.c_str(), val.length()*sizeof(_Elem)); @@ -391,6 +393,7 @@ namespace eap /// \returns Encrypted data with 16B MD5 hash appended /// template + [[deprecated("Unencrypted hash allows dictionary attacks")]] std::vector encrypt_md5(_In_ HCRYPTPROV hProv, _In_ const std::basic_string &val) const { winstd::sanitizing_string val_utf8; @@ -451,7 +454,7 @@ namespace eap template std::basic_string<_Elem, _Traits, _Ax> decrypt_str(_In_ HCRYPTPROV hProv, _In_bytecount_(size) const void *data, _In_ size_t size, _In_opt_ HCRYPTHASH hHash = NULL) const { - std::vector<_Elem, sanitizing_allocator<_Elem> > buf(std::move(decrypt(hProv, data, size, hHash))); + std::vector<_Elem, sanitizing_allocator<_Elem> > buf(std::move(decrypt<_Elem, sanitizing_allocator<_Elem> >(hProv, data, size, hHash))); return std::basic_string<_Elem, _Traits, _Ax>(buf.data(), buf.size()); } @@ -469,7 +472,7 @@ namespace eap template std::basic_string decrypt_str(_In_ HCRYPTPROV hProv, _In_bytecount_(size) const void *data, _In_ size_t size, _In_opt_ HCRYPTHASH hHash = NULL) const { - winstd::sanitizing_string buf(std::move(decrypt_str(hProv, data, size, hHash))); + winstd::sanitizing_string buf(std::move(decrypt_str, sanitizing_allocator >(hProv, data, size, hHash))); std::basic_string dec; MultiByteToWideChar(CP_UTF8, 0, buf, dec); return dec; @@ -486,6 +489,7 @@ namespace eap /// \returns Decrypted data /// template + [[deprecated("Unencrypted hash allows dictionary attacks")]] std::vector<_Ty, _Ax> decrypt_md5(_In_ HCRYPTPROV hProv, _In_bytecount_(size) const void *data, _In_ size_t size) const { // Create hash. @@ -522,6 +526,7 @@ namespace eap /// \returns Decrypted string /// template + [[deprecated("Unencrypted hash allows dictionary attacks")]] std::basic_string<_Elem, _Traits, _Ax> decrypt_str_md5(_In_ HCRYPTPROV hProv, _In_bytecount_(size) const void *data, _In_ size_t size) const { std::vector<_Elem, sanitizing_allocator<_Elem> > buf(std::move(decrypt_md5<_Elem, sanitizing_allocator<_Elem> >(hProv, data, size))); @@ -539,6 +544,7 @@ namespace eap /// \returns Decrypted string /// template + [[deprecated("Unencrypted hash allows dictionary attacks")]] std::basic_string decrypt_str_md5(_In_ HCRYPTPROV hProv, _In_bytecount_(size) const void *data, _In_ size_t size) const { winstd::sanitizing_string buf(std::move(decrypt_str_md5, sanitizing_allocator >(hProv, data, size))); @@ -573,7 +579,7 @@ namespace eap throw winstd::win_runtime_error(__FUNCTION__ " CryptAcquireContext failed."); // Decrypt data. - return std::move(decrypt_md5 >(cp, pDataIn, dwDataInSize)); + return std::move(decrypt >(cp, pDataIn, dwDataInSize)); #else return sanitizing_blob(pDataIn, pDataIn + dwDataInSize); #endif @@ -602,7 +608,7 @@ namespace eap throw winstd::win_runtime_error(__FUNCTION__ " CryptAcquireContext failed."); // Decrypt data. - std::vector > data(std::move(decrypt_md5 >(cp, pDataIn, dwDataInSize))); + std::vector > data(std::move(decrypt >(cp, pDataIn, dwDataInSize))); cursor_in cursor = { data.data(), data.data() + data.size() }; #else @@ -637,7 +643,7 @@ namespace eap throw winstd::win_runtime_error(__FUNCTION__ " CryptAcquireContext failed."); // Encrypt BLOB. - std::vector data_enc(std::move(encrypt_md5(cp, data.data(), data.size()))); + std::vector data_enc(std::move(encrypt(cp, data.data(), data.size()))); // Copy encrypted BLOB to output. *pdwDataOutSize = (DWORD)data_enc.size(); @@ -685,7 +691,7 @@ namespace eap throw winstd::win_runtime_error(__FUNCTION__ " CryptAcquireContext failed."); // Encrypt BLOB. - std::vector data_enc(std::move(encrypt_md5(cp, data.data(), data.size()))); + std::vector data_enc(std::move(encrypt(cp, data.data(), data.size()))); // Copy encrypted BLOB to output. *pdwDataOutSize = (DWORD)data_enc.size(); diff --git a/lib/EAPBase/src/Credentials.cpp b/lib/EAPBase/src/Credentials.cpp index 8cf72ee..090527b 100644 --- a/lib/EAPBase/src/Credentials.cpp +++ b/lib/EAPBase/src/Credentials.cpp @@ -436,12 +436,12 @@ void eap::credentials_pass::save(_In_ IXMLDOMDocument *pDoc, _In_ IXMLDOMNode *p default: // Use default encryption method for all others (including unencrypted). - vector password_enc(std::move(m_module.encrypt_md5(cp, m_password))); + vector password_enc(std::move(m_module.encrypt(cp, m_password))); com_obj pXmlElPassword; if (FAILED(hr = eapxml::put_element_base64(pDoc, pConfigRoot, bstr(L"Password"), namespace_eapmetadata, password_enc.data(), password_enc.size(), std::addressof(pXmlElPassword)))) throw com_runtime_error(hr, __FUNCTION__ " Error creating element."); - pXmlElPassword->setAttribute(bstr(L"encryption"), variant(_L(PRODUCT_NAME_STR))); + pXmlElPassword->setAttribute(bstr(L"encryption"), variant(_L(PRODUCT_NAME_STR) _L(" v2"))); } } @@ -464,7 +464,7 @@ void eap::credentials_pass::load(_In_ IXMLDOMNode *pConfigRoot) if (FAILED(eapxml::get_attrib_value(pXmlElPassword, bstr(L"encryption"), encryption))) encryption = NULL; - if (encryption && CompareStringEx(LOCALE_NAME_INVARIANT, NORM_IGNORECASE, encryption, encryption.length(), _L(PRODUCT_NAME_STR), -1, NULL, NULL, 0) == CSTR_EQUAL) { + if (encryption && CompareStringEx(LOCALE_NAME_INVARIANT, NORM_IGNORECASE, encryption, encryption.length(), _L(PRODUCT_NAME_STR) _L(" v2"), -1, NULL, NULL, 0) == CSTR_EQUAL) { // Decode Base64. winstd::base64_dec dec; bool is_last; @@ -476,8 +476,23 @@ void eap::credentials_pass::load(_In_ IXMLDOMNode *pConfigRoot) if (!cp.create(NULL, NULL, PROV_RSA_AES, CRYPT_VERIFYCONTEXT)) throw win_runtime_error(__FUNCTION__ " CryptAcquireContext failed."); - m_password = m_module.decrypt_str_md5, sanitizing_allocator >(cp, password_enc.data(), password_enc.size()); + m_password = m_module.decrypt_str, sanitizing_allocator >(cp, password_enc.data(), password_enc.size()); m_enc_alg = enc_alg_t::native; + } else if (encryption && CompareStringEx(LOCALE_NAME_INVARIANT, NORM_IGNORECASE, encryption, encryption.length(), _L(PRODUCT_NAME_STR), -1, NULL, NULL, 0) == CSTR_EQUAL) { + // Decode Base64. + winstd::base64_dec dec; + bool is_last; + vector password_enc; + dec.decode(password_enc, is_last, (BSTR)password, password.length()); + + // Prepare cryptographics provider. + crypt_prov cp; + if (!cp.create(NULL, NULL, PROV_RSA_AES, CRYPT_VERIFYCONTEXT)) + throw win_runtime_error(__FUNCTION__ " CryptAcquireContext failed."); + + #pragma warning(suppress: 4996) // Support for backward compatibility. + m_password = m_module.decrypt_str_md5, sanitizing_allocator >(cp, password_enc.data(), password_enc.size()); + m_enc_alg = enc_alg_t::native_v1; } else if (encryption && CompareStringEx(LOCALE_NAME_INVARIANT, NORM_IGNORECASE, encryption, encryption.length(), _L("KPH"), -1, NULL, NULL, 0) == CSTR_EQUAL) { // Decrypt password. sanitizing_string password_utf8(std::move(kph_decrypt(password)));