1
\$\begingroup\$

I wondering if anyone could give me feedback on whether or not this is a secure way to implement AES/RSA hybrid encryption and signatures in python 3 with the PyCryptodome module.

All of the encoding and to/from hex format is just there because that's the format I intend to transfer it in.

"""Hybrid AES/RSA encryption, integrity, and repudiation proof of concept.
    Requires the PyCryptodome module but is imported as Crypto"""

from hashlib import sha512
from Crypto.Cipher import PKCS1_OAEP
from Crypto.Cipher import AES
from Crypto.PublicKey import RSA
from Crypto.Random import get_random_bytes


def generate_keys():
    """ Generates the rsa key pair and returns them.
        In actual usage private key is exported to .pem secured with a passphrase"""
    while True:
        privatekey = RSA.generate(2048)
        publickey = privatekey.publickey()

        return privatekey, publickey


def encrypt_msg(msg_body, publickey, privatekey):
    """Generates the session key, then creates the nonce and cipher.
       encrypts the message body with AES 256.
       encrypts the session key with RSA 2048
       returns the cipher text, tag, nonce, and the encrypted session key in hex format
       Hashes clear text message and creates signature"""
    session_key = get_random_bytes(32)

    aes_cipher = AES.new(session_key, AES.MODE_EAX)
    nonce = aes_cipher.nonce
    aes_cipher_text, tag = aes_cipher.encrypt_and_digest(msg_body)

    rsa_cipher = PKCS1_OAEP.new(publickey)
    enc_session_key = rsa_cipher.encrypt(session_key)

    msg_hash = int.from_bytes(sha512(msg_body).digest(), byteorder='big')
    signature = pow(msg_hash, privatekey.d, privatekey.n)

    return aes_cipher_text.hex(), tag.hex(), nonce.hex(), enc_session_key.hex(), signature


def decrypt_msg(aes_cipher_text, tag, nonce, enc_session_key, privatekey, publickey, signature):
    """encrypted session key is encoded and returned from hex
       Uses private key to decrypt the session key
       nonce is encoded and returned from hex
       cipher is created with supplied key, and nonce
       cipher text is decrypted
       tag is encoded and returned from hex
       message is checked for authenticity
       clear text is returned and authenticity status is returned
       hashes clear text and compares to signature
       """

    decrypt = PKCS1_OAEP.new(privatekey)
    session_key = decrypt.decrypt(enc_session_key.encode().fromhex(enc_session_key))

    aes_cipher = AES.new(session_key, AES.MODE_EAX, nonce=nonce.encode().fromhex(nonce))
    clear_text = aes_cipher.decrypt(aes_cipher_text.encode().fromhex(aes_cipher_text))

    try:
        aes_cipher.verify(tag.encode().fromhex(tag))
        authentic = True

    except ValueError:
        authentic = False

    msg_hash = int.from_bytes(sha512(clear_text).digest(), byteorder='big')
    hashfromsignature = pow(signature, publickey.e, publickey.n)
    if msg_hash == hashfromsignature:
        valid = True

    else:
        valid = False

    return clear_text, authentic, valid


TEST_MSG = b'this is a short test message.'

# Generates and assigns public and private keys
# Person A's key pair
PRIVATE_KEY, PUBLIC_KEY = generate_keys()
# Person B's key pair
PRIVATE_KEY_1, PUBLIC_KEY_1 = generate_keys()

# Encrypts the message, and creates the tag, nonce and encrypted session key
# Note same key pair is used only for testing purposes in implementation
# both sides will have own key pairs
msg_aes_cipher_text, msg_tag, msg_nonce, encrypted_session_key, signature = encrypt_msg(TEST_MSG, PUBLIC_KEY_1, PRIVATE_KEY)

# Decrypts the message with the supplied data and check for authenticity
msg_clear_text, authentic_status, valid = decrypt_msg(msg_aes_cipher_text, msg_tag, msg_nonce, encrypted_session_key, PRIVATE_KEY_1, PUBLIC_KEY, signature)
if valid:
    print('Valid signature.')
else:
    print('Invalid signature!')
if msg_tag:
    print('Valid tag.')
else:
    print('Invalid tag.')
print(msg_clear_text)
```
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Please do not update the code in questions to include feedback from answers. This breaks the question and answer nature of the site and makes the answers confusing. Thank you. \$\endgroup\$ – Peilonrayz Apr 9 '20 at 21:56
  • \$\begingroup\$ @greybeard Don't edit code in questions, any and all feedback should go in answers. \$\endgroup\$ – Peilonrayz Apr 9 '20 at 21:57
4
\$\begingroup\$

While this question is an off-topic programming question on Crypto.SE, I doubt whether it'll get a better answer on StackOverflow or Code Review.

The part worries me is this:

...
    msg_hash = int.from_bytes(sha512(clear_text).digest(), byteorder='big')
    hashfromsignature = pow(signature, publickey.e, publickey.n)
    if msg_hash == hashfromsignature:
        valid = True

    else:
        valid = False
...

You seem to be verifying unpadded textbook RSA signature. You should change this part to using RSA-PSS signature scheme - it's a newer RSA signature scheme in PKCS#1 ver2.

\$\endgroup\$
1
  • \$\begingroup\$ Updated the signature scheme to PSS based on your recommendations. \$\endgroup\$ – Joe Smith Apr 9 '20 at 21:47
2
\$\begingroup\$

Just something for clarify the code

if msg_hash == hashfromsignature:
    valid = True

else:
    valid = False

On an easy statement

valid = msg_hash == hashfromsignature

Also this while is not looping, not sure if you want to use a yield here

def generate_keys():
    while True:
        privatekey = RSA.generate(2048)
        publickey = privatekey.publickey()

        return privatekey, publickey

Could be rewrite as

def generate_keys():
    privatekey = RSA.generate(2048)
    publickey = privatekey.publickey()

    return privatekey, publickey

Not sure of your intention here with the while but I suspect that you wanted to use a generator function.

\$\endgroup\$
1
  • \$\begingroup\$ The while statement wasn't supposed to be there for this bit of code, in the full project there is a check that works with that while, however I just forgot to remove it when posting this code snippet. \$\endgroup\$ – Joe Smith Apr 9 '20 at 12:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.