MimeKit icon indicating copy to clipboard operation
MimeKit copied to clipboard

Split MimeKit into MimeKit[.Core] and MimeKit.Cryptography

Open wrall opened this issue 3 years ago • 9 comments

Is your feature request related to a problem? Please describe. Right now, different projects can end up with weird conflicts when one dependency uses MimeKitLite and another uses MimeKit. When this happens, there can be an error about not being sure which version to use.

Describe the solution you'd like It would be conceptually simpler to have MimeKit simply depend on MimeKitLite and add the additional functionality on top of it. Of course this would probably require a major version change.

Describe alternatives you've considered The workaround at the moment is to use aliased references that allows you to select which reference to pull a type from. Another dev approach would be to use a different namespaces, but that is far uglier.

wrall avatar Jul 15 '22 00:07 wrall

What projects are you talking about, specifically?

I would argue that those projects should be using MimeKit always and not MimeKitLite.

jstedfast avatar Jul 15 '22 12:07 jstedfast

To add context: I just had the problem, that Wiremock.net added the dependency to MimekitLite to parse emails. I was using Mailkit to send emails. I think I can downgrade to Lite, but I will see.

Here is also a Wiremock.net issue: https://github.com/WireMock-Net/WireMock.Net/issues/990

ggmueller avatar Oct 24 '23 11:10 ggmueller

This has been on my radar for a while now but I don't know of a good way to do it that wouldn't require the user manually register the fact that they have the crypto stuff available to the MimeParser.

The MimeVisitor class would also need changes because right now, MimeKitLite's MimeVisitor API is not identical to the MimeVisitor API in MimeKit.

MimeMessage also has ties to the crypto APIs that are conditionally built depending on MimeKit vs MimeKitLite build configuration.

jstedfast avatar Oct 24 '23 13:10 jstedfast

Also been thinking a while of this, reading a bit into the code. Not quite sure of everything though.

At first glance, it looks to me the biggest issue is the ParserOptions.Default field. As the default options would need to be different when CRYPTO is enabled.

I guess it would be doable to dynamically load a known assembly by name and call an additional static initializer method there that would additionally register the additional mimeparts, eg via the existing RegisterMimeType method. If the Crypto assembly is not available, it would skip this initialization.

It could also set some kind of FeatureFlag or something on the ParserOptions indicating if Crypto is available.

Then MimeParser and MimeVisitor could maybe make a runtime decision if they support Crypto.

Would this make sense?

ggmueller avatar Jan 04 '24 19:01 ggmueller

Yea, that's more or less what I've also concluded.

MimeParser isn't a problem - it uses ParserOptions to instantiate the various MIME type classes.

MimeVisitor is/was the biggest problem because MimeKitLite does not have any of the crypto classes like ApplicationPkcs7Mime or MultipartEncrypted or MultipartSigned and obviously that is not so easy to make work via reflection or any sort of runtime decision making.

jstedfast avatar Jan 05 '24 02:01 jstedfast

Having the same issue. I am using StrongGrid v0.102.0 as well as MailKit v4.3.0 and I get conflicts everywhere now.

Suneeh avatar Jan 08 '24 10:01 Suneeh

I'll put this on the roadmap for 5.0 but I don't have definite plans for that yet.

In the meantime, there is also a MailKitLite if that helps.

Or ask the developers of StrongGrid to publish a version built against MimeKit instead of MimeKitLite. That would be a LOT easier to do than rearchitect MimeKit.

jstedfast avatar Jan 08 '24 13:01 jstedfast

What I would propose is that we completely get rid of MimeKitLite (mostly I hate the naming) and either:

  1. Replace MimeKitLite with a MimeKit.Core assembly/package, move the classes that depend on BouncyCastle into a MimeKit.Cryptography assembly and create a dummy MimeKit nuget that can be used to get both MimeKit.Core and MimeKit.Cryptography.
  2. Kill off MimeKitLite, keep MimeKit (as the equivalent of the Core library) and move the classes that depend on BouncyCastle into MimeKit.Cryptography.

Either way, MimeKitLite ceases to exist.

The next question is: should Microsoft.Extensions.DependencyInjection be used? Or should stuff like CryptographyContext.Register() continue to be the way the SecureMimeContext and OpenPgpContext get registered?

The problem with the .Register() approach is that we'd need something similar for DKIM/ARC.

So much of what exists in MimeKit.Cryptography is deeply tangled with BouncyCastle which is going to make this split extremely painful.

I already have some local branches that have started:

  • core
    • moved the MimeMessage.Sign/Encrypt/etc APIs into extension methods
    • updated MimeVisitor to use the new IMimePart/IMultipart/etc interfaces so that it doesn't depend, per se, on ApplicationPkcs7Mime which heavily depends on BouncyCastle (unfortunately, the IApplicationPkcs7Mime interface also still does...)
  • dkim-abstractions
    • abstracting out the DKIM/ARC stuff into interfaces that have no dependency on BouncyCastle as well as updating DkimVerifier/ArcVerifier/etc to use the abstract interfaces.
  • crypto-abstractions
    • abstracting out OpenPgpContext (which means IPgpPublicKey and IPgpPrivateKey so far) into an interface detangled from BouncyCastle, but yikes, this one especially is deeply tangled.
    • SecureMimeContext is tangled with BouncyCastle, but extracting an ISecureMimeContext interface wasn't too bad, but classes like CmsSigner and CmsRecipient will be a bit of work.

This is going to be more difficult than I thought 😦

jstedfast avatar Jan 13 '24 22:01 jstedfast