lieser / dkim_verifier

DKIM Verifier Extension for Mozilla Thunderbird
MIT License
213 stars 36 forks source link

Details about verifed DKIM #160

Open xsistema opened 5 years ago

xsistema commented 5 years ago

Is it possible to see the details of verified DKIM? For example key size, algorithm? It would be nice to have such a feature in menu.

lieser commented 5 years ago

The signing algorithm can be seen by looking at the source of the e-mail (The a=... tag in the dkim signature header). Note that it can only be rsa-sha256 or rsa-sha1 (until #142 is implemented). An thanks to #141, the deprecated use of rsa-sha1 will be easily spotted in the upcoming 2.1.0 version.

There is currently no easy way to see the key size (you need to use external tools). But the add-on warns if the key size is to low (currently 1024, in 2.1.0 it will be 2048).

I may make it easier in the feature to see detail, but currently I see my priority for adding something like this as low. See also the related #143.

xsistema commented 5 years ago

Yes, I know how to examine the details, but add some info would be nice and it's pretty easy: "Insecure signature algorithm (rsa-sha1)" "Signature is insecure (key size too small - 1024 bits)". But overall - the add-on is superb.

gene-git commented 5 years ago

Please be careful with key size, by itself its not a good marker of security.

ECC is part of the 2017 DKIM draft[1] and ECC key lengths are way shorter than RSA keys. Algo dependent, but 256 ECC keys are roughly equivalent to 3072 bit RSA key length

[1] https://tools.ietf.org/html/draft-ietf-dcrup-dkim-ecc-01#page-3

lieser commented 6 months ago

I have a first working draft for this. Please let me know if anyone is interested in trying it out and giving early feedback on it. Both about what additional information is shown and how it is presented.

gene-git commented 6 months ago

Thank you for working on this - I gave up on Thunderbird earlier this year and moved to Evolution - so unfortunately I am no longer in position to test things any longer.

xsistema commented 6 months ago

I can try it - let me know what to do.

lieser commented 6 months ago

@xsistema Just trying out the current state and giving some feedback that comes in mind would be enough. Also not urgent, don't plan to release it already in the next few days. Find a packed version here: dkim_verifier@pl-2024-05-09-376c719.zip

A preview on how it currently looks: grafik

In particular I'm interested in:

xsistema commented 6 months ago

Can not install - TB shows that it is not compatible with TB Beta 126.0 version..

lieser commented 6 months ago

Version marked to be compatible with up to TB 127: dkim_verifier@pl-2024-05-09-9e54739.zip

xsistema commented 5 months ago

New feature is excellent - main information of DKIM signature is shown. About UI - simple, and as should be - works without noticeable problems. But one thing - on some emails DKIM "Signed headers" (and some other fields) missing - only shows when button "Reverify DKIM signature" is pressed. Perhaps it should be fixed (but not critical).

dodmi commented 5 months ago

@xsistema Sounds, like DKIM verifier displays a cached result (which doesn't contain the needed data)

@lieser I like the idea much and will try to backport it. I think:

lieser commented 5 months ago

Thank you both for the feedback.

@xsistema

But one thing - on some emails DKIM "Signed headers" (and some other fields) missing - only shows when button "Reverify DKIM signature" is pressed.

Like @dodmi already mentioned this is currently the expected behavior if a result saved with an older version is viewed. The old result is simply missing the information.


@dodmi

Result should be on top of the list

Could you maybe elaborate on why you think it should be on the top? I made it the first as kind of a identifier to distinguish between multiple signatures.

I'm not really sure about the order of SDID, AUID and Algorithm; I guess, I'd place Algorithm as second field

I know the algorithm was probably the information most requested to see. But to me the algorithm is more of a technical detail than the SDID, AUID or the result. The SDID and result should I think definitely be the two at the top (and everything else being up to debate, including the order of this two). I can see that in most cases the AUID seems like just noise because it is essentially the same as the SDID. But that is just because practically no one uses it, not because it is not potentially important information.

Maybe don't show fields, which aren't populated (Expiration date)

The idea here was to make it possible to distinguish between the signature not having an expiration date, and the information not being available, e.g. because an older saved result is shown that did not save the expiration date or because of an parsing error. Currently only the sign and expiration date behave in this way because they are the only information show that is optional in the signature, But maybe that is overkill and just a wast of space, was myself unsure on that.

Displaying results of multiple signatures may be challenging for the users if the verification fails on some signatures (but is pretty interesting on the other hand)

Making all signatures visible for the users that are interested in it was one of the main reasons for this feature. A few already thought the add-on is not handling multiple signatures at all, because only one is visible. I agree that to people new to DKIM seeing all this signatures (and some of them maybe failing) could be confusing. But that could probably said for most of the information in this view. I see it as a view targeting mainly advanced users. And it being kind of hidden behind the button hopefully will prevent beginners from getting easily confused by all this information.

dodmi commented 5 months ago

Could you maybe elaborate on why you think it should be on the top? I made it the first as kind of a identifier to distinguish between multiple signatures.

Well, in my point of view, the most important information of a signature, is if it's valid or not. And I'd place this information on top.

Making all signatures visible for the users that are interested in it was one of the main reasons for this feature. A few already thought the add-on is not handling multiple signatures at all, because only one is visible. I agree that to people new to DKIM seeing all this signatures (and some of them maybe failing) could be confusing. But that could probably said for most of the information in this view. I see it as a view targeting mainly advanced users. And it being kind of hidden behind the button hopefully will prevent beginners from getting easily confused by all this information.

Point for you. All signatures should be visible.

dodmi commented 5 months ago

What about DNSSec information? Is it already included, but missing in your screenshot? I think it maybe be interesting.

Also, while you're arguing against adding signature key length, I think, it may be valuable information, as it's important for assessing the quality of a RSA signature.

dodmi commented 5 months ago

What do you think of putting SDID and AUID in one line: image

dodmi commented 5 months ago

I'm pretty happy with that version: image

First there's the important "What", "Who" and "When" (the order is my personal preference, but I'd be also fine with switching "Who" and "What")

After that, there's the technical "How"

lieser commented 5 months ago

I will probably move the result to the top, and the time also aboth the algorithm (so the order as in your screenshots).

And I think I like the / separation in the algorithm in your screenshot more than what I did.

What about DNSSec information? Is it already included, but missing in your screenshot? I think it maybe be interesting.

It is not explicitly mentioned, but should show up similar to the normal view.

Also, while you're arguing against adding signature key length, I think, it may be valuable information, as it's important for assessing the quality of a RSA signature.

My point was more about the algorithm in general, not the key length in specific. And I still think that for most the important part should only be if it is secure, not what it is specific. But I agree that many will probably find it nice to easily see, and that for RSA it probably makes sense to show the key size too, as it is important for how secure an RSA key is.

What do you think of putting SDID and AUID in one line:

Did you check how that works with long domains? Doesn't look bad in your examples, but unsure if saving one line is worth the less structured view and longer lines. Similar unsure about combining signing and expiration time in one line. Will have to think about it a little more.

Maybe @xsistema could also say what he likes more.

I'm going for the shorter local date/time

In my draft I just printed the time without much considering the formatting, but yeah the long version is maybe a little overkill here.

I'd like to add the key length in brackets after the signing algorithm, in future: i.e. RSA (2048bit) / SHA256

Would have done it similar. And only show the key length for RSA.


@dodmi Note that it would be OK for me if you make the advanced view for 2.x look more how you like it the most, instead of it needing to be a 1:1 copy. But you should know that I still value your input on how it will look in 5.x. So please keep the feedback coming, even if you decide to change some things for 2.x.

dodmi commented 5 months ago

@lieser: I'm happy to provide my input - I guess, I'll have to upgrade some time ;)

I've implemented the following GUI options: image pref("extensions.dkim_verifier.advancedInfo.show", false); pref("extensions.dkim_verifier.advancedInfo.allSignatures", false); pref("extensions.dkim_verifier.advancedInfo.includeHeaders", false);

If advancedInfo.show is false, no advanced info is shown. If advancedInfo.show is true, the from and statusbar tooltip is replaced and an additional header tooltip is added.

With advancedInfo.allSignatures it's possible to control if only a detailed result for the primary signature or for all signatures is shown. advancedInfo.includeHeaders lets you disable the detailed information about signed headers, as this blows up my tooltips in some cases. I'm not sure, that I'll like it in future... This is the compact version for all signatures, no expiration tim: image This is the extended version: image

Here for Google Groups - many signed headers and an expiration time: image

Here a signature without timestamp: image

Here, signature key length is not available, since an error was thrown, before the key was fetched: image