Add support for encoding/decoding compressed EC keys #57

huskcasaca · 2024-12-13T03:51:47Z

This PR add support for Compressed and Uncompressed(default) format under RAW EC Public Key.

JDK(BC)
WebCrypto
Apple (Not Supported)
OpenSSL3

huskcasaca · 2024-12-13T04:02:41Z

cryptography-providers/jdk/src/jvmMain/kotlin/algorithms/JdkEc.kt

+internal fun ECParameterSpec.decodePoint(bytes: ByteArray): ECPoint {
+    val fieldSize = curveOrderSize()
+    return when (bytes[0].toInt()) {
+        0x02, // compressed evenY
+        0x03, // compressed oddY
+             -> {
+            check(bytes.size == fieldSize + 1) { "Wrong compressed key size ${bytes.size}" }
+            val p = (curve.field as ECFieldFp).p
+            val a = curve.a
+            val b = curve.b
+            val x = BigInteger(1, bytes.copyOfRange(1, bytes.size))
+            var y = x.multiply(x).add(a).multiply(x).add(b).mod(p).modSqrt(p)
+            if (y.testBit(0) != (bytes[0].toInt() == 0x03)) {
+                y = p.subtract(y)
+            }
+            ECPoint(x, y)
+        }
+        0x04, // uncompressed
+             -> {
+            check(bytes.size == fieldSize * 2 + 1) { "Wrong uncompressed key size ${bytes.size}" }
+            val x = bytes.copyOfRange(1, fieldSize + 1)
+            val y = bytes.copyOfRange(fieldSize + 1, fieldSize + 1 + fieldSize)
+            ECPoint(BigInteger(1, x), BigInteger(1, y))
+        }
+        else -> error("Unsupported key type ${bytes[0].toInt()}")
+    }
+}
+
+internal fun BigInteger.modSqrt(p: BigInteger): BigInteger {
+    check(p.testBit(0) && p.testBit(1)) { "Unsupported curve modulus" }  // p ≡ 3 (mod 4)
+    return modPow(p.add(BigInteger.ONE).shiftRight(2), p) // Tonelli-Shanks
+}
+


BC is added as a provider without some API access so it requires manually calculating y.

The implementation looks legit, thanks!
The only thing I would like to add is some link to the reference based on what the implementation is written.
For example link to a https://www.secg.org/sec1-v2.pdf or anything else?

whyoleg

Thanks for the PR!
Looks nice!
Here is the first round of review. I still need to check JDK implementation, as it contains more hand-written logic :)

whyoleg · 2025-01-19T22:36:53Z

cryptography-providers-tests-api/src/commonMain/kotlin/support.kt

-        else                          -> null
+        format.name == "JWK" && !provider.isWebCrypto
+             -> "JWK key format"
+        format == EC.PublicKey.Format.RAW.Compressed && provider.isApple


could you please mention this in limitations of apple provider?

cryptography-providers/webcrypto/src/commonMain/kotlin/algorithms/WebCryptoEc.kt

whyoleg · 2025-02-19T19:30:56Z

cryptography-providers/openssl3/api/src/commonMain/kotlin/algorithms/Openssl3Ec.kt

+    val ecKey = checkError(EVP_PKEY_get1_EC_KEY(key))
+    val ecGroup = checkError(EC_KEY_get0_group(ecKey))
+    val ecPoint = checkError(EC_KEY_get0_public_key(ecKey))


Those functions are deprecated in openssl:

https://docs.openssl.org/3.1/man3/EC_KEY_new/#synopsis

https://docs.openssl.org/3.1/man3/EVP_PKEY_set1_RSA/#synopsis

Could you please replace them?

Looks like we need to find a way to provide OPENSSL_API_COMPAT to cinterop to hide those declarations to avoid usage them in future

FYI: I was able to enforce OPENSSL_NO_DEPRECATED in cinterop in eeea305 and now those declarations are not available.
This commit will be soon merged into main, so if you willing to proceed with changing those you can temporary cherry-pick it from whyoleg/dev branch

Sorry for the late reply. I will check when free.

No problem, take your time.
FYI: openssl changes mentioned are already in main, so you can just rebase.
FYI2: At the current moment, I plan to do a release in mid-June, so it would be really nice to include those changes in it, along with your other contribution of RIPEMD!

whyoleg

Thanks for the PR one more and sorry for long review.
Finally I was able to validate the JDK implementation and left some small remarks.
Let's resolve them, migrate OpenSSL implementation to use non-deprecated APIs and we are good to go!

whyoleg · 2025-05-20T20:27:45Z

cryptography-providers/jdk/src/jvmMain/kotlin/algorithms/JdkEc.kt

+            EC.PublicKey.Format.RAW            -> encodeToRaw(false)
+            EC.PublicKey.Format.RAW.Compressed -> encodeToRaw(true)


It would be nice to use named parameters here:

Suggested change

EC.PublicKey.Format.RAW -> encodeToRaw(false)

EC.PublicKey.Format.RAW.Compressed -> encodeToRaw(true)

EC.PublicKey.Format.RAW -> encodeToRaw(compressed = false)

EC.PublicKey.Format.RAW.Compressed -> encodeToRaw(compressed = true)

whyoleg · 2025-05-20T20:31:43Z

cryptography-providers/jdk/src/jvmMain/kotlin/algorithms/JdkEc.kt


+            if (compressed) {
+                val output = ByteArray(fieldSize + 1)
+                output[0] = if (!BigInteger(1, y).testBit(0)) 0x02 else 0x03


minor: maybe it’s possible to use key.w.affineY here? no need to re-create BigInteger from bytes, as we already have original value? :)

whyoleg · 2025-05-20T20:33:30Z

cryptography-providers/jdk/src/jvmMain/kotlin/algorithms/JdkEc.kt

-                val x = bytes.copyOfRange(1, fieldSize + 1)
-                val y = bytes.copyOfRange(fieldSize + 1, fieldSize + 1 + fieldSize)
-                val point = ECPoint(BigInteger(1, x), BigInteger(1, y))
+        // use RawEncodedKeySpec


this comment is a bit missleading, as there is no such class. Do you mind just dropping it?

whyoleg · 2025-05-20T20:46:58Z

cryptography-providers/jdk/src/jvmMain/kotlin/algorithms/JdkEc.kt

+internal fun ECParameterSpec.decodePoint(bytes: ByteArray): ECPoint {
+    val fieldSize = curveOrderSize()
+    return when (bytes[0].toInt()) {
+        0x02, // compressed evenY
+        0x03, // compressed oddY
+             -> {
+            check(bytes.size == fieldSize + 1) { "Wrong compressed key size ${bytes.size}" }
+            val p = (curve.field as ECFieldFp).p
+            val a = curve.a
+            val b = curve.b
+            val x = BigInteger(1, bytes.copyOfRange(1, bytes.size))
+            var y = x.multiply(x).add(a).multiply(x).add(b).mod(p).modSqrt(p)
+            if (y.testBit(0) != (bytes[0].toInt() == 0x03)) {
+                y = p.subtract(y)
+            }
+            ECPoint(x, y)
+        }
+        0x04, // uncompressed
+             -> {
+            check(bytes.size == fieldSize * 2 + 1) { "Wrong uncompressed key size ${bytes.size}" }
+            val x = bytes.copyOfRange(1, fieldSize + 1)
+            val y = bytes.copyOfRange(fieldSize + 1, fieldSize + 1 + fieldSize)
+            ECPoint(BigInteger(1, x), BigInteger(1, y))
+        }
+        else -> error("Unsupported key type ${bytes[0].toInt()}")
+    }
+}
+
+internal fun BigInteger.modSqrt(p: BigInteger): BigInteger {
+    check(p.testBit(0) && p.testBit(1)) { "Unsupported curve modulus" }  // p ≡ 3 (mod 4)
+    return modPow(p.add(BigInteger.ONE).shiftRight(2), p) // Tonelli-Shanks
+}
+


The implementation looks legit, thanks!
The only thing I would like to add is some link to the reference based on what the implementation is written.
For example link to a https://www.secg.org/sec1-v2.pdf or anything else?

huskcasaca added 2 commits December 13, 2024 11:28

Add support for compressed EC public keys on JDK(BC)

57ce1b1

Add support for compressed EC public keys on Openssl3

5bd40b5

huskcasaca commented Dec 13, 2024

View reviewed changes

huskcasaca added 4 commits December 13, 2024 18:16

Fix OpenSSL ECDH compressed key

7f3d070

Add support for compressed EC public keys on WebCrypto

77266e7

Add API for compressed ec public key format

75a39c4

Exclude compressed ec public key format from Apple

938d50c

huskcasaca marked this pull request as ready for review December 18, 2024 03:51

whyoleg self-requested a review January 19, 2025 22:32

whyoleg requested changes Feb 19, 2025

View reviewed changes

whyoleg linked an issue May 13, 2025 that may be closed by this pull request

Add support for encoding/decoding compressed EC keys #56

Open

whyoleg added this to cryptography-kotlin May 17, 2025

whyoleg removed this from cryptography-kotlin May 17, 2025

whyoleg requested changes May 20, 2025

View reviewed changes

whyoleg added the enhancement New feature or request label May 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for encoding/decoding compressed EC keys #57

Add support for encoding/decoding compressed EC keys #57

Uh oh!

huskcasaca commented Dec 13, 2024 •

edited

Loading

Uh oh!

huskcasaca Dec 13, 2024

Uh oh!

whyoleg May 20, 2025

Uh oh!

whyoleg left a comment

Uh oh!

whyoleg Jan 19, 2025

Uh oh!

Uh oh!

whyoleg Feb 19, 2025

Uh oh!

whyoleg Apr 24, 2025

Uh oh!

huskcasaca May 3, 2025

Uh oh!

whyoleg May 20, 2025

Uh oh!

whyoleg left a comment

Uh oh!

whyoleg May 20, 2025

Uh oh!

whyoleg May 20, 2025

Uh oh!

whyoleg May 20, 2025

Uh oh!

whyoleg May 20, 2025

Uh oh!

Uh oh!

		EC.PublicKey.Format.RAW -> encodeToRaw(false)
		EC.PublicKey.Format.RAW.Compressed -> encodeToRaw(true)

Add support for encoding/decoding compressed EC keys #57

Are you sure you want to change the base?

Add support for encoding/decoding compressed EC keys #57

Uh oh!

Conversation

huskcasaca commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whyoleg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whyoleg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

huskcasaca commented Dec 13, 2024 •

edited

Loading