verify() function doesn't differentiate between invalid and not present

Asked by David

I wish to use dkimpy to verify the authenticity of some emails, however not all of them have a DKIM check present in the header. I want to be able to differentiate between an invalid DKIM signature and no signature present.

I've looked at the source code for the verify() function (The one on line 848 of __init__.py) and it seems the first return is only triggered when no DKIM signature is present.

I've modified this function locally for my own purposes and it works great, but I was wondering if a "dkim.present()" feature could be added to the official dkimpy module so it becomes standard. I imagine it would have similar parameters to the verify() function, but it would just return "len([(x,y) for x,y in self.headers if x.lower() == b"dkim-signature"]) == 0" instead.

Question information

Language:
English Edit question
Status:
Solved
For:
dkimpy Edit question
Assignee:
No assignee Edit question
Solved by:
Scott Kitterman
Solved:
Last query:
Last reply:
Revision history for this message
Scott Kitterman (kitterman) said :
#1

If you can provide a patch, I'll consider it, but I'm concerned that if it's too complex it might cause more maintenance overhead than it's worth.

Scott K

Revision history for this message
David (protofall) said :
#2

I've never made a patch before and I am rather new to Python so I'm not sure how that works, but this is what the new function would look like

  #: Checks if any DKIM signature is present
  #: @return: True if there is one or more DKIM signatures present or False otherwise
  def present(self):
    return (len([(x,y)] for x,y in self.headers if x.lower() == b"dkim-signature"]) > 0)

This function is a part of the dkim class (I put it just above the verify function. For reference here is how I call it:

message = *INSERT MESSAGE HERE*
d = dkim.DKIM(message)
present = d.present()
verify = d.verify()

I did have issues when trying to do "present = dkim.present(message)" so it seems I'm missing something (Claims module 'dkim' has no attribute 'present')

Revision history for this message
Scott Kitterman (kitterman) said :
#3

OK. I see what you're after and I don't think it would be a maintenance burden. I am somewhat reluctant because it seems to invite behavior that's contrary to a SHOULD NOT requirement from the DKIM RFC (Section 6.1):

   Survivability of signatures after transit is not guaranteed, and
   signatures can fail to verify through no fault of the Signer.
   Therefore, a Verifier SHOULD NOT treat a message that has one or more
   bad signatures and no good signatures differently from a message with
   no signature at all

https://tools.ietf.org/html/rfc6376

What's the use case?

Scott K

Revision history for this message
David (protofall) said :
#4

I'm making an email filter that sits at the user's firewall and decides, based on user-defined settings and different checks, which emails get received and which ones get destroyed. The end goal is attempting to remove as many malicious emails as possible while keeping as many legitimate emails as possible. There are multiple layers to this program to check validity and one of them is the dkim checker. Some of the settings can change the dkim checker behaviour, for example we can make it so only verify() is run (The no signatures case will be treated the same as invalid signatures) or we can run present() and treat them differently. These options are controlled by the user. Also note we will be applying weightings, just because an email has an invalid dkim signature doesn't mean we will instantly reject it. We consider all the layers before deciding.

I understand the reasoning for that SHOULD NOT requirement with packets dropping/corrupting and such, but we specifically need to have the option to differentiate between the two cases.

Revision history for this message
Best Scott Kitterman (kitterman) said :
#5

I've created https://bugs.launchpad.net/dkimpy/+bug/1851141 to track this feature request.

Revision history for this message
David (protofall) said :
#6

Thanks Scott Kitterman, that solved my question.