From 44c71394c9b817164603f00a34e1001b853e9c88 Mon Sep 17 00:00:00 2001 From: ngie Date: Tue, 21 May 2019 02:30:43 +0000 Subject: [PATCH] Fix `KAT(CCM)?Parser` file descriptor leaks Make `KAT(CCM)?Parser` into a context suite-capable object by implementing `__enter__` and `__exit__` methods which manage opening up the file descriptors and closing them on context exit. This implementation was decided over adding destructor logic to a `__del__` method, as there are a number of issues around object lifetimes when dealing with threading cleanup, atexit handlers, and a number of other less obvious edgecases. Plus, the architected solution is more pythonic and clean. Complete the iterator implementation by implementing a `__next__` method for both classes which handles iterating over the data using a generator pattern, and by changing `__iter__` to return the object instead of the data which it would iterate over. Alias the `__next__` method to `next` when working with python 2.x in order to maintain functional compatibility between the two major versions. As part of this work and to ensure readability, push the initialization of the parser objects up one layer and pass it down to a helper function. This could have been done via a decorator, but I was trying to keep it simple for other developers to make it easier to modify in the future. This fixes ResourceWarnings with python 3. PR: 237403 MFC after: 1 week Tested with: python 2.7.16 (amd64), python 3.6.8 (amd64) --- tests/sys/opencrypto/cryptodev.py | 57 +++++++++++++++++++++------- tests/sys/opencrypto/cryptotest.py | 61 ++++++++++++++++++++++-------- 2 files changed, 90 insertions(+), 28 deletions(-) diff --git a/tests/sys/opencrypto/cryptodev.py b/tests/sys/opencrypto/cryptodev.py index 3dfba66dc18..618de24a97a 100644 --- a/tests/sys/opencrypto/cryptodev.py +++ b/tests/sys/opencrypto/cryptodev.py @@ -323,11 +323,23 @@ class MismatchError(Exception): class KATParser: def __init__(self, fname, fields): - self.fp = open(fname) self.fields = set(fields) self._pending = None + self.fname = fname + self.fp = None + + def __enter__(self): + self.fp = open(self.fname) + return self + + def __exit__(self, exc_type, exc_value, exc_tb): + if self.fp is not None: + self.fp.close() def __iter__(self): + return self + + def __next__(self): while True: didread = False if self._pending is not None: @@ -340,12 +352,13 @@ class KATParser: if didread and not i: return - if (i and i[0] == '#') or not i.strip(): - continue - if i[0] == '[': - yield i[1:].split(']', 1)[0], self.fielditer() - else: - raise ValueError('unknown line: %r' % repr(i)) + if not i.startswith('#') and i.strip(): + break + + if i[0] == '[': + yield i[1:].split(']', 1)[0], self.fielditer() + else: + raise ValueError('unknown line: %r' % repr(i)) def eatblanks(self): while True: @@ -400,9 +413,18 @@ class KATParser: # section. class KATCCMParser: def __init__(self, fname): - self.fp = open(fname) self._pending = None + self.fname = fname + self.fp = None + + def __enter__(self): + self.fp = open(self.fname) self.read_globals() + return self + + def __exit__(self, exc_type, exc_value, exc_tb): + if self.fp is not None: + self.fp.close() def read_globals(self): self.global_values = {} @@ -463,6 +485,9 @@ class KATCCMParser: self.section_values[f] = v def __iter__(self): + return self + + def __next__(self): while True: if self._pending: line = self._pending @@ -503,6 +528,10 @@ class KATCCMParser: def _spdechex(s): return binascii.hexlify(''.join(s.split())) +if sys.version_info[0] < 3: + KATCCMParser.next = KATCCMParser.__next__ + KATParser.next = KATParser.__next__ + if __name__ == '__main__': if True: try: @@ -518,11 +547,13 @@ if __name__ == '__main__': except IOError: pass elif False: - kp = KATParser('/usr/home/jmg/aesni.testing/format tweak value input - data unit seq no/XTSGenAES128.rsp', [ 'COUNT', 'DataUnitLen', 'Key', 'DataUnitSeqNumber', 'PT', 'CT' ]) - for mode, ni in kp: - print(i, ni) - for j in ni: - print(j) + columns = [ 'COUNT', 'DataUnitLen', 'Key', 'DataUnitSeqNumber', 'PT', 'CT' ] + fname = '/usr/home/jmg/aesni.testing/format tweak value input - data unit seq no/XTSGenAES128.rsp' + with KATParser(fname, columns) as kp: + for mode, ni in kp: + print(i, ni) + for j in ni: + print(j) elif False: key = _spdechex('c939cc13397c1d37de6ae0e1cb7c423c') iv = _spdechex('00000000000000000000000000000001') diff --git a/tests/sys/opencrypto/cryptotest.py b/tests/sys/opencrypto/cryptotest.py index 2d58a67b106..9b379474aad 100644 --- a/tests/sys/opencrypto/cryptotest.py +++ b/tests/sys/opencrypto/cryptotest.py @@ -104,8 +104,12 @@ def GenTestCase(cname): else: raise RuntimeError('unknown mode: %r' % repr(mode)) - for bogusmode, lines in cryptodev.KATParser(fname, - [ 'Count', 'Key', 'IV', 'CT', 'AAD', 'Tag', 'PT', ]): + columns = [ 'Count', 'Key', 'IV', 'CT', 'AAD', 'Tag', 'PT', ] + with cryptodev.KATParser(fname, columns) as parser: + self.runGCMWithParser(parser, mode) + + def runGCMWithParser(self, parser, mode): + for _, lines in next(parser): for data in lines: curcnt = int(data['Count']) cipherkey = binascii.unhexlify(data['Key']) @@ -166,9 +170,13 @@ def GenTestCase(cname): repr(data)) def runCBC(self, fname): + columns = [ 'COUNT', 'KEY', 'IV', 'PLAINTEXT', 'CIPHERTEXT', ] + with cryptodev.KATParser(fname, columns) as parser: + self.runCBCWithParser(parser) + + def runCBCWithParser(self, parser): curfun = None - for mode, lines in cryptodev.KATParser(fname, - [ 'COUNT', 'KEY', 'IV', 'PLAINTEXT', 'CIPHERTEXT', ]): + for mode, lines in next(parser): if mode == 'ENCRYPT': swapptct = False curfun = Crypto.encrypt @@ -193,10 +201,14 @@ def GenTestCase(cname): self.assertEqual(r, ct) def runXTS(self, fname, meth): + columns = [ 'COUNT', 'DataUnitLen', 'Key', 'DataUnitSeqNumber', 'PT', + 'CT'] + with cryptodev.KATParser(fname, columns) as parser: + self.runXTSWithParser(parser, meth) + + def runXTSWithParser(self, parser, meth): curfun = None - for mode, lines in cryptodev.KATParser(fname, - [ 'COUNT', 'DataUnitLen', 'Key', 'DataUnitSeqNumber', 'PT', - 'CT' ]): + for mode, lines in next(parser): if mode == 'ENCRYPT': swapptct = False curfun = Crypto.encrypt @@ -231,7 +243,11 @@ def GenTestCase(cname): self.assertEqual(r, ct) def runCCMEncrypt(self, fname): - for data in cryptodev.KATCCMParser(fname): + with cryptodev.KATCCMParser(fname) as parser: + self.runCCMEncryptWithParser(parser) + + def runCCMEncryptWithParser(self, parser): + for data in next(parser): Nlen = int(data['Nlen']) if Nlen != 12: # OCF only supports 12 byte IVs @@ -266,11 +282,15 @@ def GenTestCase(cname): repr(data) + " on " + cname) def runCCMDecrypt(self, fname): + with cryptodev.KATCCMParser(fname) as parser: + self.runCCMDecryptWithParser(parser) + + def runCCMDecryptWithParser(self, parser): # XXX: Note that all of the current CCM # decryption test vectors use IV and tag sizes # that aren't supported by OCF none of the # tests are actually ran. - for data in cryptodev.KATCCMParser(fname): + for data in next(parser): Nlen = int(data['Nlen']) if Nlen != 12: # OCF only supports 12 byte IVs @@ -326,9 +346,13 @@ def GenTestCase(cname): self.runTDES(i) def runTDES(self, fname): + columns = [ 'COUNT', 'KEYs', 'IV', 'PLAINTEXT', 'CIPHERTEXT', ] + with cryptodev.KATParser(fname, columns) as parser: + self.runTDESWithParser(parser) + + def runTDESWithParser(self, parser): curfun = None - for mode, lines in cryptodev.KATParser(fname, - [ 'COUNT', 'KEYs', 'IV', 'PLAINTEXT', 'CIPHERTEXT', ]): + for mode, lines in next(parser): if mode == 'ENCRYPT': swapptct = False curfun = Crypto.encrypt @@ -365,9 +389,12 @@ def GenTestCase(cname): # Skip SHA512_(224|256) tests if fname.find('SHA512_') != -1: return + columns = [ 'Len', 'Msg', 'MD' ] + with cryptodev.KATParser(fname, columns) as parser: + self.runSHAWithParser(parser) - for hashlength, lines in cryptodev.KATParser(fname, - [ 'Len', 'Msg', 'MD' ]): + def runSHAWithParser(self, parser): + for hashlength, lines in next(parser): # E.g., hashlength will be "L=20" (bytes) hashlen = int(hashlength.split("=")[1]) @@ -413,8 +440,12 @@ def GenTestCase(cname): self.runSHA1HMAC(i) def runSHA1HMAC(self, fname): - for hashlength, lines in cryptodev.KATParser(fname, - [ 'Count', 'Klen', 'Tlen', 'Key', 'Msg', 'Mac' ]): + columns = [ 'Count', 'Klen', 'Tlen', 'Key', 'Msg', 'Mac' ] + with cryptodev.KATParser(fname, columns) as parser: + self.runSHA1HMACWithParser(parser) + + def runSHA1HMACWithParser(self, parser): + for hashlength, lines in next(parser): # E.g., hashlength will be "L=20" (bytes) hashlen = int(hashlength.split("=")[1]) -- 2.42.0