From 3b66ec1a22e10e817e43d3594f4d0edc39386a50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Douglas=20Mendiz=C3=A1bal?= Date: Thu, 25 Feb 2021 10:12:46 -0600 Subject: [PATCH] Allow multiple token labels for PKCS#11 driver This patch changes the slot selection logic to look for more than one token label in the list of slots, using the first token that is found. This change is required to enable load balancing with devices that use separate tokens for this feature. This patch adds a new option in the [p11_crypto_plugin] section "token_labels", and deprecates the "token_label" option. For backwards compatibility, the "token_label" option will still be used if present. Change-Id: Ic2b85246c37e856c38cb47613313b19e653118de (cherry picked from commit 1ca03610d7257c782b11d6bcf54074d66a79c545) (cherry picked from commit b41ceaa369e2bdd5b75d76c0a3368854293ed03a) (cherry picked from commit 7cf483e205736483b4999abfe4baf6f626fff862) --- barbican/cmd/barbican_manage.py | 2 +- barbican/plugin/crypto/p11_crypto.py | 75 ++++++++++++------- barbican/plugin/crypto/pkcs11.py | 42 +++++------ .../tests/plugin/crypto/test_p11_crypto.py | 20 +++-- barbican/tests/plugin/crypto/test_pkcs11.py | 14 ++-- ...-pkcs11-token-labels-61b63e34b7c8cc1a.yaml | 14 ++++ 6 files changed, 103 insertions(+), 64 deletions(-) create mode 100644 releasenotes/notes/allow-multiple-pkcs11-token-labels-61b63e34b7c8cc1a.yaml diff --git a/barbican/cmd/barbican_manage.py b/barbican/cmd/barbican_manage.py index ebdba8d35..e78e7173c 100644 --- a/barbican/cmd/barbican_manage.py +++ b/barbican/cmd/barbican_manage.py @@ -336,7 +336,7 @@ class HSMCommands(object): encryption_mechanism='CKM_AES_CBC', hmac_keywrap_mechanism=hmacwrap, token_serial_number=conf.p11_crypto_plugin.token_serial_number, - token_label=conf.p11_crypto_plugin.token_label + token_labels=conf.p11_crypto_plugin.token_labels ) self.session = self.pkcs11.get_session() diff --git a/barbican/plugin/crypto/p11_crypto.py b/barbican/plugin/crypto/p11_crypto.py index a9f7606a4..90b421b3e 100644 --- a/barbican/plugin/crypto/p11_crypto.py +++ b/barbican/plugin/crypto/p11_crypto.py @@ -38,12 +38,17 @@ p11_crypto_plugin_opts = [ help=u._('Path to vendor PKCS11 library')), cfg.StrOpt('token_serial_number', help=u._('Token serial number used to identify the token to be ' - 'used. Required when the device has multiple tokens ' - 'with the same label.')), + 'used.')), cfg.StrOpt('token_label', - help=u._('Token label used to identify the token to ' - 'be used. Required when token_serial_number is ' - 'not specified.')), + deprecated_for_removal=True, + help=u._('DEPRECATED: Use token_labels instead. ' + 'Token label used to identify the token to ' + 'be used.')), + cfg.ListOpt('token_labels', + help=u._('List of labels for one or more tokens to be used. ' + 'Typically this is a single label, but some HSM ' + 'devices may require more than one label for Load ' + 'Balancing or High Availability configurations.')), cfg.StrOpt('login', help=u._('Password to login to PKCS11 session'), secret=True), @@ -128,23 +133,41 @@ class P11CryptoPlugin(plugin.CryptoPluginBase): def __init__(self, conf=CONF, ffi=None, pkcs11=None): self.conf = conf plugin_conf = conf.p11_crypto_plugin - if plugin_conf.library_path is None: - raise ValueError(u._("library_path is required")) - - # Use specified or create new pkcs11 object - self.pkcs11 = pkcs11 or self._create_pkcs11(plugin_conf, ffi) # Save conf arguments + if plugin_conf.library_path is None: + raise ValueError(u._("library_path is required")) + self.library_path = plugin_conf.library_path + self.encryption_mechanism = plugin_conf.encryption_mechanism + self.generate_iv = plugin_conf.aes_gcm_generate_iv + self.cka_sensitive = plugin_conf.always_set_cka_sensitive self.mkek_key_type = 'CKK_AES' self.mkek_length = plugin_conf.mkek_length self.mkek_label = plugin_conf.mkek_label self.hmac_label = plugin_conf.hmac_label self.hmac_key_type = plugin_conf.hmac_key_type self.hmac_keygen_mechanism = plugin_conf.hmac_keygen_mechanism + self.hmac_keywrap_mechanism = plugin_conf.hmac_keywrap_mechanism + self.os_locking_ok = plugin_conf.os_locking_ok self.pkek_length = plugin_conf.pkek_length self.pkek_cache_ttl = plugin_conf.pkek_cache_ttl self.pkek_cache_limit = plugin_conf.pkek_cache_limit + self.rw_session = plugin_conf.rw_session + self.seed_file = plugin_conf.seed_file + self.seed_length = plugin_conf.seed_length + self.slot_id = plugin_conf.slot_id + self.login = plugin_conf.login + self.token_serial_number = plugin_conf.token_serial_number + self.token_labels = plugin_conf.token_labels or list() + if plugin_conf.token_label: + LOG.warning('Using deprecated option "token_label". Please update ' + 'your configuration file.') + if plugin_conf.token_label not in self.token_labels: + self.token_labels.append(plugin_conf.token_label) + + # Use specified or create new pkcs11 object + self.pkcs11 = pkcs11 or self._create_pkcs11(ffi) self._configure_object_cache() @@ -315,25 +338,25 @@ class P11CryptoPlugin(plugin.CryptoPluginBase): else: break - def _create_pkcs11(self, plugin_conf, ffi=None): + def _create_pkcs11(self, ffi=None): seed_random_buffer = None - if plugin_conf.seed_file: - with open(plugin_conf.seed_file, 'rb') as f: - seed_random_buffer = f.read(plugin_conf.seed_length) + if self.seed_file: + with open(self.seed_file, 'rb') as f: + seed_random_buffer = f.read(self.seed_length) return pkcs11.PKCS11( - library_path=plugin_conf.library_path, - login_passphrase=plugin_conf.login, - rw_session=plugin_conf.rw_session, - slot_id=plugin_conf.slot_id, - encryption_mechanism=plugin_conf.encryption_mechanism, + library_path=self.library_path, + login_passphrase=self.login, + rw_session=self.rw_session, + slot_id=self.slot_id, + encryption_mechanism=self.encryption_mechanism, ffi=ffi, seed_random_buffer=seed_random_buffer, - generate_iv=plugin_conf.aes_gcm_generate_iv, - always_set_cka_sensitive=plugin_conf.always_set_cka_sensitive, - hmac_keywrap_mechanism=plugin_conf.hmac_keywrap_mechanism, - token_serial_number=plugin_conf.token_serial_number, - token_label=plugin_conf.token_label, - os_locking_ok=plugin_conf.os_locking_ok + generate_iv=self.generate_iv, + always_set_cka_sensitive=self.cka_sensitive, + hmac_keywrap_mechanism=self.hmac_keywrap_mechanism, + token_serial_number=self.token_serial_number, + token_labels=self.token_labels, + os_locking_ok=self.os_locking_ok ) def _reinitialize_pkcs11(self): @@ -350,7 +373,7 @@ class P11CryptoPlugin(plugin.CryptoPluginBase): with self.mk_cache_lock: self.mk_cache.clear() - self.pkcs11 = self._create_pkcs11(self.conf.p11_crypto_plugin) + self.pkcs11 = self._create_pkcs11() self._configure_object_cache() def _get_session(self): diff --git a/barbican/plugin/crypto/pkcs11.py b/barbican/plugin/crypto/pkcs11.py index c05ac7d2a..cd43b05e8 100644 --- a/barbican/plugin/crypto/pkcs11.py +++ b/barbican/plugin/crypto/pkcs11.py @@ -12,7 +12,6 @@ # limitations under the License. import collections -import itertools import textwrap import cffi @@ -433,7 +432,7 @@ class PKCS11(object): generate_iv=None, always_set_cka_sensitive=None, hmac_keywrap_mechanism='CKM_SHA256_HMAC', token_serial_number=None, - token_label=None, + token_labels=None, os_locking_ok=False): if algorithm: LOG.warning("WARNING: Using deprecated 'algorithm' argument.") @@ -467,7 +466,7 @@ class PKCS11(object): self.rw_session = rw_session self.slot_id = self._get_slot_id( token_serial_number, - token_label, + token_labels, slot_id) # Algorithm options @@ -485,10 +484,9 @@ class PKCS11(object): self._seed_random(session, seed_random_buffer) self._rng_self_test(session) self.return_session(session) - LOG.debug("Connected to PCKS11 sn: %s label: %s slot: %s", - token_serial_number, token_label, self.slot_id) + LOG.debug("Connected to PCKS#11 Token in Slot %s", self.slot_id) - def _get_slot_id(self, token_serial_number, token_label, slot_id): + def _get_slot_id(self, token_serial_number, token_labels, slot_id): # First find out how many slots with tokens are available slots_ptr = self.ffi.new("CK_ULONG_PTR") rv = self.lib.C_GetSlotList(CK_TRUE, self.ffi.NULL, slots_ptr) @@ -525,10 +523,10 @@ class PKCS11(object): LOG.debug("Found token sn: %s in slot %s", token.serial_number, token.slot_id) - if token_label: + if token_labels: LOG.warning( - "Ignoring token_label: %s from barbican.conf", - token_label + "Ignoring token_labels: %s from barbican.conf", + token_labels ) if slot_id: LOG.warning("Ignoring slot_id: %s from barbican.conf", @@ -537,22 +535,16 @@ class PKCS11(object): raise ValueError("Token Serial Number not found.") # Label match is next, raises an error if there's not exactly one match - if token_label: - matched = list(itertools.dropwhile( - lambda x: x.label != token_label, - tokens - )) - if len(matched) > 1: - raise ValueError("More than one matching token label found") - if len(matched) < 1: - raise ValueError("Token Label not found.") - - token = matched.pop() - LOG.debug("Found token label: %s in slot %s", token.label, - token.slot_id) - if slot_id: - LOG.warning("Ignoring slot_id: %s from barbican.conf", slot_id) - return token.slot_id + if token_labels: + for token in tokens: + if token.label in token_labels: + LOG.debug("Found token label: %s in slot %s", token.label, + token.slot_id) + if slot_id: + LOG.warning("Ignoring slot_id: %s from barbican.conf", + slot_id) + return token.slot_id + raise ValueError("Token Labels not found.") # If we got this far, slot_id was the only param given, so we return it return slot_id diff --git a/barbican/tests/plugin/crypto/test_p11_crypto.py b/barbican/tests/plugin/crypto/test_p11_crypto.py index 58aa3445e..9b5ffb498 100644 --- a/barbican/tests/plugin/crypto/test_p11_crypto.py +++ b/barbican/tests/plugin/crypto/test_p11_crypto.py @@ -58,6 +58,7 @@ class WhenTestingP11CryptoPlugin(utils.BaseTestCase): self.cfg_mock.p11_crypto_plugin.slot_id = 1 self.cfg_mock.p11_crypto_plugin.token_serial_number = None self.cfg_mock.p11_crypto_plugin.token_label = None + self.cfg_mock.p11_crypto_plugin.token_labels = None self.cfg_mock.p11_crypto_plugin.rw_session = True self.cfg_mock.p11_crypto_plugin.pkek_length = 32 self.cfg_mock.p11_crypto_plugin.pkek_cache_ttl = 900 @@ -72,9 +73,19 @@ class WhenTestingP11CryptoPlugin(utils.BaseTestCase): self.cfg_mock.p11_crypto_plugin.plugin_name = self.plugin_name self.plugin = p11_crypto.P11CryptoPlugin( - conf=self.cfg_mock, pkcs11=self.pkcs11 + conf=self.cfg_mock, + pkcs11=self.pkcs11 ) + def test_backwards_compatibility_for_token_label(self): + self.cfg_mock.p11_crypto_plugin.token_label = 'deprecatedLabel' + self.cfg_mock.p11_crypto_plugin.token_labels = ['label1', 'label2'] + plugin = p11_crypto.P11CryptoPlugin( + conf=self.cfg_mock, + pkcs11=self.pkcs11 + ) + self.assertIn('deprecatedLabel', plugin.token_labels) + def test_invalid_library_path(self): cfg = self.cfg_mock.p11_crypto_plugin cfg.library_path = None @@ -301,19 +312,18 @@ class WhenTestingP11CryptoPlugin(utils.BaseTestCase): ffi = pkcs11.build_ffi() setattr(ffi, 'dlopen', lambda x: lib) - p11 = self.plugin._create_pkcs11(self.cfg_mock.p11_crypto_plugin, ffi) + p11 = self.plugin._create_pkcs11(ffi) self.assertIsInstance(p11, pkcs11.PKCS11) # test for when plugin_conf.seed_file is not None - self.cfg_mock.p11_crypto_plugin.seed_file = 'seed_file' + self.plugin.seed_file = 'seed_file' d = '01234567' * 4 mo = mock.mock_open(read_data=d) with mock.patch(six.moves.builtins.__name__ + '.open', mo, create=True): - p11 = self.plugin._create_pkcs11( - self.cfg_mock.p11_crypto_plugin, ffi) + p11 = self.plugin._create_pkcs11(ffi) self.assertIsInstance(p11, pkcs11.PKCS11) mo.assert_called_once_with('seed_file', 'rb') diff --git a/barbican/tests/plugin/crypto/test_pkcs11.py b/barbican/tests/plugin/crypto/test_pkcs11.py index 8e13daeb5..fcd0b76d0 100644 --- a/barbican/tests/plugin/crypto/test_pkcs11.py +++ b/barbican/tests/plugin/crypto/test_pkcs11.py @@ -178,11 +178,11 @@ class WhenTestingPKCS11(utils.BaseTestCase): return pkcs11.CKR_OK def test_get_slot_id_from_serial_number(self): - slot_id = self.pkcs11._get_slot_id('111111', None, 1) + slot_id = self.pkcs11._get_slot_id('111111', None, 2) self.assertEqual(1, slot_id) def test_get_slot_id_from_label(self): - slot_id = self.pkcs11._get_slot_id(None, 'myLabel', 1) + slot_id = self.pkcs11._get_slot_id(None, ['myLabel'], 2) self.assertEqual(1, slot_id) def test_get_slot_id_backwards_compatibility(self): @@ -190,7 +190,7 @@ class WhenTestingPKCS11(utils.BaseTestCase): self.assertEqual(5, slot_id) def test_get_slot_id_from_serial_ignores_label(self): - slot_id = self.pkcs11._get_slot_id('111111', 'badLabel', 1) + slot_id = self.pkcs11._get_slot_id('111111', ['badLabel'], 2) self.assertEqual(1, slot_id) def test_get_slot_id_from_serial_ignores_given_slot(self): @@ -198,7 +198,7 @@ class WhenTestingPKCS11(utils.BaseTestCase): self.assertEqual(1, slot_id) def test_get_slot_id_from_label_ignores_given_slot(self): - slot_id = self.pkcs11._get_slot_id(None, 'myLabel', 3) + slot_id = self.pkcs11._get_slot_id(None, ['myLabel'], 3) self.assertEqual(1, slot_id) def test_get_slot_id_serial_not_found(self): @@ -207,14 +207,14 @@ class WhenTestingPKCS11(utils.BaseTestCase): def test_get_slot_id_label_not_found(self): self.assertRaises(ValueError, - self.pkcs11._get_slot_id, None, 'badLabel', 1) + self.pkcs11._get_slot_id, None, ['myLabelbad'], 1) def test_get_slot_id_two_tokens_same_label(self): self.lib.C_GetSlotList.side_effect = self._get_two_slot_list self.lib.C_GetTokenInfo.side_effect = \ self._get_two_token_info_same_label - self.assertRaises(ValueError, - self.pkcs11._get_slot_id, None, 'myLabel', 1) + slot_id = self.pkcs11._get_slot_id(None, ['myLabel'], 3) + self.assertEqual(1, slot_id) def test_public_get_session(self): self.lib.C_GetSessionInfo.side_effect = self._get_session_public diff --git a/releasenotes/notes/allow-multiple-pkcs11-token-labels-61b63e34b7c8cc1a.yaml b/releasenotes/notes/allow-multiple-pkcs11-token-labels-61b63e34b7c8cc1a.yaml new file mode 100644 index 000000000..23c5acbde --- /dev/null +++ b/releasenotes/notes/allow-multiple-pkcs11-token-labels-61b63e34b7c8cc1a.yaml @@ -0,0 +1,14 @@ +--- +features: + - | + A new "token_labels" option has been added to the PKCS#11 driver which + supersedes the previous "token_label" option. The new option is used to + specify a list of tokens that can be used by Barbican. This is required + for some HSM devices that use separate tokens for load balancing. For most + use cases the new option will just have a single token. The old option + is deprecated, but will still be used if present. +deprecations: + - | + The "token_label" option in the PKCS#11 driver is deprecated. Th new + "token_labels" option should be used instead. If present, "token_label" + will still be used by appending it to "token_labels".