Compare commits

..

7 Commits

Author SHA1 Message Date
link2xt
d407c2ad6a Add NOTIFY capability
Delta Chat does not use it now,
but should: <https://github.com/deltachat/deltachat-core-rust/issues/4983>
Having no capability will confuse whoever develops it.
2023-11-12 04:30:52 +00:00
link2xt
9e14a741c3 Autoformat tests with black 2023-11-08 20:29:44 +00:00
link2xt
01fcb9ae0e Fix None dereference in benchmarks 2023-11-08 20:29:21 +00:00
link2xt
064f6d36ad Fix path in scripts/bench.sh 2023-11-08 20:23:14 +00:00
holger krekel
6b3590e7c8 test: test concurrent user creation 2023-11-08 19:36:38 +00:00
link2xt
251aac18fb fix(dictproxy): check that user exists and create it in a transaction
Otherwise user may be already created by another connection
as checking if the user exists happens
in a different read-only transaction.
This happens when Delta Chat connects IMAP and SMTP at the same time.

Also update last_login time on login.
2023-11-08 19:34:17 +00:00
link2xt
f46bf2f670 Remove authentication logs from dictproxy
They log the passwords and make it difficult to spot actual exceptions.
2023-11-07 21:04:33 +01:00
9 changed files with 134 additions and 45 deletions

View File

@@ -33,13 +33,6 @@ class Connection:
def cursor(self): def cursor(self):
return self._sqlconn.cursor() return self._sqlconn.cursor()
def create_user(self, addr: str, password: str):
"""Create a row in the users table."""
self.execute("PRAGMA foreign_keys=on")
q = """INSERT INTO users (addr, password, last_login)
VALUES (?, ?, ?)"""
self.execute(q, (addr, password, int(time.time())))
def get_user(self, addr: str) -> {}: def get_user(self, addr: str) -> {}:
"""Get a row from the users table.""" """Get a row from the users table."""
q = "SELECT addr, password, last_login from users WHERE addr = ?" q = "SELECT addr, password, last_login from users WHERE addr = ?"

View File

@@ -1,5 +1,6 @@
import logging import logging
import os import os
import time
import sys import sys
import json import json
import crypt import crypt
@@ -46,17 +47,6 @@ def is_allowed_to_create(user, cleartext_password) -> bool:
return True return True
def create_user(db, user, encrypted_password):
with db.write_transaction() as conn:
conn.create_user(user, encrypted_password)
return dict(
home=f"/home/vmail/{user}",
uid="vmail",
gid="vmail",
password=encrypted_password,
)
def get_user_data(db, user): def get_user_data(db, user):
with db.read_connection() as conn: with db.read_connection() as conn:
result = conn.get_user(user) result = conn.get_user(user)
@@ -71,14 +61,30 @@ def lookup_userdb(db, user):
def lookup_passdb(db, user, cleartext_password): def lookup_passdb(db, user, cleartext_password):
userdata = get_user_data(db, user) with db.write_transaction() as conn:
if not userdata: userdata = conn.get_user(user)
if userdata:
# Update last login time.
conn.execute(
"UPDATE users SET last_login=? WHERE addr=?", (int(time.time()), user)
)
userdata["uid"] = "vmail"
userdata["gid"] = "vmail"
return userdata
if not is_allowed_to_create(user, cleartext_password): if not is_allowed_to_create(user, cleartext_password):
return return
encrypted_password = encrypt_password(cleartext_password) encrypted_password = encrypt_password(cleartext_password)
userdata = create_user(db=db, user=user, encrypted_password=encrypted_password) q = """INSERT INTO users (addr, password, last_login)
userdata["password"] = userdata["password"].strip() VALUES (?, ?, ?)"""
return userdata conn.execute(q, (user, encrypted_password, int(time.time())))
return dict(
home=f"/home/vmail/{user}",
uid="vmail",
gid="vmail",
password=encrypted_password,
)
def handle_dovecot_request(msg, db, mail_domain): def handle_dovecot_request(msg, db, mail_domain):

View File

@@ -19,7 +19,7 @@ mail_plugins = quota
# these are the capabilities Delta Chat cares about actually # these are the capabilities Delta Chat cares about actually
# so let's keep the network overhead per login small # so let's keep the network overhead per login small
# https://github.com/deltachat/deltachat-core-rust/blob/master/src/imap/capabilities.rs # https://github.com/deltachat/deltachat-core-rust/blob/master/src/imap/capabilities.rs
imap_capability = IMAP4rev1 IDLE MOVE QUOTA CONDSTORE imap_capability = IMAP4rev1 IDLE MOVE QUOTA CONDSTORE NOTIFY
# Authentication for system users. # Authentication for system users.

View File

@@ -1,4 +1,4 @@
#!/bin/bash #!/bin/bash
set -e set -e
venv/bin/pytest online-tests/benchmark.py -vrx venv/bin/pytest tests/online/benchmark.py -vrx

View File

@@ -1,4 +1,4 @@
#!/bin/bash #!/bin/bash
venv/bin/tox -c chatmaild venv/bin/tox -c chatmaild
venv/bin/tox -c deploy-chatmail venv/bin/tox -c deploy-chatmail
venv/bin/pytest tests/online -vrx --durations=5 $@ venv/bin/pytest tests/online -rs -vrx --durations=5 $@

View File

@@ -1,21 +1,15 @@
import os
import json import json
import pytest import pytest
import threading
import queue
import traceback
import chatmaild.dictproxy import chatmaild.dictproxy
from chatmaild.dictproxy import get_user_data, lookup_passdb, handle_dovecot_request from chatmaild.dictproxy import get_user_data, lookup_passdb, handle_dovecot_request
from chatmaild.database import Database, DBError from chatmaild.database import Database, DBError
@pytest.fixture()
def db(tmpdir):
db_path = tmpdir / "passdb.sqlite"
print("database path:", db_path)
return Database(db_path)
def test_basic(db): def test_basic(db):
lookup_passdb(db, "link2xt@c1.testrun.org", "Pieg9aeToe3eghuthe5u") lookup_passdb(db, "link2xt@c1.testrun.org", "Pieg9aeToe3eghuthe5u")
data = get_user_data(db, "link2xt@c1.testrun.org") data = get_user_data(db, "link2xt@c1.testrun.org")
@@ -53,8 +47,10 @@ def test_too_high_db_version(db):
def test_handle_dovecot_request(db): def test_handle_dovecot_request(db):
msg = ('Lshared/passdb/laksjdlaksjdlaksjdlk12j3l1k2j3123/' msg = (
'some42@c3.testrun.org\tsome42@c3.testrun.org') "Lshared/passdb/laksjdlaksjdlaksjdlk12j3l1k2j3123/"
"some42@c3.testrun.org\tsome42@c3.testrun.org"
)
res = handle_dovecot_request(msg, db, "c3.testrun.org") res = handle_dovecot_request(msg, db, "c3.testrun.org")
assert res assert res
assert res[0] == "O" and res.endswith("\n") assert res[0] == "O" and res.endswith("\n")
@@ -62,3 +58,29 @@ def test_handle_dovecot_request(db):
assert userdata["home"] == "/home/vmail/some42@c3.testrun.org" assert userdata["home"] == "/home/vmail/some42@c3.testrun.org"
assert userdata["uid"] == userdata["gid"] == "vmail" assert userdata["uid"] == userdata["gid"] == "vmail"
assert userdata["password"].startswith("{SHA512-CRYPT}") assert userdata["password"].startswith("{SHA512-CRYPT}")
def test_100_concurrent_lookups(db):
num = 100
dbs = [Database(db.path) for i in range(num)]
print(f"created {num} databases")
results = queue.Queue()
def lookup(db):
try:
lookup_passdb(db, "something@c1.testrun.org", "Pieg9aeToe3eghuthe5u")
except Exception:
results.put(traceback.format_exc())
else:
results.put(None)
threads = [threading.Thread(target=lookup, args=(db,), daemon=True) for db in dbs]
print(f"created {num} threads, starting them and waiting for results")
for thread in threads:
thread.start()
for _ in dbs:
res = results.get()
if res is not None:
pytest.fail(f"concurrent lookup failed\n{res}")

View File

@@ -9,9 +9,10 @@ import itertools
from email.parser import BytesParser from email.parser import BytesParser
from email import policy from email import policy
from pathlib import Path from pathlib import Path
from math import ceil
import pytest import pytest
from chatmaild.database import Database
conftestdir = Path(__file__).parent conftestdir = Path(__file__).parent
@@ -71,7 +72,7 @@ def pytest_report_header():
@pytest.fixture @pytest.fixture
def benchmark(request): def benchmark(request):
def bench(func, num, name=None): def bench(func, num, name=None, reportfunc=None):
if name is None: if name is None:
name = func.__name__ name = func.__name__
durations = [] durations = []
@@ -80,7 +81,7 @@ def benchmark(request):
func() func()
durations.append(time.time() - now) durations.append(time.time() - now)
durations.sort() durations.sort()
request.config._benchresults[name] = durations request.config._benchresults[name] = (reportfunc, durations)
return bench return bench
@@ -101,7 +102,9 @@ def pytest_terminal_summary(terminalreporter):
headers = f"{'benchmark name': <30} " + fcol(float_names) headers = f"{'benchmark name': <30} " + fcol(float_names)
tr.write_line(headers) tr.write_line(headers)
tr.write_line("-" * len(headers)) tr.write_line("-" * len(headers))
for name, durations in results.items(): summary_lines = []
for name, (reportfunc, durations) in results.items():
measures = [ measures = [
sorted(durations)[len(durations) // 2], sorted(durations)[len(durations) // 2],
min(durations), min(durations),
@@ -110,6 +113,16 @@ def pytest_terminal_summary(terminalreporter):
line = f"{name: <30} " line = f"{name: <30} "
line += fcol(f"{float: 2.2f}" for float in measures) line += fcol(f"{float: 2.2f}" for float in measures)
tr.write_line(line) tr.write_line(line)
vmedian, vmin, vmax = measures
if reportfunc:
for line in reportfunc(vmin=vmin, vmedian=vmedian, vmax=vmax):
summary_lines.append(line)
if summary_lines:
tr.write_line("")
tr.section("benchmark summary measures")
for line in summary_lines:
tr.write_line(line)
@pytest.fixture @pytest.fixture
@@ -117,6 +130,16 @@ def imap(maildomain):
return ImapConn(maildomain) return ImapConn(maildomain)
@pytest.fixture
def make_imap_connection(maildomain):
def make_imap_connection():
conn = ImapConn(maildomain)
conn.connect()
return conn
return make_imap_connection
class ImapConn: class ImapConn:
AuthError = imaplib.IMAP4.error AuthError = imaplib.IMAP4.error
logcmd = "journalctl -f -u dovecot" logcmd = "journalctl -f -u dovecot"
@@ -157,6 +180,16 @@ def smtp(maildomain):
return SmtpConn(maildomain) return SmtpConn(maildomain)
@pytest.fixture
def make_smtp_connection(maildomain):
def make_smtp_connection():
conn = SmtpConn(maildomain)
conn.connect()
return conn
return make_smtp_connection
class SmtpConn: class SmtpConn:
AuthError = smtplib.SMTPAuthenticationError AuthError = smtplib.SMTPAuthenticationError
logcmd = "journalctl -f -t postfix/smtpd -t postfix/smtp -t postfix/lmtp" logcmd = "journalctl -f -t postfix/smtpd -t postfix/smtp -t postfix/lmtp"
@@ -202,6 +235,13 @@ def gencreds(maildomain):
return lambda domain=None: next(gen(domain)) return lambda domain=None: next(gen(domain))
@pytest.fixture()
def db(tmpdir):
db_path = tmpdir / "passdb.sqlite"
print("database path:", db_path)
return Database(db_path)
# #
# Delta Chat testplugin re-use # Delta Chat testplugin re-use
# use the cmfactory fixture to get chatmail instance accounts # use the cmfactory fixture to get chatmail instance accounts
@@ -272,7 +312,7 @@ class Remote:
self.sshdomain = sshdomain self.sshdomain = sshdomain
def iter_output(self, logcmd=""): def iter_output(self, logcmd=""):
getjournal = f"journalctl -f" if not logcmd else logcmd getjournal = "journalctl -f" if not logcmd else logcmd
self.popen = subprocess.Popen( self.popen = subprocess.Popen(
["ssh", f"root@{self.sshdomain}", getjournal], ["ssh", f"root@{self.sshdomain}", getjournal],
stdout=subprocess.PIPE, stdout=subprocess.PIPE,

View File

@@ -1,5 +1,6 @@
import pytest import pytest
import smtplib import threading
import queue
def test_login_basic_functioning(imap_or_smtp, gencreds, lp): def test_login_basic_functioning(imap_or_smtp, gencreds, lp):
@@ -23,7 +24,7 @@ def test_login_basic_functioning(imap_or_smtp, gencreds, lp):
with pytest.raises(imap_or_smtp.AuthError): with pytest.raises(imap_or_smtp.AuthError):
imap_or_smtp.login(user, password + "wrong") imap_or_smtp.login(user, password + "wrong")
lp.sec(f"creating users with a short password is not allowed") lp.sec("creating users with a short password is not allowed")
user, _password = gencreds() user, _password = gencreds()
with pytest.raises(imap_or_smtp.AuthError): with pytest.raises(imap_or_smtp.AuthError):
imap_or_smtp.login(user, "admin") imap_or_smtp.login(user, "admin")
@@ -40,3 +41,30 @@ def test_login_same_password(imap_or_smtp, gencreds):
imap_or_smtp.login(user1, password1) imap_or_smtp.login(user1, password1)
imap_or_smtp.connect() imap_or_smtp.connect()
imap_or_smtp.login(user2, password1) imap_or_smtp.login(user2, password1)
def test_concurrent_logins_same_account(
make_imap_connection, make_smtp_connection, gencreds
):
"""Test concurrent smtp and imap logins
and check remote server succeeds on each connection.
"""
user1, password1 = gencreds()
login_results = queue.Queue()
def login_smtp_imap(smtp, imap):
try:
imap.login(user1, password1)
except Exception:
login_results.put(False)
else:
login_results.put(True)
conns = [(make_smtp_connection(), make_imap_connection()) for i in range(10)]
for args in conns:
thread = threading.Thread(target=login_smtp_imap, args=args, daemon=True)
thread.start()
for _ in conns:
assert login_results.get()

View File

@@ -91,7 +91,7 @@ class TestEndToEndDeltaChat:
lp.sec("setup encrypted comms between ac1 and ac2 on different instances") lp.sec("setup encrypted comms between ac1 and ac2 on different instances")
qr = ac1.get_setup_contact_qr() qr = ac1.get_setup_contact_qr()
ch = ac2.qr_setup_contact(qr) ac2.qr_setup_contact(qr)
msg = ac2.wait_next_incoming_message() msg = ac2.wait_next_incoming_message()
assert "verified" in msg.text assert "verified" in msg.text