Default canonicalization ("c" parameter) must be "simple/simple", not "relaxed/relaxed"

Bug #1839299 reported by Cyril N.
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
dkimpy
Fix Released
High
Scott Kitterman

Bug Description

Hi!

I just spent an awful lot of time trying to figure out why Dkimpy was refusing messages that, when tested on other services (like https://www.appmaildev.com/site/testfile/dkim) were accepted as valid.

After hours of searching, I discovered that by default, dkim py set the canonicalization policy to "relaxed/relaxed" (on the DomainSigner.verify_sig function) instead of "simple/simple" that is considered the default by the RFC:

https://www.ietf.org/rfc/rfc4871.txt

> c= Message canonicalization (plain-text; OPTIONAL, default is "simple/simple").

This is why some messages are refused by dkimpy where they shouldn't.

Revision history for this message
Cyril N. (cnicodeme) wrote :

Here's a quick fix in the meantime:

```
def patch_dkim():
    from dkim import get_txt, parse_tag_value, InvalidTagValueList, MessageFormatError, validate_signature_fields, re
    def new_verify(self,idx=0,dnsfunc=get_txt):
        sigheaders = [(x,y) for x,y in self.headers if x.lower() == b"dkim-signature"]
        if len(sigheaders) <= idx:
            return False

        # By default, we validate the first DKIM-Signature line found.
        try:
            sig = parse_tag_value(sigheaders[idx][1])
            self.signature_fields = sig
        except InvalidTagValueList as e:
            raise MessageFormatError(e)

        self.logger.debug("sig: %r" % sig)

        validate_signature_fields(sig)
        self.domain = sig[b'd']
        self.selector = sig[b's']

        include_headers = [x.lower() for x in re.split(br"\s*:\s*", sig[b'h'])]
        self.include_headers = tuple(include_headers)

        if b'c' not in sig:
            sig[b'c'] = b'simple/simple'

        return self.verify_sig(sig, include_headers, sigheaders[idx], dnsfunc)

    dkim.DKIM.verify = new_verify

patch_dkim()
```

Changed in dkimpy:
status: New → In Progress
importance: Undecided → High
assignee: nobody → Scott Kitterman (kitterman)
milestone: none → 0.9.3
status: In Progress → Fix Committed
Revision history for this message
Scott Kitterman (kitterman) wrote :

2019-08-09 Version 0.9.3
    - Fix linesep setting in arcsign script (LP: #1838262) (Thanks to Gowtham
      Gopalakrishnan for the report and the patch)
    - Fix default canonicalization for DKIM signature verification to be
      simple/simple per RFC 6376 (LP: #1839299) (Thanks to Cyril Nicodème for
      the report and a suggested fix)

Changed in dkimpy:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.