From 3283b1284475cf6c79a7329aee8bd7443cc72672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 3 Jun 2020 14:39:23 +0200 Subject: Fix CVE-2020-13757: detect cyphertext modifications by prepending zero bytes Reject cyphertexts that have been modified by prepending zero bytes, by checking the cyphertext length against the expected size (given the decryption key). This resolves CVE-2020-13757. The same approach is used when verifying a signature. Thanks Carnil for pointing this out on https://github.com/sybrenstuvel/python-rsa/issues/146 --- CHANGELOG.txt | 3 +++ rsa/pkcs1.py | 9 +++++++++ tests/test_pkcs1.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 2704e69..96a380d 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -11,6 +11,9 @@ in `setup.py`. Two security fixes have also been backported, so 4.3 = 4.0 + these two fixes. - Choose blinding factor relatively prime to N. Thanks Christian Heimes for pointing this out. +- Reject cyphertexts (when decrypting) and signatures (when verifying) that have + been modified by prepending zero bytes. This resolves CVE-2020-13757. Thanks + Carnil for pointing this out. Version 4.0 - released 2018-09-16 diff --git a/rsa/pkcs1.py b/rsa/pkcs1.py index 84f0e3b..c05239a 100644 --- a/rsa/pkcs1.py +++ b/rsa/pkcs1.py @@ -234,6 +234,12 @@ def decrypt(crypto, priv_key): decrypted = priv_key.blinded_decrypt(encrypted) cleartext = transform.int2bytes(decrypted, blocksize) + # Detect leading zeroes in the crypto. These are not reflected in the + # encrypted value (as leading zeroes do not influence the value of an + # integer). This fixes CVE-2020-13757. + if len(crypto) > blocksize: + raise DecryptionError('Decryption failed') + # If we can't find the cleartext marker, decryption failed. if cleartext[0:2] != b'\x00\x02': raise DecryptionError('Decryption failed') @@ -331,6 +337,9 @@ def verify(message, signature, pub_key): cleartext = HASH_ASN1[method_name] + message_hash expected = _pad_for_signing(cleartext, keylength) + if len(signature) != keylength: + raise VerificationError('Verification failed') + # Compare with the signed one if expected != clearsig: raise VerificationError('Verification failed') diff --git a/tests/test_pkcs1.py b/tests/test_pkcs1.py index 5377b30..9f7dcea 100644 --- a/tests/test_pkcs1.py +++ b/tests/test_pkcs1.py @@ -17,6 +17,7 @@ """Tests string operations.""" import struct +import sys import unittest import rsa @@ -66,6 +67,37 @@ class BinaryTest(unittest.TestCase): self.assertNotEqual(encrypted1, encrypted2) +class ExtraZeroesTest(unittest.TestCase): + def setUp(self): + # Key, cyphertext, and plaintext taken from https://github.com/sybrenstuvel/python-rsa/issues/146 + self.private_key = rsa.PrivateKey.load_pkcs1( + "-----BEGIN RSA PRIVATE KEY-----\nMIIEowIBAAKCAQEAs1EKK81M5kTFtZSuUFnhKy8FS2WNXaWVmi/fGHG4CLw98+Yo\n0nkuUarVwSS0O9pFPcpc3kvPKOe9Tv+6DLS3Qru21aATy2PRqjqJ4CYn71OYtSwM\n/ZfSCKvrjXybzgu+sBmobdtYm+sppbdL+GEHXGd8gdQw8DDCZSR6+dPJFAzLZTCd\nB+Ctwe/RXPF+ewVdfaOGjkZIzDoYDw7n+OHnsYCYozkbTOcWHpjVevipR+IBpGPi\n1rvKgFnlcG6d/tj0hWRl/6cS7RqhjoiNEtxqoJzpXs/Kg8xbCxXbCchkf11STA8u\ndiCjQWuWI8rcDwl69XMmHJjIQAqhKvOOQ8rYTQIDAQABAoIBABpQLQ7qbHtp4h1Y\nORAfcFRW7Q74UvtH/iEHH1TF8zyM6wZsYtcn4y0mxYE3Mp+J0xlTJbeVJkwZXYVH\nL3UH29CWHSlR+TWiazTwrCTRVJDhEoqbcTiRW8fb+o/jljVxMcVDrpyYUHNo2c6w\njBxhmKPtp66hhaDpds1Cwi0A8APZ8Z2W6kya/L/hRBzMgCz7Bon1nYBMak5PQEwV\nF0dF7Wy4vIjvCzO6DSqA415DvJDzUAUucgFudbANNXo4HJwNRnBpymYIh8mHdmNJ\n/MQ0YLSqUWvOB57dh7oWQwe3UsJ37ZUorTugvxh3NJ7Tt5ZqbCQBEECb9ND63gxo\n/a3YR/0CgYEA7BJc834xCi/0YmO5suBinWOQAF7IiRPU+3G9TdhWEkSYquupg9e6\nK9lC5k0iP+t6I69NYF7+6mvXDTmv6Z01o6oV50oXaHeAk74O3UqNCbLe9tybZ/+F\ndkYlwuGSNttMQBzjCiVy0+y0+Wm3rRnFIsAtd0RlZ24aN3bFTWJINIsCgYEAwnQq\nvNmJe9SwtnH5c/yCqPhKv1cF/4jdQZSGI6/p3KYNxlQzkHZ/6uvrU5V27ov6YbX8\nvKlKfO91oJFQxUD6lpTdgAStI3GMiJBJIZNpyZ9EWNSvwUj28H34cySpbZz3s4Xd\nhiJBShgy+fKURvBQwtWmQHZJ3EGrcOI7PcwiyYcCgYEAlql5jSUCY0ALtidzQogW\nJ+B87N+RGHsBuJ/0cxQYinwg+ySAAVbSyF1WZujfbO/5+YBN362A/1dn3lbswCnH\nK/bHF9+fZNqvwprPnceQj5oK1n4g6JSZNsy6GNAhosT+uwQ0misgR8SQE4W25dDG\nkdEYsz+BgCsyrCcu8J5C+tUCgYAFVPQbC4f2ikVyKzvgz0qx4WUDTBqRACq48p6e\n+eLatv7nskVbr7QgN+nS9+Uz80ihR0Ev1yCAvnwmM/XYAskcOea87OPmdeWZlQM8\nVXNwINrZ6LMNBLgorfuTBK1UoRo1pPUHCYdqxbEYI2unak18mikd2WB7Fp3h0YI4\nVpGZnwKBgBxkAYnZv+jGI4MyEKdsQgxvROXXYOJZkWzsKuKxVkVpYP2V4nR2YMOJ\nViJQ8FUEnPq35cMDlUk4SnoqrrHIJNOvcJSCqM+bWHAioAsfByLbUPM8sm3CDdIk\nXVJl32HuKYPJOMIWfc7hIfxLRHnCN+coz2M6tgqMDs0E/OfjuqVZ\n-----END RSA PRIVATE KEY-----", + format='PEM') + cyphertext = "4501b4d669e01b9ef2dc800aa1b06d49196f5a09fe8fbcd037323c60eaf027bfb98432be4e4a26c567ffec718bcbea977dd26812fa071c33808b4d5ebb742d9879806094b6fbeea63d25ea3141733b60e31c6912106e1b758a7fe0014f075193faa8b4622bfd5d3013f0a32190a95de61a3604711bc62945f95a6522bd4dfed0a994ef185b28c281f7b5e4c8ed41176d12d9fc1b837e6a0111d0132d08a6d6f0580de0c9eed8ed105531799482d1e466c68c23b0c222af7fc12ac279bc4ff57e7b4586d209371b38c4c1035edd418dc5f960441cb21ea2bedbfea86de0d7861e81021b650a1de51002c315f1e7c12debe4dcebf790caaa54a2f26b149cf9e77d" + plaintext = "54657374" + + if sys.version_info < (3, 0): + self.cyphertext = cyphertext.decode("hex") + self.plaintext = plaintext.decode('hex') + else: + self.cyphertext = bytes.fromhex(cyphertext) + self.plaintext = bytes.fromhex(plaintext) + + def test_unmodified(self): + message = rsa.decrypt(self.cyphertext, self.private_key) + self.assertEqual(message, self.plaintext) + + def test_prepend_zeroes(self): + cyphertext = b'\00\00' + self.cyphertext + with self.assertRaises(rsa.DecryptionError): + rsa.decrypt(cyphertext, self.private_key) + + def test_append_zeroes(self): + cyphertext = self.cyphertext + b'\00\00' + with self.assertRaises(rsa.DecryptionError): + rsa.decrypt(cyphertext, self.private_key) + + class SignatureTest(unittest.TestCase): def setUp(self): (self.pub, self.priv) = rsa.newkeys(512) @@ -132,3 +164,21 @@ class SignatureTest(unittest.TestCase): signature = pkcs1.sign_hash(msg_hash, self.priv, 'SHA-224') self.assertTrue(pkcs1.verify(message, signature, self.pub)) + + def test_prepend_zeroes(self): + """Prepending the signature with zeroes should be detected.""" + + message = b'je moeder' + signature = pkcs1.sign(message, self.priv, 'SHA-256') + signature = b'\00\00' + signature + with self.assertRaises(rsa.VerificationError): + pkcs1.verify(message, signature, self.pub) + + def test_apppend_zeroes(self): + """Apppending the signature with zeroes should be detected.""" + + message = b'je moeder' + signature = pkcs1.sign(message, self.priv, 'SHA-256') + signature = signature + b'\00\00' + with self.assertRaises(rsa.VerificationError): + pkcs1.verify(message, signature, self.pub) -- cgit v1.2.1