Add pyadb keygen command to generate keypairs

[joeleong]

Should address #95 and #131

• (I think) I reimplemented the custom public key structure from AOSP.
• Add pyadb keygen command to mimic adb keygen

Note: I did update the dependencies since the pubkey code uses pycryptodome and six.
I was having problems similar to #109, so I think you need M2Crypto or rsa and pycryptodome (2 libraries). There is probably a way to implement the pubkey stuff with the other crypto libraries but I didn't bother. Happy to change things if necessary.

[coveralls]

Coverage decreased (-2.2%) to 41.385% when pulling 01614ea on joeleong:add-custom-pubkey-encoding into d0be33c on google:master.

[Halastra]

Let's help this thing reach master!

1. I'm not experienced in creating proper test cases, but I'm willing to try.
2. I have a minor improvement suggestion for struct usage in decode_pubkey (will post proposed patch soon).
3. I'd also like to try and make this work with the cryptography library (recently supported by this project).

Hopefully I'll be able to do it soon.
Thanks for your great work!

[Halastra]

Started a review for adb/android_pubkey.py.
This is currently just a suggested cleanup, to increase readability for future reviews.
I will soon be inspecting the RSA-related code, with regards to:

• The correctness of the algorithm
• Library usage (specifically, porting to cryptography if possible).

Let me know what you think!

[Halastra]

Verify endianness argument:
assert endianess in ('little', 'big')

[joeleong]

Hey! Thanks for the review. I like your changes overall.

The only ones I'm unsure about are the size_in changes.
I'm not sure it's much clearer that way and might just be adding some unnecessary machinery, especially because it looks like its only used in two places. I'm probably open to it if you feel strongly about it, but how would you feel about leaving it the way it is, but maybe adding some comments to make it clearer?

I'll take a closer look when I get a chance and probably incorporate most/all of them.
It should be pretty easy to port to the other crypto lib but I haven't looked at it specifically.
I can help with that if you'd like.
The tests are probably most important.

[Halastra]

Hi. I completely agree :)
Let me explain, and then tell me what you think. Maybe it's redundant after all.

The size_in function lets you convert sizes between different units.
In our case, we convert 2048 bits to 256 bytes and then 64 words.

But what if the modulus length is not a multiple of 32 or 8 bits?
e.g. 2047 bits would still require 256 bytes, and 2040 bits would require 255 bytes which are still 64 words.

The size_in is a simple formula which handles these cases.
I don't really know cryptography really well. Perhaps these cases are irrelevant by design.

Also agreed on the tests, probably most important. I'll try to get to those soon 👍

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[joeleong]

I rebased and incorporated most of your suggestions in 0791b9.
I think I'd prefer to skip the size_in suggestion for now, just because they're constants.
I also forgot to include your suggestion about asserting the endianess argument, but I don't think that's a big deal since it defaults to little if gibberish is passed and its a local function anyway.

I also ported it to use py-cryptography instead of pycryptodome.

Let me know what you think!

[Halastra]

@googlebot I consent

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[Halastra]

Well that took longer than expected.. but here goes.
A few quick notes:

• This test won't actually run on Python 3+ because of TypeError: from_buffer() cannot return the address of a unicode object #159 (I will address this issue in Update sign_cryptography.py #157 or a new PR)
• I'm misusing this platform because I don't know how to add commits to this PR. My apologies.
• This test is simple and naive, but should be enough to cover most of your added functionality.
• Let's hope TemporaryDirectory is allowed to run on the CI server :0

Add the following code under test/test_cryptography.py:

import unittest
from adb import sign_cryptography
from adb import android_pubkey
from tempfile import TemporaryDirectory
from os import path

class CryptographyTest(unittest.TestCase):
    def testKeygenAndSigner(self):
        private_key_file_name = 'adbkey'
        with TemporaryDirectory() as temp_dir:
            private_key_file_path = path.join(
                temp_dir, 
                private_key_file_name
            )
            android_pubkey.keygen(private_key_file_path)
            signer = sign_cryptography.CryptographySigner(private_key_file_path)

Regarding your previous notes:

• I really like your work!
• I might be tempted to add the size_in function in a future PR.
• Same goes for the endianess (+ better default to 'little', to match bytes.fromhex)

Please add the above changes. I will review and approve them (promise!).

[joeleong]

Hey thanks for your suggestions!

I haven't really been keeping up with this project, particularly #167 and all the stuff @JeffLIrion has been contributing, so it is a little unclear to me what the best path forward is. It's very possible his PR's could supersede this one.

I'm happy to incorporate your suggestions to this PR either directly or you can maybe submit a PR to https://github.com/joeleong/python-adb/tree/add-custom-pubkey-encoding and I'll push your commits here.

[Halastra]

You are right in your observations. His work is indeed an improvement in many areas!
But since there's still no apparent USB support in adb-shell, I'll be using this package.

So I guess I'll submit a PR to your repository as you suggested.

[JeffLIrion]

This pull request contains @joeleong's key generation code as well as tests: #170

I also copied it over to adb-shell (and attributed it to @joeleong), along with tests.

• keygen.py
• test_adb_device.py
• test_keygen.py
• Coverage of keygen.py