From 75488ba870188e0920f19a05a18ae731a57f3fc3 Mon Sep 17 00:00:00 2001 From: Simon Rozman Date: Thu, 6 Feb 2020 09:39:57 +0100 Subject: [PATCH] credentials: Move user impersonation to peer::get_identity() To retrieve user credentials, EapHost provides us the interactive user's token we can use to impersonate. By doing the impersonation early in peer::get_identity(), we don't need to pass the token down the lower methods. This is rather a simplification than a performance optimization. Signed-off-by: Simon Rozman --- lib/EAPBase/include/Config.h | 2 ++ lib/EAPBase/include/Credentials.h | 36 +++++++++++++------------------ lib/EAPBase/include/Module.h | 5 +++-- lib/EAPBase/src/Credentials.cpp | 8 ------- lib/EAPBase/src/Module.cpp | 5 ++++- lib/EapHost/include/Credentials.h | 12 +++++------ lib/EapHost/src/Credentials.cpp | 6 +----- lib/TLS/include/Credentials.h | 12 +++++------ lib/TLS/src/Credentials.cpp | 4 ---- lib/TTLS/include/Credentials.h | 12 +++++------ lib/TTLS/include/Module.h | 3 +-- lib/TTLS/src/Credentials.cpp | 3 --- lib/TTLS/src/Module.cpp | 5 +---- lib/TTLS_UI/src/Module.cpp | 2 -- 14 files changed, 42 insertions(+), 73 deletions(-) diff --git a/lib/EAPBase/include/Config.h b/lib/EAPBase/include/Config.h index 93e1cb7..3982313 100644 --- a/lib/EAPBase/include/Config.h +++ b/lib/EAPBase/include/Config.h @@ -308,6 +308,8 @@ namespace eap /// /// Generates public identity using current configuration and given credentials /// + /// Must be called in the connecting user context. + /// std::wstring get_public_identity(const credentials &cred) const; public: diff --git a/lib/EAPBase/include/Credentials.h b/lib/EAPBase/include/Credentials.h index 8dcab2b..f71d6ab 100644 --- a/lib/EAPBase/include/Credentials.h +++ b/lib/EAPBase/include/Credentials.h @@ -202,13 +202,12 @@ namespace eap /// /// 1. Cached credentials /// 2. Configured credentials (if \p cfg is derived from `config_method_with_cred`) - /// 3. Stored credentials + /// 3. Stored credentials (must be called in the connecting user context) /// - /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior - /// \param[in] hTokenImpersonateUser Impersonation token for a logged-on user to collect user-related information - /// \param[in] cred_cached Cached credentials (optional, can be \c NULL, must be the same type of credentials as `this`) - /// \param[in] cfg Method configuration (must be the same type of configuration as `this` credentials belong to) - /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) + /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior + /// \param[in] cred_cached Cached credentials (optional, can be \c NULL, must be the same type of credentials as `this`) + /// \param[in] cfg Method configuration (must be the same type of configuration as `this` credentials belong to) + /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) /// /// \returns /// - \c source_t::cache Credentials were obtained from EapHost cache @@ -217,7 +216,6 @@ namespace eap /// virtual source_t combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName) = 0; @@ -297,13 +295,12 @@ namespace eap /// /// 1. Cached credentials /// 2. Configured credentials (if \p cfg is derived from `config_method_with_cred`) - /// 3. Stored credentials + /// 3. Stored credentials (must be called in the connecting user context) /// - /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior - /// \param[in] hTokenImpersonateUser Impersonation token for a logged-on user to collect user-related information - /// \param[in] cred_cached Cached credentials (optional, can be \c NULL) - /// \param[in] cfg Method configuration (when derived from `config_method_with_cred`, metod attempt to load credentials from \p cfg) - /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) + /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior + /// \param[in] cred_cached Cached credentials (optional, can be \c NULL) + /// \param[in] cfg Method configuration (when derived from `config_method_with_cred`, metod attempt to load credentials from \p cfg) + /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) /// /// \returns /// - \c source_t::cache Credentials were obtained from EapHost cache @@ -312,7 +309,6 @@ namespace eap /// virtual source_t combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName); @@ -409,13 +405,12 @@ namespace eap /// /// 1. Cached credentials /// 2. Configured credentials (if \p cfg is derived from `config_method_with_cred`) - /// 3. Stored credentials + /// 3. Stored credentials (must be called in the connecting user context) /// - /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior - /// \param[in] hTokenImpersonateUser Impersonation token for a logged-on user to collect user-related information - /// \param[in] cred_cached Cached credentials (optional, can be \c NULL) - /// \param[in] cfg Method configuration (when derived from `config_method_with_cred`, metod attempt to load credentials from \p cfg) - /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) + /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior + /// \param[in] cred_cached Cached credentials (optional, can be \c NULL) + /// \param[in] cfg Method configuration (when derived from `config_method_with_cred`, metod attempt to load credentials from \p cfg) + /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) /// /// \returns /// - \c source_t::cache Credentials were obtained from EapHost cache @@ -424,7 +419,6 @@ namespace eap /// virtual source_t combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName); diff --git a/lib/EAPBase/include/Module.h b/lib/EAPBase/include/Module.h index 4e2c007..5add54d 100644 --- a/lib/EAPBase/include/Module.h +++ b/lib/EAPBase/include/Module.h @@ -1074,13 +1074,14 @@ namespace eap /// /// Checks all configured providers and tries to combine credentials. /// + /// Must be called in the connecting user context. + /// _Success_(return != 0) virtual const config_method_with_cred* combine_credentials( _In_ DWORD dwFlags, _In_ const config_connection &cfg, _In_count_(dwUserDataSize) const BYTE *pUserData, _In_ DWORD dwUserDataSize, - _Inout_ credentials_connection& cred_out, - _In_ HANDLE hTokenImpersonateUser) = 0; + _Inout_ credentials_connection& cred_out) = 0; protected: /// diff --git a/lib/EAPBase/src/Credentials.cpp b/lib/EAPBase/src/Credentials.cpp index 9f8dfa5..5f465c9 100644 --- a/lib/EAPBase/src/Credentials.cpp +++ b/lib/EAPBase/src/Credentials.cpp @@ -297,7 +297,6 @@ LPCTSTR eap::credentials_identity::target_suffix() const eap::credentials::source_t eap::credentials_identity::combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName) @@ -320,9 +319,6 @@ eap::credentials::source_t eap::credentials_identity::combine( } if (pszTargetName) { - // Switch user context. - user_impersonator impersonating(hTokenImpersonateUser); - try { credentials_identity cred_loaded(m_module); cred_loaded.retrieve(pszTargetName, cfg.m_level); @@ -603,7 +599,6 @@ LPCTSTR eap::credentials_pass::target_suffix() const eap::credentials::source_t eap::credentials_pass::combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName) @@ -626,9 +621,6 @@ eap::credentials::source_t eap::credentials_pass::combine( } if (pszTargetName) { - // Switch user context. - user_impersonator impersonating(hTokenImpersonateUser); - try { credentials_pass cred_loaded(m_module); cred_loaded.retrieve(pszTargetName, cfg.m_level); diff --git a/lib/EAPBase/src/Module.cpp b/lib/EAPBase/src/Module.cpp index 827781f..240174e 100644 --- a/lib/EAPBase/src/Module.cpp +++ b/lib/EAPBase/src/Module.cpp @@ -367,9 +367,12 @@ void eap::peer::get_identity( config_connection cfg(*this); unpack(cfg, pConnectionData, dwConnectionDataSize); + // Switch user context. + user_impersonator impersonating(hTokenImpersonateUser); + // Combine credentials. credentials_connection cred_out(*this, cfg); - auto cfg_method = combine_credentials(dwFlags, cfg, pUserData, dwUserDataSize, cred_out, hTokenImpersonateUser); + auto cfg_method = combine_credentials(dwFlags, cfg, pUserData, dwUserDataSize, cred_out); if (cfg_method) { // No UI will be necessary. diff --git a/lib/EapHost/include/Credentials.h b/lib/EapHost/include/Credentials.h index 15bcda3..ecbc28c 100644 --- a/lib/EapHost/include/Credentials.h +++ b/lib/EapHost/include/Credentials.h @@ -121,13 +121,12 @@ namespace eap /// /// 1. Cached credentials /// 2. Configured credentials (if \p cfg is derived from `config_method_with_cred`) - /// 3. Stored credentials + /// 3. Stored credentials (must be called in the connecting user context) /// - /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior - /// \param[in] hTokenImpersonateUser Impersonation token for a logged-on user to collect user-related information - /// \param[in] cred_cached Cached credentials (optional, can be \c NULL, must be `credentials_eaphost*` type) - /// \param[in] cfg Method configuration (unused, as must be as config_method_eaphost is not derived from `config_method_with_cred`) - /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) + /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior + /// \param[in] cred_cached Cached credentials (optional, can be \c NULL, must be `credentials_eaphost*` type) + /// \param[in] cfg Method configuration (unused, as must be as config_method_eaphost is not derived from `config_method_with_cred`) + /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) /// /// \returns /// - \c source_t::cache Credentials were obtained from EapHost cache @@ -136,7 +135,6 @@ namespace eap /// virtual source_t combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName); diff --git a/lib/EapHost/src/Credentials.cpp b/lib/EapHost/src/Credentials.cpp index 25eb754..9e652d1 100644 --- a/lib/EapHost/src/Credentials.cpp +++ b/lib/EapHost/src/Credentials.cpp @@ -220,7 +220,6 @@ LPCTSTR eap::credentials_eaphost::target_suffix() const eap::credentials::source_t eap::credentials_eaphost::combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName) @@ -253,9 +252,6 @@ eap::credentials::source_t eap::credentials_eaphost::combine( } if (src == source_t::unknown && pszTargetName) { - // Switch user context. - user_impersonator impersonating(hTokenImpersonateUser); - try { credentials_eaphost cred_loaded(m_module); cred_loaded.retrieve(pszTargetName, cfg.m_level); @@ -281,7 +277,7 @@ eap::credentials::source_t eap::credentials_eaphost::combine( cfg_eaphost->get_type(), (DWORD)cfg_eaphost->m_cfg_blob.size(), cfg_eaphost->m_cfg_blob.data(), src != source_t::unknown ? (DWORD)m_cred_blob.size() : 0, src != source_t::unknown ? m_cred_blob.data() : NULL, - hTokenImpersonateUser, + NULL, &fInvokeUI, &cred_data_size, get_ptr(cred_data), get_ptr(identity), diff --git a/lib/TLS/include/Credentials.h b/lib/TLS/include/Credentials.h index 61a8fdd..625b32e 100644 --- a/lib/TLS/include/Credentials.h +++ b/lib/TLS/include/Credentials.h @@ -123,13 +123,12 @@ namespace eap /// /// 1. Cached credentials /// 2. Configured credentials (if \p cfg is derived from `config_method_with_cred`) - /// 3. Stored credentials + /// 3. Stored credentials (must be called in the connecting user context) /// - /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior - /// \param[in] hTokenImpersonateUser Impersonation token for a logged-on user to collect user-related information - /// \param[in] cred_cached Cached credentials (optional, can be \c NULL, must be `credentials_tls*` type) - /// \param[in] cfg Method configuration (unused, as must be as config_method_tls is not derived from `config_method_with_cred`) - /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) + /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior + /// \param[in] cred_cached Cached credentials (optional, can be \c NULL, must be `credentials_tls*` type) + /// \param[in] cfg Method configuration (unused, as must be as config_method_tls is not derived from `config_method_with_cred`) + /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) /// /// \returns /// - \c source_t::cache Credentials were obtained from EapHost cache @@ -138,7 +137,6 @@ namespace eap /// virtual source_t combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName); diff --git a/lib/TLS/src/Credentials.cpp b/lib/TLS/src/Credentials.cpp index f83a93f..9a77f6c 100644 --- a/lib/TLS/src/Credentials.cpp +++ b/lib/TLS/src/Credentials.cpp @@ -267,7 +267,6 @@ std::wstring eap::credentials_tls::get_identity() const eap::credentials::source_t eap::credentials_tls::combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName) @@ -290,9 +289,6 @@ eap::credentials::source_t eap::credentials_tls::combine( } if (pszTargetName) { - // Switch user context. - user_impersonator impersonating(hTokenImpersonateUser); - try { credentials_tls cred_loaded(m_module); cred_loaded.retrieve(pszTargetName, cfg.m_level); diff --git a/lib/TTLS/include/Credentials.h b/lib/TTLS/include/Credentials.h index e7c6010..dd27087 100644 --- a/lib/TTLS/include/Credentials.h +++ b/lib/TTLS/include/Credentials.h @@ -112,13 +112,12 @@ namespace eap /// /// 1. Cached credentials /// 2. Configured credentials (if \p cfg is derived from `config_method_with_cred`) - /// 3. Stored credentials + /// 3. Stored credentials (must be called in the connecting user context) /// - /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior - /// \param[in] hTokenImpersonateUser Impersonation token for a logged-on user to collect user-related information - /// \param[in] cred_cached Cached credentials (optional, can be \c NULL, must be `credentials_tls_tunnel*` type) - /// \param[in] cfg Method configuration (unused, as must be as config_method_tls_tunnel is not derived from `config_method_with_cred`) - /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) + /// \param[in] dwFlags A combination of [EAP flags](https://msdn.microsoft.com/en-us/library/windows/desktop/bb891975.aspx) that describe the EAP authentication session behavior + /// \param[in] cred_cached Cached credentials (optional, can be \c NULL, must be `credentials_tls_tunnel*` type) + /// \param[in] cfg Method configuration (unused, as must be as config_method_tls_tunnel is not derived from `config_method_with_cred`) + /// \param[in] pszTargetName The name in Windows Credential Manager to retrieve credentials from (optional, can be \c NULL) /// /// \returns /// - \c source_t::cache Credentials were obtained from EapHost cache @@ -127,7 +126,6 @@ namespace eap /// virtual source_t combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName); diff --git a/lib/TTLS/include/Module.h b/lib/TTLS/include/Module.h index 322bfb1..49b4fe6 100644 --- a/lib/TTLS/include/Module.h +++ b/lib/TTLS/include/Module.h @@ -64,8 +64,7 @@ namespace eap _In_ const config_connection &cfg, _In_count_(dwUserDataSize) const BYTE *pUserData, _In_ DWORD dwUserDataSize, - _Inout_ credentials_connection& cred_out, - _In_ HANDLE hTokenImpersonateUser); + _Inout_ credentials_connection& cred_out); }; diff --git a/lib/TTLS/src/Credentials.cpp b/lib/TTLS/src/Credentials.cpp index 722790d..0422309 100644 --- a/lib/TTLS/src/Credentials.cpp +++ b/lib/TTLS/src/Credentials.cpp @@ -180,7 +180,6 @@ wstring eap::credentials_tls_tunnel::get_identity() const eap::credentials::source_t eap::credentials_tls_tunnel::combine( _In_ DWORD dwFlags, - _In_opt_ HANDLE hTokenImpersonateUser, _In_opt_ const credentials *cred_cached, _In_ const config_method &cfg, _In_opt_z_ LPCTSTR pszTargetName) @@ -188,7 +187,6 @@ eap::credentials::source_t eap::credentials_tls_tunnel::combine( // Combine outer credentials. source_t src_outer = credentials_tls::combine( dwFlags, - hTokenImpersonateUser, cred_cached, cfg, pszTargetName); @@ -196,7 +194,6 @@ eap::credentials::source_t eap::credentials_tls_tunnel::combine( // Combine inner credentials. source_t src_inner = m_inner->combine( dwFlags, - hTokenImpersonateUser, cred_cached ? dynamic_cast(cred_cached)->m_inner.get() : NULL, *dynamic_cast(cfg).m_inner, pszTargetName); diff --git a/lib/TTLS/src/Module.cpp b/lib/TTLS/src/Module.cpp index f1b941d..6f74b21 100644 --- a/lib/TTLS/src/Module.cpp +++ b/lib/TTLS/src/Module.cpp @@ -66,8 +66,7 @@ _Success_(return != 0) const eap::config_method_with_cred* eap::peer_tls_tunnel: _In_ const config_connection &cfg, _In_count_(dwUserDataSize) const BYTE *pUserData, _In_ DWORD dwUserDataSize, - _Inout_ credentials_connection& cred_out, - _In_ HANDLE hTokenImpersonateUser) + _Inout_ credentials_connection& cred_out) { #if EAP_USE_NATIVE_CREDENTIAL_CACHE // Unpack cached credentials. @@ -102,7 +101,6 @@ _Success_(return != 0) const eap::config_method_with_cred* eap::peer_tls_tunnel: LPCTSTR _target_name = (dwFlags & EAP_FLAG_GUEST_ACCESS) == 0 ? target_name.c_str() : NULL; eap::credentials::source_t src_outer = cred->credentials_tls::combine( dwFlags, - hTokenImpersonateUser, #if EAP_USE_NATIVE_CREDENTIAL_CACHE has_cached ? cred_in.m_cred.get() : NULL, #else @@ -118,7 +116,6 @@ _Success_(return != 0) const eap::config_method_with_cred* eap::peer_tls_tunnel: // Combine inner credentials. eap::credentials::source_t src_inner = cred->m_inner->combine( dwFlags, - hTokenImpersonateUser, #if EAP_USE_NATIVE_CREDENTIAL_CACHE has_cached ? dynamic_cast(cred_in.m_cred.get())->m_inner.get() : NULL, #else diff --git a/lib/TTLS_UI/src/Module.cpp b/lib/TTLS_UI/src/Module.cpp index 2755438..d5c1350 100644 --- a/lib/TTLS_UI/src/Module.cpp +++ b/lib/TTLS_UI/src/Module.cpp @@ -174,7 +174,6 @@ void eap::peer_ttls_ui::invoke_identity_ui( wstring target_name(std::move(cfg_prov->get_id())); eap::credentials::source_t src_outer = cred->credentials_tls::combine( dwFlags, - NULL, #if EAP_USE_NATIVE_CREDENTIAL_CACHE has_cached ? cred_in.m_cred.get() : NULL, #else @@ -222,7 +221,6 @@ void eap::peer_ttls_ui::invoke_identity_ui( // Combine inner credentials. eap::credentials::source_t src_inner = cred->m_inner->combine( dwFlags, - NULL, #if EAP_USE_NATIVE_CREDENTIAL_CACHE has_cached ? dynamic_cast(cred_in.m_cred.get())->m_inner.get() : NULL, #else