Skip to content

Make KeyBundle update() thread safe #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 7, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 28 additions & 26 deletions src/cryptojwt/key_bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import json
import logging
import os
import threading
import time
from datetime import datetime
from functools import cmp_to_key
Expand Down Expand Up @@ -507,34 +508,35 @@ def update(self):
:return: True if update was ok or False if we encountered an error during update.
"""
if self.source:
_old_keys = self._keys # just in case
with threading.Lock():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need a global variable with the lock. This will create a new Lock() for each call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix, thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix in c9e59bc, will address the lookup while updating issue as well

_old_keys = self._keys # just in case

# reread everything
self._keys = []
updated = None
# reread everything
self._keys = []
updated = None

try:
if self.local:
if self.fileformat in ["jwks", "jwk"]:
updated = self.do_local_jwk(self.source)
elif self.fileformat == "der":
updated = self.do_local_der(self.source, self.keytype, self.keyusage)
elif self.remote:
updated = self.do_remote()
except Exception as err:
LOGGER.error("Key bundle update failed: %s", err)
self._keys = _old_keys # restore
return False

if updated:
now = time.time()
for _key in _old_keys:
if _key not in self._keys:
if not _key.inactive_since: # If already marked don't mess
_key.inactive_since = now
self._keys.append(_key)
else:
self._keys = _old_keys
try:
if self.local:
if self.fileformat in ["jwks", "jwk"]:
updated = self.do_local_jwk(self.source)
elif self.fileformat == "der":
updated = self.do_local_der(self.source, self.keytype, self.keyusage)
elif self.remote:
updated = self.do_remote()
except Exception as err:
LOGGER.error("Key bundle update failed: %s", err)
self._keys = _old_keys # restore
return False

if updated:
now = time.time()
for _key in _old_keys:
if _key not in self._keys:
if not _key.inactive_since: # If already marked don't mess
_key.inactive_since = now
self._keys.append(_key)
else:
self._keys = _old_keys

return True

Expand Down