lieser / dkim_verifier

DKIM Verifier Extension for Mozilla Thunderbird
MIT License
210 stars 35 forks source link

add public key pinning/storing #18

Closed lieser closed 10 years ago

lieser commented 10 years ago

split from #15 Joey3000:

One more option for future could be "public key pinning" (i.e. storage of the public key and the selector per From: domain). That would be for the paranoid assuming being under an active MITM attack, with the MITM sending an e-mail to the recipient and spoofing their DNS requests. (Like the "Certificate Patrol" add-on does on Firefox, except that they use "certificate pinning" instead of just "public key pinning".) One could then (additionally to key and selector) also store the signing domain. Which would be useful for From: domains which use a signing domain different from the From: domain, but which are nevertheless legitimate. If one "pins" their signing domain down, one can trust them too. (One would possibly also need to be able to define exceptions for certain From: domains, if they use several legitimate keys/selectors/signing domains and therefore cannot be "pinned" down to just one. Like Google does with its website certificates.)

lieser commented 10 years ago

@Joey3000 I have now added the two options:

New keys are at the moment always accepted. Because a domain can have more then one key by using different selectors, this means an attacker can still insert his own key for a domain. Is this enough for you, or do you also want the ability to manually accept any new key that is not yet stored?

lieser commented 10 years ago

This will be added in version 1.0.0. A prerelease can be downloaded from https://github.com/lieser/dkim_verifier/releases.

If you want to check every DKIM key, you can use DNSSEC as a workaround. First enable storing of DKIM keys. Then set under the advanced options, that a DKIM key not signed by DNSSEC should be treated as warning or error. If a new DKIM key is now downloaded, that is not signed by DNSSEC (default for the JavaScript DNS resolver), an error/warning will be shown if it is used. After you ensure yourself that the DKIM key is correct, you can then manually enable the DNSSEC flag of the stored key to "1" (true), so the error/warning disappears.

Would be nice if you could test it. If you encounter any problems, or need additional features related to this, please reopen the issue. Thanks.

Joey3000 commented 10 years ago

Thank you very much!

I haven't tested, but based on your post, the two new options would already provide some key change feedback by:

It's close, but not quite what I had in mind. This is where the problem is:

New keys are at the moment always accepted. Because a domain can have more then one key by using different selectors, this means an attacker can still insert his own key for a domain.

As I see it, the key pinning is used for key change notification, and has following manifestation on key change:

So, a user confirmation to accept the changed key is definitely needed. To list what I had in mind in more detail, which I missed earlier (points numbered just for easier reference in future responses):

1) On From domain where no key has been stored yet: just silently store the key. (Although ideally, there should be a neutral notification, too - i.e. without influence on verification result. The notification could be enabled in the config, if the user desires. And it should be possible for the user (via the "Other Actions" drop-down menu) to prevent storage of the new key (just like they can prevent storage of changed key - see point 2.2 below). Reason: If I see such a "DKIM key stored" notification on "facebokmail.com", I might notice that it's not the "facebookmail.com", from which I know I receive verified e-mails daily.)

2) On change of key: 2.1) Warning.

2.2) An additional "Other Actions" drop-down menu point: "Accept changed DKIM public key". Which then also triggers a re-verification using the new key. Note: Before accepting, the user would need to check the key via a secure different channel. E.g. using one of the online DKIM checkers over HTTPS, with its cert pinned in browser.

3) Storage of only one key per From domain. Reason: I think domains using several keys (at the same time) are an exception, not the rule. And if the domain changes its key after a while, then it has a reason for that (compromise, precaution).

4) Allow domains to be excepted from pinning, configurable in the config. That would be for domains using more than one key at the same time, in case there are any.

5) Thinking about this a bit longer, I don't think that storage of the selector or signing domain (additionally to the key) is necessary (for the purpose of this enhancement request). Because I don't see any security improvement in that - they key is what matters cryptographically. No matter if it changes alone, or additionally with the selector or signing domain. (And you already implemented "pinning" of the SDID in #15, also allowing disabling of the warning on the "pinned" SDID which differs from the From domain.)

Additional discussion, not for implementation:

A) Problem is, that would show a "changed key" on old e-mails (using different key), if the user doesn't use the "save verification result" option. Or if they recently re-downloaded the whole mailbox to a new TB profile, or "repaired" the inbox folder, and therefore don't have verification results of old e-mails. Possible solution: Maybe some sort of "Ignore (read) e-mails older than [ ] days" option? But on the other hand, that is similar to a domain changing key/selector and taking the old selector offline. It would also cause verification of old e-mails to fail. So, I guess the user would just need to ignore verification failures on old e-mails due to the nature of client-side DKIM verification. (If done server-side, the server could store the verification result in an e-mail header - after deleting a possibly present old result and running a verification on e-mail reception. But a client can't do that - it would not be reliable, because it wouldn't know where the result has come from.)

B) Or: An "Ignore older e-mails from same domain" option. Which would require saving e-mail date with the key, and would allow ignoring of e-mails with different key older than the one saved. But I don't think it's a good thing from security prospective, as e-mail dates can be forged. One does sometimes receive e-mails supposedly dated a while ago. Which can be legitimate - due to "delay sending" feature, etc. - or not.

C) I disregarded DNSSEC for this discussion. DNSSEC, in relation to this, would provide another, additional authentication layer. Specifically when the DKIM key has been compromised and replaced by the domain, but the user, being under an active local targeted MITM attack, does not yet know about it and receives an e-mail which uses the old compromised key. But then one could argue that DNSSEC in such a (local targeted attack) case could be compromised by the attacker, too. By replacing its own public key - because itself not "pinned" (which could be difficult on changing DNS resolvers due to changing connections - there could be new keys all the time). A better solution would be a DKIM key check using a different known (at least locally) authenticated pinned channel, e.g. like what "Convergence" does (or via a VPN, etc.). Which would though, I think, go too far for this add-on. Because with DKIM verified e-mails, there seems to be greater risk of falling victim to other, easier executable attacks (e.g. social engineering or SW exploits) than to such a local targeted MITM attack on a DKIM verification with a pinned key, with a compromise of the key (i.e. knowledge of the private key corresponding to the pinned public key). Anyway, DNSSEC is a separate story. (Additional discussion for other readers: https://github.com/lieser/dkim_verifier/issues/15#issuecomment-25213992 )

What do you think? Sorry, as I wrote, I haven't tested your implementation yet.

Edit: Updated some points for more clarity.

lieser commented 10 years ago

As I don't really see a big advantage of your proposal over the DNSSEC workaround, I will probably not implement this. The only think I will probably add are an entry in the "Other Actions" menu for marking the currently used key as secure, and maybe on for deleting it (for key change, so that the new one gets stored).

My detailed thoughts about you proposal below:

1) On From domain where no key has been stored yet: just silently store the key. (Although ideally, there should be a neutral notification, too - i.e. without influence on verification result. The notification could be enabled in the config, if the user desires. And it should be possible for the user (via the "Other Actions" drop-down menu) to prevent storage of the new key (just like they can prevent storage of changed key - see point 2.2 below). Reason: If I see such a "DKIM key stored" notification on "facebokmail.com", I might notice that it's not the "facebookmail.com", from which I know I receive verified e-mails daily.)

Although with the DNSSEC workaround you don't get a notification, you will get, based on the settings, an invalid signature or a valid signature with a warning, until you mark the key as secure. The (possibly wrong) key is still stored, but I don't see any harm in this, as it will be marked as insecure. In case libunbound is used as an resolver, and the key is signed by DNSSEC, it will be marked automaticly as secure.

2) On change of key:

This will produce at the moment an error, not a waring, and will not go away until you delete the old key. Unlike in SSL/TSL, a DKIM key will probably nether change, unless the public RSA key is revoked. The reason for this is, that you can have more then on DKIM key per domain, and if a domain changes it's public/private RSA key pair, it will probably change the selector to (and by doing so will create a new DKIM key, and not change the old one). So unless I am wrong, and many domain do not change the selector, if they change the public key, or key revocation happens often, I don't see a big issue in the lack of good support for a key change.

3) Storage of only one key per From domain. Reason: I think domains using several keys (at the same time) are an exception, not the rule. And if the domain changes its key after a while, then it has a reason for that (compromise, precaution).

I don't see the advantage of this at all. If a second key is used by an attacker, it will produce a warning/error with the DNSSEC workaround like every new key, so you can detect this.

4) Allow domains to be excepted from pinning, configurable in the config. That would be for domains using more than one key at the same time, in case there are any.

Irrelevant as long as 3) is not used.

5) Thinking about this a bit longer, I don't think that storage of the selector or signing domain (additionally to the key) is necessary (for the purpose of this enhancement request). Because I don't see any security improvement in that - they key is what matters cryptographically. No matter if it changes alone, or additionally with the selector or signing domain. (And you already implemented "pinning" of the SDID in #15, also allowing disabling of the warning on the "pinned" SDID which differs from the From domain.)

The storing of the selector and domain is not used for security. It's that identifies a DKIM key, which includes the public RSA key, and is therefore definitely necessary. Otherwise you wouldn't now which key to use.

Joey3000 commented 10 years ago

I am fine with all of that. Thank you very much for providing a detailed explanation.