-
Notifications
You must be signed in to change notification settings - Fork 315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support for parsing implicit DER tags #356
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a suggestion, which you can add before merge or ignore.
# data with context-specific tag class | ||
cls.data_context_specific = b"\x86\x02\x0a\x0b" | ||
# data with private tag class | ||
cls.data_private = b"\xc6\x02\x0a\x0b" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably should have a test for length > 128 to make sure the llen encoding/decoding is happening correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is reusing common code for encoding and decoding length, so I'm quite sure that it will handle it correctly
we have test coverage for it here:
python-ecdsa/src/ecdsa/test_der.py
Lines 90 to 125 in 55d2b56
class TestReadLength(unittest.TestCase): | |
# DER requires the lengths between 0 and 127 to be encoded using the short | |
# form and lengths above that encoded with minimal number of bytes | |
# necessary | |
def test_zero_length(self): | |
self.assertEqual((0, 1), read_length(b"\x00")) | |
def test_two_byte_zero_length(self): | |
with self.assertRaises(UnexpectedDER): | |
read_length(b"\x81\x00") | |
def test_two_byte_small_length(self): | |
with self.assertRaises(UnexpectedDER): | |
read_length(b"\x81\x7f") | |
def test_long_form_with_zero_length(self): | |
with self.assertRaises(UnexpectedDER): | |
read_length(b"\x80") | |
def test_smallest_two_byte_length(self): | |
self.assertEqual((128, 2), read_length(b"\x81\x80")) | |
def test_zero_padded_length(self): | |
with self.assertRaises(UnexpectedDER): | |
read_length(b"\x82\x00\x80") | |
def test_two_three_byte_length(self): | |
self.assertEqual((256, 3), read_length(b"\x82\x01\x00")) | |
def test_empty_string(self): | |
with self.assertRaises(UnexpectedDER): | |
read_length(b"") | |
def test_length_overflow(self): | |
with self.assertRaises(UnexpectedDER): | |
read_length(b"\x83\x01\x00") |
While not needed for ECDSA/ECDH PKCS#8 files, it looks like it will be necessary for the PQC algorithms, so provide that too, so we can do some code reuse.