trusteddomainproject / OpenDKIM

Other
91 stars 50 forks source link

Handle multiple dns TXT keys #123

Open dilyanpalauzov opened 3 years ago

dilyanpalauzov commented 3 years ago
$ dig @dion.ns.cloudflare.com txt mail._domainkey.nra.bg

;; ANSWER SECTION:
mail._domainkey.nra.bg. 300     IN      TXT     "v=DKIM1; k=rsa;\" \"p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAjOgbkiLjexuOVMXdBX3XWgPXSbkBvzdN\" \"GfkEYjixS1dpmFuo6GEY23Td8ISUNgDJrzkjEZ3jma6EWxwgmXmn24ACSdLlLG/fNxt0h0ExEb5kqbihWq8yRN13XcL2ZikqMh0KFJiiBnDbcfqjM3J7XR5\" \"AXBRuJU5E7E1ZI4emfrWbT3uWU/gX5r2jH" "JqQAp301VIZMI8g8Y6Cb5RnKNr+o/5t/ClziaY33K8AKMVbi7d0aipqK7dSdQ/NVvSovurqCTXEWbJXFpeOxRI\" \"1BZ+tA4u3y3YbWHktisBuCKwQrg2FpeuTBktgB3ATyXDDXQlPJ9/gxToKn88J4XAoV5tCywIDAQAB"
mail._domainkey.nra.bg. 300     IN      TXT     "v=DKIM1; k=rsa;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAjOgbkiLjexuOVMXdBX3XWgPXSbkBvzdNGfkEYjixS1dpmFuo6GEY23Td8ISUNgDJrzkjEZ3jma6EWxwgmXmn24ACSdLlLG/fNxt0h0ExEb5kqbihWq8yRN13XcL2ZikqMh0KFJiiBnDbcfqjM3J7XR5AXBRuJU5E7E1ZI4emfrWbT3uWU/gX5r2jHJqQAp301V" "IZMI8g8Y6Cb5RnKNr+o/5t/ClziaY33K8AKMVbi7d0aipqK7dSdQ/NVvSovurqCTXEWbJXFpeOxRI1BZ+tA4u3y3YbWHktisBuCKwQrg2FpeuTBktgB3ATyXDDXQlPJ9/gxToKn88J4XAoV5tCywIDAQAB"

So the query returns two answers and RFC DomainKeys Identified Mail (DKIM) SignaturesSection “Get the Public Key” says

  1. If the query for the public key returns multiple key records, the Verifier can choose one of the key records or may cycle through the key records, performing the remainder of these steps on each record at the discretion of the implementer. The order of the key records is unspecified. If the Verifier chooses to cycle through the key records, then the "return ..." wording in the remainder of this section means "try the next key record, if any; if none, return to try another signature in the usual way".

libopendkim/dkim-keys.c:dkim_get_key_dns() around line 300 contains code, that triggers an error if multiple DNS TXT records are found.

thegushi commented 1 year ago

I'm not sure what the behavior is supposed to be when we are able to retrieve a key at a given selector and it fails to validate, and if we should continue trying again.

I think there's no space in the DKIM spec for multiple distinct keys at the same selector dns record -- in your example, you've simply formatted them differently but they're the same key. So either they all validate or none of them do.

I feel like the best logic we could apply here and be compliant with the spec is: choose the first one (whichever DNS hands us) and use that -- but even that would cause debugging issues.

Let me bring this up internally, and see what's warranted.

dilyanpalauzov commented 1 year ago

Currently dig @dion.ns.cloudflare.com txt mail._domainkey.nra.bg returns one key. Such configurations are in the wild - not created by me.

thegushi commented 1 year ago

Right, so currently something different is returned from when this issue was first opened.

Tacking this on for refernce.

$ dig @dion.ns.cloudflare.com txt mail._domainkey.nra.bg

; <<>> DiG 9.18.9 <<>> @dion.ns.cloudflare.com txt mail._domainkey.nra.bg
; (6 servers found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 4434
;; flags: qr aa rd; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1
;; WARNING: recursion requested but not available

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 1232
;; QUESTION SECTION:
;mail._domainkey.nra.bg.                IN      TXT

;; ANSWER SECTION:
mail._domainkey.nra.bg. 300     IN      TXT     "v=DKIM1; k=rsa;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAjOgbkiLjexuOVMXdBX3XWgPXSbkBvzdNGfkEYjixS1dpmFuo6GEY23Td8ISUNgDJrzkjEZ3jma6EWxwgmXmn24ACSdLlLG/fNxt0h0ExEb5kqbihWq8yRN13XcL2ZikqMh0KFJiiBnDbcfqjM3J7XR5AXBRuJU5E7E1ZI4emfrWbT3uWU/gX5r2jHJqQAp301V" "IZMI8g8Y6Cb5RnKNr+o/5t/ClziaY33K8AKMVbi7d0aipqK7dSdQ/NVvSovurqCTXEWbJXFpeOxRI1BZ+tA4u3y3YbWHktisBuCKwQrg2FpeuTBktgB3ATyXDDXQlPJ9/gxToKn88J4XAoV5tCywIDAQAB"

;; Query time: 3 msec
;; SERVER: 2606:4700:58::adf5:3b9c#53(dion.ns.cloudflare.com) (UDP)
;; WHEN: Sat Jan 07 21:01:55 UTC 2023
;; MSG SIZE  rcvd: 474
thegushi commented 1 year ago

Our behavior is going to be that we use the first key, and emit a warning when ancount > 1.