Skip to content

Conversation

@gstarovo
Copy link
Contributor

The changes expand encoding/decoding of the ECDSA signature in DER format: expanded ECDSA-Sig-Value format and ECDSA-Full-R.
The definition of the formats can be found in document section C.5, page 114.

The changes bring supporting functions like encoding and decoding of boolean according to ASN.1 standard.
Beside that, the changes add parameter and slightly modifies ECDSA signing function for enabling return of the whole point (instead of 'r').
The testing coverage of encoding/decoding with new function is also present.

Also, there are some formatting changes made with black formatter.

@gstarovo
Copy link
Contributor Author

I will provide more coverage for the newly added code, as the results of the checks indicate coverage decline.

@gstarovo gstarovo force-pushed the ecdsa-sig-extend branch 5 times, most recently from 0961801 to e17d333 Compare October 23, 2025 09:39
@gstarovo gstarovo force-pushed the ecdsa-sig-extend branch 2 times, most recently from 19ac7b2 to f173f56 Compare October 27, 2025 11:58
@gstarovo gstarovo force-pushed the ecdsa-sig-extend branch 4 times, most recently from 627de8b to 9c23305 Compare October 27, 2025 16:09
"""
if not string:
raise UnexpectedDER(
"Empty string is an invalid " "encoding of a boolean"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to split it?

body = string[1 + lengthlength : 1 + lengthlength + length]
rest = string[1 + lengthlength + length :]
if not body:
raise UnexpectedDER("Empty object identifier")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not an object identifier

if body == b"\x00":
return False, rest
# the workaround due to instrumental, that
# saves the binary data as UF-8 string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method shouldn't be passed Unicode strings ever...

point = ellipticcurve.AbstractPoint.from_bytes(
self.generator.curve(), r
)
r = point[0] % n
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change though?

:param accelerate: an indicator for ECDSA sign operation to return
an ECPoint instead of a number of "r" parameter.
Applicable only for ECDSA key.
:type accelerate: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we already are using the sigencode for specifying the output format...

:param accelerate: an indicator for ECDSA sign operation to return
an ECPoint instead of a number of "r" parameter.
Applicable only for ECDSA key.
:type accelerate: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, sigencode should specify the format we want...

leak the key. Caller should try a better entropy source, retry with
different ``k``, or use the
:func:`~SigningKey.sign_deterministic` in such case.
:param accelerate: an indicator for ECDSA sign operation to return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

self.assertIn("Empty object identifier", str(e.exception))

def test_several_boolean_octets(self):
data = b"\x01\x02\x01\x01"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you also test with b"\x01\x00 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants