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

Support parsing Office 365 Authentication-Results #490

Closed
The-Nutty opened this issue Jun 21, 2019 · 7 comments
Closed

Support parsing Office 365 Authentication-Results #490

The-Nutty opened this issue Jun 21, 2019 · 7 comments

Comments

@The-Nutty
Copy link

Is your feature request related to a problem? Please describe.
Some email providers (notably Office 365) generate a, Authentication-Results without a authserv-id at the beginning of the authentication-results header.

Example (redacted) Authentication-Results: spf=fail (sender IP is 1.1.1.1) smtp.mailfrom=eu-west-1.amazonses.com; recevingdomain.com; dkim=pass (signature was verified) header.d=domain.com;domain1.com; dmarc=bestguesspass action=none header.from=domain.com;

Currently AuthenticationResults.Parse throws

MimeKit.ParseException: Invalid instance token at offset 0
  at MimeKit.Cryptography.AuthenticationResults.TryParse(Byte[] text, Int32& index, Int32 endIndex, Boolean throwOnError, AuthenticationResults& authres)
  at MimeKit.Cryptography.AuthenticationResults.Parse(Byte[] buffer)

Describe the solution you'd like
AuthenticationResults.Parse should handle Office365 Authentication-Result values without throwing.

@jstedfast
Copy link
Owner

Ugh, this is worse than you thought.

Not only is it missing the authserv-id token (which I have worked around locally), but it also has stuff like smtp.mailfrom=eu-west-1.amazonses.com; recevingdomain.com; and header.d=domain.com;domain1.com;

I have no idea how to even start trying to parse that... What are the second domains?

This format doesn't seem to follow the spec at all :(

@The-Nutty
Copy link
Author

Yeah i understand what you mean, they are a bit of a mess. Here are 3 more examples, let me know if i can help at all.

So i have another example as well (email that is internal to organisation)

recevingdomain.com; dkim=none (message not signed)
header.d=none;recevingdomain.com; dmarc=none action=none header.from=recevingdomain.com;

Another from gmail -> recevingdomain.com

spf=softfail (sender IP is 1.1.1.1)
smtp.mailfrom=gmail.com; recevingdomain.com; dkim=pass (signature was verified)
header.d=domain.com;recevingdomain.com; dmarc=fail
action=none header.from=gmail.com;

So domain.com/domain1.com is the domain(s) the email is dkimed with, so in that last case domain.com is a mail gateway (hence why spf fails).

An example not using a mail gateway hubspot.com -> 123.onmicrosoft.com:

spf=pass (sender IP is 1.1.1.1)
 smtp.mailfrom=notifybf1.hubspot.com; 123.onmicrosoft.com;
 dkim=pass (signature was verified)
 header.d=notifybf1.hubspot.com;123.onmicrosoft.com; dmarc=pass
 action=none header.from=hubspot.com;compauth=pass reason=100

@jstedfast
Copy link
Owner

These examples also introduce action=... which is not in the spec :-\

And the last one also has the reason= after the property specs :-\

@jstedfast
Copy link
Owner

Oh, wait, no it doesn't (reason= after properties). Nevermind... I misread that.

I added a sort of hack that handles the action=, but have no idea if you can have an action and a reason and if you do have both, which one has to come first, etc.

@jstedfast
Copy link
Owner

I think the above commit fixes all of the issues you've seen. If you can test out the myget nuget packages, let me know how it goes (might take 30 minutes or so for the fix to roll out).

@The-Nutty
Copy link
Author

Just tested the latest myget package, works fine with everything office 365 throws at it.

Thanks

@jstedfast
Copy link
Owner

Awesome 😎

jstedfast added a commit that referenced this issue Dec 29, 2019
Fixes issue #490 and issue #527 in a better way by parsing the
method-specific authserv-id tokens into a new property called
Office365AuthenticationServiceIdentifier located on the
AuthenticationMethodResult class.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants