Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Authentication-Results to transaction #322

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cmadamsgit
Copy link

The authentication results from SPF/DKIM/etc. are part of a single message transaction, not the whole SMTP connection. Without this change, when multiple messages are received during a single SMTP connection, the SPF/DKIM/etc. results just keep getting appended to an Authentication-Results header.

The authentication results from SPF/DKIM/etc. are part of a single
message transaction, not the whole SMTP connection.  Without this
change, when multiple messages are received during a single SMTP
connection, the SPF/DKIM/etc. results just keep getting appended to an
Authentication-Results header.
@msimerson
Copy link
Member

Please refer to RFC 7001. Specifically:

At the time of publication of this document, the following are published, domain-level email authentication methods in common use:

  • Author Domain Signing Practices ([ADSP])
  • SMTP Service Extension for Authentication ([AUTH])
  • DomainKeys Identified Mail Signatures ([DKIM])
  • Sender Policy Framework ([SPF])
  • Vouch By Reference ([VBR])
  • reverse IP address name validation ("iprev", defined in Section 3)

I would point out that at least iprev, SPF helo and AUTH are all connection properties that are set before the first transaction is created. Therefore, a more correct implementation would be to collate the connection and transaction auth results when assembling the header.

@msimerson
Copy link
Member

Having looked into this a little more, I think A Pretty Good Solution looks like:

  1. deprecate store_auth_results in Plugin.pm. Squawk loudly if it gets called.
  2. add store_auth_results to Connection.pm and Transaction.pm
  3. Update authentication_results in SMTP to collate the connection and transaction results into an AR header
  4. Update plugins to call store_auth_results on the connection or transaction, as is appropriate

@cmadamsgit
Copy link
Author

Yeah, I think something like that is needed. I probably am not the person to do all that to be honest though... my particular use case happens to only be transaction-oriented things (so that's why my original patch is the way it is and works for me), and I don't think I'd have a good way to actually test a comprehensive patch. It's the work of going through all the plugins to understand them and make sure they're doing the right thing...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants