Skip to content
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

Support for elliptic curve private keys #158

Merged
merged 4 commits into from
Dec 24, 2023
Merged

Conversation

schorschii
Copy link
Contributor

Currently, when trying to use a elliptic curve key to sign a PDF, the following error appears:

  File "/home/georg/.local/lib/python3.10/site-packages/endesive/pdf/cms.py", line 990, in sign
    return cls.sign(
  File "/home/georg/.local/lib/python3.10/site-packages/endesive/pdf/cms.py", line 679, in sign
    contents = signer.sign(
  File "/home/georg/.local/lib/python3.10/site-packages/endesive/signer.py", line 360, in sign
    signed_value_signature = key.sign(
TypeError: _EllipticCurvePrivateKey.sign() takes 3 positional arguments but 4 were given

This change adds support for elliptic curve private keys. It works in general but sometimes (using the same certificate, PDF file and code) the following error appears:

Traceback (most recent call last):
  File "/home/georg/.local/lib/python3.10/site-packages/endesive/pdf/cms.py", line 990, in sign
    return cls.sign(
  File "/home/georg/.local/lib/python3.10/site-packages/endesive/pdf/cms.py", line 782, in sign
    assert len(zeros) == len(contents)
AssertionError

I'm not sure where the problem is here. My guess is that it is something related with the timestamp used for signing. Maybe you can help with this issue?

@m32
Copy link
Owner

m32 commented Dec 21, 2023

the change looks ok, but please add a test to verify the change, e.g. in tests/test_pdf.py, place the test key and certificate in tests/fixtures

assert len(zeros) == len(contents)
is used to check whether the generated signature fits within the declared buffer.
The buffer size is controlled by the aligned parameter, if it is not provided, endesive generates the signature twice. The second time, the signature size must be identical to first one.

@schorschii
Copy link
Contributor Author

I tried to execute the existing tests before implementing a new test, but I get the following error. Can you please give me a hint how to get them working?

$ python -m unittest
EEEEE
======================================================================
ERROR: test_cert (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_cert
Traceback (most recent call last):
  File "/usr/lib/python3.10/unittest/loader.py", line 436, in _find_test_path
    module = self._get_module_from_name(name)
  File "/usr/lib/python3.10/unittest/loader.py", line 377, in _get_module_from_name
    __import__(name)
  File "/media/georg/Data2/Sourcecode/Linux/endesive/tests/test_cert.py", line 54, in <module>
    class CA(object):
  File "/media/georg/Data2/Sourcecode/Linux/endesive/tests/test_cert.py", line 255, in CA
    def pk12_load(self, fname: str, password: str) -> pkcs12.PKCS12KeyAndCertificates:
AttributeError: module 'cryptography.hazmat.primitives.serialization.pkcs12' has no attribute 'PKCS12KeyAndCertificates'

@m32
Copy link
Owner

m32 commented Dec 22, 2023

I run tests and coverage via"
python3 -m coverage run
--omit
"endesive/pdf/PyPDF2/",
"endesive/pdf/PyPDF2_annotate/
",
"endesive/pdf/fpdf/*",
"endesive/pdf/pdf.py"
-m unittest discover tests

Ran 37 tests in 7.692s
cryptography.hazmat.primitives.serialization.pkcs12 has no attribute PKCS12KeyAndCertificates - something with cryptography version, mine is 41.0.5

@schorschii
Copy link
Contributor Author

schorschii commented Dec 23, 2023

Thanks, I used the cryptography lib v3.4.8 from my Ubuntu system. After I upgraded to 41.0.7 via pip, the test error was gone.

I added tests for signing and verifying with EC keys. This perfectly demonstrates the AssertionError mentioned in my first post. It sometimes works, but one minute later, it fails. One minute later, it starts working again and so on. Do you have any idea how to fix this?

@m32
Copy link
Owner

m32 commented Dec 23, 2023

Best answer for AssertionError
https://stackoverflow.com/questions/68959590/why-does-ecdsa-produce-different-signatures-for-the-same-data-whereas-rsa-doesn

https://transactionfee.info/charts/bitcoin-script-ecdsa-length/
Signature Hash results in a signature length of 71 bytes. If either the r or the S-value has the highest bit set then it needs extra padding of 1 byte which results in a 72-byte-signature. If the highest bits of both values are set, then both need padding of one byte each resulting in a 73-byte-signature.

The simplest workaround is to set the value for the aligned variable to something other than zero, in the given test 4096 is sufficient.
Maybe it is worth forcing this parameter (aligned) to be set for this type of key? What do you think ?

@schorschii
Copy link
Contributor Author

Thanks for the explanation. I added 4096 as default for EC keys and adjusted the test dct. It works now reliably.

@m32 m32 merged commit 3a18bea into m32:master Dec 24, 2023
@schorschii schorschii deleted the ec-support branch December 27, 2023 20:29
@schorschii
Copy link
Contributor Author

Do you plan to create a new release? I'd like to use EC keys with your library.

@m32
Copy link
Owner

m32 commented Jan 7, 2024

sorry, I forgot about the tag in git, 2.17.0 is in pypi

@schorschii
Copy link
Contributor Author

Thanks!

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.

None yet

2 participants