Add filter verifing DKIM signature #170
Conversation
|
@aidecoe Thanks again for the contribution - will have a look ASAP :-) Do you by any chance have the time to add a small testing harness, so we could run a chain of filters with a specific configuration on an email and check if tags get applied correctly? I'd like to refactor some parts of the config parsing area (which is quite a mess), but am afraid this will break other things we might not catch… |
afew/filters/DKIMValidityFilter.py
Outdated
|
|
||
| def handle_message(self, message): | ||
| if message.get_header(self.header): | ||
| dkim_ok = all(map(verify_dkim, message.get_filenames())) |
There was a problem hiding this comment.
not sure about message.get_filenames(). This could yield multiple file names, and we need to open the file in python, which can easily be done wrongly.
Is it possible to simply pass message.get_message_parts() to dkim.verify?
There was a problem hiding this comment.
This is different. dkim.verify takes full raw e-mail message (as bytes, not as string). get_filenames() may yield multiple messages that's why for sanity all() is used, so we don't have false positive.
There was a problem hiding this comment.
In other words that's the way to do it and there's no issue here.
afew/filters/DKIMValidityFilter.py
Outdated
| # WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN | ||
| # ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF | ||
| # OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. | ||
| # |
There was a problem hiding this comment.
Can you use SPDX, and simply add a single
# SPDX-License-Identifier: ISC
instead of the license text?
I still have a PR lying around fixing this in other places as well :-)
026f57d to
7977d4e
Compare
|
@flokli It would be nice to have some test suites but I don't have too much spare time. Of course I will provide test suites for any other changes I will make in the future if the framework is in place. |
7977d4e to
bef6fff
Compare
|
There are currently conflicts. Could you rebase? In addition, I'd prefer to use If you don't have the time to add tests, could you still add an example of a message with a valid DKIM signature, so we can add this to the test harness later? |
bef6fff to
f03e8c1
Compare
|
@flokli Just let me know when test framework is in place and I will write tests for those filters myself. |
The filter verifies integrity of the message. So if any of those is wrong then that's integrity violation which should be signalled. |
Filter tags e-mail having DKIM header with either `dkim-ok` or `dkim-fail`.
f03e8c1 to
9e32573
Compare
|
Hey @aidecoe |
|
@GuillaumeSeren, I believe it is rebased onto afewmail master, isn't it? |
|
@aidecoe Ah ok I was a bit confused but the changes requested. @flokli Completing the test suite is a long task and we don't have it right now. |
|
👍 🍻 |
Filter tags e-mail having DKIM header with either
dkim-okordkim-fail.