From 1a7b4bac42c0c0f4dc9d0081c462d62f193e0da8 Mon Sep 17 00:00:00 2001 From: Joel Sing Date: Thu, 12 Mar 2020 16:20:46 +1100 Subject: [PATCH] Improve logging for the m_ldap and m_ldapauth modules (#1757). Currently, it is difficult to diagnose LDAP authentication failures, since the logs do not provide sufficient information about what is actually being queried and what actually failed. This increases logging details so that information about the LDAP query is included, for example: Fri Mar 06 2020 08:02:59 ANNOUNCEMENT: Error binding as manager to LDAP server: Invalid credentials (bind dn=cn=adminz,dc=nodomain) Rather than: Fri Mar 06 2020 08:02:59 ANNOUNCEMENT: Error binding as manager to LDAP server: Invalid credentials Same with connection logging: Fri Mar 06 2020 07:59:53 CONNECT: Forbidden connection from jsing!jsing@192.168.200.1 (Invalid credentials (bind dn=uid=jsing,dc=nodomain)) Fri Mar 06 2020 08:01:19 CONNECT: Successful connection from jsing!jsing@192.168.200.1 (dn=uid=jsing,dc=nodomain) --- src/modules/extra/m_ldap.cpp | 38 +++++++++++++++++++++++++++++++++++- src/modules/m_ldapauth.cpp | 8 +++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/modules/extra/m_ldap.cpp b/src/modules/extra/m_ldap.cpp index 488208a5e..874306e62 100644 --- a/src/modules/extra/m_ldap.cpp +++ b/src/modules/extra/m_ldap.cpp @@ -78,6 +78,7 @@ class LDAPRequest } virtual int run() = 0; + virtual std::string info() = 0; }; class LDAPBind : public LDAPRequest @@ -94,6 +95,7 @@ class LDAPBind : public LDAPRequest } int run() CXX11_OVERRIDE; + std::string info() CXX11_OVERRIDE; }; class LDAPSearch : public LDAPRequest @@ -113,6 +115,7 @@ class LDAPSearch : public LDAPRequest } int run() CXX11_OVERRIDE; + std::string info() CXX11_OVERRIDE; }; class LDAPAdd : public LDAPRequest @@ -130,6 +133,7 @@ class LDAPAdd : public LDAPRequest } int run() CXX11_OVERRIDE; + std::string info() CXX11_OVERRIDE; }; class LDAPDel : public LDAPRequest @@ -145,6 +149,7 @@ class LDAPDel : public LDAPRequest } int run() CXX11_OVERRIDE; + std::string info() CXX11_OVERRIDE; }; class LDAPModify : public LDAPRequest @@ -162,6 +167,7 @@ class LDAPModify : public LDAPRequest } int run() CXX11_OVERRIDE; + std::string info() CXX11_OVERRIDE; }; class LDAPCompare : public LDAPRequest @@ -179,6 +185,7 @@ class LDAPCompare : public LDAPRequest } int run() CXX11_OVERRIDE; + std::string info() CXX11_OVERRIDE; }; class LDAPService : public LDAPProvider, public SocketThread @@ -396,7 +403,7 @@ class LDAPService : public LDAPProvider, public SocketThread if (res != LDAP_SUCCESS) { - ldap_result->error = ldap_err2string(res); + ldap_result->error = InspIRCd::Format("%s (%s)", ldap_err2string(res), req->info().c_str()); return; } @@ -646,11 +653,21 @@ int LDAPBind::run() return i; } +std::string LDAPBind::info() +{ + return "bind dn=" + who; +} + int LDAPSearch::run() { return ldap_search_ext_s(service->GetConnection(), base.c_str(), searchscope, filter.c_str(), NULL, 0, NULL, NULL, &tv, 0, &message); } +std::string LDAPSearch::info() +{ + return "search base=" + base + " filter=" + filter; +} + int LDAPAdd::run() { LDAPMod** mods = LDAPService::BuildMods(attributes); @@ -659,11 +676,21 @@ int LDAPAdd::run() return i; } +std::string LDAPAdd::info() +{ + return "add dn=" + dn; +} + int LDAPDel::run() { return ldap_delete_ext_s(service->GetConnection(), dn.c_str(), NULL, NULL); } +std::string LDAPDel::info() +{ + return "del dn=" + dn; +} + int LDAPModify::run() { LDAPMod** mods = LDAPService::BuildMods(attributes); @@ -672,6 +699,11 @@ int LDAPModify::run() return i; } +std::string LDAPModify::info() +{ + return "modify base=" + base; +} + int LDAPCompare::run() { berval cred; @@ -683,7 +715,11 @@ int LDAPCompare::run() free(cred.bv_val); return ret; +} +std::string LDAPCompare::info() +{ + return "compare dn=" + dn + " attr=" + attr; } MODULE_INIT(ModuleLDAP) diff --git a/src/modules/m_ldapauth.cpp b/src/modules/m_ldapauth.cpp index b612fe8b2..fb5c69d0d 100644 --- a/src/modules/m_ldapauth.cpp +++ b/src/modules/m_ldapauth.cpp @@ -118,6 +118,9 @@ class BindInterface : public LDAPInterface if (!checkingAttributes && requiredattributes.empty()) { + if (verbose) + ServerInstance->SNO->WriteToSnoMask('c', "Successful connection from %s (dn=%s)", user->GetFullRealHost().c_str(), DN.c_str()); + // We're done, there are no attributes to check SetVHost(user, DN); authed->set(user, 1); @@ -134,6 +137,9 @@ class BindInterface : public LDAPInterface // Only one has to pass passed = true; + if (verbose) + ServerInstance->SNO->WriteToSnoMask('c', "Successful connection from %s (dn=%s)", user->GetFullRealHost().c_str(), DN.c_str()); + SetVHost(user, DN); authed->set(user, 1); } @@ -171,7 +177,7 @@ class BindInterface : public LDAPInterface if (!attrCount) { if (verbose) - ServerInstance->SNO->WriteToSnoMask('c', "Forbidden connection from %s (unable to validate attributes)", user->GetFullRealHost().c_str()); + ServerInstance->SNO->WriteToSnoMask('c', "Forbidden connection from %s (dn=%s) (unable to validate attributes)", user->GetFullRealHost().c_str(), DN.c_str()); ServerInstance->Users->QuitUser(user, killreason); delete this; } -- 2.39.5