Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Fix load pem private key with the PEM encoded data #162

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

g1im2
Copy link

@g1im2 g1im2 commented Jun 17, 2019

fix problem like this issues "pyca/cryptography#2730", and it also happened in here.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 43.623% when pulling f1c3d24 on g1im2:master into 0e50795 on google:master.

Copy link
Contributor

@greateggsgreg greateggsgreg left a comment

Choose a reason for hiding this comment

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

This definitely needs to be fixed. An alternative method is to open the files in binary (mode "rb") so we don't need to convert to bytes after the fact. Example:

with open(rsa_key_path + '.pub', 'rb') as rsa_pub_file:
       self.public_key = rsa_pub_file.read()

with open(rsa_key_path, 'rb') as rsa_prv_file:
       self.rsa_key = serialization.load_pem_private_key(
             rsa_prv_file.read(), None, default_backend())

@JeffLIrion
Copy link
Contributor

I created a new package adb-shell (see #167) and incorporated @greateggsgreg's changes into sign_cryptography.py.

However, the CryptographySigner.Sign method raises an exception, and in the test_sign test I'm actually checking that the Sign method is broken. By contrast, the corresponding tests for PycryptodomeAuthSigner.Sign and PythonRSASigner.Sign are identical (aside from the exception catching) and they pass just fine.

If you submit a pull request that fixes this issue, that would be much appreciated!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants