Adding CORECLR Cross-Platform support to ECDH-ES

[digaomatias]

Changing the code to make it .NET Core compatible cross-platform now supporting:

• ECDH-ES^* with A128CBC-HS256, A128GCM, A192GCM, A256GCM
• ECDH-ES+A128KW^*, ECDH-ES+A192KW^*, ECDH-ES+A256KW^* with A128CBC-HS256, A128GCM, A192GCM, A256GCM

The main reason why these algorithms were not supported cross platform on CORECLR was because it makes use of CngKey, which is Windows specific. The main place where this type is required is very downstream when using ConcatKDF.DeriveKey.

I've followed the instructions from the user polewskm, where he specified on this issue:

- Refactor your ConcatKDF.DeriveKey to NOT use ECDiffieHellmanCng (notice the ...Cng suffix here)
- There is no equivalent implementation of SP800_56A_CONCAT in Linux
- You have to implement the functionality of SP800_56A_CONCAT yourself
- That is what my snippet is showing, how to manually implement SP800_56A_CONCAT on Linux using ECDiffieHellman (notice that lack of ...Cng in the suffix here)
- ...Cng classes cannot be used on Linux as they are Windows only

I've created a new method on ConcatKDF named DeriveKeyNonCng. It gets an ECDiffieHellman key instead of a CngKey.

Because of that, I had to modify everything upstream to use and work with ECDiffieHellman instead of CngKey.

The points of question on this PR are:

• Do we still need to support CngKey or can we make it a breaking change for newer versions of the library?
• If we do need support to CngKey, I would have to change this PR to have two versions of all the Elliptic Key flows, one that follows the CngKey path down to the ConcatKDF.DeriveKey, and another that follows the ECDiffieHellman path, that follows the path down to the ConcatKDF.DeriveKeyNonCng.

If you think we need to support CngKey and go for the dual path option, I can't promise a PR in a timely manner, because I have to focus on my work here, and our needs are resolved with the ECDiffieHellman only path.

[dvsekhvalnov]

Hi @digaomatias , thanks for PR !

Well, i would say backward compatibility is important thing. Give me sometime to wrap my head around changes next week and i'll get back to you with questions.

[dvsekhvalnov]

Hey @digaomatias , did first pass over commit, left couple comments here & there.

In general i'd split EcdhKeyManagement.cs into EcdhKeyManagementWin.cs & EcdhKeyManagementRest.cs and register appropriate implementation inside https://github.com/dvsekhvalnov/jose-jwt/blob/master/jose-jwt/JWTSettings.cs#L88C41 based on something like RuntimeInformation.IsOSPlatform(OSPlatform.Windows)

Will be cleaner code wise and will preserve existing contract for library.

Library already supports multiple key types almost everywhere, so having CngKey (plus probably additionally ECDiffieHellman) on windows and ECDiffieHellman on linux for some key management algos - sounds just fine.

BTW, how you envision to load ECDiffieHellman in real life? Typically key is a file on disk (.pem, e.t.c.) or stored in OS keychain?

[digaomatias]

Hey mate, I really like the ideas! It will take me a bit longer to get all that lined up. I'll see if i can get all that by the end of this week or the next.

Regarding how I envision to load ECDiffieHellman: in our particular scenario, we have the keys stored in a Cloud Secrets Manager as plain JsonObjects. The keys are loaded by our application as needed from the secrets manager. From there we just deserialize into a JsonWebKeys object and map the X,Y,D parameters into the ECDiffieHellman.Create.

But creating from a PEM file should work fine as well. Here's what I found:

Here's a general outline of the steps involved:

1. Read the PEM file and extract the base64-encoded portion.
2. Decode the base64 content to get the ASN.1 DER-encoded key.
3. Parse the DER encoding to get the key parameters.
4. Create an ECDiffieHellman object using the extracted key parameters.

On Tue, Sep 26, 2023 at 2:41 AM DV @.***> wrote:

Hey @digaomatias https://github.com/digaomatias , did first pass over commit, left couple comments here & there.

In general i'd split EcdhKeyManagement.cs into EcdhKeyManagementWin.cs & EcdhKeyManagementRest.cs and register appropriate implementation inside https://github.com/dvsekhvalnov/jose-jwt/blob/master/jose-jwt/JWTSettings.cs#L88C41 based on something like RuntimeInformation.IsOSPlatform(OSPlatform.Windows)

Will be cleaner code wise and will preserve existing contract for library.

Library already supports multiple key types almost everywhere, so having CngKey (plus probably additionally ECDiffieHellman) on windows and ECDiffieHellman on linux for some key management algos - sounds just fine.

BTW, how you envision to load ECDiffieHellman in real life? Typically key is a file on disk (.pem, e.t.c.) or stores in OS keystore?

— Reply to this email directly, view it on GitHub https://github.com/dvsekhvalnov/jose-jwt/pull/232#issuecomment-1733738165, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJZX6PXODQVFU764YS3UTX4GC2DANCNFSM6AAAAAA5AVNFFU . You are receiving this because you were mentioned.Message ID: @.***>

-- Abraço!

Rodrigo Matias Leote .NET Developer

"Whatever the mind can conceive and believe, the mind can achieve." - Dr. Napoleon Hill

[dvsekhvalnov]

@digaomatias if you short on time, i can probably pick up your work and adjust, let me know.

[digaomatias]

Hey @dvsekhvalnov I made some progress on it, but there are still some unit tests failing that I'm looking at. Just letting you know that I haven't forgotten this PR, it's just that I've been quite busy the last few weeks. Meanwhile feel free to look at the current PR and leave any comments so I can work on them on the next push.

[dvsekhvalnov]

Yeah, no worries, i totally get what it mean to be busy :) have your time.

[jimmyiv]

@digaomatias if you need any help implementing this let me know what I can do.

[cyungmann]

@digaomatias sorry to bother you, but have you had any time to look at this or will you have time soon? If not, could one of the people who offered to help pick it up? I am hoping to use the non-Windows support very soon in a project I am working on. Thanks for the contribution!

[digaomatias]

Good news, I just pushed the rest of the changes. Now we need to wait for @dvsekhvalnov approval :)

[dvsekhvalnov]

Thanks, i'll take a look next week.

[dvsekhvalnov]

Hey guys, i've added some comments. In general i'm fine with a change, my only real concern is duplicating unit tests, it's quite challenging to maintain, will appreciate if we can use some xUnit magic to avoid it.

Once we good, i'll run cross compatibility tests with some other libraries and prepare release.

[cyungmann]

Anything I can do to help move this along?

[dvsekhvalnov]

hi @digaomatias , i think we need to ping @digaomatias :)

I just don't know if he is planning to work on comments or if we can pick up where he left to finalize. Sometimes open source is confusing is that regard.

[digaomatias]

Sorry guys, my work got extremely busy these last few days and I couldn’t work on it. And probably won’t have time to do it for the next week at least. If you want to pick it up I’m fine with that. Otherwise I would be able to continue the work in one week roughly.

Thanks for understanding.

On Tue, 21 Nov 2023 at 10:42 PM, DV @.***> wrote:

hi @digaomatias https://github.com/digaomatias , i think we need to ping @digaomatias https://github.com/digaomatias :)

I just don't know if he is planning to work on comments or if we can pick up where he left to finalize. Sometimes open source is confusing is that regard.

— Reply to this email directly, view it on GitHub https://github.com/dvsekhvalnov/jose-jwt/pull/232#issuecomment-1820564524, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJZX32L37STW72RVW6MG3YFRZQPAVCNFSM6AAAAAA5AVNFFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRQGU3DINJSGQ . You are receiving this because you were mentioned.Message ID: @.***>

[digaomatias]

All right guys, sorry for the long delay. New year's January resolution is to finish this! 👯 I'll get the ball rolling this week.

[digaomatias]

I've finished working on the changes for the PR comments. If the unit tests work well on the Unix, it should be good to go.

[dvsekhvalnov]

yeah..looking, give some time :)

[dvsekhvalnov]

Code wise it looks good to me. Will take some time to play around on Linux and Mac and do cross-compatibility checks with other libraries.

If somebody have FreeBSD VM would be nice to try it too.

And then no reason not to merge & release :)

[digaomatias]

Take your time man!

Yeah, I hope it's all good everywhere. It will be great to have ECDH working fine on Unix platforms.

On Wed, Jan 17, 2024 at 7:16 AM DV @.***> wrote:

Code wise it looks good to me. Will take some time to play around on Linux and Mac and do cross-compatibility checks with other libraries.

If somebody have FreeBSD VM would be nice to try it too.

And then no reason not to merge & release :)

— Reply to this email directly, view it on GitHub https://github.com/dvsekhvalnov/jose-jwt/pull/232#issuecomment-1894268832, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJZXYNDMXQO4YVDP33MFDYO27ZRAVCNFSM6AAAAAA5AVNFFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJUGI3DQOBTGI . You are receiving this because you were mentioned.Message ID: @.***>

-- Abraço!

Rodrigo Matias Leote .NET Developer

"Whatever the mind can conceive and believe, the mind can achieve." - Dr. Napoleon Hill

[dvsekhvalnov]

Post some update here, still struggling to get it compiled, looks like ECCurve available only for .net 4.7 and .net standard 2.1, so it doesn't want to build older targets at the moment.

[dvsekhvalnov]

Need to figure out how to update your pull :)

@digaomatias can you enable allowed changes form maintainers for your PR? (maybe it's enabled by default though).

[digaomatias]

I couldn't find the option "allowed changes from maintainers" in the PR. I've added you as a contributor in my repository. Maybe that will allow you to commit to the branch from the PR.

On Fri, Jan 26, 2024 at 7:01 AM DV @.***> wrote:

Need to figure out how to update your pull :)

@digaomatias https://github.com/digaomatias can you enable allowed changes form maintainers for your PR? (maybe it's enabled by default though).

— Reply to this email directly, view it on GitHub https://github.com/dvsekhvalnov/jose-jwt/pull/232#issuecomment-1910721216, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJZX26P5NN6UBACYRIM4TYQKMWRAVCNFSM6AAAAAA5AVNFFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJQG4ZDCMRRGY . You are receiving this because you were mentioned.Message ID: @.***>

-- Abraço!

Rodrigo Matias Leote .NET Developer

"Whatever the mind can conceive and believe, the mind can achieve." - Dr. Napoleon Hill

[dvsekhvalnov]

Yeah, cool, i can update PR :) Fixed compilation on all targets, gonna test stuff now.

[digaomatias]

That's great news! Thanks mate!

On Wed, Jan 31, 2024 at 6:19 AM DV @.***> wrote:

Yeah, cool, i can update PR :) Fixed compilation on all targets, gonna test stuff now.

— Reply to this email directly, view it on GitHub https://github.com/dvsekhvalnov/jose-jwt/pull/232#issuecomment-1917526537, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJZX3WOX3DLV5FIUNJXQLYRETQPAVCNFSM6AAAAAA5AVNFFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJXGUZDMNJTG4 . You are receiving this because you were mentioned.Message ID: @.***>

-- Abraço!

Rodrigo Matias Leote .NET Developer

"Whatever the mind can conceive and believe, the mind can achieve." - Dr. Napoleon Hill

[JhaAEisenhour]

Anything I can do to help on this PR? I've little expertise here but I'm interested in this functionality and could at least run some tests.

[dvsekhvalnov]

Hey @JhaAEisenhour, well if you have freebsd vm, would be nice to try it :)

Was on PTO last week, but should get some time to move it forward given week.

[JhaAEisenhour]

@dvsekhvalnov Afraid I don't.

[dvsekhvalnov]

Interesting, found some tests are not passing on real Ubuntu VM, seems curves are named differently, ECDSA_P256 vs nistP256, gonna fix them.

[jerwinjha]

@dvsekhvalnov , do you have specific version/configuration guidelines for said FreeBSD VM? I might be able to put one together.

[dvsekhvalnov]

Hi @jerwinjha , guess nothing special, any version of FreeBSD that can run .netstandard 2.1 runtime to run tests on it.

[dvsekhvalnov]

Seems i finally fixed Linux tests, gonna try round over OSX now.

[dvsekhvalnov]

Got couple failed tests on OSX 🤦

Failed!  - Failed:     4, Passed:   375, Skipped:    40, Total:   419

cross platform is a joke..

[dvsekhvalnov]

I literally hate issues due to openssl mismatch versions :(

[dvsekhvalnov]

Look at that, all tests passing on Win/Ubuntu/MacOS :)

[digaomatias]

Amazing work!

I couldn’t have done that myself 😅

Thanks for all the effort!

Rodrigo Matias Leote .NET Developer

"Whatever the mind can conceive and believe, the mind can achieve." - Dr. Napoleon Hill

On Mon, 19 Feb 2024 at 11:59 PM, DV @.***> wrote:

Look at that, all tests passing on Win/Ubuntu/MacOS :)

— Reply to this email directly, view it on GitHub https://github.com/dvsekhvalnov/jose-jwt/pull/232#issuecomment-1952202822, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJZX4ZITJRN222PJAHZO3YUMWAZAVCNFSM6AAAAAA5AVNFFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJSGIYDEOBSGI . You are receiving this because you were mentioned.Message ID: @.***>

[dvsekhvalnov]

Wow, somehow it works on FreeBSD, at least on latest with net80:

Passed!  - Failed:     0, Passed:   380, Skipped:    39, Total:   419, Duration: 37 s - UnitTests.dll (net8.0)
[dv@freebsd]$ freebsd-version
14.0-RELEASE

okay, think we good, gonna do some cross-compatibility tests with other libraries.

[JhaAEisenhour]

That's awesome! When do you think it will merge in, if you were guessing?

[dvsekhvalnov]

Hey @JhaAEisenhour , well hope soonish :)

trying to make a progress every day, posting here details. Imo ~70% ready, doing cross-compatibility tests, then add some docs and ready for nuget.

[dvsekhvalnov]

Interesting, while back porting tests i found that ConcatKDF.DeriveEcdhKey() is probably implemented using wrong assumption that max key size bit is 256. While Nist521 curve key has 65-66 bytes length which is 528 bits max.

And therefore ECDH alg not working on P384 and P521 curves (yeah and corresponding unit tests were lacking too by other reason).

Need to look into.

[dvsekhvalnov]

Okay, i think i fixed ConcatKDF. The good news it's finally working correctly (e.g. cross-compatible) on nist P384 and P521 curves compared to unusable CNG implementation by Microsoft.

But have to put more unit tests now and may be need to default JWK implementation to new ecdh keys to make library fully compatible with default settings.

[dvsekhvalnov]

✅ Win NetFX cross compatibility tests ✅ Win NetCore cross compatibility tests

[dvsekhvalnov]

✅ OSX NetCore cross compatibility tests

2 more to go :)

[dvsekhvalnov]

✅ Linux NetCore cross compatibility tests

[dvsekhvalnov]

✅ FreeBSD NetCore cross compatibility tests

[dvsekhvalnov]

Okay, it looks amazing, definitely worth version 5 :)

Just docs update and we good to go.

[dvsekhvalnov]

While we are here, does anybody have feeling library need helpers to load all kinds of keys from common file formats, like .pem, .p12, e.t.c. ?

It's relatively trivial but some gotchas here and there if you want to export private parts, e.t.c.

Something like:

   ECDsa pk = EcdhKey.LoadPrivate("my.p12", "password");

[dvsekhvalnov]

Ok, i think that's it. About to merge it now :)

Want to say huge thank you to @polewskm and @digaomatias for great idea and bringing it to live. That's fixing 10yo compatibility issues with Microsoft ncrypt.dll and making library fully compliant on all major operating systems. Truly amazing.

Before we close it off, some stats: was around 70 commits and 6K lines changed, which is not an small fix by all means :)

P.S> Gonna release it soonish to nuget along with https://github.com/dvsekhvalnov/jose-jwt/issues/237 (should be quick 🤞), as you know zero tolerance to security issues ;)

[lucabarbi]

great job guys. Thank you very much

[digaomatias]

Amazing work! Well done!

Abraço!

Rodrigo Matias Leote .NET Developer

"Whatever the mind can conceive and believe, the mind can achieve." - Dr. Napoleon Hill

On Mon, 18 Mar 2024 at 10:17 PM, lucabarbi @.***> wrote:

great job guys

— Reply to this email directly, view it on GitHub https://github.com/dvsekhvalnov/jose-jwt/pull/232#issuecomment-2003286318, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJZX2WC4YTTOBZLRSB4YTYY2WJLAVCNFSM6AAAAAA5AVNFFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBTGI4DMMZRHA . You are receiving this because you were mentioned.Message ID: @.***>

[dvsekhvalnov]

v5.0.0 released to nuget.org

For those who in air gapped environments copy uploaded: https://github.com/dvsekhvalnov/jose-jwt/releases/tag/v5.0.0