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

Bug #1839299 reported by Cyril N. on 2019-08-07
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
dkimpy
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.

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
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  Edit
Everyone can see this information.

Other bug subscribers