From: Philipp Gesang Date: Thu, 16 Apr 2020 15:40:08 +0000 (+0200) Subject: clarify possible IV reuse with multiple Encrypt handles X-Git-Tag: v2.2~1^2~7 X-Git-Url: http://developer.intra2net.com/git/?p=python-delta-tar;a=commitdiff_plain;h=66b1c6f4ef81f0c6b3e3af491f016ae2572a22d3 clarify possible IV reuse with multiple Encrypt handles Address Deltatar audit item 2.2: Unsafe to create more than 2^32 instances of Encrypt using the same key and salt Extend the documentation section on handling of IVs with a paragraph concerning multiple Encrypt handles. Also add a method to obtain the list of IVs for checks performed by the caller. --- diff --git a/deltatar/crypto.py b/deltatar/crypto.py index 3c3a816..388e9c7 100755 --- a/deltatar/crypto.py +++ b/deltatar/crypto.py @@ -70,6 +70,14 @@ e. g. when traversing the same object multiple times. Since the crypto context has no notion of a position in a PDT encrypted archive, this condition must be sorted out downstream. +When encrypting with more than one Encrypt context special care must be taken +to prevent accidental reuse of IVs. The builtin protection against reuse is +only effective for objects encrypted with the same Encrypt handle. If multiple +Encrypt handles are used to encrypt with the same combination of password and +salt, the encryption becomes susceptible to birthday attacks (bound = 2^32 due +to the 64-bit random iv). Thus the use of multiple handles is discouraged. + + Command Line Utility ------------------------------------------------------------------------------- @@ -971,7 +979,7 @@ class Crypto (object): elif cnt == AES_GCM_IV_CNT_INDEX: if self.index_counter_used is True: raise InvalidFileCounter ("attempted to reuse index file " - " counter %d: must be unique" % cnt) + "counter %d: must be unique" % cnt) self.index_counter_used = True if cnt <= AES_GCM_IV_CNT_MAX: self.cnt = cnt @@ -1091,6 +1099,15 @@ class Crypto (object): self.enc = None + def get_used_ivs (self): + """ + Get the set of IVs that were used so far during the lifetime of + this context. Useful to check for IV reuse if multiple encryption + contexts were used independently. + """ + return self.used_ivs + + class Encrypt (Crypto): lastinfo = None @@ -1103,7 +1120,6 @@ class Encrypt (Crypto): The ctor will throw immediately if one of the parameters does not conform to our expectations. - counter=AES_GCM_IV_CNT_DATA, strict_ivs=True): :type version: int to fit uint16_t :type paramversion: int to fit uint16_t :param password: mutually exclusive with ``key`` @@ -1117,6 +1133,16 @@ class Encrypt (Crypto): and cannot be reused even with different fixed parts. :type strict_ivs: bool :type insecure: bool, whether to permit passthrough mode + + *Security considerations*: The ``class Encrypt`` handle guarantees that + all random parts (first eight bytes) of the IVs used for encrypting + objects are unique. This guarantee does *not* apply across handles if + multiple handles are used with the same combination of password and + salt. Thus, use of multiple handles with the same combination of password + and salt is subject to birthday attacks with a bound of 2^32. To avoid + collisions, the application should keep the number of handles as low + as possible and check for reuse by comparing the set of IVs used of all + handles that were created (accessible using the ``get_used_ivs`` method). """ if password is None and key is None \ or password is not None and key is not None : @@ -1333,7 +1359,8 @@ class Decrypt (Crypto): ``AES_GCM_IV_CNT_INDEX`` are unique in each backup set and cannot be reused even with different fixed parts. :type fixedparts: bytes list - :type insecure: bool, whether to process objects encrypted in + :type insecure: bool + :param insecure: whether to process objects encrypted in passthrough mode (*``paramversion`` < 1*) """ if password is None and key is None \ diff --git a/testing/test_crypto.py b/testing/test_crypto.py index f47c7c7..3f444f9 100644 --- a/testing/test_crypto.py +++ b/testing/test_crypto.py @@ -158,6 +158,56 @@ class AESGCMTest (CryptoLayerTest): assert len (header) == crypto.PDTCRYPT_HDR_SIZE + def test_crypto_aes_gcm_enc_multi_ivs_ok_explicit_counter (self): + """ + Access the list of IVs used during encryption and check reuse. Start + from explicit counters so the inputs don’t overlap. IVs must not have + been reused. + """ + + def enc (start_count, data): + password = str (os.urandom (42)) + encryptor = crypto.Encrypt (TEST_VERSION, + TEST_PARAMVERSION, + password=password, + nacl=TEST_STATIC_NACL, + counter=start_count) + + for i, blob in enumerate (data, 1): + fname = "%s_%d" % (TEST_DUMMY_FILENAME, i) + header_dummy = encryptor.next (fname) + _, _ = encryptor.process (blob) + _, header, _ = encryptor.done (header_dummy) + assert len (encryptor.get_used_ivs ()) == i + + return encryptor.get_used_ivs () + + ivs1 = enc (0x0042, [TEST_PLAINTEXT, b"none of your business"]) + ivs2 = enc (0x1337, [b"read me if you can", b"for British eyes only!"]) + + # No reuse in general. + assert len (ivs1 & ivs2) == 0 + + ivs1 = list (ivs1) + ivs2 = list (ivs2) + + # Counters of used IVs must match what we passed explicitly. + def extract_counters (ivs): + def getcount (iv): + _, cnt = struct.unpack (crypto.FMT_I2N_IV, iv) + return cnt + return list (map (getcount, ivs)) + + cnt1 = extract_counters (ivs1) + cnt2 = extract_counters (ivs2) + + assert 0x0042 in cnt1 + assert 0x0043 in cnt1 + + assert 0x1337 in cnt2 + assert 0x1338 in cnt2 + + def test_crypto_aes_gcm_enc_chunk_size (self): password = str (os.urandom (42)) encryptor = crypto.Encrypt (TEST_VERSION, @@ -795,35 +845,10 @@ class AESGCMTest (CryptoLayerTest): return ct + cnk, header, fixed ct_1, hdr_1, _____ = enc (orig_pt_1) - ct_2, hdr_2, fixed = enc (orig_pt_2) - - mut_hdr_2 = bytearray (hdr_2) - mut_hdr_2_vw = memoryview (mut_hdr_2) - # get IV - iv_lo = crypto.HDR_OFF_IV - iv_hi = crypto.HDR_OFF_IV + crypto.PDTCRYPT_HDR_SIZE_IV - iv_1 = hdr_1 [iv_lo : iv_hi] - # transplant into other header - mut_hdr_2_vw [iv_lo : iv_hi] = iv_1 - hdr_2_mod = bytes (mut_hdr_2) - decryptor = crypto.Decrypt (password=password, fixedparts=fixed, - strict_ivs=True) - - def dec (hdr, ct): - decryptor.next (hdr) - off = 0 - pt = b"" - while off < len (ct): - upto = min (off + cnksiz, len (ct)) - cnk = decryptor.process (ct [off:upto]) - pt += cnk - off += cnksiz - return pt + decryptor.done () + encryptor.cnt -= 1 # induce error by forcing an identical IV on next step - decr_pt_1 = dec (hdr_1, ct_1) - decr_pt_2 = dec (hdr_2, ct_2) # good header, different IV - with self.assertRaises (crypto.DuplicateIV): # bad header, reuse detected - decr_pt_2 = dec (hdr_2_mod, ct_2) + with self.assertRaises (crypto.DuplicateIV): # reuse detected + ct_2, hdr_2, fixed = enc (orig_pt_2) class HeaderTest (CryptoLayerTest):