diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c466c5..6594af4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Unreleased +- Fix: with `Mail.queue(message, encrypt: true)`, a send error could name the recipient (eg. an SMTP reply `550 : mailbox unavailable`). The worker wrote that error to its log and to the failed jobs in plain text. Now the encrypted mail job throws a `RedactedMailError`. Its text has only the error type and the SMTP or HTTP code. The full error text, with the answer of the server, is encrypted with the app key (at most the first 64K characters, counted in UTF-16 code units). Read it with `RedactedMailError.decrypt(job.error)`. When the error can not be encrypted, only the type and the code are kept. Mail jobs without `encrypt` keep the full error, as before. - New: a built-in job for `Mail.queue()` and `Mail.later()` of dox_mailer. `Queue.configure()` turns it on. Apps do not add it to `lib/generated/jobs.g.dart`: every `JobRegistry` finds the built-in jobs (`JobRegistry.builtIn`) after its own jobs. With `encrypt: true`, the mail is saved like a `ShouldBeEncrypted` job. - Now depends on dox_mailer. - Deploy the queue worker together with the web app. An old worker does not know the new mail job, so it fails the queued emails. diff --git a/README.md b/README.md index 8d6dcaa..71feab2 100644 --- a/README.md +++ b/README.md @@ -277,6 +277,9 @@ dox_queue has a built-in job for it. It is not in `lib/generated/jobs.g.dart`, a Deploy the worker together with the web app. An old worker does not know this job, so it fails the queued emails. +With `encrypt: true`, a send error is logged and saved as its type and code only, eg. `RedactedMailError: MailException (code 550). Details are encrypted: ...`. +The full error text, with the answer of the server, is encrypted with the app key (at most the first 64K characters, counted in UTF-16 code units). To read it, take a `FailedJob` from `Queue.driver.failed()` and call `RedactedMailError.decrypt(job.error)`. + ## Run the worker ```bash diff --git a/lib/dox_queue.dart b/lib/dox_queue.dart index 13e416b..9201ca6 100644 --- a/lib/dox_queue.dart +++ b/lib/dox_queue.dart @@ -14,4 +14,5 @@ export 'src/queue_driver.dart'; export 'src/queue_provider.dart'; export 'src/queue_tables.dart'; export 'src/queue_worker.dart'; +export 'src/send_queued_mail.dart' show RedactedMailError; export 'src/worker.dart'; diff --git a/lib/src/send_queued_mail.dart b/lib/src/send_queued_mail.dart index 31294cd..2a855ee 100644 --- a/lib/src/send_queued_mail.dart +++ b/lib/src/send_queued_mail.dart @@ -1,4 +1,6 @@ -import 'package:dox_mailer/dox_mailer.dart' show Mail, MailQueue, Message; +import 'package:dox_core/dox_core.dart' show AESEncryptor, Dox; +import 'package:dox_mailer/dox_mailer.dart' + show Mail, MailException, MailQueue, Message; import 'job.dart'; import 'job_registry.dart'; @@ -62,6 +64,19 @@ class SendEncryptedQueuedMail extends SendQueuedMail with ShouldBeEncrypted { _decode, ); + /// Sends the mail. When the send fails, it throws a [RedactedMailError] + /// in place of the error. An SMTP reply or a [MailException] can name + /// an address, and the worker writes the error to its log and to the + /// failed jobs in plain text. + @override + Future handle() async { + try { + await super.handle(); + } catch (error, stack) { + Error.throwWithStackTrace(RedactedMailError.of(error), stack); + } + } + static DoxJob _decode(Map data) { return SendEncryptedQueuedMail( mailer: data['mailer'] as String, @@ -72,6 +87,107 @@ class SendEncryptedQueuedMail extends SendQueuedMail with ShouldBeEncrypted { } } +/// The error of a mail of `Mail.queue(message, encrypt: true)` that +/// could not be sent. The worker logs and saves it in place of the +/// first error, so no address or text of the mail is written in plain +/// text. +/// +/// Its text has only the type of the first error and the SMTP or HTTP +/// code. The full text of the first error (and the answer of the server, +/// `MailException.body`) is encrypted with the app key, like the job +/// data. At most the first 64K characters (UTF-16 code units) are +/// encrypted, so a huge error does not fill the log. Read it with +/// [decrypt]. +/// ``` +/// RedactedMailError: MailException (code 550). Details are encrypted: +/// ``` +class RedactedMailError implements Exception { + RedactedMailError(this.type, {this.statusCode, this.details}); + + /// [error] with its text encrypted. When that fails, only the type + /// (and the code) is kept, never the text. Everything is read from + /// [error] inside the `try`, so an error that throws can not leak. + factory RedactedMailError.of(Object error) { + String type = 'unknown'; + int? code; + String? details; + try { + type = '${error.runtimeType}'; + code = error is MailException ? error.statusCode : null; + String text = '$error'; + if (error is MailException && (error.body ?? '').isNotEmpty) { + text = '$text\nbody: ${error.body}'; + } + details = Queue.encryptData(_cut(text), _purpose); + } catch (_) { + details = null; + } + return RedactedMailError(type, statusCode: code, details: details); + } + + /// the most characters (UTF-16 code units) of the error that are + /// encrypted + static const int maxDetails = 64 * 1024; + + /// the first [maxDetails] characters of [text]. It does not cut an + /// emoji (a surrogate pair) in two. + static String _cut(String text) { + if (text.length <= maxDetails) return text; + int end = maxDetails; + int last = text.codeUnitAt(end - 1); + if (last >= 0xD800 && last <= 0xDBFF) end--; + return text.substring(0, end); + } + + /// bound to the encrypted details, so job data can not be read as them + static const String _purpose = 'dox_queue:RedactedMailError'; + + static const String _marker = 'Details are encrypted: '; + + /// the class of the first error, eg. `MailException` + final String type; + + /// the SMTP reply code or the HTTP status, when there is one + final int? statusCode; + + /// the encrypted text of the first error, or null when it could not + /// be encrypted + final String? details; + + /// The text of the first error, from a saved or logged [error] text + /// (eg. the `error` of a failed job). Needs the same app key, or one + /// of `AppConfig.previousKeys`. Null when it has no details. Throws a + /// `DecryptException` when the key is wrong, or the details are not + /// real encrypted details. + /// ``` + /// for (FailedJob job in await Queue.driver.failed()) { + /// print(RedactedMailError.decrypt(job.error)); + /// } + /// ``` + static String? decrypt(String error) { + int at = error.indexOf(_marker); + if (at < 0) return null; + String rest = error.substring(at + _marker.length); + String details = rest.split(RegExp(r'\s')).first; + return AESEncryptor.decode( + details, + Dox().config.appKey, + associatedData: _purpose, + purpose: Queue.encryptPurpose, + previousSecrets: Dox().config.previousKeys, + ); + } + + @override + String toString() { + String code = statusCode == null ? '' : ' (code $statusCode)'; + String text = details == null + ? 'details could not be encrypted' + : '$_marker$details'; + return 'RedactedMailError: $type$code. $text'; + } +} + /// Puts the mails of `Mail.queue()` on this queue. `Queue.configure()` /// sets it with `Mail.useQueue()`. class QueueMailQueue implements MailQueue { diff --git a/test/queue_mail_test.dart b/test/queue_mail_test.dart index b2f2f8d..9879a73 100644 --- a/test/queue_mail_test.dart +++ b/test/queue_mail_test.dart @@ -1,3 +1,6 @@ +import 'dart:async'; + +import 'package:dox_core/dox_core.dart' show AppConfig, Dox, LoggerConfig; import 'package:dox_mailer/dox_mailer.dart'; import 'package:dox_queue/dox_queue.dart'; import 'package:test/test.dart'; @@ -20,6 +23,49 @@ class BrokenTransport extends Transport { late BrokenTransport brokenMailer; +/// a mail server that says no, and names the recipient and the subject, +/// like many SMTP servers do +class RejectTransport extends Transport { + @override + Future send(Message message) async { + throw MailException( + '550 5.1.1 : mailbox unavailable ' + '(subject "Your invoice", for Ann, forwarded to ann.lee@other.org)', + statusCode: 550, + ); + } +} + +/// a mail server that quotes the whole body in its answer +class QuoteTransport extends Transport { + @override + Future send(Message message) async { + throw MailException( + '552 message refused: ${message.html}', + statusCode: 552, + ); + } +} + +/// every line the worker logs while [body] runs +Future _logOf(Future Function() body) async { + StringBuffer log = StringBuffer(); + void keep(Zone self, ZoneDelegate parent, Zone zone, String line) { + log.writeln(line); + } + + await runZoned(body, zoneSpecification: ZoneSpecification(print: keep)); + return log.toString(); +} + +/// the parts of the test mail that must not be in a log +const List _secrets = [ + 'ann@example.com', + 'ann.lee@other.org', + 'Your invoice', + 'Ann', +]; + Message _message() => Message( to:
[const Address('ann@example.com', 'Ann')], bcc:
[const Address('boss@example.com')], @@ -45,7 +91,17 @@ Future _work([String queue = 'default', int tries = 1]) async { } void main() { - setUpAll(setUpTestApp); + setUpAll(() { + setUpTestApp(); + // log on, so the tests can read what the worker logs + Dox().initialize( + AppConfig( + appKey: 'k' * 32, + totalIsolate: 1, + logger: LoggerConfig(enabled: true), + ), + ); + }); setUp(() { Mail.restore(); @@ -60,6 +116,8 @@ void main() { 'main': mainMailer, 'other': otherMailer, 'broken': brokenMailer, + 'reject': RejectTransport(), + 'quote': QuoteTransport(), }, ), ); @@ -123,6 +181,47 @@ void main() { expect(await Queue.driver.pop('default'), isNull); }); + test('encrypt: true: a send error shows no mail data', () async { + await Mail.mailer('reject').queue(_message(), encrypt: true); + String log = await _logOf(_retryThenFail); + List failed = await Queue.driver.failed(); + String error = failed.single.error; + for (String secret in _secrets) { + expect(log, isNot(contains(secret))); + expect(error, isNot(contains(secret))); + } + // still enough to find the problem + printOnFailure(log); + expect(log, contains('failed try 1 of 2')); + expect(log, contains('RedactedMailError: MailException (code 550).')); + expect(error, contains('RedactedMailError: MailException (code 550).')); + // the app key gives back the full error + expect( + RedactedMailError.decrypt(error), + startsWith('MailException: 550 5.1.1 '), + ); + }); + + test('encrypt: true: a 100 KB body is not in the error', () async { + String html = '

Your reset code is 918273

${'x' * 100 * 1024}'; + Message message = _message().copyWith(html: html); + await Mail.mailer('quote').queue(message, encrypt: true); + String log = await _logOf(_retryThenFail); + String error = (await Queue.driver.failed()).single.error; + for (String text in [log, error]) { + expect(text, isNot(contains('918273'))); + expect(text, isNot(contains('xxxx'))); + expect(text, contains('MailException (code 552). Details are')); + } + }); + + test('a plain mail job keeps the full error, as before', () async { + await Mail.mailer('reject').queue(_message()); + await _logOf(_retryThenFail); + List failed = await Queue.driver.failed(); + expect(failed.single.error, contains('')); + }); + test('encrypt: true saves the mail encrypted', () async { await Mail.queue(_message(), encrypt: true); JobPayload payload = await _work(); @@ -135,3 +234,9 @@ void main() { }); } } + +/// two tries: the first is released, the second fails +Future _retryThenFail() async { + await _work('default', 2); + await _work('default', 2); +} diff --git a/test/redacted_mail_error_test.dart b/test/redacted_mail_error_test.dart new file mode 100644 index 0000000..425b528 --- /dev/null +++ b/test/redacted_mail_error_test.dart @@ -0,0 +1,183 @@ +import 'dart:io'; + +import 'package:dox_core/dox_core.dart' show AppConfig, DecryptException, Dox; +import 'package:dox_mailer/dox_mailer.dart'; +import 'package:dox_queue/dox_queue.dart'; +import 'package:test/test.dart'; + +/// an error whose text can not be made, so it can not be encrypted +class BadTextError extends MailException { + BadTextError() : super('ann@example.com', statusCode: 421); + + @override + String toString() => throw StateError('no text'); +} + +/// an error with a given text +class TextError implements Exception { + TextError(this.text); + + final String text; + + @override + String toString() => text; +} + +/// an error whose type can not be read +class BadTypeError implements Exception { + @override + Type get runtimeType => throw StateError('ann@example.com'); + + @override + String toString() => 'ann@example.com'; +} + +/// the texts that must never be in the plain text of the error +const List _secrets = [ + 'ann@example.com', + 'ANN@EXAMPLE.COM', + 'Ann Lee', + 'Zoë', + '918273', + 'xxxx', + 'LLLL', + 'other.org', +]; + +/// Many kinds of error text that leaked with text redaction: names in +/// curly quotes, texts that overlap, a 1 MB body, a long local part. +List _errors = [ + '550 5.1.1 : mailbox unavailable', + 'rejected for “Ann Lee” and ‘Zoë’', + 'Ann Leeann@example.com overlaps Ann Lee', + '552 refused: Hi Ann, your code is 918273. ${'x' * 1024 * 1024}', + '550 <${'L' * 200}@other.org>: no mailbox', +]; + +/// the plain text part: the error without its encrypted details +String _plain(String error) => error.split('Details are encrypted: ').first; + +void main() { + setUpAll(() { + Dox().initialize(AppConfig(appKey: 'k' * 32, totalIsolate: 1)); + }); + + for (String text in _errors) { + test('no mail data in: ${text.substring(0, 30)}', () { + String error = + '${RedactedMailError.of(MailException(text, statusCode: 550))}'; + for (String secret in _secrets) { + expect(_plain(error), isNot(contains(secret))); + expect(error, isNot(contains(secret))); + } + expect( + error, + startsWith('RedactedMailError: MailException (code 550). '), + ); + }); + } + + test('decrypt gives back the full error text', () { + String text = '550 5.1.1 : mailbox unavailable'; + String error = + '${RedactedMailError.of(MailException(text, statusCode: 550))}'; + // like a failed job: the error, then the stack trace + String saved = '$error\n#0 main (file:///app.dart:1:1)'; + expect(RedactedMailError.decrypt(saved), 'MailException: $text'); + }); + + test('only the first 64K characters of a huge error are encrypted', () { + String text = 'x' * (5 * 1024 * 1024); + RedactedMailError error = RedactedMailError.of(MailException(text)); + expect('$error'.length, lessThan(100 * 1024)); + expect( + RedactedMailError.decrypt('$error'), + hasLength(RedactedMailError.maxDetails), + ); + }); + + test('a wrong key can not read the details', () { + String error = '${RedactedMailError.of(MailException('ann@example.com'))}'; + Dox().initialize(AppConfig(appKey: 'z' * 32, totalIsolate: 1)); + addTearDown(_resetKey); + expect( + () => RedactedMailError.decrypt(error), + throwsA(isA()), + ); + }); + + test('an error with no details gives null', () { + expect(RedactedMailError.decrypt('StateError: boom'), isNull); + }); + + test('when the text can not be made, only type and code stay', () { + RedactedMailError error = RedactedMailError.of(BadTextError()); + expect(error.type, 'BadTextError'); + expect(error.statusCode, 421); + expect(error.details, isNull); + expect( + '$error', + 'RedactedMailError: BadTextError (code 421). ' + 'details could not be encrypted', + ); + }); + + test('a SocketException keeps its type, with no code', () { + RedactedMailError error = RedactedMailError.of( + SocketException( + 'Connection refused', + address: InternetAddress('10.0.0.5'), + port: 587, + ), + ); + expect(error.type, 'SocketException'); + expect(error.statusCode, isNull); + expect('$error', startsWith('RedactedMailError: SocketException. ')); + expect(RedactedMailError.decrypt('$error'), contains('10.0.0.5')); + }); + + test('the cut does not split an emoji', () { + int max = RedactedMailError.maxDetails; + String text = '${'x' * (max - 1)}😀 and more'; + String? back = RedactedMailError.decrypt( + '${RedactedMailError.of(TextError(text))}', + ); + expect(back, 'x' * (max - 1)); + }); + + test('the answer of the server is in the encrypted details', () { + RedactedMailError error = RedactedMailError.of( + MailException( + 'the API said no', + statusCode: 422, + body: '{"to":"ann@example.com"}', + ), + ); + expect('$error', isNot(contains('ann@example.com'))); + expect( + RedactedMailError.decrypt('$error'), + 'MailException: the API said no\nbody: {"to":"ann@example.com"}', + ); + }); + + test('an error whose type throws gives only "unknown"', () { + RedactedMailError error = RedactedMailError.of(BadTypeError()); + expect( + '$error', + 'RedactedMailError: unknown. details could not be encrypted', + ); + }); + + test('decrypt only takes real encrypted details', () { + expect( + () => RedactedMailError.decrypt( + 'RedactedMailError: X. Details are encrypted: {"to":"ann@example.com"}', + ), + throwsA(isA()), + ); + }); +} + +void _resetKey() { + Dox().initialize(AppConfig(appKey: 'k' * 32, totalIsolate: 1)); +}