Fixed a bug in retrieving the DKIM public key

[jayden-sudo]

Description

For some emails, the DKIM public key is not stored TXT record directly in selector._domainkey.domain, but instead points to a CNAME.

For example, when retrieving the DKIM public key for the following domain: protonmail._domainkey.proton.me, we first need to resolve the CNAME for this domain: protonmail.domainkey.drfeyjwh4gwlal4e2rhajsytrp6auv2nhenecpzigu7muak6lw6ya.domains.proton.ch.

However, using the "dns" npm package, the CNAME cannot be resolved.

With this update, all DNS resolutions will be performed using google DNS-over-HTTPS (DoH).

Type of Change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist:

• I have discussed with the team prior to submitting this PR
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• New and existing unit tests pass locally with my changes

[saleel]

However, using the "dns" npm package, the CNAME cannot be resolved.

Are you sure this is the case? I thought the dns package resolve CNAME records as well 🤔

Moving to HTTP DNS can solve another problem though: NextJS users were facing issues around this code due to conflicts in server-side vs frontend rendering. Was thinking of making this change to fix that.
We add a trust assumption on Google DNS HTTPS service when generating proofs (even for server side now), but the verifier is free to validate the "pubkey" by their own means.

@Divide-By-0 what do you think?

@jayden-sudo Btw, we should also remove the import of dns package if we are doing this. And maybe some refactoring since we dont have two cases now (Call let list = await resolveDNSHTTP(name, "TXT"); directly?)

[Divide-By-0]

We add a trust assumption on Google DNS HTTPS service when generating proofs (even for server side now), but the verifier is free to validate the "pubkey" by their own means.

This is fine for most cases but not ideal; in the ideal scenario, we default to this Google key, but also try to fetch the key directly via a DNS query if we can -- if that fails, fine we go with google, if it succeeds then we verify that both keys are the same and output a scary warning if they aren't (but still proceed with Google as a default). Basically, we want a user to be able to tell if something is going wrong i.e. Google is censoring that key for whatever reason. IMO, we can add this as a future feature request, and merge this in for now in order to unblock and immediately resolve SSR rendering issues. Do you agree?

[jayden-sudo]

Are you sure this is the case? I thought the dns package resolve CNAME records as well 🤔

The dns package can resolve the CNAME in some cases, but the example ‘protonmail._domainkey.proton.me’, which I provided above, can't resolve the CNAME through the dns package. (After my testing, I found that there are many CNAME that can't be resolved by the dns package, and I can't resolve the CNAME by using dig and nslookup in the shell, but I can resolve it by using google public DNS).

[jayden-sudo]

We add a trust assumption on Google DNS HTTPS service when generating proofs (even for server side now), but the verifier is free to validate the "pubkey" by their own means.

This is fine for most cases but not ideal; in the ideal scenario, we default to this Google key, but also try to fetch the key directly via a DNS query if we can -- if that fails, fine we go with google, if it succeeds then we verify that both keys are the same and output a scary warning if they aren't (but still proceed with Google as a default). Basically, we want a user to be able to tell if something is going wrong i.e. Google is censoring that key for whatever reason. IMO, we can add this as a future feature request, and merge this in for now in order to unblock and immediately resolve SSR rendering issues. Do you agree?

I think we can use google & cloudflare at the same time to make sure DNS is accurate (both via DNS over HTTPS)

[saleel]

IMO, we can add this as a future feature request

Yes, that make sense. DNS from local ISPs sometimes get attacked (or maybe even prone to censoring) and many people tend to use public DNS like Google/Cloudflare. But its good to have multiple options/fallbacks - maybe even multiple http dns.

We can merge this for now. We can remove dns import (as its not needed until we do the above feature) in this case so it fix NextJS rendering issues as well.

[saleel]

I think we can use google & cloudflare at the same time to make sure DNS is accurate (both via DNS over HTTPS)

Yea 👍 @Divide-By-0 If you agree, please add an issue for that.

[jayden-sudo]

#212
I just modified the code to use both Google and Cloudflare DoH server. If the DKIM public key data does not match, it will log: console.error("DKIM record mismatch!");.

[saleel]

LGTM 🚀

Added a minor comment.