jstedfast / MimeKit

A .NET MIME creation and parser library with support for S/MIME, PGP, DKIM, TNEF and Unix mbox spools.
http://www.mimekit.net
MIT License
1.83k stars 372 forks source link

Expose a way to tell why Arc Verification fails #591

Closed The-Nutty closed 4 years ago

The-Nutty commented 4 years ago

Is your feature request related to a problem? Please describe. It would be very good to be able to tell why Arc verification fails as currently there is no way to differentiate what failed when attempting to validate arc.

Describe the solution you'd like ArcVerifier.GetArcHeaderSets https://github.com/jstedfast/MimeKit/blob/778ca3f1ac1d3edb16f9adb8894097218d37f742/MimeKit/Cryptography/ArcVerifier.cs#L546 already takes a bool throwOnError parameter that always false, this could be exposed in a Verify/VerifyAsync overload You then could make a few changes to VerifyAsync to throw if Signature or Seal validation fail or not, im my use case its really if GetArcHeaderSets throws that i care about. This should be quite simple and would not break existing functionality.

Describe alternatives you've considered Could expand ArcSignatureValidationResult to contain other fail states but im not sure thats appropriate.

The-Nutty commented 4 years ago

Furthure more another thing that would make the arc Verifier a big more usable is being able to verify signatures/seals even if cv=fail somewhere in the chain.

Currently if cv=fail https://github.com/jstedfast/MimeKit/blob/778ca3f1ac1d3edb16f9adb8894097218d37f742/MimeKit/Cryptography/ArcVerifier.cs#L529 then mimekit does not bother to verify the seals, when infact for our use case that would be very useful.

Im happy to raise a new feature request for this if you would like but seemed logical here.

jstedfast commented 4 years ago

Can you explain why you need this? Maybe if I had more information about what you are trying to do, I could come up with a better API?

I really don't want to expose a virtual method that takes a throwOnError argument. That would be a HUGE design failure in my opinion.

The typical use-case is "Does this message have a valid ARC signature chain? yes/no"

The-Nutty commented 4 years ago

Yeah i understand that, for me it would be useful to know why arc failed. Some of this is routed in https://github.com/jstedfast/MimeKit/issues/590 as if ARC is failing we need to know why so it can be fixed.

Our other use case is in an arc chain compromising of 3 intermediary. Where i=1 cv=none, i=2 cv=fail and i=3 cv=pass. It would be good to know if the signature/seal of i=3 passed even though for i=2 cv=fail.

jstedfast commented 4 years ago

I guess what I'm asking is: do you want this for diagnostic purposes (i.e. to debug issues)? Or do you want it for some other reason?

I always find Exceptions are fine for diagnostics issues but are horrible for real usability (custom Exception classes with various state fields can close the gap a bit, but even then, they tend to suck).

jstedfast commented 4 years ago

I think I may have some ideas on how to make the information you need available...

The-Nutty commented 4 years ago

Yeah its really 2 fold diagnostic information primarily and also the ability to very the rest of an arc chain even if one part was cv=fail.

I agree with your sentiment on exceptions, i appreciate you trying to find a solution.

jstedfast commented 4 years ago

If you could review the PR I linked above, that would be helpful. Let me know if that will meet your needs.

Thanks!

jstedfast commented 3 years ago

I've started working on a vnext branch and figured I should poke you to provide me with a list of things you might like as improvements to the ARC/DKIM/Authentication-Results APIs that exist in 2.x since you are probably the 1 person who has provided me with the most feedback in this area.

If you have any thoughts, please feel free to share over at https://github.com/jstedfast/MimeKit/issues/675