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

opendkim verify header with semicolon not recognized #1232

Open
minami-o opened this issue Oct 25, 2016 · 0 comments
Open

opendkim verify header with semicolon not recognized #1232

minami-o opened this issue Oct 25, 2016 · 0 comments

Comments

@minami-o
Copy link

@minami-o minami-o commented Oct 25, 2016

RainLoop version, browser, OS:
1.10.4.183
Expected behavior and actual behavior:
When opening an email with DKIM "Authentication-Results" header and value dkim=pass, a green check sign should appear next to the sender email address.
When I do so on my server with a standard installation of opendkim, the header is not correctly interpreted by rainloop and I get no check sign
Steps to reproduce the problem:
I am running opendkim 2.9.1-1 with default config.
This version places a "reason" item which can contain a semicolon as shown below, between the dkim & header.* items.
Logs or screenshots:
The DKIM header is the following:

Authentication-Results: myserver; dkim=pass
reason="2048-bit key; unprotected key"
header.d=orig_tld header.i=@orig_tld header.b=V7fXSgIO;
dkim-adsp=pass; dkim-atps=neutral

The issue is that, in file app/libraries/MailSo/Mime/HeaderCollection.php, on line 379, the regular expression filters out semicolons from valid dkim sentences.
As a "reason" header item is present in my header, which contains a semicolon, the regexp fails.
I just released this filter the following way to get it working:

if (\preg_match('/dkim=[^;]+/i', $sHeaderValue, $aMatch) && !empty($aMatch[0]))

became

if (\preg_match('/dkim=.+/i', $sHeaderValue, $aMatch) && !empty($aMatch[0]))

I'm really not sure why there was a filter on semicolons, but it didn't work on my setup, so maybe it should be removed after thorough testing on different setups…
Cheers

RainLoop added a commit that referenced this issue Oct 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.