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

Add public/private key encryption and decryption #110

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

yashasvi-ranawat
Copy link
Contributor

Uses ECIES encryption decryption method; AES-128-CBC with PKCS7 is used as the cipher; hmac-sha256 is used as the mac

Uses ECIES encryption decryption method; AES-128-CBC with PKCS7 is used as the cipher; hmac-sha256 is used as the mac
Copy link
Member

@merc1er merc1er left a comment

Choose a reason for hiding this comment

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

That looks great! Thank you.
It should probably include a section for it in the docs, but I can take care of writing this :)

I might have someone else to review this before merging, as I am not knowledgeable when it comes to cryptography.

bitcash/crypto.py Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Dec 28, 2021

I don't think that this is BitCash specific and might make the most sense to have it in its own repository and Python package. If I'm not mistaken, would this code work for Bitcoin, Bitcoin Cash, and Bitcoin SV?

@yashasvi-ranawat
Copy link
Contributor Author

@sometato
No, this is not BitCash specific. Would work with any Elliptical Curve based public/private key pair crypto.

However, it adds feature parity with the Electron-Cash app. Users of BitCash get the simplicity of public/private key based encryption/decryption. Such encryption/decryption methods are very common, and have allied applications.

@merc1er
Although I don't mind waiting for the review, you can also test out the feature parity with the Electron-cash app.
>>> key = wif_to_key('cU6s7jckL3bZUUkb3Q2CD9vNu8F1o58K5R5a3JFtidoccMbhEGKZ')
>>> key.public_key.hex()
'033d5c2875c9bd116875a71a5db64cffcb13396b163d039b1d9327824891804334'
This hex of public key can be used in the Electron-Cash app to encrypt any message.
>>> encrypted_message = <string of encrypted message from the electron-cash app>
>>> key.decrypt_message(encrypted_message)
<the message in its binary format>

@@ -20,3 +23,110 @@ def ripemd160_sha256(bytestr):


hash160 = ripemd160_sha256


def sha512(bytestr):
Copy link

Choose a reason for hiding this comment

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

I think that only the minimum should contain public methods. This could be _sha512().

return base64.b64encode(encrypted + mac)


def ecies_decrypt(encrypted, secret):
Copy link

Choose a reason for hiding this comment

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

Might consider making all functions except ecies_* private, or do bitcash/_crypto.py and only expose the wallet methods. Either way is good with me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made aes_* functions private, since they're only used by ecies_* functions -- which should be public. But the other functions should be left public, since they expose hashlib functions in a simpler way, as used by Bitcash in other functions/classes.

@@ -48,7 +48,7 @@
'Programming Language :: Python :: Implementation :: PyPy'
],

install_requires=['coincurve>=4.3.0', 'requests'],
install_requires=['coincurve>=4.3.0', 'requests', 'pyaes'],
Copy link

Choose a reason for hiding this comment

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

It's very cool that pyaes has no dependencies, but it's a bit dated and possibly prone to timing attacks. I'd just like to point that out.

Is this what Electron Cash uses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Electron cash uses cryptodome; however, falls back to pyaes if cryptodome import fails.

Pyaes is simply a pythonic implementation of AES-128. I think, mitigating attack such as timing attack would be more relevant on how bitcash -- and apps/implementation that use bitcash -- uses pyaes.
So mostly, any implementation that uses bitcash's ECIES would need to mitigate timing attacks by choosing when it declares the message to be invalid, right when bitcash tells it that AES key/iv is bad, or when it further verifies the content of the "successfully" decrypted data to be bad.

@merc1er merc1er changed the title ENH: Added public/private key encryption and decryption Add public/private key encryption and decryption Apr 26, 2023
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.

2 participants