URL: https://github.com/freeipa/freeipa/pull/509 Author: tiran Title: #509: Migrate OTP import script to python-cryptography Action: opened
PR body: """ Supersedes @npmccallum PR #486 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/509/head:pr509 git checkout pr509
From a42cc54f44c48aaf105d4765af797676d1802881 Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum <npmccal...@redhat.com> Date: Mon, 31 Aug 2015 10:46:19 -0400 Subject: [PATCH 1/2] Migrate OTP import script to python-cryptography https://fedorahosted.org/freeipa/ticket/5192 --- ipaserver/install/ipa_otptoken_import.py | 104 ++++++++++-------------- ipatests/test_ipaserver/test_otptoken_import.py | 100 +++++++++-------------- 2 files changed, 80 insertions(+), 124 deletions(-) diff --git a/ipaserver/install/ipa_otptoken_import.py b/ipaserver/install/ipa_otptoken_import.py index 00939e0..d5ed12a 100644 --- a/ipaserver/install/ipa_otptoken_import.py +++ b/ipaserver/install/ipa_otptoken_import.py @@ -29,11 +29,15 @@ from lxml import etree import dateutil.parser import dateutil.tz -import nss.nss as nss import gssapi import six from six.moves import xrange +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.kdf import pbkdf2 +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes +from cryptography.hazmat.backends import default_backend + from ipapython import admintool from ipalib import api, errors from ipaserver.plugins.ldap2 import AUTOBIND_DISABLED @@ -118,13 +122,13 @@ def convertAlgorithm(value): "Converts encryption URI to (mech, ivlen)." return { - "http://www.w3.org/2001/04/xmlenc#aes128-cbc": (nss.CKM_AES_CBC_PAD, 128), - "http://www.w3.org/2001/04/xmlenc#aes192-cbc": (nss.CKM_AES_CBC_PAD, 192), - "http://www.w3.org/2001/04/xmlenc#aes256-cbc": (nss.CKM_AES_CBC_PAD, 256), - "http://www.w3.org/2001/04/xmlenc#tripledes-cbc": (nss.CKM_DES3_CBC_PAD, 64), - "http://www.w3.org/2001/04/xmldsig-more#camellia128": (nss.CKM_CAMELLIA_CBC_PAD, 128), - "http://www.w3.org/2001/04/xmldsig-more#camellia192": (nss.CKM_CAMELLIA_CBC_PAD, 192), - "http://www.w3.org/2001/04/xmldsig-more#camellia256": (nss.CKM_CAMELLIA_CBC_PAD, 256), + "http://www.w3.org/2001/04/xmlenc#aes128-cbc": (algorithms.AES, modes.CBC, 128), + "http://www.w3.org/2001/04/xmlenc#aes192-cbc": (algorithms.AES, modes.CBC, 192), + "http://www.w3.org/2001/04/xmlenc#aes256-cbc": (algorithms.AES, modes.CBC, 256), + "http://www.w3.org/2001/04/xmlenc#tripledes-cbc": (algorithms.TripleDES, modes.CBC, 64), + "http://www.w3.org/2001/04/xmldsig-more#camellia128": (algorithms.Camellia, modes.CBC, 128), + "http://www.w3.org/2001/04/xmldsig-more#camellia192": (algorithms.Camellia, modes.CBC, 192), + "http://www.w3.org/2001/04/xmldsig-more#camellia256": (algorithms.Camellia, modes.CBC, 256), # TODO: add support for these formats. # "http://www.w3.org/2001/04/xmlenc#kw-aes128": "kw-aes128", @@ -134,7 +138,7 @@ def convertAlgorithm(value): # "http://www.w3.org/2001/04/xmldsig-more#kw-camellia128": "kw-camellia128", # "http://www.w3.org/2001/04/xmldsig-more#kw-camellia192": "kw-camellia192", # "http://www.w3.org/2001/04/xmldsig-more#kw-camellia256": "kw-camellia256", - }.get(value.lower(), (None, None)) + }.get(value.lower(), (None, None, None)) def convertEncrypted(value, decryptor=None, pconv=base64.b64decode, econv=lambda x: x): @@ -169,50 +173,29 @@ def __init__(self, enckey): if params is None: raise ValueError("XML file is missing PBKDF2 parameters!") - self.salt = fetch(params, "./xenc11:Salt/xenc11:Specified/text()", base64.b64decode) - self.iter = fetch(params, "./xenc11:IterationCount/text()", int) - self.klen = fetch(params, "./xenc11:KeyLength/text()", int) - self.hmod = fetch(params, "./xenc11:PRF/@Algorithm", convertHMACType, hashlib.sha1) + salt = fetch(params, "./xenc11:Salt/xenc11:Specified/text()", base64.b64decode) + itrs = fetch(params, "./xenc11:IterationCount/text()", int) + klen = fetch(params, "./xenc11:KeyLength/text()", int) + hmod = fetch(params, "./xenc11:PRF/@Algorithm", convertHMACType, hashlib.sha1) - if self.salt is None: + if salt is None: raise ValueError("XML file is missing PBKDF2 salt!") - if self.iter is None: + if itrs is None: raise ValueError("XML file is missing PBKDF2 iteration count!") - if self.klen is None: + if klen is None: raise ValueError("XML file is missing PBKDF2 key length!") - def derive(self, masterkey): - mac = hmac.HMAC(masterkey, None, self.hmod) - - # Figure out how many blocks we will have to combine - # to expand the master key to the desired length. - blocks = self.klen // mac.digest_size - if self.klen % mac.digest_size != 0: - blocks += 1 - - # Loop through each block adding it to the derived key. - dk = [] - for i in range(1, blocks + 1): - # Set initial values. - last = self.salt + struct.pack('>I', i) - hash = [0] * mac.digest_size + self.kdf = pbkdf2.PBKDF2HMAC( + algorithm=hmod, + length=klen, + salt=salt, + iterations=itrs + ) - # Perform n iterations. - for _j in xrange(self.iter): - tmp = mac.copy() - tmp.update(last) - last = tmp.digest() - - # XOR the previous hash with the new hash. - for k in range(mac.digest_size): - hash[k] ^= ord(last[k]) - - # Add block to derived key. - dk.extend(hash) - - return ''.join([chr(c) for c in dk])[:self.klen] + def derive(self, masterkey): + return self.kdf.derive(masterkey) def convertKeyDerivation(value): @@ -229,13 +212,17 @@ class XMLDecryptor(object): * RFC 6931""" def __init__(self, key, hmac=None): - self.__key = nss.SecItem(key) + self.__key = key self.__hmac = hmac def __call__(self, element, mac=None): - (mech, ivlen) = fetch(element, "./xenc:EncryptionMethod/@Algorithm", convertAlgorithm) + (algo, mode, klen) = fetch(element, "./xenc:EncryptionMethod/@Algorithm", convertAlgorithm) data = fetch(element, "./xenc:CipherData/xenc:CipherValue/text()", base64.b64decode) + # Make sure the key is the right length. + if len(self.__key) * 8 != klen: + raise ValidationError("Invalid key length!") + # If a MAC is present, perform validation. if mac: tmp = self.__hmac.copy() @@ -243,13 +230,14 @@ def __call__(self, element, mac=None): if tmp.digest() != mac: raise ValidationError("MAC validation failed!") - # Decrypt the data. - slot = nss.get_best_slot(mech) - key = nss.import_sym_key(slot, mech, nss.PK11_OriginUnwrap, nss.CKA_ENCRYPT, self.__key) - iv = nss.param_from_iv(mech, nss.SecItem(data[0:ivlen//8])) - ctx = nss.create_context_by_sym_key(mech, nss.CKA_DECRYPT, key, iv) - out = ctx.cipher_op(data[ivlen // 8:]) - out += ctx.digest_final() + iv = data[:algo.block_size / 8] + data = data[len(iv):] + + cipher = Cipher(algo(self.__key), mode(iv)) + decryptor = cipher.decryptor() + + out = decryptor.update(data) + out += decryptor.finalize() return out @@ -469,14 +457,6 @@ class OTPTokenImport(admintool.AdminTool): usage = "%prog [options] <PSKC file> <output file>" @classmethod - def main(cls, argv): - nss.nss_init_nodb() - try: - super(OTPTokenImport, cls).main(argv) - finally: - nss.nss_shutdown() - - @classmethod def add_options(cls, parser): super(OTPTokenImport, cls).add_options(parser) diff --git a/ipatests/test_ipaserver/test_otptoken_import.py b/ipatests/test_ipaserver/test_otptoken_import.py index b885cef..19dfbf7 100644 --- a/ipatests/test_ipaserver/test_otptoken_import.py +++ b/ipatests/test_ipaserver/test_otptoken_import.py @@ -19,16 +19,13 @@ import os import pytest -from nss import nss from ipaserver.install.ipa_otptoken_import import PSKCDocument, ValidationError basename = os.path.join(os.path.dirname(__file__), "data") -@pytest.mark.skipif(True, reason="Causes NSS errors. Ticket 5192") @pytest.mark.tier1 class test_otptoken_import(object): - def test_figure3(self): doc = PSKCDocument(os.path.join(basename, "pskc-figure3.xml")) assert doc.keyname is None @@ -63,62 +60,47 @@ def test_figure5(self): assert False def test_figure6(self): - nss.nss_init_nodb() - try: - doc = PSKCDocument(os.path.join(basename, "pskc-figure6.xml")) - assert doc.keyname == 'Pre-shared-key' - doc.setKey('12345678901234567890123456789012'.decode('hex')) - assert [(t.id, t.options) for t in doc.getKeyPackages()] == \ - [(u'12345678', { - 'ipatokenotpkey': u'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ', - 'ipatokenvendor': u'Manufacturer', - 'ipatokenserial': u'987654321', - 'ipatokenhotpcounter': 0, - 'ipatokenotpdigits': 8, - 'type': u'hotp'})] - finally: - nss.nss_shutdown() + doc = PSKCDocument(os.path.join(basename, "pskc-figure6.xml")) + assert doc.keyname == 'Pre-shared-key' + doc.setKey('12345678901234567890123456789012'.decode('hex')) + assert [(t.id, t.options) for t in doc.getKeyPackages()] == \ + [(u'12345678', { + 'ipatokenotpkey': u'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ', + 'ipatokenvendor': u'Manufacturer', + 'ipatokenserial': u'987654321', + 'ipatokenhotpcounter': 0, + 'ipatokenotpdigits': 8, + 'type': u'hotp'})] def test_figure7(self): - nss.nss_init_nodb() - try: - doc = PSKCDocument(os.path.join(basename, "pskc-figure7.xml")) - assert doc.keyname == 'My Password 1' - doc.setKey('qwerty') - assert [(t.id, t.options) for t in doc.getKeyPackages()] == \ - [(u'123456', { - 'ipatokenotpkey': u'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ', - 'ipatokenvendor': u'TokenVendorAcme', - 'ipatokenserial': u'987654321', - 'ipatokenotpdigits': 8, - 'type': u'hotp'})] - finally: - nss.nss_shutdown() + doc = PSKCDocument(os.path.join(basename, "pskc-figure7.xml")) + assert doc.keyname == 'My Password 1' + doc.setKey('qwerty') + assert [(t.id, t.options) for t in doc.getKeyPackages()] == \ + [(u'123456', { + 'ipatokenotpkey': u'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ', + 'ipatokenvendor': u'TokenVendorAcme', + 'ipatokenserial': u'987654321', + 'ipatokenotpdigits': 8, + 'type': u'hotp'})] def test_figure8(self): - nss.nss_init_nodb() try: PSKCDocument(os.path.join(basename, "pskc-figure8.xml")) except NotImplementedError: # X.509 is not supported. pass else: assert False - finally: - nss.nss_shutdown() def test_invalid(self): - nss.nss_init_nodb() try: PSKCDocument(os.path.join(basename, "pskc-invalid.xml")) except ValueError: # File is invalid. pass else: assert False - finally: - nss.nss_shutdown() def test_mini(self): - nss.nss_init_nodb() try: doc = PSKCDocument(os.path.join(basename, "pskc-mini.xml")) for t in doc.getKeyPackages(): @@ -127,28 +109,22 @@ def test_mini(self): pass else: assert False - finally: - nss.nss_shutdown() def test_full(self): - nss.nss_init_nodb() - try: - doc = PSKCDocument(os.path.join(basename, "full.xml")) - assert [(t.id, t.options) for t in doc.getKeyPackages()] == \ - [(u'KID1', { - 'ipatokenotpkey': u'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ', - 'ipatokennotafter': u'20060531000000Z', - 'ipatokennotbefore': u'20060501000000Z', - 'ipatokenserial': u'SerialNo-IssueNo', - 'ipatokentotpclockoffset': 60000, - 'ipatokenotpalgorithm': u'sha1', - 'ipatokenvendor': u'iana.dummy', - 'description': u'FriendlyName', - 'ipatokentotptimestep': 200, - 'ipatokenhotpcounter': 0, - 'ipatokenmodel': u'Model', - 'ipatokenotpdigits': 8, - 'type': u'hotp', - })] - finally: - nss.nss_shutdown() + doc = PSKCDocument(os.path.join(basename, "full.xml")) + assert [(t.id, t.options) for t in doc.getKeyPackages()] == \ + [(u'KID1', { + 'ipatokenotpkey': u'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ', + 'ipatokennotafter': u'20060531000000Z', + 'ipatokennotbefore': u'20060501000000Z', + 'ipatokenserial': u'SerialNo-IssueNo', + 'ipatokentotpclockoffset': 60000, + 'ipatokenotpalgorithm': u'sha1', + 'ipatokenvendor': u'iana.dummy', + 'description': u'FriendlyName', + 'ipatokentotptimestep': 200, + 'ipatokenhotpcounter': 0, + 'ipatokenmodel': u'Model', + 'ipatokenotpdigits': 8, + 'type': u'hotp', + })] From 7f5ba739e09511b8e14e7294f079d94806e45646 Mon Sep 17 00:00:00 2001 From: Christian Heimes <chei...@redhat.com> Date: Sat, 25 Feb 2017 11:06:04 +0100 Subject: [PATCH 2/2] Finish port to PyCA cryptography * add missing default_backend * unpad encrypted data * use cryptography's hashes and HMAC construct * remove hard dependency on python-nss from setup.py Signed-off-by: Christian Heimes <chei...@redhat.com> --- ipaserver/install/ipa_otptoken_import.py | 52 +++++++++++++++++++------------- ipaserver/setup.py | 1 - ipatests/setup.py | 3 +- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/ipaserver/install/ipa_otptoken_import.py b/ipaserver/install/ipa_otptoken_import.py index d5ed12a..73c3099 100644 --- a/ipaserver/install/ipa_otptoken_import.py +++ b/ipaserver/install/ipa_otptoken_import.py @@ -20,20 +20,18 @@ import abc import base64 import datetime -import hashlib -import hmac import os import uuid -import struct from lxml import etree import dateutil.parser import dateutil.tz import gssapi import six -from six.moves import xrange -from cryptography.hazmat.primitives import hashes +from cryptography.exceptions import InvalidSignature +from cryptography.hazmat.primitives import hashes, hmac +from cryptography.hazmat.primitives.padding import PKCS7 from cryptography.hazmat.primitives.kdf import pbkdf2 from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes from cryptography.hazmat.backends import default_backend @@ -110,12 +108,12 @@ def convertHMACType(value): "Converts HMAC URI to hashlib object." return { - "http://www.w3.org/2000/09/xmldsig#hmac-sha1": hashlib.sha1, - "http://www.w3.org/2001/04/xmldsig-more#hmac-sha224": hashlib.sha224, - "http://www.w3.org/2001/04/xmldsig-more#hmac-sha256": hashlib.sha256, - "http://www.w3.org/2001/04/xmldsig-more#hmac-sha384": hashlib.sha384, - "http://www.w3.org/2001/04/xmldsig-more#hmac-sha512": hashlib.sha512, - }.get(value.lower(), hashlib.sha1) + "http://www.w3.org/2000/09/xmldsig#hmac-sha1": hashes.SHA1, + "http://www.w3.org/2001/04/xmldsig-more#hmac-sha224": hashes.SHA224, + "http://www.w3.org/2001/04/xmldsig-more#hmac-sha256": hashes.SHA256, + "http://www.w3.org/2001/04/xmldsig-more#hmac-sha384": hashes.SHA384, + "http://www.w3.org/2001/04/xmldsig-more#hmac-sha512": hashes.SHA512, + }.get(value.lower(), hashes.SHA1) def convertAlgorithm(value): @@ -176,7 +174,7 @@ def __init__(self, enckey): salt = fetch(params, "./xenc11:Salt/xenc11:Specified/text()", base64.b64decode) itrs = fetch(params, "./xenc11:IterationCount/text()", int) klen = fetch(params, "./xenc11:KeyLength/text()", int) - hmod = fetch(params, "./xenc11:PRF/@Algorithm", convertHMACType, hashlib.sha1) + hmod = fetch(params, "./xenc11:PRF/@Algorithm", convertHMACType, hashes.SHA1) if salt is None: raise ValueError("XML file is missing PBKDF2 salt!") @@ -188,10 +186,11 @@ def __init__(self, enckey): raise ValueError("XML file is missing PBKDF2 key length!") self.kdf = pbkdf2.PBKDF2HMAC( - algorithm=hmod, + algorithm=hmod(), length=klen, salt=salt, - iterations=itrs + iterations=itrs, + backend=default_backend() ) def derive(self, masterkey): @@ -216,7 +215,7 @@ def __init__(self, key, hmac=None): self.__hmac = hmac def __call__(self, element, mac=None): - (algo, mode, klen) = fetch(element, "./xenc:EncryptionMethod/@Algorithm", convertAlgorithm) + algo, mode, klen = fetch(element, "./xenc:EncryptionMethod/@Algorithm", convertAlgorithm) data = fetch(element, "./xenc:CipherData/xenc:CipherValue/text()", base64.b64decode) # Make sure the key is the right length. @@ -227,17 +226,24 @@ def __call__(self, element, mac=None): if mac: tmp = self.__hmac.copy() tmp.update(data) - if tmp.digest() != mac: - raise ValidationError("MAC validation failed!") + try: + tmp.verify(mac) + except InvalidSignature as e: + raise ValidationError("MAC validation failed!", e) iv = data[:algo.block_size / 8] data = data[len(iv):] - cipher = Cipher(algo(self.__key), mode(iv)) + algorithm = algo(self.__key) + cipher = Cipher(algorithm, mode(iv), default_backend()) decryptor = cipher.decryptor() + padded = decryptor.update(data) + padded += decryptor.finalize() + + unpadder = PKCS7(algorithm.block_size).unpadder() + out = unpadder.update(padded) + out += unpadder.finalize() - out = decryptor.update(data) - out += decryptor.finalize() return out @@ -440,7 +446,11 @@ def setKey(self, key): # Load the decryptor. self.__decryptor = XMLDecryptor(key) if self.__mkey is not None and self.__algo is not None: - tmp = hmac.HMAC(self.__decryptor(self.__mkey), digestmod=self.__algo) + tmp = hmac.HMAC( + self.__decryptor(self.__mkey), + self.__algo(), + backend=default_backend() + ) self.__decryptor = XMLDecryptor(key, tmp) def getKeyPackages(self): diff --git a/ipaserver/setup.py b/ipaserver/setup.py index 95ba5eb..d3c735c 100755 --- a/ipaserver/setup.py +++ b/ipaserver/setup.py @@ -58,7 +58,6 @@ "netaddr", "pyasn1", "pyldap", - "python-nss", "six", # not available on PyPI # "python-libipa_hbac", diff --git a/ipatests/setup.py b/ipatests/setup.py index e46e922..46d51ff 100644 --- a/ipatests/setup.py +++ b/ipatests/setup.py @@ -71,12 +71,11 @@ "pyldap", "pytest", "pytest_multihost", - "python-nss", "six", ], extras_require={ "integration": ["dbus-python", "pyyaml", "ipaserver"], - "ipaserver": ["ipaserver"], + "ipaserver": ["ipaserver", "python-nss"], "webui": ["selenium", "pyyaml", "ipaserver"], "xmlrpc": ["ipaserver"], }
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
