misc: ecdsa raw signature #1457
Conversation
| } | ||
|
|
||
| public final class aws/smithy/kotlin/runtime/hashing/EcdsaJVMKt { | ||
| public static final fun ecdsaSecp256r1 ([B[B)[B |
There was a problem hiding this comment.
Existing ecdsaSecp256r1(key, message) calls work unchanged, should not be a breaking change
There was a problem hiding this comment.
This is definitely a breaking change because you've altered the method signature. Older consumers of runtime-core will encounter a NoSuchMethodError exception at runtime. The only way to do this safely without breaking binary compatibility is to maintain the old method signature and add a new one.
I recommend one of the following approaches:
- Expose a new method
ecdsaSecp256r1Rswhich invokesecdsaSecp256r1Rsand extracts therandsvalues. This could be implemented in common since it's just byte manipulation - Expose a new method
asn1DerToRswhich converts anyByteArrayfrom ASN.1 DER format to r||s. Callers would then invokeecdsaSecp256r1Rsfirst and thenasn1DerToRs. This could also be implemented in common.
xinsong-cui
added
no-changelog
ianbotsf
left a comment
ianbotsf
left a comment
There was a problem hiding this comment.
Correctness: Please add/update tests which verify actual values.
| } | ||
|
|
||
| public final class aws/smithy/kotlin/runtime/hashing/EcdsaJVMKt { | ||
| public static final fun ecdsaSecp256r1 ([B[B)[B |
There was a problem hiding this comment.
This is definitely a breaking change because you've altered the method signature. Older consumers of runtime-core will encounter a NoSuchMethodError exception at runtime. The only way to do this safely without breaking binary compatibility is to maintain the old method signature and add a new one.
I recommend one of the following approaches:
- Expose a new method
ecdsaSecp256r1Rswhich invokesecdsaSecp256r1Rsand extracts therandsvalues. This could be implemented in common since it's just byte manipulation - Expose a new method
asn1DerToRswhich converts anyByteArrayfrom ASN.1 DER format to r||s. Callers would then invokeecdsaSecp256r1Rsfirst and thenasn1DerToRs. This could also be implemented in common.
runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/hashing/Ecdsa.kt
Outdated
Show resolved
Hide resolved
| /** | ||
| * Parses an ASN.1 DER encoded ECDSA signature and converts it to raw r||s format. | ||
| */ | ||
| private fun parseDerSignature(derSignature: ByteArray): ByteArray { | ||
| var index = 2 // Skip SEQUENCE tag and length | ||
|
|
||
| // Read r | ||
| index++ // Skip INTEGER tag | ||
| val rLength = derSignature[index++].toInt() and 0xFF | ||
| val r = derSignature.sliceArray(index until index + rLength) | ||
| index += rLength | ||
|
|
||
| // Read s | ||
| index++ // Skip INTEGER tag | ||
| val sLength = derSignature[index++].toInt() and 0xFF | ||
| val s = derSignature.sliceArray(index until index + sLength) | ||
|
|
||
| // Remove leading zero bytes and pad to 32 bytes | ||
| val rFixed = r.dropWhile { it == 0.toByte() }.toByteArray() | ||
| val sFixed = s.dropWhile { it == 0.toByte() }.toByteArray() | ||
|
|
||
| val rPadded = if (rFixed.size < 32) ByteArray(32 - rFixed.size) + rFixed else rFixed | ||
| val sPadded = if (sFixed.size < 32) ByteArray(32 - sFixed.size) + sFixed else sFixed | ||
|
|
||
| return rPadded + sPadded | ||
| } |
There was a problem hiding this comment.
Question: Where did we find this algorithm? Can we link to a reliable source?
There was a problem hiding this comment.
I don't really find any available algorithm, I find this is helpful: the structure of DER (which tells where is r and s located in DER)
xinsong-cui
removed
the
acknowledge-api-break
| /** | ||
| * ECDSA on the SECP256R1 curve returning raw r||s format. | ||
| */ | ||
| public fun ecdsaSecp256r1Rs(key: ByteArray, message: ByteArray): ByteArray { | ||
| val derSignature = ecdsaSecp256r1(key, message) | ||
| return parseDerSignature(derSignature) | ||
| } |
There was a problem hiding this comment.
Can we create some unit tests for this?
There was a problem hiding this comment.
Yes, tests added.
Note: I have to disable signature verification test on JVM8 since SHA256withECDSAinP1363Format is not supported
runtime/runtime-core/jvm/test/aws/smithy/kotlin/runtime/hashing/EcdsaJVMTest.kt
Show resolved
Hide resolved
3 participants
Issue #
Description of changes
Adds support for generating ECDSA signatures in raw r||s format (64 bytes) in addition to the existing ASN.1 DER format.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.