diff --git a/ironic/common/auth_basic.py b/ironic/common/auth_basic.py index 85856ec0b5..43a60f46bf 100644 --- a/ironic/common/auth_basic.py +++ b/ironic/common/auth_basic.py @@ -15,6 +15,7 @@ import base64 import binascii +import functools import logging import bcrypt @@ -91,6 +92,16 @@ def authenticate(auth_file, username, password): unauthorized() +@functools.lru_cache(maxsize=256) +def _checkpw(password, hashed): + """Wrapped bcrypt.checkpw for caching + + Keep an in-memory cache of bcrypt.checkpw responses to avoid the + high CPU cost of repeatedly checking the same values + """ + return bcrypt.checkpw(password, hashed) + + def auth_entry(entry, password): """Compare a password with a single user auth file entry @@ -102,7 +113,7 @@ def auth_entry(entry, password): """ username, encrypted = parse_entry(entry) - if not bcrypt.checkpw(password, encrypted): + if not _checkpw(password, encrypted): LOG.info('Password for %s does not match', username) unauthorized() diff --git a/ironic/tests/unit/api/test_middleware.py b/ironic/tests/unit/api/test_middleware.py index 80f768fd13..ca0ec1cd89 100644 --- a/ironic/tests/unit/api/test_middleware.py +++ b/ironic/tests/unit/api/test_middleware.py @@ -15,15 +15,16 @@ Tests to assert that various incorporated middleware works as expected. """ +import bcrypt from http import client as http_client import os import tempfile +from unittest import mock from oslo_config import cfg import oslo_middleware.cors as cors_middleware from ironic.tests.unit.api import base -from ironic.tests.unit.api import utils from ironic.tests.unit.db import utils as db_utils @@ -132,7 +133,7 @@ class TestBasicAuthMiddleware(base.BaseApiTest): def setUp(self): super(TestBasicAuthMiddleware, self).setUp() - self.environ = {'fake.cache': utils.FakeMemcache()} + self.environ = {} self.fake_db_node = db_utils.get_test_node(chassis_id=None) def test_not_authenticated(self): @@ -148,6 +149,20 @@ class TestBasicAuthMiddleware(base.BaseApiTest): response = self.get_json('/chassis', headers=auth_header) self.assertEqual({'chassis': []}, response) + @mock.patch.object(bcrypt, 'checkpw', autospec=True) + def test_authenticated_cached(self, mock_checkpw): + def checkpw(password, hashed): + return password == b'myPassword' + mock_checkpw.side_effect = checkpw + + auth_header = {'Authorization': 'Basic bXlOYW1lOm15UGFzc3dvcmQ='} + for i in range(10): + response = self.get_json('/chassis', headers=auth_header) + self.assertEqual({'chassis': []}, response) + # call count will be zero or one, depending on already cached + # results from other tests + self.assertLessEqual(mock_checkpw.call_count, 1) + def test_public_unauthenticated(self): response = self.get_json('/') self.assertEqual('v1', response['id']) diff --git a/releasenotes/notes/bcrypt_cache-d78775ff02f2d970.yaml b/releasenotes/notes/bcrypt_cache-d78775ff02f2d970.yaml new file mode 100644 index 0000000000..72317f6d2b --- /dev/null +++ b/releasenotes/notes/bcrypt_cache-d78775ff02f2d970.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Performance of Basic HTTP authentication has been improved by keeping a + memory cache of bcrypt password checks. This improves the performance of + Ironic conductor with JSON-RPC, and API access when using Basic HTTP + authentication. \ No newline at end of file