From 08eaee66b572d3c205c0652d94039bcb55eb62d9 Mon Sep 17 00:00:00 2001 From: Dominik Chilla Date: Mon, 5 Jun 2023 15:57:16 +0200 Subject: [PATCH] ldap3 thread-safe + refactoring --- README.md | 29 +++++++++++++------ app/lam.py | 36 ++++++++++-------------- app/lam_config_backend.py | 2 +- app/lam_log_backend.py | 2 +- app/lam_policy_backend.py | 59 ++++++++++++++++++++------------------- tests/sasl-fail.lua | 39 ++++++++++++++++++++++++++ tests/sasl-wildcard.lua | 3 -- 7 files changed, 107 insertions(+), 63 deletions(-) create mode 100644 tests/sasl-fail.lua diff --git a/README.md b/README.md index b8a7f8c..6ca53be 100644 --- a/README.md +++ b/README.md @@ -62,14 +62,14 @@ services: #LDAP_QUERY: (&(mail=%rcpt%)(|(amavisWhitelistSender=*@%from_domain%)(amavisWhitelistSender=%from%))) # LDAP_QUERY: (&(|(mail=%rcpt%)(mail=*@%rcpt_domain%))(|(amavisWhitelistSender=*@%from_domain%)(amavisWhitelistSender=%from%))) # This enables the use of own ldap-acl-milter LDAP schema. Default: False - # Setting MILTER_SCHEMA: True disables the LDAP_QUERY parameter! - MILTER_SCHEMA: 'True' - # If MILTER_SCHEMA_WILDCARD_DOMAIN is set to True, the milter allows *@domain + # Setting MILTER_SCHEMA: true disables the LDAP_QUERY parameter! + #MILTER_SCHEMA: 'true' + # If MILTER_SCHEMA_WILDCARD_DOMAIN is set to true, the milter allows *@domain # as valid sender/recipient addresses in LDAP. # This only works if MILTER_SCHEMA is enabled! MILTER_SCHEMA_WILDCARD_DOMAIN: 'False' # default: test. Possible: test, reject - MILTER_MODE: 'reject' + #MILTER_MODE: 'reject' MILTER_NAME: some-another-milter-name # Default: UNIX-socket located under /socket/ldap-acl-milter # https://pythonhosted.org/pymilter/namespacemilter.html#a266a6e09897499d8b1ae0e20f0d2be73 @@ -79,10 +79,23 @@ services: # Expect authentication information from LDAP like allowedClientAddr, # allowedSaslUser or allowedx509CN. This is usefull if the milter handles # outbound email traffic, where senders must authenticate before submission. - # Default: False (inbound mode) - MILTER_EXPECT_AUTH: 'True' - # Blank or comma separated list of valid email recipients to whitelist, - MILTER_WHITELISTED_RCPTS: 'postmaster@example.com,hostmaster@example.org' + # Default: false (inbound mode) + #MILTER_EXPECT_AUTH: 'true' + # Blank or comma separated list of valid email recipients to whitelist (default: empty) + #MILTER_WHITELISTED_RCPTS: 'postmaster@example.com,hostmaster@example.org' + # Allow null-sender (<>) for bounces/DSNs (default: disabled) + #MILTER_ALLOW_NULL_SENDER: 'true' + # Enable recipient count limits (default: disabled) + #MILTER_MAX_RCPT_ENABLED: 'true' + #MILTER_MAX_RCPT: 1 + # Enable DKIM checks (default: disabled). + # This enables the milter to use the + # sender address placed in the 5322.from header + # within policy checks, unless the DKIM authentication results + # are invalid. + # Enabling this feature also requires a DKIM validating milter + # BEFORE the ldap-acl-milter! + #MILTER_DKIM_ENABLED: 'true' hostname: ldap-acl-milter volumes: - "lam_socket:/socket/:rw" diff --git a/app/lam.py b/app/lam.py index 5f2ce39..4d84890 100644 --- a/app/lam.py +++ b/app/lam.py @@ -65,26 +65,22 @@ class LdapAclMilter(Milter.Base): try: # this may fail, if no x509 client certificate was used. # postfix only passes this macro to milters if the TLS connection - # with the authenticating client was trusted in a x509 manner! + # with the authenticating client was trusted in a x509 manner (CA trust)! # Unfortunately, postfix only passes the CN-field of the subject/issuer DN :-/ x509_subject = self.getsymval('{cert_subject}') if x509_subject != None: self.session.set_x509_subject(x509_subject) - log_debug( - "x509_subject={}".format(self.session.get_x509_subject()), - self.session - ) - else: - log_debug("No x509_subject registered", self.session) + log_debug( + "x509_subject={}".format(self.session.get_x509_subject()), + self.session + ) x509_issuer = self.getsymval('{cert_issuer}') if x509_issuer != None: self.session.set_x509_issuer(x509_issuer) - log_debug( - "x509_issuer={}".format(self.session.get_x509_issuer()), - self.session - ) - else: - log_debug("No x509_issuer registered", self.session) + log_debug( + "x509_issuer={}".format(self.session.get_x509_issuer()), + self.session + ) except: log_error( "x509 exception: {}".format(traceback.format_exc()), @@ -95,12 +91,10 @@ class LdapAclMilter(Milter.Base): sasl_user = self.getsymval('{auth_authen}') if sasl_user != None: self.session.set_sasl_user(sasl_user) - log_debug( - "sasl_user={}".format(self.session.get_sasl_user()), - self.session - ) - else: - log_debug("No sasl_user registered", self.session) + log_debug( + "sasl_user={}".format(self.session.get_sasl_user()), + self.session + ) except: log_error( "sasl_user exception: {}".format(traceback.format_exc()), @@ -253,7 +247,7 @@ class LdapAclMilter(Milter.Base): self.session ) except Exception as e: - log_info("AR-parse exception: {0}".format(str(e)), self.session) + log_warning("AR-parse exception: {0}".format(str(e)), self.session) return self.milter_action(action = 'continue') # Not registered/used callbacks @@ -289,7 +283,7 @@ class LdapAclMilter(Milter.Base): if self.session.get_hdr_from_domain().lower() == passed_dkim_sdid.lower(): self.session.set_dkim_aligned(True) log_info( - "Found aligned DKIM signature for SDID: {0}".format( + "Found aligned DKIM signature for SDID={0}".format( passed_dkim_sdid ), self.session diff --git a/app/lam_config_backend.py b/app/lam_config_backend.py index fde5685..f2f9b6c 100644 --- a/app/lam_config_backend.py +++ b/app/lam_config_backend.py @@ -112,7 +112,7 @@ class LamConfigBackend(): log_info("ENV[MILTER_EXPECT_AUTH]: {}".format(self.milter_expect_auth)) if 'MILTER_WHITELISTED_RCPTS' in os.environ: - # A blank separated list is expected + # A blank or comma separated list is expected whitelisted_rcpts_str = os.environ['MILTER_WHITELISTED_RCPTS'] for whitelisted_rcpt in re.split(',|\s', whitelisted_rcpts_str): if g_rex_email.match(whitelisted_rcpt) == None: diff --git a/app/lam_log_backend.py b/app/lam_log_backend.py index 615f20b..944b297 100644 --- a/app/lam_log_backend.py +++ b/app/lam_log_backend.py @@ -24,7 +24,7 @@ def init_log_backend(): logging.info("Logger initialized") def do_log(level: str, log_message: str, session: Optional[LamSession] = None): - log_line = '' + log_line = '-' if session is not None: if hasattr(session, 'mconn_id'): log_line = "{}".format(session.get_mconn_id()) diff --git a/app/lam_policy_backend.py b/app/lam_policy_backend.py index c6c89b2..df67a79 100644 --- a/app/lam_policy_backend.py +++ b/app/lam_policy_backend.py @@ -1,12 +1,13 @@ import re -from lam_rex import g_rex_domain from ldap3 import ( - Server, Connection, NONE, set_config_parameter + Server, Connection, NONE, set_config_parameter, + SAFE_RESTARTABLE ) from ldap3.core.exceptions import LDAPException from lam_exceptions import ( LamPolicyBackendException, LamHardException, LamSoftException ) +from lam_rex import g_rex_domain from lam_config_backend import LamConfigBackend from lam_session import LamSession from lam_log_backend import log_info, log_debug @@ -29,12 +30,12 @@ class LamPolicyBackend(): self.config.ldap_bindpw, auto_bind = True, raise_exceptions = True, - client_strategy = 'RESTARTABLE' + client_strategy = SAFE_RESTARTABLE ) - log_info("Connected to LDAP-server: {}".format(self.config.ldap_server)) + log_info("policy: connected to LDAP-server: {}".format(self.config.ldap_server)) except LDAPException as e: raise LamPolicyBackendException( - "Connection to LDAP-server failed: {}".format(str(e)) + "policy: Connection to LDAP-server failed: {}".format(str(e)) ) from e def check_policy(self, session: LamSession, **kwargs): @@ -44,19 +45,19 @@ class LamPolicyBackend(): m = g_rex_domain.match(from_addr) if m == None: raise LamHardException( - "Could not determine domain of from={}".format(from_addr) + "policy: Could not determine domain of from={}".format(from_addr) ) from_domain = m.group(1) - log_debug("from_domain={}".format(from_domain), session) + log_debug("policy: from_domain={}".format(from_domain), session) m = g_rex_domain.match(rcpt_addr) if m == None: raise LamHardException( - "Could not determine domain of rcpt={}".format( + "policy: Could not determine domain of rcpt={}".format( rcpt_addr ) ) rcpt_domain = m.group(1) - log_debug("rcpt_domain={}".format(rcpt_domain), session) + log_debug("policy: rcpt_domain={}".format(rcpt_domain), session) try: if self.config.milter_schema == True: # LDAP-ACL-Milter schema enabled @@ -82,7 +83,7 @@ class LamPolicyBackend(): ) else: auth_method = auth_method.replace('%X509_AUTH%','') - log_debug("auth_method: {}".format(auth_method), session) + log_debug("policy: auth_method: {}".format(auth_method), session) if self.config.milter_schema_wildcard_domain == True: # The asterisk (*) character is in term of local part # RFC5322 compliant and expected as a wildcard literal in this code. @@ -92,15 +93,15 @@ class LamPolicyBackend(): # In this case *@ cannot be a real address! if re.match(r'^\*@.+$', from_addr, re.IGNORECASE): raise LamHardException( - "Literal wildcard sender (*@) is not " + + "policy: Literal wildcard sender (*@) is not " + "allowed in wildcard mode!" ) if re.match(r'^\*@.+$', rcpt_addr, re.IGNORECASE): raise LamHardException( - "Literal wildcard recipient (*@) is not " + + "policy: Literal wildcard recipient (*@) is not " + "allowed in wildcard mode!" ) - self.ldap_conn.search(self.config.ldap_base, + _, _, ldap_response, _ = self.ldap_conn.search(self.config.ldap_base, "(&" + auth_method + "(|" + @@ -131,7 +132,7 @@ class LamPolicyBackend(): # Asterisk (*) must be ASCII-HEX encoded for LDAP queries query_from = from_addr.replace("*","\\2a") query_to = rcpt_addr.replace("*","\\2a") - self.ldap_conn.search(self.config.ldap_base, + _, _, ldap_response, _ = self.ldap_conn.search(self.config.ldap_base, "(&" + auth_method + "(allowedSenders=" + query_from + ")" + @@ -141,33 +142,33 @@ class LamPolicyBackend(): ")", attributes=['policyID'] ) - if len(self.ldap_conn.entries) == 0: + if len(ldap_response) == 0: # Policy not found in LDAP raise LamHardException( - "mismatch: from_src={0} from={1} rcpt={2}".format( + "policy: mismatch: from_src={0} from={1} rcpt={2}".format( from_source, from_addr, rcpt_addr ) ) - elif len(self.ldap_conn.entries) == 1: + elif len(ldap_response) == 1: if from_source == 'from-header': log_info( - "5322.from_domain={} authorized by DKIM signature".format( + "policy: 5322.from_domain={} authorized by DKIM signature".format( from_domain ), session ) # Policy found in LDAP, but which one? - entry = self.ldap_conn.entries[0] + entry = ldap_response[0]['attributes'] log_info( - "match='{0}' from_src={1}".format( - entry.policyID.value, from_source + "policy: match='{0}' from_src={1}".format( + entry['PolicyID'][0], from_source ), session ) - elif len(self.ldap_conn.entries) > 1: + elif len(ldap_response) > 1: # Something went wrong!? There shouldn´t be more than one entries! raise LamHardException( - "More than one policies found! from={0} rcpt={1} auth_method={2}".format( + "policy: More than one policies found! from={0} rcpt={1} auth_method={2}".format( from_addr, rcpt_addr, auth_method ) ) @@ -182,14 +183,14 @@ class LamPolicyBackend(): query = query.replace("%sasl_user%", session.get_sasl_user()) query = query.replace("%from_domain%", from_domain) query = query.replace("%rcpt_domain%", rcpt_domain) - log_debug("LDAP query: {}".format(query), session) - self.ldap_conn.search(self.config.ldap_base, query) - if len(self.ldap_conn.entries) == 0: + log_debug("policy: LDAP query: {}".format(query), session) + _, _, ldap_response, _ = self.ldap_conn.search(self.config.ldap_base, query) + if len(ldap_response) == 0: raise LamHardException( - "mismatch from_src={0} from={1} rcpt={2}".format( + "policy: mismatch from_src={0} from={1} rcpt={2}".format( from_source, from_addr, rcpt_addr ) ) - log_info("match from_src={}".format(from_source), session) + log_info("policy: match from_src={}".format(from_source), session) except LDAPException as e: - raise LamSoftException("LDAP exception: " + str(e)) from e + raise LamSoftException("policy: LDAP exception: " + str(e)) from e diff --git a/tests/sasl-fail.lua b/tests/sasl-fail.lua new file mode 100644 index 0000000..6a4ed42 --- /dev/null +++ b/tests/sasl-fail.lua @@ -0,0 +1,39 @@ +-- https://mopano.github.io/sendmail-filter-api/constant-values.html#com.sendmail.milter.MilterConstants +-- http://www.opendkim.org/miltertest.8.html + +-- socket must be defined as miltertest global variable (-D) +conn = mt.connect(socket) +if conn == nil then + error "mt.connect() failed" +end +if mt.conninfo(conn, "localhost", "::1") ~= nil then + error "mt.conninfo() failed" +end + +mt.set_timeout(60) + +-- 5321.FROM+MACROS +mt.macro(conn, SMFIC_MAIL, "{auth_authen}", "blubb-user1") +if mt.mailfrom(conn, "tester-fail@test.blah") ~= nil then + error "mt.mailfrom() failed" +end + +-- 5321.RCPT+MACROS +mt.macro(conn, SMFIC_RCPT, "i", "4CgSNs5Q9sz7SllQ") +if mt.rcptto(conn, "") ~= nil then + error "mt.rcptto() failed" +end + +-- EOM +if mt.eom(conn) ~= nil then + error "mt.eom() failed" +end +mt.echo("EOM: " .. mt.getreply(conn)) +if mt.getreply(conn) == SMFIR_CONTINUE then + mt.echo("EOM-continue") +elseif mt.getreply(conn) == SMFIR_REPLYCODE then + mt.echo("EOM-reject") +end + +-- DISCONNECT +mt.disconnect(conn) \ No newline at end of file diff --git a/tests/sasl-wildcard.lua b/tests/sasl-wildcard.lua index b93d207..5059075 100644 --- a/tests/sasl-wildcard.lua +++ b/tests/sasl-wildcard.lua @@ -28,9 +28,6 @@ end if mt.header(conn, "fRoM", '"Blah Blubb" ') ~= nil then error "mt.header(From) failed" end -if mt.header(conn, "Authentication-RESULTS", "my-auth-serv-id;\n dkim=pass header.d=test.blah header.s=selector1-test-blah header.b=mumble") ~= nil then - error "mt.header(Authentication-Results) failed" -end -- EOM if mt.eom(conn) ~= nil then