diff --git a/email_server/auth.py b/email_server/auth.py index 08ad04d..8c6c5e1 100644 --- a/email_server/auth.py +++ b/email_server/auth.py @@ -32,7 +32,7 @@ class EnhancedAuthenticator: def __call__(self, server, session, envelope, mechanism, auth_data): if not isinstance(auth_data, LoginPassword): logger.warning(f'Invalid auth data format: {type(auth_data)}') - return AuthResult(success=False, handled=True, message='535 Authentication failed') + return AuthResult(success=False, handled=False, message='535 Authentication failed') # Decode bytes to string if necessary username = auth_data.login @@ -73,7 +73,7 @@ class EnhancedAuthenticator: message=f'Invalid credentials for {username}' ) logger.warning(f'Authentication failed for {username}: invalid credentials') - return AuthResult(success=False, handled=True, message='535 Authentication failed') + return AuthResult(success=False, handled=False, message='535 Authentication failed') except Exception as e: logger.error(f'Authentication error for {username}: {e}') @@ -84,7 +84,7 @@ class EnhancedAuthenticator: success=False, message=f'Authentication error: {str(e)}' ) - return AuthResult(success=False, handled=True, message='451 Internal server error') + return AuthResult(success=False, handled=False, message='451 Internal server error') class EnhancedIPAuthenticator: """ diff --git a/email_server/server_runner.py b/email_server/server_runner.py index e4dce02..d2a065c 100644 --- a/email_server/server_runner.py +++ b/email_server/server_runner.py @@ -117,7 +117,7 @@ async def start_server(shutdown_event=None): ) controller_tls.start() logger.debug(f' - Plain SMTP (IP whitelist): {BIND_IP}:{SMTP_PORT}') - logger.debug(f' - STARTTLS SMTP (auth required): {BIND_IP}:{SMTP_TLS_PORT}') + logger.debug(f' - Direct TLS SMTP (SMTPS, auth required): {BIND_IP}:{SMTP_TLS_PORT}') logger.debug('Management available via web interface at: http://localhost:5000/email') try: diff --git a/email_server/settings_loader.py b/email_server/settings_loader.py index 8e84ae5..24ae971 100644 --- a/email_server/settings_loader.py +++ b/email_server/settings_loader.py @@ -14,8 +14,8 @@ DEFAULTS = { '; Server configuration for SMTP ports and hostname': None, '; Plain SMTP port for internal/whitelisted IPs': None, 'SMTP_PORT': '4025', - '; STARTTLS SMTP port for authenticated users': None, - 'SMTP_TLS_PORT': '40587', + '; TLS SMTP port for authenticated users': None, + 'SMTP_TLS_PORT': '40465', '; Server hostname for HELO/EHLO identification': None, 'HOSTNAME': 'mail.example.com', '; Override HELO hostname': None, diff --git a/email_server/smtp_handler.py b/email_server/smtp_handler.py index db100a1..f8d44c5 100644 --- a/email_server/smtp_handler.py +++ b/email_server/smtp_handler.py @@ -21,7 +21,7 @@ from email_server.tool_box import get_logger logger = get_logger() class CustomSMTP(AIOSMTP): - """Custom SMTP class with configurable banner.""" + """Custom SMTP class with configurable banner and secure AUTH handling.""" def __init__(self, *args, **kwargs): # Sets Custom SMTP banner from settings @@ -30,11 +30,31 @@ class CustomSMTP(AIOSMTP): if _banner_message == '""': _banner_message = '' self.custom_banner = _banner_message - + + # Store authenticator and auth_require_tls for later use + self._custom_authenticator = kwargs.get('authenticator', None) + self._custom_auth_require_tls = kwargs.get('auth_require_tls', False) super().__init__(*args, **kwargs) # Override the __ident__ to use our custom banner self.__ident__ = self.custom_banner + def _get_auth_methods(self): + # Only advertise AUTH if authenticator is set and (not auth_require_tls or connection is secure) + if self._custom_authenticator and (not self._custom_auth_require_tls or self.session and self.session.ssl): + return super()._get_auth_methods() + return [] + + async def smtp_AUTH(self, arg): + """ + Override AUTH command to close connection after failed authentication. + """ + result = await super().smtp_AUTH(arg) + # If authentication failed, close the connection immediately + if isinstance(result, AuthResult) and not result.success: + if hasattr(self, 'session') and hasattr(self.session, 'transport') and self.session.transport: + self.session.transport.close() + return result + class EnhancedCombinedAuthenticator: """ Enhanced combined authenticator with sender validation support. @@ -353,7 +373,9 @@ class EnhancedCustomSMTPHandler: return '250 OK' class TLSController(Controller): - """Custom controller with TLS support - modeled after the working original.""" + """ + Custom controller for direct TLS (SMTPS, port 465) support. + """ def __init__(self, handler, ssl_context, hostname='localhost', port=40587): logger.debug(f"TLSController __init__: ssl_context={ssl_context is not None}") @@ -365,10 +387,11 @@ class TLSController(Controller): logger.debug(f"TLSController factory: ssl_context={self._ssl_context is not None}") logger.debug(f"TLSController factory: ssl_context object={self._ssl_context}") logger.debug(f"TLSController factory: hostname={self.smtp_hostname}") + # This is direct TLS (SMTPS, port 465 style) smtp_instance = CustomSMTP( self.handler, tls_context=self._ssl_context, - require_starttls=False, # Don't require STARTTLS immediately, but make it available + require_starttls=False, # Direct TLS: do not advertise or require STARTTLS auth_require_tls=True, # If auth is used, require TLS authenticator=self.handler.combined_authenticator, decode_data=True, @@ -378,17 +401,18 @@ class TLSController(Controller): return smtp_instance class PlainController(Controller): - """Controller for plain SMTP with username/password and IP-based authentication.""" + """Controller for plain SMTP with authentication and IP whitelist fallback.""" def __init__(self, handler, hostname='localhost', port=4025): self.smtp_hostname = hostname # Store for HELO identification super().__init__(handler, hostname='0.0.0.0', port=port) # Bind to all interfaces def factory(self): + # Pass authenticator and set auth_require_tls=False to enable AUTH on plain port return CustomSMTP( self.handler, authenticator=self.handler.combined_authenticator, - auth_require_tls=False, # Allow AUTH over plain text (not recommended for production) + auth_require_tls=False, # Allow AUTH on plain port decode_data=True, hostname=self.smtp_hostname # Use proper hostname for HELO ) diff --git a/migrations/README b/migrations/README deleted file mode 100644 index 0e04844..0000000 --- a/migrations/README +++ /dev/null @@ -1 +0,0 @@ -Single-database configuration for Flask. diff --git a/migrations/alembic.ini b/migrations/alembic.ini deleted file mode 100644 index ec9d45c..0000000 --- a/migrations/alembic.ini +++ /dev/null @@ -1,50 +0,0 @@ -# A generic, single database configuration. - -[alembic] -# template used to generate migration files -# file_template = %%(rev)s_%%(slug)s - -# set to 'true' to run the environment during -# the 'revision' command, regardless of autogenerate -# revision_environment = false - - -# Logging configuration -[loggers] -keys = root,sqlalchemy,alembic,flask_migrate - -[handlers] -keys = console - -[formatters] -keys = generic - -[logger_root] -level = WARN -handlers = console -qualname = - -[logger_sqlalchemy] -level = WARN -handlers = -qualname = sqlalchemy.engine - -[logger_alembic] -level = INFO -handlers = -qualname = alembic - -[logger_flask_migrate] -level = INFO -handlers = -qualname = flask_migrate - -[handler_console] -class = StreamHandler -args = (sys.stderr,) -level = NOTSET -formatter = generic - -[formatter_generic] -format = %(levelname)-5.5s [%(name)s] %(message)s -datefmt = %H:%M:%S diff --git a/migrations/env.py b/migrations/env.py deleted file mode 100644 index 4c97092..0000000 --- a/migrations/env.py +++ /dev/null @@ -1,113 +0,0 @@ -import logging -from logging.config import fileConfig - -from flask import current_app - -from alembic import context - -# this is the Alembic Config object, which provides -# access to the values within the .ini file in use. -config = context.config - -# Interpret the config file for Python logging. -# This line sets up loggers basically. -fileConfig(config.config_file_name) -logger = logging.getLogger('alembic.env') - - -def get_engine(): - try: - # this works with Flask-SQLAlchemy<3 and Alchemical - return current_app.extensions['migrate'].db.get_engine() - except (TypeError, AttributeError): - # this works with Flask-SQLAlchemy>=3 - return current_app.extensions['migrate'].db.engine - - -def get_engine_url(): - try: - return get_engine().url.render_as_string(hide_password=False).replace( - '%', '%%') - except AttributeError: - return str(get_engine().url).replace('%', '%%') - - -# add your model's MetaData object here -# for 'autogenerate' support -# from myapp import mymodel -# target_metadata = mymodel.Base.metadata -config.set_main_option('sqlalchemy.url', get_engine_url()) -target_db = current_app.extensions['migrate'].db - -# other values from the config, defined by the needs of env.py, -# can be acquired: -# my_important_option = config.get_main_option("my_important_option") -# ... etc. - - -def get_metadata(): - if hasattr(target_db, 'metadatas'): - return target_db.metadatas[None] - return target_db.metadata - - -def run_migrations_offline(): - """Run migrations in 'offline' mode. - - This configures the context with just a URL - and not an Engine, though an Engine is acceptable - here as well. By skipping the Engine creation - we don't even need a DBAPI to be available. - - Calls to context.execute() here emit the given string to the - script output. - - """ - url = config.get_main_option("sqlalchemy.url") - context.configure( - url=url, target_metadata=get_metadata(), literal_binds=True - ) - - with context.begin_transaction(): - context.run_migrations() - - -def run_migrations_online(): - """Run migrations in 'online' mode. - - In this scenario we need to create an Engine - and associate a connection with the context. - - """ - - # this callback is used to prevent an auto-migration from being generated - # when there are no changes to the schema - # reference: http://alembic.zzzcomputing.com/en/latest/cookbook.html - def process_revision_directives(context, revision, directives): - if getattr(config.cmd_opts, 'autogenerate', False): - script = directives[0] - if script.upgrade_ops.is_empty(): - directives[:] = [] - logger.info('No changes in schema detected.') - - conf_args = current_app.extensions['migrate'].configure_args - if conf_args.get("process_revision_directives") is None: - conf_args["process_revision_directives"] = process_revision_directives - - connectable = get_engine() - - with connectable.connect() as connection: - context.configure( - connection=connection, - target_metadata=get_metadata(), - **conf_args - ) - - with context.begin_transaction(): - context.run_migrations() - - -if context.is_offline_mode(): - run_migrations_offline() -else: - run_migrations_online() diff --git a/migrations/script.py.mako b/migrations/script.py.mako deleted file mode 100644 index 2c01563..0000000 --- a/migrations/script.py.mako +++ /dev/null @@ -1,24 +0,0 @@ -"""${message} - -Revision ID: ${up_revision} -Revises: ${down_revision | comma,n} -Create Date: ${create_date} - -""" -from alembic import op -import sqlalchemy as sa -${imports if imports else ""} - -# revision identifiers, used by Alembic. -revision = ${repr(up_revision)} -down_revision = ${repr(down_revision)} -branch_labels = ${repr(branch_labels)} -depends_on = ${repr(depends_on)} - - -def upgrade(): - ${upgrades if upgrades else "pass"} - - -def downgrade(): - ${downgrades if downgrades else "pass"} diff --git a/migrations/versions/3ce273a1be20_initial_migration.py b/migrations/versions/3ce273a1be20_initial_migration.py deleted file mode 100644 index 6ef2a27..0000000 --- a/migrations/versions/3ce273a1be20_initial_migration.py +++ /dev/null @@ -1,108 +0,0 @@ -"""Initial migration - -Revision ID: 3ce273a1be20 -Revises: -Create Date: 2025-06-07 15:25:35.603295 - -""" -from alembic import op -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = '3ce273a1be20' -down_revision = None -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_table('esrv_dkim_keys') - op.drop_table('esrv_auth_logs') - op.drop_table('esrv_users') - op.drop_table('esrv_domains') - op.drop_table('esrv_whitelisted_ips') - op.drop_table('esrv_email_logs') - op.drop_table('esrv_custom_headers') - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_table('esrv_custom_headers', - sa.Column('id', sa.INTEGER(), nullable=False), - sa.Column('domain_id', sa.INTEGER(), nullable=False), - sa.Column('header_name', sa.VARCHAR(), nullable=False), - sa.Column('header_value', sa.VARCHAR(), nullable=False), - sa.Column('is_active', sa.BOOLEAN(), nullable=True), - sa.Column('created_at', sa.DATETIME(), nullable=True), - sa.PrimaryKeyConstraint('id') - ) - op.create_table('esrv_email_logs', - sa.Column('id', sa.INTEGER(), nullable=False), - sa.Column('message_id', sa.VARCHAR(), nullable=False), - sa.Column('timestamp', sa.DATETIME(), nullable=False), - sa.Column('peer', sa.VARCHAR(), nullable=False), - sa.Column('mail_from', sa.VARCHAR(), nullable=False), - sa.Column('rcpt_tos', sa.VARCHAR(), nullable=False), - sa.Column('content', sa.TEXT(), nullable=False), - sa.Column('status', sa.VARCHAR(), nullable=False), - sa.Column('dkim_signed', sa.BOOLEAN(), nullable=True), - sa.Column('from_address', sa.VARCHAR(), server_default=sa.text("'unknown'"), nullable=False), - sa.Column('to_address', sa.VARCHAR(), server_default=sa.text("'unknown'"), nullable=False), - sa.Column('subject', sa.TEXT(), nullable=True), - sa.Column('message', sa.TEXT(), nullable=True), - sa.Column('created_at', sa.DATETIME(), nullable=True), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('message_id') - ) - op.create_table('esrv_whitelisted_ips', - sa.Column('id', sa.INTEGER(), nullable=False), - sa.Column('ip_address', sa.VARCHAR(), nullable=False), - sa.Column('domain_id', sa.INTEGER(), nullable=False), - sa.Column('is_active', sa.BOOLEAN(), nullable=True), - sa.Column('created_at', sa.DATETIME(), nullable=True), - sa.PrimaryKeyConstraint('id') - ) - op.create_table('esrv_domains', - sa.Column('id', sa.INTEGER(), nullable=False), - sa.Column('domain_name', sa.VARCHAR(), nullable=False), - sa.Column('is_active', sa.BOOLEAN(), nullable=True), - sa.Column('created_at', sa.DATETIME(), nullable=True), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('domain_name') - ) - op.create_table('esrv_users', - sa.Column('id', sa.INTEGER(), nullable=False), - sa.Column('email', sa.VARCHAR(), nullable=False), - sa.Column('password_hash', sa.VARCHAR(), nullable=False), - sa.Column('domain_id', sa.INTEGER(), nullable=False), - sa.Column('can_send_as_domain', sa.BOOLEAN(), nullable=True), - sa.Column('is_active', sa.BOOLEAN(), nullable=True), - sa.Column('created_at', sa.DATETIME(), nullable=True), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('email') - ) - op.create_table('esrv_auth_logs', - sa.Column('id', sa.INTEGER(), nullable=False), - sa.Column('auth_type', sa.VARCHAR(), nullable=False), - sa.Column('identifier', sa.VARCHAR(), nullable=False), - sa.Column('ip_address', sa.VARCHAR(), nullable=True), - sa.Column('success', sa.BOOLEAN(), nullable=False), - sa.Column('message', sa.TEXT(), nullable=True), - sa.Column('created_at', sa.DATETIME(), nullable=True), - sa.PrimaryKeyConstraint('id') - ) - op.create_table('esrv_dkim_keys', - sa.Column('id', sa.INTEGER(), nullable=False), - sa.Column('domain_id', sa.INTEGER(), nullable=False), - sa.Column('selector', sa.VARCHAR(), nullable=False), - sa.Column('private_key', sa.TEXT(), nullable=False), - sa.Column('public_key', sa.TEXT(), nullable=False), - sa.Column('is_active', sa.BOOLEAN(), nullable=True), - sa.Column('created_at', sa.DATETIME(), nullable=True), - sa.Column('replaced_at', sa.DATETIME(), nullable=True), - sa.PrimaryKeyConstraint('id') - ) - # ### end Alembic commands ### diff --git a/tests/authentication_order_fix.md b/tests/authentication_order_fix.md new file mode 100644 index 0000000..eeabfb4 --- /dev/null +++ b/tests/authentication_order_fix.md @@ -0,0 +1,69 @@ +# SMTP Server Authentication Order and Best Practices + +## Summary of Fixes (June 2025) + +This document describes the authentication logic and order for the SMTP server, as well as the recent fixes applied to ensure correct sender authentication and IP whitelisting behavior. + +### What Was Fixed +- **Authentication Response:** + - The server now immediately responds with an SMTP error (e.g., `535 Authentication failed`) if the username or password is incorrect, instead of hanging the session. This is achieved by returning `AuthResult(success=False, handled=False, message='535 Authentication failed')` from the authenticator, allowing the aiosmtpd framework to send the error to the client. +- **No Forced Connection Close:** + - The server does not forcibly close the connection after failed authentication, but lets the SMTP client decide whether to retry or quit, as per SMTP protocol best practices. +- **AUTH on Both Ports:** + - Both the plain SMTP port (`smtp_port`) and the secure TLS port (`smtp_tls_port`) now advertise and allow authentication (AUTH LOGIN/PLAIN). IP whitelist fallback is also available on both ports. + +## Authentication Order and Logic + +1. **Connection Handling** + - If a client connects to the plain SMTP port, both AUTH and IP whitelisting are available. + - If a client connects to the TLS SMTP port, the connection is immediately secured with TLS. Both AUTH and IP whitelisting are available. + +2. **Sender Authentication (Username/Password)** + - When a client issues the AUTH command (LOGIN or PLAIN) on either port: + - The server checks the username and password against the database. + - If valid, the session is marked as authenticated and the sender can send as their own address or, if permitted, as any address in their domain. + - If invalid, the server responds with `535 Authentication failed` and does not hang the session. + + **Code Snippet for Immediate Authentication Failure Response:** + ```python + # In email_server/auth.py + def __call__(self, server, session, envelope, mechanism, auth_data): + # ...existing code... + if not isinstance(auth_data, LoginPassword): + logger.warning(f'Invalid auth data format: {type(auth_data)}') + return AuthResult(success=False, handled=False, message='535 Authentication failed') + # ...existing code... + try: + sender = get_sender_by_email(username) + if sender and check_password(password, sender.password_hash): + # ...success logic... + return AuthResult(success=True, handled=True) + else: + # ...failure logging... + return AuthResult(success=False, handled=False, message='535 Authentication failed') + except Exception as e: + # ...error logging... + return AuthResult(success=False, handled=False, message='451 Internal server error') + ``` + - Returning `handled=False` ensures the SMTP client is immediately informed of the failure and does not hang. + +3. **IP Whitelisting (Secondary/Fallback)** + - If no AUTH is provided, the server checks if the client's IP is whitelisted for the target domain. + - If the IP is whitelisted, the session is authorized to send for that domain. + - If not, the server rejects the mail transaction. + +## Best Practices for Future Development + +- **Always return `handled=False` in `AuthResult` for failed authentication** to ensure the SMTP client receives an error and the session does not hang. +- **Advertise AUTH on both the plain SMTP and TLS ports**; allow both user authentication and IP whitelist fallback. +- **Do not use or advertise STARTTLS** on any port if only direct TLS is desired. +- **Log all authentication attempts** (success and failure) for auditing and troubleshooting. +- **Keep authentication and IP whitelisting logic modular** for easy updates and security reviews. + +## Example Client Setup +- For user authentication, connect to either the plain SMTP port (e.g., 25 or 4025) or the TLS port (e.g., 40587) and use the correct username and password. +- For IP whitelisting, connect from an authorized IP to either port; no authentication is required, but the sender must be allowed for the domain. + +--- + +**This document should be updated if the authentication logic or port usage changes in the future.** diff --git a/tests/bash_send_email.sh b/tests/bash_send_email.sh index 105ed0e..9a5b5cb 100644 --- a/tests/bash_send_email.sh +++ b/tests/bash_send_email.sh @@ -1,69 +1,110 @@ #!/bin/bash -sender="test@example.com" receiver="info@example.com" -password="testpass123" +EMAIL_SERVER="localhost" #"pymta.example.com" "localhost" +EMAIL_SERVER_auth="10.100.111.1" # IP for authenticated server ( not localhost), use your main interface ip + +sender="test@example.com" +username="test@example.com" +password="ZjDvcjPSs-nwK2Ghj5vQY7L4LdmTpmn_AEZMokJTFS" # password you setup for the user! domain="example.com" body_content_file="@tests/email_body.txt" SMTP_PORT=4025 -SMTP_TLS_PORT=40587 -cc_recipient="targetcc@example.com" -bcc_recipient="targetbcc@example.com" +SMTP_TLS_PORT=40465 +cc_recipient="ccrecipient@example.com" +bcc_recipient="bccrecipient@example.com" <