load_pk_from_dns fails when response contains quotes

[Cyril N.]

When loading the data from the DNS TXT entry (xxx._domainkey.domain.tld), if that result contains surounding quotes, the DKIM test fails, even if everything else is correct.

My suggestion is to remove the quotes at the beginning and end of the DNS response when they are quotes:

```
def load_pk_from_dns(name, dnsfunc=get_txt):
  s = dnsfunc(name)
  if not s:
      raise KeyFormatError("missing public key: %s"%name)
  try:
      if type(s) is str:
        s = s.encode('ascii')

      if s[0:1] == b'"':
        s = s[1:]
      if s[-1:] == b'"':
        s = s[0:-1]

      pub = parse_tag_value(s)
  except InvalidTagValueList as e:
      raise KeyFormatError(e)

  # ... next of the function
```

[Scott Kitterman (kitterman)]

Can you give an example of a domain that does this?

On Tuesday, January 22, 2019 09:50:24 AM you wrote:
> Public bug reported:
>
> When loading the data from the DNS TXT entry
> (xxx._domainkey.domain.tld), if that result contains surounding quotes,
> the DKIM test fails, even if everything else is correct.
>
> My suggestion is to remove the quotes at the beginning and end of the
> DNS response when they are quotes:
>
>
> ```
> def load_pk_from_dns(name, dnsfunc=get_txt):
> s = dnsfunc(name)
> if not s:
> raise KeyFormatError("missing public key: %s"%name)
> try:
> if type(s) is str:
> s = s.encode('ascii')
>
> if s[0:1] == b'"':
> s = s[1:]
> if s[-1:] == b'"':
> s = s[0:-1]
>
> pub = parse_tag_value(s)
> except InvalidTagValueList as e:
> raise KeyFormatError(e)
>
> # ... next of the function
> ```
>
> ** Affects: dkimpy
> Importance: Undecided
> Status: New

[Cyril N. (cnicodeme)]

It's this one:

default._domainkey.boomtownig.com

Thanks :)

[Stuart Gathman (stuart-gathman)]

That domain is not a DKIM domain. Yes, IF you removed the *apparently* erroneous quotes, it might be a DKIM domain. But it is not. The quotes may be there on purpose to sniff for bad implementations of DKIM, which dkimpy would be if it tried to pretend this was really a DKIM domain by removing the quotes. Or, the owner of the domain may be so overworked that they forgot to run the most basic smoke test after configuring DKIM. Or maybe they are using some Microsoft package that does everything for them automatically - and wrong.

No matter the reason, your proposed patch would introduce an error, not fix one. The domain does not actually implement DKIM.

[Stuart Gathman (stuart-gathman)]

If it is not too complicated, we could add some code to check for common misconfigurations (like this one), and while the correct result is no DKIM record, there might be some sort of debugging log that could help someone. E.g. some heuristic result like "Did default._domainkey.example.com use spurious quotes?" could be added to a field in the DKIM object, and software like pymilter could check that when there is no DKIM record, and send a DSN. Some mail admins might appreciate it. Others would simply block your domain as "spam" because they don't know a DSN when they see it any more than how to configure DKIM.

[Stuart Gathman (stuart-gathman)]

I recommend this bug be closed as "invalid".

[Scott Kitterman (kitterman)]

I agree.

Postel's Law about being liberal in what you receive only applies to
ambiguity. The DKIM spec isn't ambiguous. Adjusting to account for quirks
like this will only make dkimpy less maintainable (like much of the mail
system that is pile upon pile of workarounds that we are now stuck with).

Scott K

[Cyril N. (cnicodeme)]

Yes I understand, it makes sense.