Page MenuHomeSoftware Heritage
Differential D7499

inbound_email: add support for signed email addresses
ClosedPublic
Actions

Authored by olasd on Apr 4 2022, 5:05 PM.

Details

Reviewers
ardumont
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T3999: [Add forge now] Design a feedback tracking solution
Commits
rDWAPPS841919a3c8a6: inbound_email: add support for signed email addresses
Summary

These utilities allow us to generate addresses of the form
<localpart>+<integer>.<signature>@<domain>, where the integer is the
primary key of a given object for which we want to track email
exchanges. The signature prevents the addresses from being forged, that
is, the addresses have to be explicitly generated and displayed by the
web app to be discovered.

The counterpart function retrieves all relevant email addresses from the
list of recipients of an email message, and validates the signatures to
recover the integer values that won't have been tampered with.

These new utilities are expected to be used in the views and signal
handlers pertaining to processing of inbound email messages.

Related to T3999

Depends on D7498

Test Plan

targeted tests have been added

Diff Detail

Repository
rDWAPPS Web applications
Branch
inbound-email-signed-addresses
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 28119
Build 44042: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 44041: arc lint + arc unit

Event Timeline

olasd created this revision.Apr 4 2022, 5:05 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 4 2022, 5:05 PM
swh-public-ci added a comment.Apr 4 2022, 5:29 PM

Build has FAILED

Patch application report for D7499 (id=27204)

Could not rebase; Attempt merge onto 902039b683...

Merge made by the 'recursive' strategy.
 requirements-test.txt                     |   2 +-
 swh/web/inbound_email/utils.py            | 119 +++++++++++++++++++++++++--
 swh/web/tests/inbound_email/test_utils.py | 130 ++++++++++++++++++++++++++++++
 3 files changed, 242 insertions(+), 9 deletions(-)
Changes applied before test
commit ed005afb1d1b0fed9430514843bfd6318c440f60
Merge: 902039b6 83cd2ce9
Author: Jenkins user <jenkins@localhost>
Date:   Mon Apr 4 15:06:46 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 83cd2ce985bf4e209799c21717a0f333a808381e
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Mar 31 16:01:56 2022 +0200

    inbound_email: add support for signed email addresses
    
    These utilities allow us to generate addresses of the form
    `<localpart>+<integer>.<signature>@<domain>`, where the integer is the
    primary key of a given object for which we want to track email
    exchanges. The signature prevents the addresses from being forged, that
    is, the addresses have to be explicitly generated and displayed by the
    web app to be discovered.
    
    The counterpart function retrieves all relevant email addresses from the
    list of recipients of an email message, and validates the signatures to
    recover the integer values that won't have been tampered with.
    
    These new utilities are expected to be used in the views and signal
    handlers pertaining to processing of inbound email messages.

commit 104d8a87f09c8ed892d8dcf7af2ec08870e0b80e
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Apr 1 15:16:11 2022 +0200

    inbound_email: split recipient matching logic out
    
    This allows calling the function on a single recipient rather than on a
    whole message, when one isn't available.

commit 30ea00efd7e0f2a34f7f70b1c6984fc743b82405
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Apr 1 15:15:05 2022 +0200

    Restrict pytest-postgresql to < 4.0.0
    
    Other modules still need psycopg2 and pytest-postgresql 4 introduced a
    hard dependency on psycopg3.
    
    This restriction has only been needed since a recent dependency
    upgrade (or maybe a pip upgrade); pip has stopped being able to solve it
    itself for some reason.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1668/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1668/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 4 2022, 5:29 PM
Harbormaster failed remote builds in B28107: Diff 27204!
olasd updated this revision to Diff 27217.Apr 5 2022, 1:16 PM

rebase

olasd added a child revision: D7503: inbound_email: add function to extract the plaintext from a mail.Apr 5 2022, 1:17 PM
swh-public-ci added a comment.Apr 5 2022, 1:43 PM

Build was aborted

Patch application report for D7499 (id=27217)

Could not rebase; Attempt merge onto 902039b683...

Updating 902039b6..9a5e6bc4
Fast-forward
 requirements-test.txt                     |   2 +-
 swh/web/inbound_email/utils.py            | 119 +++++++++++++++++++++++++--
 swh/web/tests/inbound_email/test_utils.py | 130 ++++++++++++++++++++++++++++++
 3 files changed, 242 insertions(+), 9 deletions(-)
Changes applied before test
commit 9a5e6bc41eb1255648883085981befaa08cbee90
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Mar 31 16:01:56 2022 +0200

    inbound_email: add support for signed email addresses
    
    These utilities allow us to generate addresses of the form
    `<localpart>+<integer>.<signature>@<domain>`, where the integer is the
    primary key of a given object for which we want to track email
    exchanges. The signature prevents the addresses from being forged, that
    is, the addresses have to be explicitly generated and displayed by the
    web app to be discovered.
    
    The counterpart function retrieves all relevant email addresses from the
    list of recipients of an email message, and validates the signatures to
    recover the integer values that won't have been tampered with.
    
    These new utilities are expected to be used in the views and signal
    handlers pertaining to processing of inbound email messages.

commit a594dd506f581afddf1ca483ba49afe3e6695daf
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Apr 1 15:16:11 2022 +0200

    inbound_email: split recipient matching logic out
    
    This allows calling the function on a single recipient rather than on a
    whole message, when one isn't available.

commit e46b75a9c5afe5ae58b67d1bf4a5c67e8363c1a0
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Apr 1 15:15:05 2022 +0200

    Restrict pytest-postgresql to < 4.0.0
    
    Other modules still need psycopg2 and pytest-postgresql 4 introduced a
    hard dependency on psycopg3.
    
    This restriction has only been needed since a recent dependency
    upgrade (or maybe a pip upgrade); pip has stopped being able to solve it
    itself for some reason.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1672/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1672/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 5 2022, 1:43 PM
Harbormaster failed remote builds in B28119: Diff 27217!
swh-public-ci added a comment.Apr 5 2022, 2:04 PM

Build is green

Patch application report for D7499 (id=27217)

Could not rebase; Attempt merge onto 902039b683...

Updating 902039b6..9a5e6bc4
Fast-forward
 requirements-test.txt                     |   2 +-
 swh/web/inbound_email/utils.py            | 119 +++++++++++++++++++++++++--
 swh/web/tests/inbound_email/test_utils.py | 130 ++++++++++++++++++++++++++++++
 3 files changed, 242 insertions(+), 9 deletions(-)
Changes applied before test
commit 9a5e6bc41eb1255648883085981befaa08cbee90
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Mar 31 16:01:56 2022 +0200

    inbound_email: add support for signed email addresses
    
    These utilities allow us to generate addresses of the form
    `<localpart>+<integer>.<signature>@<domain>`, where the integer is the
    primary key of a given object for which we want to track email
    exchanges. The signature prevents the addresses from being forged, that
    is, the addresses have to be explicitly generated and displayed by the
    web app to be discovered.
    
    The counterpart function retrieves all relevant email addresses from the
    list of recipients of an email message, and validates the signatures to
    recover the integer values that won't have been tampered with.
    
    These new utilities are expected to be used in the views and signal
    handlers pertaining to processing of inbound email messages.

commit a594dd506f581afddf1ca483ba49afe3e6695daf
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Apr 1 15:16:11 2022 +0200

    inbound_email: split recipient matching logic out
    
    This allows calling the function on a single recipient rather than on a
    whole message, when one isn't available.

commit e46b75a9c5afe5ae58b67d1bf4a5c67e8363c1a0
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Apr 1 15:15:05 2022 +0200

    Restrict pytest-postgresql to < 4.0.0
    
    Other modules still need psycopg2 and pytest-postgresql 4 introduced a
    hard dependency on psycopg3.
    
    This restriction has only been needed since a recent dependency
    upgrade (or maybe a pip upgrade); pip has stopped being able to solve it
    itself for some reason.

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1675/ for more details.

Harbormaster completed remote builds in B28119: Diff 27217.Apr 5 2022, 2:04 PM
olasd requested review of this revision.Apr 5 2022, 2:04 PM
ardumont accepted this revision.Apr 5 2022, 2:26 PM
ardumont added a subscriber: ardumont.

lgtm, one (non-blocking signature) suggestion inlne.

swh/web/inbound_email/utils.py
93
This revision is now accepted and ready to land.Apr 5 2022, 2:26 PM
olasd updated this revision to Diff 27231.Apr 5 2022, 4:21 PM

rebase and apply @ardumont's comment

swh-public-ci added a comment.Apr 5 2022, 4:49 PM

Build was aborted

Patch application report for D7499 (id=27231)

Rebasing onto 10f8ba1467...

Current branch diff-target is up to date.
Changes applied before test
commit 620d14c60e2c3152bacf9ccaa1d23edbc4a5f98b
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Mar 31 16:01:56 2022 +0200

    inbound_email: add support for signed email addresses
    
    These utilities allow us to generate addresses of the form
    `<localpart>+<integer>.<signature>@<domain>`, where the integer is the
    primary key of a given object for which we want to track email
    exchanges. The signature prevents the addresses from being forged, that
    is, the addresses have to be explicitly generated and displayed by the
    web app to be discovered.
    
    The counterpart function retrieves all relevant email addresses from the
    list of recipients of an email message, and validates the signatures to
    recover the integer values that won't have been tampered with.
    
    These new utilities are expected to be used in the views and signal
    handlers pertaining to processing of inbound email messages.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1682/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1682/console

Harbormaster failed remote builds in B28134: Diff 27231!Apr 5 2022, 4:49 PM
vlorentz accepted this revision.Apr 6 2022, 10:51 AM
vlorentz added a subscriber: vlorentz.

The signature scheme looks good, nice.

It makes me a little uncomfortable that the validator needs access to the same secret key that is used for generating signatures; because the validator runs in a frontend app, and a leak of that secret key allows someone to spoof any address.

But I guess the alternative (having the secret key in a different backend service used only for generating new ids) is overkill :/

swh/web/tests/inbound_email/test_utils.py
241

forgot this print

olasd added a comment.Apr 6 2022, 5:17 PM
In D7499#196252, @vlorentz wrote:

It makes me a little uncomfortable that the validator needs access to the same secret key that is used for generating signatures; because the validator runs in a frontend app, and a leak of that secret key allows someone to spoof any address.

A leak of the Django secret key will compromise much more than this trivial email processing.

Now, your comment makes me wonder if we shouldn't just store the generated extension in the database, and use that for matching inbound emails, rather than validating the signature and extracting primary keys from that. This would keep already published addresses valid even after doing a secret key rotation. And we could have an action to invalidate an existing extension if it ends up used for spam.

I guess we can file that under "stuff that we should improve in the future".

olasd updated this revision to Diff 27264.Apr 6 2022, 6:02 PM
  • Rebase
  • Remove spurious print
olasd marked an inline comment as done.Apr 6 2022, 6:04 PM
olasd marked an inline comment as done.
swh-public-ci added a comment.Apr 6 2022, 6:25 PM

Build is green

Patch application report for D7499 (id=27264)

Rebasing onto 120076eea4...

Current branch diff-target is up to date.
Changes applied before test
commit 841919a3c8a6a74b063d8c2b97f458ba6318e71a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Mar 31 16:01:56 2022 +0200

    inbound_email: add support for signed email addresses
    
    These utilities allow us to generate addresses of the form
    `<localpart>+<integer>.<signature>@<domain>`, where the integer is the
    primary key of a given object for which we want to track email
    exchanges. The signature prevents the addresses from being forged, that
    is, the addresses have to be explicitly generated and displayed by the
    web app to be discovered.
    
    The counterpart function retrieves all relevant email addresses from the
    list of recipients of an email message, and validates the signatures to
    recover the integer values that won't have been tampered with.
    
    These new utilities are expected to be used in the views and signal
    handlers pertaining to processing of inbound email messages.

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1688/ for more details.

Harbormaster completed remote builds in B28192: Diff 27264.Apr 6 2022, 6:25 PM
Closed by commit rDWAPPS841919a3c8a6: inbound_email: add support for signed email addresses (authored by olasd). · Explain WhyApr 6 2022, 6:37 PM
This revision was automatically updated to reflect the committed changes.
olasd added a commit: rDWAPPS841919a3c8a6: inbound_email: add support for signed email addresses.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
web/
inbound_email/
88 lines98%
Loading...
tests/
inbound_email/
111 lines100%
Loading...

Diff 27217

View Options

swh/web/inbound_email/utils.py

Loading...
View Options

swh/web/tests/inbound_email/test_utils.py

Loading...
About · Developer information · Legal