From: Philipp Gesang Date: Mon, 6 Mar 2017 15:51:13 +0000 (+0100) Subject: redo stream decryption X-Git-Tag: v2.2~7^2~233 X-Git-Url: http://developer.intra2net.com/git/?a=commitdiff_plain;h=e4e5d0b8fc92c69260d26f54b3d8cb14d02143dd;p=python-delta-tar redo stream decryption When decrypting, the size of the encrypted object is known, as is the length of the appended authentication tag; there is no ambiguity regarding the end of an object. Thus the old string matching logic with its linear search behavior can go. --- diff --git a/deltatar/crypto.py b/deltatar/crypto.py index cc57c7d..529988c 100755 --- a/deltatar/crypto.py +++ b/deltatar/crypto.py @@ -54,7 +54,7 @@ from cryptography.hazmat.backends import default_backend __all__ = [ "ENCRYPT", "DECRYPT" , "AES_GCM_context" , "hdr_make", "hdr_read", "hdr_fmt", "hdr_fmt_pretty" - , "I2N_HDR_SIZE" ] + , "I2N_HDR_SIZE", "I2N_TLR_SIZE_TAG" ] ############################################################################### @@ -282,7 +282,7 @@ class AES_GCM_context (object): if tag is None: ret = self.ctx.finalize () return True, ret, self.ctx.tag - ret = self.ctx.finalize_with_tag (tag) # XXX this fails if tags don’t match + ret = self.ctx.finalize_with_tag (tag) # XXX this raises “InvalidTag” if tags don’t match return True, ret, None diff --git a/deltatar/tarfile.py b/deltatar/tarfile.py index 13fb489..4266a40 100644 --- a/deltatar/tarfile.py +++ b/deltatar/tarfile.py @@ -316,6 +316,9 @@ class SubsequentHeaderError(HeaderError): class InvalidEncryptionError(TarError): """Exception for undefined crypto modes and combinations.""" pass +class DecryptionError(TarError): + """Exception for error during decryption.""" + pass #--------------------------- # internal stream interface @@ -394,7 +397,7 @@ class _Stream: self.encver = encver self.password = password self.last_block_offset = 0 - self.dbuf = b"" + self.dbuf = b"" # ??? self.aes_buf = b"" # ??? self.exception = None self.compresslevel = compresslevel @@ -785,11 +788,64 @@ class _Stream: """ c = len(self.dbuf) t = [self.dbuf] + l_buf = self.bufsize # not mutated + + if self.encver is not None: + tag1 = None # carry if spanning block bounds + l_tag = crypto.I2N_TLR_SIZE_TAG + b_tag = size - l_tag + e_tag = size + while c < size: - buf = self.__read(self.bufsize) + buf = self.__read(l_buf) if not buf: break + if self.encver is not None: + tag = None + r = len (buf) + cr = c + r + rem = size - cr + + if tag1 is not None: # read rest of tag + tag = tag1 + buf + assert len (tag) == l_tag + elif rem == 0: + split = l_buf - l_tag + ctxt = buf [0 : split] + tag = buf [split : ] + elif rem < 0: + if r == l_tag: # read entire tag only + ctxt = None + tag = buf + else: # r > l_tag + split = r - l_buf + ctxt = buf [0 : split] + tag = buf [split : ] + elif cr > b_tag: # rem > 0 ∧ tag bleeding into next block + got = cr - b_tag + split = r - got + ctxt = buf [0 : split] + tag1 = buf [split : ] # see “scope” above + else: # entire buffer is data + ctxt = buf + + if ctxt is not None: + ok, buf = self.encryption.process_chunk (ctxt) + if ok is False: + raise + raise DecryptionError("error decrypting [%d:%d)" + % (c, cr)) + if tag is not None: + try: + ok, ret, _ = self.encryption.done (tag) + except cryptography.InvalidTag as exn: + raise DecryptionError("authentication tag mismatch") + if ok is False: + raise DecryptionError("error finalizing stream: %s" + % ret) + break # tag valid ∧ no further data + if self.comptype != "tar": try: buf = self.cmp.decompress(buf) @@ -798,7 +854,7 @@ class _Stream: except Exception as e: # happens at the end of the file # _init_read_gz failed in the previous iteration so - # sel.cmp.descompress fails here + # self.cmp.decompress fails here if self.concat_stream: pass else: @@ -821,6 +877,7 @@ class _Stream: self.dbuf = t[size:] return t[:size] + def __read(self, size): """Return size bytes from stream. If internal buffer is empty, read another block from the stream. @@ -828,7 +885,7 @@ class _Stream: c = len(self.buf) t = [self.buf] while c < size: - buf = self.__dec_read(self.bufsize) + buf = self.fileobj.read(size) if not buf: ## XXX stream terminated prematurely; this should be an error break @@ -840,81 +897,6 @@ class _Stream: return t[:size] - def __dec_read(self, size): - ''' - This function reads directly from the file and returns the data - decrypted. This means that if the file is not encrypted, this function - is trivial. - - If the data in the file is encrypted, then the process is different: - first we have to read the raw encrypted data, then decrypt it and - return. But the decryption process is not straightforward because the - self.fileobj stream contains multiple encrypted files one after the - other. We need to detect each separate file, which is detected because - they are separated by the "Salted__" keyword. - - It gets more complicated, because we decrypt chunk by chunk, and to - correctly decrypt one chunk we need to set a "last" variable that - specifies if it's the last chunk of a file, because the end of a file is - handled differently, as it gets padded. - - Knowing if the current chunk is the last part of a file is usually done - just by detecting if it's followed by a "Salted__" keyword or if we - cannot read more bytes from the stream. BUT there's a pretty particular - case, in which the current chunk ends exactly with one file, so that - the next chunk starts with "Salted__". - - To fix that rare case, we just read N bytes from the stream, and check - if the last bytes correspond with the string "Salted__". Then we save - those last characters for next call to __dec_read. If the last bytes - were "Salted__", then we set "last" to True. - - Well, actually we not only substract the length of "Salted__", but 16/32 - chars because the file is decrypted in multiples of the key size. - ''' - if self.encver is not None: - ## XXX - ## PHG: this logic doesn’t map to our header-based approach - ## and requires adjustment - buf = self.fileobj.read(size) - last = len(buf) < size - buf = self.aes_buf + buf - self.aes_buf = b"" - - # prevent setting last to False when it shouldn't - if not last: - kl = 16 ## XXX key length; obsolete - last = buf[-kl:].startswith(b'Salted__') - self.aes_buf = buf[-kl:] - buf = buf[:-kl] - - return self.__split_enc_file(buf, last) - else: - buf = self.fileobj.read(size) - return buf - - def __split_enc_file(self, buf, last): - if not buf: # what else? - return buf ## XXX WTF‽ - - idx = buf.find(b'Salted__') ## wat? - if idx == -1: - return self.encryption.process(buf, last) # decrypt - - b1 = buf[:idx] - b2 = buf[idx:] - if b1: - buf = self.encryption.decrypt(b1, True) - else: - buf = b'' - - self.encryption.get_salt_str(b2) - self.encryption.init() - b2 = b2[len(self.encryption.salt_str):] - buf += self.__split_enc_file(b2, last) - - return buf -# class _Stream class _StreamProxy(object): """Small proxy class that enables transparent compression