Bugfix - sign_cryptography.CryptographySigner - keyfile opened as text (breaks on Python 3)

[Halastra]

This should fix an issue caused by the CryptographySigner initializer method,
which opens the RSA public/private key files for reading in "text mode" instead of "binary mode".

Python 2.x does work with this, but on Python 3 this breaks pretty quickly.

[coveralls]

Coverage increased (+0.3%) to 44.039% when pulling 50ce722 on Halastra:bugfix_sign_cryptography into f4e597f on google:master.

[JeffLIrion]

The signer still doesn't work. I have these changes in adb-shell, but the Sign() method raises an exception.

• sign_cryptography.py
• test_sign_cryptography.py (notice that the test asserts that an exception was raised)

[Halastra]

That's odd. I have tested ConnectDevice on multiple machines using cryptography and it's always worked for me.
I assumed that meant the signer was working.
An you elaborate on the details of the failure?

[JeffLIrion]

I haven't investigated why the test fails, but here's the Travis CI output from this draft pull request:

2215======================================================================
2216ERROR: test_sign (tests.test_sign_cryptography.TestCryptographySigner)
2217Test that the ``Sign`` method does not raise an exception.
2218----------------------------------------------------------------------
2219Traceback (most recent call last):
2220  File "/home/travis/build/JeffLIrion/adb_shell/tests/test_sign_cryptography.py", line 19, in test_sign
2221    self.signer.Sign(b'notadb')
2222  File "/home/travis/build/JeffLIrion/adb_shell/adb_shell/auth/sign_cryptography.py", line 84, in Sign
2223    return self.rsa_key.sign(data, padding.PKCS1v15(), utils.Prehashed(hashes.SHA1()))
2224  File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/cryptography/hazmat/backends/openssl/rsa.py", line 416, in sign
2225    self._backend, data, algorithm
2226  File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/cryptography/hazmat/backends/openssl/utils.py", line 48, in _calculate_digest_and_algorithm
2227    "The provided data must be the same length as the hash "
2228ValueError: The provided data must be the same length as the hash algorithm's digest size.

[Halastra]

Alright, I'm not really crypto-savvy, but I looked a bit into it.
Indeed, passing 'notadb' to CryptographySigner.Sign fails.

So, I went and debugged python-adb with cryptography, to try and see why it worked.
It seems the banner is read from the device and then signed.
As it happens, banner is the right size and signer.Sign(banner) succeeds. I assume the Android device just sends you a hash, which is always the right size.

But why does it fail when it's not the right size?
I found this answer on stackoverflow, which led me to the explanation.

Notice how you intantiate the signer object, with utils.Prehashed(hashes.SHA1()). Apparently, this means the signer is told to expect a prehashed value (i.e. a hash), so naturally it should fail when it gets 'notadb'.

My conclusion is that there is no bug in CryptographySigner.Sign.
You're just supposed to be signing a prehashed blob (read from the device), not any raw arbitrary data.
If anything, your test should check that the signer does fail with input such as 'notadb'.

[JeffLIrion]

Thanks for investigating this! I fixed my unit test:

https://github.com/JeffLIrion/adb_shell/blob/cf246bfdc7f023d466fb4a2d899967c61c857af6/tests/test_sign_cryptography.py#L23