Adding CORECLR Cross-Platform support to ECDH-ES #232

digaomatias · 2023-09-20T23:44:29Z

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.

…tible.

… method supports both ECDiffieHellman and ECDsa.

Adding back the supported frameworks to the project's properties.

dvsekhvalnov · 2023-09-22T09:06:56Z

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 · 2023-09-25T13:41:42Z

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 · 2023-09-25T21:30:01Z

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 <#232 (comment)>, 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 · 2023-09-26T06:26:26Z

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

Added a TestSuiteUnix.cs file containing the Unix execution flow. Changed the TestSuite.cs to make the tests skippable if it's runninf on non-Windows platforms. Changed JWTSettings to load the proper ECDH strategy based on the OSPlatform.

digaomatias · 2023-10-16T05:32:30Z

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 · 2023-10-19T15:13:06Z

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

jimmyiv · 2023-10-24T15:47:28Z

jose-jwt/jwa/EcdhKeyManagementWinWithAesKeyWrap.cs

+
+namespace Jose
+{
+    public class EcdhKeyManagementWinWithAesKeyWrap : EcdhKeyManagementUnix


This should fix a couple of your tests.

Suggested change

public class EcdhKeyManagementWinWithAesKeyWrap : EcdhKeyManagementUnix

public class EcdhKeyManagementWinWithAesKeyWrap : EcdhKeyManagementWin

jimmyiv · 2023-10-24T18:50:45Z

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

cyungmann · 2023-10-31T14:15:54Z

@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 · 2023-11-03T01:03:01Z

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

dvsekhvalnov · 2023-11-03T15:06:12Z

Thanks, i'll take a look next week.

dvsekhvalnov · 2023-09-25T11:23:33Z

jose-jwt/jws/netstandard1_4/EcdsaUsingSha.cs

Were there any real changes in the file? Sounds like formatting only (though i can't even figure out a difference)?

dvsekhvalnov · 2023-09-25T13:28:35Z

jose-jwt/jwa/EcdhKeyManagement.cs

+                if (jwk.Kty == Jwk.KeyTypes.EC)
+                {
+                    privateKey = jwk.EcDiffieHellmanKey();
+                }
            }


Not checking for ECDsa ? Missing symmetry with wrap flow.

dvsekhvalnov · 2023-09-25T13:33:31Z

jose-jwt/jwa/EcdhKeyManagement.cs

+        {
+            throw new JoseException("(Direct) ECDH-ES key management cannot use existing CEK.");
+        }
+
        private byte[] NewKey(int keyLength, object key, IDictionary<string, object> header)


Sounds like the only differentiator here is OS: Win vs Everything else? We can't branch based on key type (CngKey vs ECDiffieHellman) because JWK hiding actual implementation?

dvsekhvalnov · 2023-11-07T16:31:35Z

jose-jwt/crypto/ConcatKDF.cs

+#endif
+        }
+
+        public static byte[] DeriveKeyNonCng(ECDiffieHellman externalPubKey, ECDiffieHellman privateKey, int keyBitLength, byte[] algorithmId, byte[] partyVInfo, byte[] partyUInfo, byte[] suppPubInfo)


funny name :)

hahaha true. I'll change to make it more explicit, like DeriveEcdhKey

jose-jwt/jwa/EcdhKeyManagementWin.cs

dvsekhvalnov · 2023-11-07T16:42:17Z

jose-jwt/keys/EccKey.cs

Same here, any functional changes?

It used to have changes, but it was reverted.
We can discard it.

dvsekhvalnov · 2023-11-07T16:46:49Z

UnitTests/TestSuiteUnix.cs

can't we somehow have single TestSuite.cs (e.g. not splited by OS type) leveraging xUnit conditions?

Something like: https://josephwoodward.co.uk/2019/01/skipping-xunit-tests-based-on-runtime-conditions

Maintaining 2 suites by 250+ tests quite challenging.

dvsekhvalnov · 2023-11-07T16:49:40Z

UnitTests/jwk/JwkUnixTest.cs

Same comment as with TestSuite.cs. Can we collapse JwkTest.cs and JwkUnixTest.cs into JwkTest.cs with xUnit conditions?

dvsekhvalnov · 2023-11-07T16:50:20Z

UnitTests/jwe/JweUnixTest.cs

Same comment as with TestSuite.cs. Can we collapse JweTest.cs and JweUnixTest.cs into JweTest.cs with xUnit conditions?

UnitTests/SecurityVulnerabilitiesTest.cs

dvsekhvalnov · 2023-11-07T16:56:33Z

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 · 2023-11-20T21:09:34Z

Anything I can do to help move this along?

dvsekhvalnov · 2023-11-21T09:42:20Z

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 · 2023-11-21T10:01:33Z

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 <#232 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABJZX32L37STW72RVW6MG3YFRZQPAVCNFSM6AAAAAA5AVNFFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRQGU3DINJSGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

digaomatias · 2024-01-07T21:06:03Z

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.

…CDH and CngKey test runs on windows, where test will run only with ECDH on Unix. Resolving other PR Comments.

digaomatias · 2024-01-08T06:16:52Z

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.

…W AES-CBC JWK unit tests

…tests

… unit tests

dvsekhvalnov · 2024-03-13T16:32:33Z

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

dvsekhvalnov · 2024-03-13T18:23:17Z

✅ OSX NetCore cross compatibility tests

2 more to go :)

dvsekhvalnov · 2024-03-14T18:04:07Z

✅ Linux NetCore cross compatibility tests

dvsekhvalnov · 2024-03-14T18:24:15Z

✅ FreeBSD NetCore cross compatibility tests

dvsekhvalnov · 2024-03-14T18:25:25Z

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

Just docs update and we good to go.

dvsekhvalnov · 2024-03-15T16:51:08Z

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 · 2024-03-18T09:10:12Z

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 #237 (should be quick 🤞), as you know zero tolerance to security issues ;)

lucabarbi · 2024-03-18T09:16:46Z

great job guys. Thank you very much

digaomatias · 2024-03-18T20:54:12Z

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 <#232 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABJZX2WC4YTTOBZLRSB4YTYY2WJLAVCNFSM6AAAAAA5AVNFFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBTGI4DMMZRHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

dvsekhvalnov · 2024-04-03T18:30:05Z

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

Rodrigo Leote and others added 7 commits September 18, 2023 16:18

Changing the code to support Linux encryption.

3bd8fe5

Replacing CngKey for ECDiffieHellman to make it fully .Net Core compa…

cd7e464

…tible.

Changing Skippable unit tests to run on the MAC.

d9e958a

Adding ECDsa handling to the EcdhKeyManagement.cs, since the original…

20e716a

… method supports both ECDiffieHellman and ECDsa.

Fixing unit tests on Windows.

d7bb64e

Removing ECDiffieHellman from EcdsaUsingSha.cs.

5e60e69

Adding back the supported frameworks to the project's properties.

Adding to README

83cbb5a

jimmyiv reviewed Oct 24, 2023

View reviewed changes

Finishing changes to allow for Linux and Windows ECDH key handling.

b6b9806

dvsekhvalnov reviewed Nov 7, 2023

View reviewed changes

Rodrigo Leote added 3 commits January 8, 2024 18:19

Removing Unix TestSuite and using SkippableTheories to support both E…

e8c8b20

…CDH and CngKey test runs on windows, where test will run only with ECDH on Unix. Resolving other PR Comments.

Added InvalidCurveAttack test for the ECDH key implementation.

18524b8

Unifying the JWE and JWK tests for unix and windows implementations.

cbabc0b

dvsekhvalnov added 12 commits March 7, 2024 20:28

make last bits of encryption with Jwk to use Ecdh by default, ECDH AK…

410935f

…W AES-CBC JWK unit tests

Added ECDH_ES-AKW-GCM JWK encryption tests

226030a

Support for ECDSa keys in ECDH encryption, ECDH AES-CBC and GCM unit …

4b23562

…tests

fixed .net4.6 compilation

6b4caa7

Support for ECDSa keys in ECDH KW encryption, ECDH KW AES-CBC and GCM…

2302f24

… unit tests

Support for ECDSa keys in ECDH decryption, ECDH AES-CBC unit tests

8215f94

ECDSa ECDH AES-GCM unit tests

8e0f169

ECDSa ECDH KW AES-CBC unit tests

5ff04a1

ECDSa ECDH KW AES-GCM unit tests

dc54f51

fixed compilation errors

f1e9fea

minor touches, refactoring EccKeyUnix into EcdhKeys, test updates

1bd993f

fixed .net4.6 project file

00be482

Reverted NETCOREAPP3 conditional compilation, as targeting broken

c2aa7a8

dvsekhvalnov added 2 commits March 15, 2024 15:58

docs

704afb4

docs

24b3a89

dvsekhvalnov added 4 commits March 15, 2024 19:57

docs

840c634

docs

e01d38a

docs

fbc565b

docs

5ba7b57

dvsekhvalnov merged commit 8912025 into dvsekhvalnov:master Mar 18, 2024
12 checks passed

	public class EcdhKeyManagementWinWithAesKeyWrap : EcdhKeyManagementUnix
	public class EcdhKeyManagementWinWithAesKeyWrap : EcdhKeyManagementWin

Adding CORECLR Cross-Platform support to ECDH-ES #232

Adding CORECLR Cross-Platform support to ECDH-ES #232

Conversation

digaomatias commented Sep 20, 2023

dvsekhvalnov commented Sep 22, 2023

dvsekhvalnov commented Sep 25, 2023 • edited Loading

digaomatias commented Sep 25, 2023 via email

dvsekhvalnov commented Sep 26, 2023

digaomatias commented Oct 16, 2023

dvsekhvalnov commented Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

jimmyiv commented Oct 24, 2023

cyungmann commented Oct 31, 2023

digaomatias commented Nov 3, 2023

dvsekhvalnov commented Nov 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvsekhvalnov commented Nov 7, 2023

cyungmann commented Nov 20, 2023

dvsekhvalnov commented Nov 21, 2023

digaomatias commented Nov 21, 2023 via email

digaomatias commented Jan 7, 2024

digaomatias commented Jan 8, 2024

dvsekhvalnov commented Mar 13, 2024 • edited Loading

dvsekhvalnov commented Mar 13, 2024

dvsekhvalnov commented Mar 14, 2024

dvsekhvalnov commented Mar 14, 2024

dvsekhvalnov commented Mar 14, 2024

dvsekhvalnov commented Mar 15, 2024

dvsekhvalnov commented Mar 18, 2024

lucabarbi commented Mar 18, 2024 • edited Loading

digaomatias commented Mar 18, 2024 via email

dvsekhvalnov commented Apr 3, 2024

dvsekhvalnov commented Sep 25, 2023 •

edited

Loading

dvsekhvalnov commented Oct 19, 2023 •

edited

Loading

dvsekhvalnov commented Mar 13, 2024 •

edited

Loading

lucabarbi commented Mar 18, 2024 •

edited

Loading