Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Details about verifed DKIM #160

Open
xsistema opened this issue Jul 25, 2019 · 18 comments
Open

Details about verifed DKIM #160

xsistema opened this issue Jul 25, 2019 · 18 comments
Assignees
Milestone

Comments

@xsistema
Copy link

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
Copy link
Owner

lieser commented Jul 31, 2019

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 xsistema changed the title Details about veriifed DKIM Details about verifed DKIM Jul 31, 2019
@xsistema
Copy link
Author

xsistema commented Jul 31, 2019

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
Copy link

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
Copy link
Owner

lieser commented May 9, 2024

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
Copy link

gene-git commented May 9, 2024

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
Copy link
Author

xsistema commented May 9, 2024

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

@lieser
Copy link
Owner

lieser commented May 9, 2024

@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: [email protected]

A preview on how it currently looks:
grafik

In particular I'm interested in:

  • Is there any information you are still missing and you would like to be added? Best also shorty write why the information would be useful for you. I deliberately did not add all possible information about the DKIM signatures. Instead I tried to only select the information that may be of actual interest to any advanced user (who still mainly just use it for viewing e-mails - i.e. not for debugging DKIM issues).
  • Are there any noticeable UI issues? Some things I already noticed:
    • The results show only up if the verification was already finished when opening the DKIM popup. How important would it even be for anyone this gets implemented?
    • If a lot of DKIM results are shown and a scroll bar appears, the buttons scroll too.
  • Do you see any improvements on how the the DKIM results are presented? I don't plan to win any awards with my GUI - defiantly not my area of expertise - but it should also not look ugly or the information hard to read, and try to look not to different from the rest of Thunderbird.

@xsistema
Copy link
Author

xsistema commented May 9, 2024

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

@lieser
Copy link
Owner

lieser commented May 9, 2024

Version marked to be compatible with up to TB 127: [email protected]

@xsistema
Copy link
Author

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
Copy link
Contributor

dodmi commented May 16, 2024

@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:

  • Result should be on top of the list
  • I'm not really sure about the order of SDID, AUID and Algorithm; I guess, I'd place Algorithm as second field
  • Maybe don't show fields, which aren't populated (Expiration date)
  • 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)

@lieser
Copy link
Owner

lieser commented May 17, 2024

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
Copy link
Contributor

dodmi commented May 18, 2024

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
Copy link
Contributor

dodmi commented May 20, 2024

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
Copy link
Contributor

dodmi commented May 21, 2024

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

@dodmi
Copy link
Contributor

dodmi commented May 24, 2024

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")

  • I'm going for the shorter local date/time
  • I'm adding either times or a hint, that no signing time is included, so this are always three lines for a valid signature

After that, there's the technical "How"

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

@lieser
Copy link
Owner

lieser commented May 27, 2024

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
Copy link
Contributor

dodmi commented May 28, 2024

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants