Peer correctly returns providers configuration instead of method configuration in method_tls::get_result()

lib/EAPBase/include/Method.h

@@ -51,9 +51,10 @@ namespace eap
         /// Constructs an EAP method
         ///
         /// \param[in] mod   EAP module to use for global services
-        /// \param[in] cfg  Method configuration
+        /// \param[in] cfg   Providers configuration
+        /// \param[in] cred  User credentials
         ///
-        method(_In_ module &module, _In_ config_method &cfg, _In_ credentials &cred);
+        method(_In_ module &module, _In_ config_provider_list &cfg, _In_ credentials &cred);
 
         ///
         /// Copies an EAP method
@@ -140,7 +141,7 @@ namespace eap
 
     public:
         module &m_module;               ///< EAP module
-        config_method &m_cfg;   ///< Method configuration
+        config_provider_list &m_cfg;    ///< Providers configuration
         credentials &m_cred;            ///< User credentials
     };
 }

lib/EAPBase/src/Method.cpp

@@ -28,7 +28,7 @@ using namespace winstd;
 // eap::method
 //////////////////////////////////////////////////////////////////////
 
-eap::method::method(_In_ module &module, _In_ config_method &cfg, _In_ credentials &cred) :
+eap::method::method(_In_ module &module, _In_ config_provider_list &cfg, _In_ credentials &cred) :
     m_module(module),
     m_cfg(cfg),
     m_cred(cred)

lib/TLS/include/Method.h

@@ -143,10 +143,10 @@ namespace eap
         /// Constructs an EAP method
         ///
         /// \param[in] mod   EAP module to use for global services
-        /// \param[in] cfg   Method configuration
+        /// \param[in] cfg   Providers configuration
         /// \param[in] cred  User credentials
         ///
-        method_tls(_In_ module &module, _In_ config_method_tls &cfg, _In_ credentials_tls &cred);
+        method_tls(_In_ module &module, _In_ config_provider_list &cfg, _In_ credentials_tls &cred);
 
         ///
         /// Copies an EAP method
@@ -503,7 +503,6 @@ namespace eap
             _In_                              size_t    size_secret);
 
     protected:
-        config_method_tls &m_cfg;                               ///< EAP-TLS method configuration
         credentials_tls &m_cred;                                ///< EAP-TLS user credentials
 
         packet m_packet_req;                                    ///< Request packet

lib/TLS/src/Method.cpp

@@ -93,8 +93,7 @@ void eap::method_tls::packet::clear()
 // eap::method_tls
 //////////////////////////////////////////////////////////////////////
 
-eap::method_tls::method_tls(_In_ module &module, _In_ config_method_tls &cfg, _In_ credentials_tls &cred) :
+eap::method_tls::method_tls(_In_ module &module, _In_ config_provider_list &cfg, _In_ credentials_tls &cred) :
-    m_cfg(cfg),
     m_cred(cred),
     m_certificate_req(false),
     m_server_hello_done(false),
@@ -109,7 +108,6 @@ eap::method_tls::method_tls(_In_ module &module, _In_ config_method_tls &cfg, _I
 
 
 eap::method_tls::method_tls(_In_ const method_tls &other) :
-    m_cfg(other.m_cfg),
     m_cred(other.m_cred),
     m_packet_req(other.m_packet_req),
     m_packet_res(other.m_packet_res),
@@ -136,7 +134,6 @@ eap::method_tls::method_tls(_In_ const method_tls &other) :
 
 
 eap::method_tls::method_tls(_Inout_ method_tls &&other) :
-    m_cfg(other.m_cfg),
     m_cred(other.m_cred),
     m_packet_req(std::move(other.m_packet_req)),
     m_packet_res(std::move(other.m_packet_res)),
@@ -172,7 +169,6 @@ eap::method_tls::~method_tls()
 eap::method_tls& eap::method_tls::operator=(_In_ const method_tls &other)
 {
     if (this != std::addressof(other)) {
-        assert(std::addressof(m_cfg ) == std::addressof(other.m_cfg )); // Copy method with same configuration only!
         assert(std::addressof(m_cred) == std::addressof(other.m_cred)); // Copy method with same credentials only!
         (method&)*this              = other;
         m_packet_req                = other.m_packet_req;
@@ -203,7 +199,6 @@ eap::method_tls& eap::method_tls::operator=(_In_ const method_tls &other)
 eap::method_tls& eap::method_tls::operator=(_Inout_ method_tls &&other)
 {
     if (this != std::addressof(other)) {
-        assert(std::addressof(m_cfg ) == std::addressof(other.m_cfg )); // Move method with same configuration only!
         assert(std::addressof(m_cred) == std::addressof(other.m_cred)); // Move method with same credentials only!
         (method&)*this              = std::move(other);
         m_packet_req                = std::move(other.m_packet_req);
@@ -243,8 +238,15 @@ void eap::method_tls::begin_session(
     if (!m_cp.create(NULL, MS_ENHANCED_PROV, PROV_RSA_FULL))
         throw win_runtime_error(__FUNCTION__ " Error creating cryptographics provider.");
 
-    m_session_id = m_cfg.m_session_id;
+    if (m_cfg.m_providers.empty() || m_cfg.m_providers.front().m_methods.empty())
-    m_state.m_master_secret = m_cfg.m_master_secret;
+        throw invalid_argument(__FUNCTION__ " Configuration has no providers and/or methods.");
+
+    const config_provider &cfg_prov(m_cfg.m_providers.front());
+    const config_method_tls *cfg_method = dynamic_cast<const config_method_tls*>(cfg_prov.m_methods.front().get());
+    assert(cfg_method);
+
+    m_session_id = cfg_method->m_session_id;
+    m_state.m_master_secret = cfg_method->m_master_secret;
 }
 
 
@@ -510,6 +512,10 @@ void eap::method_tls::get_result(
 {
     assert(ppResult);
 
+    config_provider &cfg_prov(m_cfg.m_providers.front());
+    config_method_tls *cfg_method = dynamic_cast<config_method_tls*>(cfg_prov.m_methods.front().get());
+    assert(cfg_method);
+
     switch (reason) {
     case EapPeerMethodResultSuccess: {
         if (!m_server_finished)
@@ -535,23 +541,27 @@ void eap::method_tls::get_result(
         ppResult->fIsSuccess = TRUE;
 
         // Update configuration with session resumption data and prepare BLOB.
-        m_cfg.m_session_id    = m_session_id;
+        cfg_method->m_session_id    = m_session_id;
-        m_cfg.m_master_secret = m_state.m_master_secret;
+        cfg_method->m_master_secret = m_state.m_master_secret;
 
         break;
     }
 
     case EapPeerMethodResultFailure:
-        // :(
+        // Clear session resumption data.
-        m_cfg.m_session_id.clear();
+        cfg_method->m_session_id.clear();
-        m_cfg.m_master_secret.clear();
+        cfg_method->m_master_secret.clear();
-        ppResult->fSaveConnectionData = TRUE;
+
+        ppResult->fIsSuccess = FALSE;
+        ppResult->dwFailureReasonCode = EAP_E_AUTHENTICATION_FAILED;
+
         break;
 
     default:
         throw win_runtime_error(ERROR_NOT_SUPPORTED, __FUNCTION__ " Not supported.");
     }
 
+    // Always ask EAP host to save the connection data.
     ppResult->fSaveConnectionData = TRUE;
     m_module.pack(m_cfg, &ppResult->pConnectionData, &ppResult->dwSizeofConnectionData);
     if (m_blob_cfg)
@@ -610,15 +620,7 @@ eap::sanitizing_blob eap::method_tls::make_client_hello() const
 eap::sanitizing_blob eap::method_tls::make_client_cert() const
 {
     // Select client certificate.
-    PCCERT_CONTEXT cert;
+    PCCERT_CONTEXT cert = m_cred.m_cert ? m_cred.m_cert : NULL;
-    if (m_cfg.m_use_preshared) {
-        // Using pre-shared credentials.
-        const credentials_tls *preshared = dynamic_cast<credentials_tls*>(m_cfg.m_preshared.get());
-        cert = preshared && preshared->m_cert ? preshared->m_cert : NULL;
-    } else {
-        // Using own credentials.
-        cert = m_cred.m_cert ? m_cred.m_cert : NULL;
-    }
 
     size_t size_data, size_list;
     sanitizing_blob msg;
@@ -1104,14 +1106,18 @@ void eap::method_tls::verify_server_trust() const
     assert(!m_server_cert_chain.empty());
     const cert_context &cert = m_server_cert_chain.front();
 
-    if (!m_cfg.m_server_names.empty()) {
+    const config_provider &cfg_prov(m_cfg.m_providers.front());
+    const config_method_tls *cfg_method = dynamic_cast<const config_method_tls*>(cfg_prov.m_methods.front().get());
+    assert(cfg_method);
+
+    if (!cfg_method->m_server_names.empty()) {
         // Check server name.
 
         string subj;
         if (!CertGetNameStringA(cert, CERT_NAME_SIMPLE_DISPLAY_TYPE, 0, NULL, subj))
             throw win_runtime_error(__FUNCTION__ " Error retrieving server's certificate subject name.");
 
-        for (list<string>::const_iterator s = m_cfg.m_server_names.cbegin(), s_end = m_cfg.m_server_names.cend();; ++s) {
+        for (list<string>::const_iterator s = cfg_method->m_server_names.cbegin(), s_end = cfg_method->m_server_names.cend();; ++s) {
             if (s != s_end) {
                 const char
                     *a = s->c_str(),
@@ -1135,7 +1141,7 @@ void eap::method_tls::verify_server_trust() const
     cert_store store;
     if (!store.create(CERT_STORE_PROV_MEMORY, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, NULL, 0, NULL))
         throw win_runtime_error(ERROR_INVALID_DOMAINNAME, __FUNCTION__ " Error creating temporary certificate store.");
-    for (list<cert_context>::const_iterator c = m_cfg.m_trusted_root_ca.cbegin(), c_end = m_cfg.m_trusted_root_ca.cend(); c != c_end; ++c)
+    for (list<cert_context>::const_iterator c = cfg_method->m_trusted_root_ca.cbegin(), c_end = cfg_method->m_trusted_root_ca.cend(); c != c_end; ++c)
         CertAddCertificateContextToStore(store, *c, CERT_STORE_ADD_REPLACE_EXISTING, NULL);
 
     // Add all certificates from the server's certificate chain, except the first one.
@@ -1164,10 +1170,10 @@ void eap::method_tls::verify_server_trust() const
 
     // Check chain validation error flags. Ignore CERT_TRUST_IS_UNTRUSTED_ROOT flag when we check root CA explicitly.
     if (context->TrustStatus.dwErrorStatus != CERT_TRUST_NO_ERROR &&
-        (m_cfg.m_trusted_root_ca.empty() || (context->TrustStatus.dwErrorStatus & ~CERT_TRUST_IS_UNTRUSTED_ROOT) != CERT_TRUST_NO_ERROR))
+        (cfg_method->m_trusted_root_ca.empty() || (context->TrustStatus.dwErrorStatus & ~CERT_TRUST_IS_UNTRUSTED_ROOT) != CERT_TRUST_NO_ERROR))
         throw win_runtime_error(context->TrustStatus.dwErrorStatus, "Error validating certificate chain.");
 
-    if (!m_cfg.m_trusted_root_ca.empty()) {
+    if (!cfg_method->m_trusted_root_ca.empty()) {
         // Verify Root CA against our trusted root CA list
         if (context->cChain != 1)
             throw win_runtime_error(ERROR_NOT_SUPPORTED, __FUNCTION__ " Multiple chain verification not supported.");
@@ -1175,7 +1181,7 @@ void eap::method_tls::verify_server_trust() const
             throw win_runtime_error(ERROR_NOT_SUPPORTED, __FUNCTION__ " Can not verify empty certificate chain.");
 
         PCCERT_CONTEXT cert_root = context->rgpChain[0]->rgpElement[context->rgpChain[0]->cElement-1]->pCertContext;
-        for (list<cert_context>::const_iterator c = m_cfg.m_trusted_root_ca.cbegin(), c_end = m_cfg.m_trusted_root_ca.cend();; ++c) {
+        for (list<cert_context>::const_iterator c = cfg_method->m_trusted_root_ca.cbegin(), c_end = cfg_method->m_trusted_root_ca.cend();; ++c) {
             if (c != c_end) {
                 if (cert_root->cbCertEncoded == (*c)->cbCertEncoded &&
                     memcmp(cert_root->pbCertEncoded, (*c)->pbCertEncoded, cert_root->cbCertEncoded) == 0)

lib/TTLS/include/Config.h

@@ -148,7 +148,7 @@ namespace eap {
         std::wstring get_public_identity(const credentials_ttls &cred) const;
 
     public:
-        std::unique_ptr<config> m_inner;    ///< Inner authentication configuration
+        std::unique_ptr<config_method_with_cred> m_inner;   ///< Inner authentication configuration
         std::wstring m_anonymous_identity;                  ///< Anonymous identity
     };
 }

lib/TTLS/include/Method.h

@@ -57,9 +57,10 @@ namespace eap
         /// Constructs an EAP method
         ///
         /// \param[in] mod  EAP module to use for global services
-        /// \param[in] cfg  Method configuration
+        /// \param[in] cfg   Providers configuration
+        /// \param[in] cred  User credentials
         ///
-        method_ttls(_In_ module &module, _In_ config_method_ttls &cfg, _In_ credentials_ttls &cred);
+        method_ttls(_In_ module &module, _In_ config_provider_list &cfg, _In_ credentials_ttls &cred);
 
         ///
         /// Copies an EAP method
@@ -115,6 +116,15 @@ namespace eap
             _Inout_bytecap_(*dwSendPacketSize) EapPacket *pSendPacket,
             _Inout_                            DWORD     *pdwSendPacketSize);
 
+        ///
+        /// Obtains the result of an authentication session from the EAP method.
+        ///
+        /// \sa [EapPeerGetResult function](https://msdn.microsoft.com/en-us/library/windows/desktop/aa363611.aspx)
+        ///
+        virtual void get_result(
+            _In_    EapPeerMethodResultReason reason,
+            _Inout_ EapPeerMethodResult       *ppResult);
+
         /// @}
 
         ///

lib/TTLS/include/Module.h

@@ -221,7 +221,7 @@ namespace eap
             {}
 
         public:
-            config_method_ttls m_cfg;   ///< Method configuration
+            config_provider_list m_cfg; ///< Providers configuration
             credentials_ttls m_cred;    ///< User credentials
             method_ttls m_method;       ///< EAP-TTLS method
         };

lib/TTLS/src/Config.cpp

@@ -35,7 +35,7 @@ eap::config_method_ttls::config_method_ttls(_In_ module &mod) :
 
 
 eap::config_method_ttls::config_method_ttls(const _In_ config_method_ttls &other) :
-    m_inner(other.m_inner ? (config_method*)other.m_inner->clone() : nullptr),
+    m_inner(other.m_inner ? (config_method_with_cred*)other.m_inner->clone() : nullptr),
     m_anonymous_identity(other.m_anonymous_identity),
     config_method_tls(other)
 {
@@ -54,7 +54,7 @@ eap::config_method_ttls& eap::config_method_ttls::operator=(const _In_ config_me
 {
     if (this != &other) {
         (config_method_tls&)*this = other;
-        m_inner.reset(other.m_inner ? (config_method*)other.m_inner->clone() : nullptr);
+        m_inner.reset(other.m_inner ? (config_method_with_cred*)other.m_inner->clone() : nullptr);
         m_anonymous_identity  = other.m_anonymous_identity;
     }
 

lib/TTLS/src/Method.cpp

@@ -28,7 +28,7 @@ using namespace winstd;
 // eap::method_ttls
 //////////////////////////////////////////////////////////////////////
 
-eap::method_ttls::method_ttls(_In_ module &module, _In_ config_method_ttls &cfg, _In_ credentials_ttls &cred) :
+eap::method_ttls::method_ttls(_In_ module &module, _In_ config_provider_list &cfg, _In_ credentials_ttls &cred) :
     m_cred(cred),
     m_version(version_0),
     method_tls(module, cfg, cred)
@@ -127,6 +127,25 @@ void eap::method_ttls::get_response_packet(
 }
 
 
+void eap::method_ttls::get_result(
+    _In_    EapPeerMethodResultReason reason,
+    _Inout_ EapPeerMethodResult       *ppResult)
+{
+    if (!m_server_finished) {
+        // Do the TLS.
+        method_tls::get_result(reason, ppResult);
+    } else {
+        // The TLS was OK.
+        method_tls::get_result(EapPeerMethodResultSuccess, ppResult);
+
+        if (reason == EapPeerMethodResultFailure) {
+            ppResult->fIsSuccess = FALSE;
+            ppResult->dwFailureReasonCode = EAP_E_AUTHENTICATION_FAILED;
+        }
+    }
+}
+
+
 void eap::method_ttls::derive_msk()
 {
     static const unsigned char s_label[] = "ttls keying material";

lib/TTLS/src/Module.cpp

@@ -83,7 +83,6 @@ void eap::peer_ttls::get_identity(
     const config_provider &cfg_prov(cfg.m_providers.front());
     const config_method_ttls *cfg_method = dynamic_cast<const config_method_ttls*>(cfg_prov.m_methods.front().get());
     assert(cfg_method);
-    const config_method_pap *cfg_inner_pap = dynamic_cast<const config_method_pap*>(cfg_method->m_inner.get());
 
     // Unpack cached credentials.
     credentials_ttls cred_in(*this);
@@ -92,11 +91,11 @@ void eap::peer_ttls::get_identity(
 
     credentials_ttls cred_out(*this);
 
-    // Determine credential storage target(s). Also used as user-friendly method name for logging.
+    // Determine credential storage target(s).
     eap_type_t type_inner;
-    if (cfg_inner_pap) {
+    if (dynamic_cast<const config_method_pap*>(cfg_method->m_inner.get()))
         type_inner = eap_type_pap;
-    } else {
+    else {
         assert(0); // Unsupported inner authentication method type.
         type_inner = eap_type_undefined;
     }
@@ -116,7 +115,7 @@ void eap::peer_ttls::get_identity(
         }
 
         if (!is_inner_set && cred_in.m_inner) {
-            // Inner PAP: Using EAP service cached credentials.
+            // Inner: Using EAP service cached credentials.
             cred_out.m_inner.reset((credentials*)cred_in.m_inner->clone());
             log_event(&EAPMETHOD_TRACE_EVT_CRED_CACHED1, event_data((unsigned int)type_inner), event_data(cred_out.m_inner->get_name()), event_data::blank);
             is_inner_set = true;
@@ -131,15 +130,12 @@ void eap::peer_ttls::get_identity(
     }
 
     if (!is_inner_set) {
-        if (cfg_inner_pap) {
+        if (cfg_method->m_inner->m_use_preshared) {
-            if (cfg_inner_pap->m_use_preshared) {
+            // Inner: Using preshared credentials.
-                // Inner PAP: Using preshared credentials.
+            cred_out.m_inner.reset((credentials*)cfg_method->m_inner->m_preshared->clone());
-                cred_out.m_inner.reset((credentials*)cfg_inner_pap->m_preshared->clone());
             log_event(&EAPMETHOD_TRACE_EVT_CRED_PRESHARED1, event_data((unsigned int)type_inner), event_data(cred_out.m_inner->get_name()), event_data::blank);
             is_inner_set = true;
         }
-        } else
-            assert(0); // Unsupported inner authentication method type.
     }
 
     if ((dwFlags & EAP_FLAG_GUEST_ACCESS) == 0 && (!is_outer_set || !is_inner_set)) {
@@ -164,12 +160,14 @@ void eap::peer_ttls::get_identity(
 
         if (!is_inner_set) {
             unique_ptr<credentials> cred_loaded;
-            if (cfg_inner_pap) cred_loaded.reset(new credentials_pap(*this));
+            switch (type_inner) {
-            else               assert(0); // Unsupported inner authentication method type.
+            case eap_type_pap: cred_loaded.reset(new credentials_pap(*this)); break;
+            default          : assert(0); // Unsupported inner authentication method type.
+            }
             try {
                 cred_loaded->retrieve(cfg_prov.m_id.c_str());
 
-                // Inner PAP: Using stored credentials.
+                // Inner: Using stored credentials.
                 cred_out.m_inner = std::move(cred_loaded);
                 log_event(&EAPMETHOD_TRACE_EVT_CRED_STORED1, event_data((unsigned int)type_inner), event_data(cred_out.m_inner->get_name()), event_data::blank);
                 is_inner_set = true;
@@ -304,14 +302,7 @@ EAP_SESSION_HANDLE eap::peer_ttls::begin_session(
     unique_ptr<session> s(new session(*this));
 
     // Unpack configuration.
-    config_provider_list cfg(*this);
+    unpack(s->m_cfg, pConnectionData, dwConnectionDataSize);
-    unpack(cfg, pConnectionData, dwConnectionDataSize);
-    if (cfg.m_providers.empty() || cfg.m_providers.front().m_methods.empty())
-        throw invalid_argument(__FUNCTION__ " Configuration has no providers and/or methods.");
-
-    // Copy method configuration.
-    const config_provider &cfg_prov(cfg.m_providers.front());
-    s->m_cfg = *dynamic_cast<const config_method_ttls*>(cfg_prov.m_methods.front().get());
 
     // Unpack credentials.
     unpack(s->m_cred, pUserData, dwUserDataSize);