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

Add support for ECDH #998

Closed
myst3rium opened this issue Jan 31, 2024 · 20 comments
Closed

Add support for ECDH #998

myst3rium opened this issue Jan 31, 2024 · 20 comments
Labels
enhancement New feature or request

Comments

@myst3rium
Copy link

RSA is getting older and older and, in view of the massive increase in computing power, it is becoming increasingly vulnerable. On the other hand, there are the "new" elliptic curves, which offer secure encryption with significantly less effort.

I tried using MimeKit with certificates with ECC instead of RSA and dug deep into the library to see that the library doesn't seem to support ECC certificates; everything seems to be designed for RSA. So I keep getting the error "unkown cipher" when using an ECC certificate.
Maybe I just missed anything anywhere but the fact that the official API documentation (http://jstedfast.github.io/MimeKit/docs) is unreachable, too, doesn't make it easier to find a solution.

I would therefore be very grateful for at least some help and information in this area.

@jstedfast
Copy link
Owner

jstedfast commented Jan 31, 2024

Maybe I just missed anything anywhere but the fact that the official API documentation (http://jstedfast.github.io/MimeKit/docs) is unreachable, too, doesn't make it easier to find a solution.

You just have bad timing - I'm in the process of moving the website from Azure to GitHub and unfortunately, things are not going smoothly for me 😦

That said, you can still use the .com and .org website URLs to get to the old Azure site:

http://www.mimekit.org/docs
http://www.mimekit.com/docs

Now on to your actual question...

I tried using MimeKit with certificates with ECC instead of RSA and dug deep into the library to see that the library doesn't seem to support ECC certificates; everything seems to be designed for RSA. So I keep getting the error "unkown cipher" when using an ECC certificate.

Where exactly are you getting this exception?

I have a sneaking suspicion (though I could be wrong) that you are trying to convert between System.Security certificates/keys and BouncyCastle certificates/keys. If so, then you are correct, there is no code that can handle ECC private key conversion from System.Security to BouncyCastle. Conversion between the 2 APIs has not been fun, even with better supported algorithms in System.Security such as RSA and DSA.

If you stick strictly with System.Security or BouncyCastle, things are more likely to work but there could be issues (I'm not 100% positive) - this is why I am asking for the location in the code that threw the exception.

@myst3rium
Copy link
Author

myst3rium commented Jan 31, 2024

Oh thank god! The documentation is back and there is a chance of getting some information <3

Well - let's start:
I had to derive a SecureMimeContext from your BouncyCastleSecureMimeContext to handle the certificatesa as we need it. There my entrance is the function "Encrypt" returning an ApplicationPkcs7Mime. On reaching the Call "CryptServices.Envelope(...)" the error returned is

  • "SecurityUtilityException: Cipher 1.2.840.10045.2.1 not recognised."
  • Org.BouncyCastle.Cms.CmsException: "can't find key generation algorithm."
    -- Org.BouncyCastle.Security.CipherUtilities.GetCipher(string) in CipherUtilities.cs
    -- Org.BouncyCastle.Security.WrapperUtilities.GetWrapper(string) in WrapperUtilities.cs

I tracked it down to CryptServices.Generate(KeyParameter, SecureRandom) where the call to "GenerateWrappedKey" fails with that error. Within that function the CrypServices tries to call "CreateWrapper" with the keyEncryptionAlgorithm ::= 1.2.840.10045.2.1 (+params).
And this is where I struggle since I'm currently not deep enough in MimeKit and BC to tell, whats going on there but it seems that either my certificate has a major issue or the "GenerateWrappedKey" function uses the wrong cert property ...?

Since I need to create the certificates myself I wrote an certificate generator that may be faulty.
I tried to get the whole process documented into an stackoverflow question, you may review for further information.
https://stackoverflow.com/questions/77906972/difficulties-with-ecc-bouncycastle-and-mimekit

Thanks for your help!

@jstedfast
Copy link
Owner

I was hoping for a StackTrace that showed a MimeKit stack frame to provide me with some context.

Based on the stack you provided, though, it appears it's not anything to do with key conversion between the 2 platforms and looks like it's an issue with BouncyCastle itself?

I'm not very knowledgeable about the internals of BouncyCastle, so I'm not sure I'm going to be much help in understanding that code or why things are failing there.

The OID that BouncyCastle is using seems right to me? https://oidref.com/1.2.840.10045.2.1

I would probably recommend asking for help at https://github.com/bcgit/bc-csharp - specifically, Peter Dettman is the maintainer and is very knowledgeable about both BouncyCastle as well as cryptography and would definitely be able to help you.

@jstedfast
Copy link
Owner

jstedfast commented Jan 31, 2024

Oh, and for what it's worth, if you open an issue in the bc-csharp repo that I linked to, if you link back to this issue in MimeKit, it'll link the 2 issues making it easier to track.

@myst3rium
Copy link
Author

myst3rium commented Jan 31, 2024

Thanks for the recommendation. I'll follow that hint and will link the issues.
For me its very unclear if this is an issue with bouncycastle or with mimekit since the problem starts when MimeKit calls "WrapperUtilities.GetWrapper" from BC with "1.2.840.10045.2.1" as algorithm and the error seems to be anywhere between:
grafik

Maybe there is a need for another exception for ECC as there is one for RSA implemented in MimeKits "CreateWrapper" function. But I'll come back to that after getting some insight from the bc guys.

@jstedfast
Copy link
Owner

Oh, I see what you are saying... the issue could be that BouncyCastleSecureMimeContext.CmsRecipientGenererator.CreateWrapper() might not be passing the correct name value to BouncyCastle's WrapperUtilities.GetWrapper().

@myst3rium
Copy link
Author

Let's see, what the bc gurus think about it: bcgit/bc-csharp#516

@myst3rium
Copy link
Author

myst3rium commented Jan 31, 2024

Without knowing if I'm right or dumb I'd think, BC would like to have passed "ECIES" as "algorithm" to provide the ECDHBasicAgreement that maybe should be fed with the ECC params from the cert in order to use the correct curve and curve params ... But thats just a wild guess.
grafik

@jstedfast
Copy link
Owner

That probably makes sense, but lets see what Peter Dettman says.

I'm currently busy with my day job, but what does CollectionUtilities.GetValueOrKey() do with the algorithm string? My guess is that it tries to look up whatever algorithm string we feed it to get back a "canonical" name, but since (as you discovered), it is sending "1.2.840.10045.2.1", the lookup table probably doesn't have a match for that.

@myst3rium
Copy link
Author

myst3rium commented Jan 31, 2024

Exactly - that one tries to lookup a friendly name and maybe that one should return the value "ECIES" instead of keeping the OID. Looking into that dictionary I doubt that one since this seems to lookup only the AES derivates and the other common symmetrical cryptography algorithms such as DES, TrippleDES, ARIA, RC4, ...

algorithm = CollectionUtilities.GetValueOrKey(Algorithms, algorithm).ToUpperInvariant();
where
'Algorithms' is:
grafik

@jstedfast
Copy link
Owner

I wonder if the MimeKit code to get the wrapper is just going about things entirely the wrong way?

As in, it seems pointless to call WrapperUtilities.GetWrapper() if none of the algorithms that MimeKit is likely to pass in are ever expected to match.

Might be worth going straight to calling CipherUtilities.GetCipher() and then creating a wrapper around that (just like WrapperUtilities.GetWrapper() does if the algorithm doesn't match any of the handled enum values).

@myst3rium
Copy link
Author

myst3rium commented Jan 31, 2024

I get your point, and it feels correct as you explain it. But from here I'm lost for now. I need to look deeper into your code in order to really understand the impact of the change, since for now the code works as intended when using RSA certificates ...

@jstedfast
Copy link
Owner

Ugh, we can't call CipherUtilities(DerObjectIdentifier) directly because we'd lose out on the handy BufferedCipherWrapper class that is a nested class within WrapperUtilities.

And I guess it wouldn't really gain us anything in that case (other than perhaps a slight optimization).

Oh well, that was just a quick idea that turned out to be a dead end. I need to get back to my day job haha, but this is more interesting.

@jstedfast
Copy link
Owner

@myst3rium are you generating your own EC keys? If so, which elliptic curve?

I'm working on trying to add some unit tests for this.

@myst3rium
Copy link
Author

myst3rium commented Feb 9, 2024

Hi @jstedfast,

[Short version] - secp256r1 (P-256), secp384r1 (P-384) and secp521r1 (P-521)
You might want to use the code I provided in the corresponding stackoverflow question: https://stackoverflow.com/questions/77906972/difficulties-with-ecc-bouncycastle-and-mimekit

[ellaborate version]
I did some research when I opened this issue. Long story short - I read about Mozilla commiting to a set of 3 curves. So these seem to be the most popular ones that should work at least.
As [1] states, Mozilla uses secp256r1 (P-256), secp384r1 (P-384) and secp521r1 (P-521).

In my research I found another article [2] from March 2015 (sorry - its german) that came to the same conclusion: Most browsers only support these three curves.

The last reference I can provide is a question on security stackexchange [3]. The answer refers to NSA Suite B that contains curves P-256 and P-384. So chrome seems to stick to this suite and removed secp521r1 for no known reason. P-521 may be used as well since it's not less secure but less efficient than P-256.

So if most browsers refer to these curves, they seem to be some kind of "silent" standard that should work properly.

[references]
[1] - Mozilla Server Side TLS - https://wiki.mozilla.org/Talk:Security/Server_Side_TLS
[2] - EC-Kryptographie (german article about using EC, 2015) - https://www.secuvera.de/blog/ec-kryptographie-fuer-den-privatmann/
[3] - Security Stackexchange about supported curves and NSA Suite B - https://security.stackexchange.com/questions/100991/why-is-secp521r1-no-longer-supported-in-chrome-others

@myst3rium
Copy link
Author

Addendum:
it seems that NSA Suite B has been replaced with the Commercial National Secutiry Algorithm Suite [4] which reccomends the use of P-384.

[references]
[4] - CNSA 2.0 - https://media.defense.gov/2022/Sep/07/2003071834/-1/-1/0/CSA_CNSA_2.0_ALGORITHMS_.PDF

jstedfast added a commit that referenced this issue Feb 9, 2024
@jstedfast
Copy link
Owner

I would love if you could give these changes a spin and make sure everything works as expected.

Hoping to make a new release in a week or so if I can.

I tried adding unit tests for DSA but those didn't work (DSA seems obsolete at this point anyway).

Then there's DiffieHellman and Edwards Curve private keys that probably need to be handled at some point?

Tried testing EC keys with the WindowsSecureMimeContext but it seems the Microsoft CMS implementation does not currently support that.

@myst3rium
Copy link
Author

myst3rium commented Feb 14, 2024

Hi @jstedfast,

I'm making a note here:
Huge success!

Having just finished a big chunk at work, I was finally able to test your implementation and it works. I am very happy that I am now able to encrypt and decrypt using the "new" standards and I am very grateful for the work you have put into it! I've tried to look through the latest commits, but I need a little more time to really understand what's going on. I am amazed at how quickly you were able to make such complex changes.

About my testing scenario:
Currently working on some mail communication I have a small setup with a local mail server (hMailServer). Since the code for sending and receiving (encrypted) mails already exists using an extended version of the BouncyCastleSecureMimeContext the setup was pretty easy. I have just created some new certificates with BouncyCastle based on the curve "secp521r1", created a new encrypted mail with an attached file and sent it to a mailbox of the hMailServer. Using Thunderbird I validated that the mail is encrypted. Finally, I retrieved and decrypted the mail using POP3 in my recipient application where I was able to retrieve the attached file.

@jstedfast
Copy link
Owner

Awesome, thanks so much for verifying that things work for your needs!

@myst3rium
Copy link
Author

That's the least I could contribute. For all the work you've done, I owe it to you to at least test the changes in my setup properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants