From patchwork Thu Oct 10 06:20:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Donnellan X-Patchwork-Id: 1174326 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46pgvl2x3fz9sPW for ; Thu, 10 Oct 2019 17:21:23 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 46pgvl1rBKzDqvW for ; Thu, 10 Oct 2019 17:21:23 +1100 (AEDT) X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=linux.ibm.com (client-ip=148.163.158.5; helo=mx0a-001b2d01.pphosted.com; envelope-from=ajd@linux.ibm.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 46pgvb112kzDqtt for ; Thu, 10 Oct 2019 17:21:14 +1100 (AEDT) Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x9A6D1go121063 for ; Thu, 10 Oct 2019 02:21:07 -0400 Received: from e06smtp04.uk.ibm.com (e06smtp04.uk.ibm.com [195.75.94.100]) by mx0a-001b2d01.pphosted.com with ESMTP id 2vhy69g681-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 10 Oct 2019 02:21:07 -0400 Received: from localhost by e06smtp04.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 10 Oct 2019 07:21:05 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (9.149.109.194) by e06smtp04.uk.ibm.com (192.168.101.134) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Thu, 10 Oct 2019 07:21:03 +0100 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id x9A6L24539911624 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 10 Oct 2019 06:21:02 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8F3A8AE04D; Thu, 10 Oct 2019 06:21:02 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 05600AE057; Thu, 10 Oct 2019 06:21:02 +0000 (GMT) Received: from ozlabs.au.ibm.com (unknown [9.192.253.14]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Thu, 10 Oct 2019 06:21:01 +0000 (GMT) Received: from intelligence.ozlabs.ibm.com (haven.au.ibm.com [9.192.254.114]) (using TLSv1.2 with cipher DHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.au.ibm.com (Postfix) with ESMTPSA id BB42AA0131; Thu, 10 Oct 2019 17:20:58 +1100 (AEDT) From: Andrew Donnellan To: patchwork@lists.ozlabs.org Subject: [PATCH] parser: Unmangle From: headers that have been mangled for DMARC purposes Date: Thu, 10 Oct 2019 17:20:47 +1100 X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 X-TM-AS-GCONF: 00 x-cbid: 19101006-0016-0000-0000-000002B6BE36 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 19101006-0017-0000-0000-00003317C844 Message-Id: <20191010062047.21549-1-ajd@linux.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-10-10_03:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=1 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1908290000 definitions=main-1910100058 X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stephen Rothwell , Alexandre Belloni Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" To avoid triggering spam filters due to failed signature validation, many mailing lists mangle the From header to change the From address to be the address of the list, typically where the sender's domain has a strict DMARC policy enabled. In this case, we should try to unmangle the From header. Add support for using the X-Original-Sender or Reply-To headers, as used by Google Groups and Mailman respectively, to unmangle the From header when necessary. Closes: #64 ("Incorrect submitter when using googlegroups") Reported-by: Alexandre Belloni Reported-by: Stephen Rothwell Signed-off-by: Andrew Donnellan --- patchwork/parser.py | 75 ++++++++++++++++++++++++++++++---- patchwork/tests/test_parser.py | 68 ++++++++++++++++++++++++++++-- 2 files changed, 130 insertions(+), 13 deletions(-) diff --git a/patchwork/parser.py b/patchwork/parser.py index 7dc66bc05a5b..79beac5617b1 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -321,12 +321,7 @@ def find_series(project, mail, author): return _find_series_by_markers(project, mail, author) -def get_or_create_author(mail): - from_header = clean_header(mail.get('From')) - - if not from_header: - raise ValueError("Invalid 'From' header") - +def split_from_header(from_header): name, email = (None, None) # tuple of (regex, fn) @@ -355,6 +350,65 @@ def get_or_create_author(mail): (name, email) = fn(match.groups()) break + return (name, email) + + +# Unmangle From addresses that have been mangled for DMARC purposes. +# +# To avoid triggering spam filters due to failed signature validation, many +# mailing lists mangle the From header to change the From address to be the +# address of the list, typically where the sender's domain has a strict +# DMARC policy enabled. +# +# Unfortunately, there's no standardised way of preserving the original +# From address. +# +# Google Groups adds an X-Original-Sender header. If present, we use that. +# +# Mailman preserves the original address by adding a Reply-To, except in the +# case where the list is set to either reply to list, or reply to a specific +# address, in which case the original From is added to Cc instead. These corner +# cases are dumb, but we try and handle things as sensibly as possible by +# looking for a name in Reply-To/Cc that matches From. It's inexact but should +# be good enough for our purposes. +def get_original_sender(mail, name, email): + if name and ' via ' in name: + # Mailman uses the format " via " + # Google Groups uses "'' via " + stripped_name = name[:name.rfind(' via ')].strip().strip("'") + + original_sender = clean_header(mail.get('X-Original-Sender', '')) + if original_sender: + new_email = split_from_header(original_sender)[1].strip()[:255] + return (stripped_name, new_email) + + addrs = [] + reply_to_headers = mail.get_all('Reply-To') or [] + cc_headers = mail.get_all('Cc') or [] + for header in reply_to_headers + cc_headers: + header = clean_header(header) + addrs = header.split(",") + for addr in addrs: + new_name, new_email = split_from_header(addr) + if new_name: + new_name = new_name.strip()[:255] + if new_email: + new_email = new_email.strip()[:255] + if new_name == stripped_name: + return (stripped_name, new_email) + + # If we can't figure out the original sender, just keep it as is + return (name, email) + + +def get_or_create_author(mail, project=None): + from_header = clean_header(mail.get('From')) + + if not from_header: + raise ValueError("Invalid 'From' header") + + name, email = split_from_header(from_header) + if not email: raise ValueError("Invalid 'From' header") @@ -362,6 +416,9 @@ def get_or_create_author(mail): if name is not None: name = name.strip()[:255] + if project and email.lower() == project.listemail.lower(): + name, email = get_original_sender(mail, name, email) + # this correctly handles the case where we lose the race to create # the person and another process beats us to it. (If the record # does not exist, g_o_c invokes _create_object_from_params which @@ -1004,7 +1061,7 @@ def parse_mail(mail, list_id=None): if not is_comment and (diff or pull_url): # patches or pull requests # we delay the saving until we know we have a patch. - author = get_or_create_author(mail) + author = get_or_create_author(mail, project) delegate = find_delegate_by_header(mail) if not delegate and diff: @@ -1099,7 +1156,7 @@ def parse_mail(mail, list_id=None): is_cover_letter = True if is_cover_letter: - author = get_or_create_author(mail) + author = get_or_create_author(mail, project) # we don't use 'find_series' here as a cover letter will # always be the first item in a thread, thus the references @@ -1159,7 +1216,7 @@ def parse_mail(mail, list_id=None): if not submission: return - author = get_or_create_author(mail) + author = get_or_create_author(mail, project) try: comment = Comment.objects.create( diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index e5a2fca3044e..85c6e7550f93 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -265,11 +265,23 @@ class SenderCorrelationTest(TestCase): """ @staticmethod - def _create_email(from_header): + def _create_email(from_header, reply_tos=None, ccs=None, + x_original_sender=None): mail = 'Message-Id: %s\n' % make_msgid() + \ - 'From: %s\n' % from_header + \ - 'Subject: Tests\n\n'\ - 'test\n' + 'From: %s\n' % from_header + + if reply_tos: + mail += 'Reply-To: %s\n' % ', '.join(reply_tos) + + if ccs: + mail += 'Cc: %s\n' % ', '.join(ccs) + + if x_original_sender: + mail += 'X-Original-Sender: %s\n' % x_original_sender + + mail += 'Subject: Tests\n\n'\ + 'test\n' + return message_from_string(mail) def test_existing_sender(self): @@ -311,6 +323,54 @@ class SenderCorrelationTest(TestCase): self.assertEqual(person_b._state.adding, False) self.assertEqual(person_b.id, person_a.id) + def test_mailman_dmarc_munging(self): + project = create_project() + real_sender = 'Existing Sender ' + munged_sender = 'Existing Sender via List <{}>'.format( + project.listemail) + other_email = 'Other Person ' + + # Unmunged author + mail = self._create_email(real_sender) + person_a = get_or_create_author(mail, project) + person_a.save() + + # Single Reply-To + mail = self._create_email(munged_sender, [real_sender]) + person_b = get_or_create_author(mail, project) + self.assertEqual(person_b._state.adding, False) + self.assertEqual(person_b.id, person_a.id) + + # Single Cc + mail = self._create_email(munged_sender, [], [real_sender]) + person_b = get_or_create_author(mail, project) + self.assertEqual(person_b._state.adding, False) + self.assertEqual(person_b.id, person_a.id) + + # Multiple Reply-Tos and Ccs + mail = self._create_email(munged_sender, [other_email, real_sender], + [other_email, other_email]) + person_b = get_or_create_author(mail, project) + self.assertEqual(person_b._state.adding, False) + self.assertEqual(person_b.id, person_a.id) + + def test_google_dmarc_munging(self): + project = create_project() + real_sender = 'Existing Sender ' + munged_sender = "'Existing Sender' via List <{}>".format( + project.listemail) + + # Unmunged author + mail = self._create_email(real_sender) + person_a = get_or_create_author(mail, project) + person_a.save() + + # X-Original-Sender header + mail = self._create_email(munged_sender, None, None, real_sender) + person_b = get_or_create_author(mail, project) + self.assertEqual(person_b._state.adding, False) + self.assertEqual(person_b.id, person_a.id) + class SeriesCorrelationTest(TestCase): """Validate correct behavior of find_series."""