Malformed XML-Report leads to traceback as bounce mail

[tschuettler]

We received a dmarc report containing:

  <policy_published>
    <domain>dmarc.example.com</domain>
    <adkim>s</adkim>
    <aspf>r</aspf>
    <p>none</p>
    <sp></sp>
    <pct>100</pct>
  </policy_published>

This is arguably a case of not following the RFC since sp is not allowed to be empy|null, but I would propose that the dmarc parser should not send a full traceback to the reporter.

This resulted in follwing excerpt of a bounce mail:

dmarc-reports@localhost
Remote Server returned '554 5.3.0 < #5.3.0 x-unix;
Importing report cb72fe$47da515=0ce221328f17c275@example.com received from example.com

Traceback (most recent call last):
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/backends/utils.py", line84,
  in _execute return self.cursor.execute(sql, params) psycopg2.errors.NotNullViolation: null value in column "policy_sp" violates not-null constraint
  DETAIL: Failing row contains (829, cb72fe$47da515=0ce221328f17c275@example.com, 2020-07-29 22:00:03+00, 2020-07-30 22:00:04+00, dmarc.example.com, s, r, none, null, 100, 17). The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/srv/modoboa/instance/manage.py", line21,
  in <module> main()
File "/srv/modoboa/instance/manage.py", line17,
  in main execute_from_command_line(sys.argv)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/__init__.py", line 381,
  in execute_from_command_line utility.execute()
File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/__init__.py", line 375,
  in execute self.fetch_command(subcommand).run_from_argv(self.argv)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/base.py", line 323,
  in  run_from_argv self.execute(*args, **cmd_options)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/base.py", line 364,
  in  execute output = self.handle(*args, **options)
File "/srv/modoboa/env/lib/python3.6/site-packages/modoboa_dmarc/management/commands/import_aggregated_report.py", line 36,
  in  handle lib.import_report_from_stdin()
File "/srv/modoboa/env/lib/python3.6/site-packages/modoboa_dmarc/lib.py", line 177,
  in  import_report_from_stdin import_report_from_email(content)
File "/srv/modoboa/env/lib/python3.6/site-packages/modoboa_dmarc/lib.py", line 155,
  in  import_report_from_email import_archive(fpo, content_type=part.get_content_type())
File "/srv/modoboa/env/lib/python3.6/site-packages/modoboa_dmarc/lib.py", line 136,
  in  import_archive import_report(zfile.read())
File "/usr/lib/python3.6/contextlib.py", line 52,
  in  inner return func(*args, **kwds)
File "/srv/modoboa/env/lib/python3.6/site-packages/modoboa_dmarc/lib.py", line 118,
  in  import_report report.save()
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/base.py", line 741,
  in  save force_update=force_update, update_fields=update_fields)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/base.py", line 779,
  in  save_base force_update, using, update_fields,
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/base.py", line 870,
  in  _save_table result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/base.py", line 908,
  in  _do_insert using=using, raw=raw)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/manager.py", line 82,
  in  manager_method return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/query.py", line 1186,
  in  _insert return query.get_compiler(using=using).execute_sql(return_id)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1375,
  in  execute_sql cursor.execute(sql, params)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/backends/utils.py", line 67,
  in  execute return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/backends/utils.py", line 76,
  in  _execute_with_wrappers return executor(sql, params, many, context)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/backends/utils.py", line 84,
  in  _execute return self.cursor.execute(sql, params)
File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/utils.py", line 89,
  in  __exit__ raise dj_exc_value.with_traceback(traceback) from exc_value
File "/srv/modoboa/env/lib/python>'

Actually browsing through modoboa_dmarc_report shows quite a few cases where the value of sp is an empty string. Most of them by not listing a sp attribute in the xml. But actually even for this offending reporter with the same xml-contents in the report. I assume that the older modoboa-dmarc under Python 2 did handle an empty xml-attribute differently than the current one under Python 3 does.

In https://github.com/modoboa/modoboa-dmarc/issues/32 there was also a case of sending the full traceback. Perhaps we could try to catch the exceptions more broadly to prevent any traceback from going back to the reporter?

[tonioo]

@tschuettler Indeed, it could be improved.

[tschuettler]

@tonioo I just stumbled upon the fix from https://github.com/modoboa/modoboa-dmarc/commit/9a4bd9268778d30a118cda8a231b41eaa02112d7. It does indeed stop sending backtraces 😄.

However, it also stops processing reports without sp attribute altogether. (Which, to my understanding, are not valid according to the RFC, but we have reports from "Yahoo", "Yahoo! Inc.", "seznam.cz a.s." and "Verizon Media" that are missing a value for that attribute. That even appears as an issue in the unit tests: https://github.com/modoboa/modoboa-dmarc/runs/6308262398?check_suite_focus=true#step:9:19 and https://github.com/modoboa/modoboa-dmarc/runs/6308262398?check_suite_focus=true#step:9:23

Perhaps we could use a fallback like dmarcian does and continue processing the report?