From 8284906284dfad4d611d406c8b2a067c5f508c83 Mon Sep 17 00:00:00 2001 From: Jakob Schlyter Date: Thu, 3 Sep 2020 09:08:49 +0200 Subject: [PATCH 01/15] remove unused exception UnknownKeytype closes #65 --- src/cryptojwt/exception.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/cryptojwt/exception.py b/src/cryptojwt/exception.py index b56fd926..74a9d8c4 100644 --- a/src/cryptojwt/exception.py +++ b/src/cryptojwt/exception.py @@ -63,10 +63,6 @@ class UpdateFailed(KeyIOError): pass -class UnknownKeytype(Invalid): - """An unknown key type""" - - class JWKException(JWKESTException): pass From abacb4dddecca64b8de7c2fb2f9ff789f0a54b3f Mon Sep 17 00:00:00 2001 From: Jakob Schlyter Date: Fri, 4 Sep 2020 15:12:51 +0200 Subject: [PATCH 02/15] add optional error holddown for KeyBundle --- src/cryptojwt/key_bundle.py | 22 +++++++++++++++++--- tests/test_03_key_bundle.py | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/cryptojwt/key_bundle.py b/src/cryptojwt/key_bundle.py index 9b2f200a..6fd16cbd 100755 --- a/src/cryptojwt/key_bundle.py +++ b/src/cryptojwt/key_bundle.py @@ -4,6 +4,7 @@ import logging import os import time +from datetime import datetime from functools import cmp_to_key import requests @@ -156,6 +157,7 @@ def __init__( keys=None, source="", cache_time=300, + error_holddown=0, fileformat="jwks", keytype="RSA", keyusage=None, @@ -188,6 +190,7 @@ def __init__( self.remote = False self.local = False self.cache_time = cache_time + self.error_holddown = error_holddown self.time_out = 0 self.etag = "" self.source = None @@ -198,6 +201,7 @@ def __init__( self.last_updated = 0 self.last_remote = None # HTTP Date of last remote update self.last_local = None # UNIX timestamp of last local update + self.last_error = None # UNIX timestamp of last error if httpc: self.httpc = httpc @@ -365,6 +369,16 @@ def do_remote(self): # if self.verify_ssl is not None: # self.httpc_params["verify"] = self.verify_ssl + if self.last_error: + t = self.last_error + self.error_holddown + if time.time() < t: + LOGGER.warning( + "Not reading remote JWKS from %s (in error holddown until %s)", + self.source, + datetime.fromtimestamp(t), + ) + return False + LOGGER.info("Reading remote JWKS from %s", self.source) try: LOGGER.debug("KeyBundle fetch keys from: %s", self.source) @@ -390,6 +404,7 @@ def do_remote(self): self.do_keys(self.imp_jwks["keys"]) except KeyError: LOGGER.error("No 'keys' keyword in JWKS") + self.last_error = time.time() raise UpdateFailed(MALFORMED.format(self.source)) if hasattr(_http_resp, "headers"): @@ -402,12 +417,13 @@ def do_remote(self): else: LOGGER.warning( - "HTTP status %d reading remote JWKS from %s", - _http_resp.status_code, - self.source, + "HTTP status %d reading remote JWKS from %s", _http_resp.status_code, self.source, ) + self.last_error = time.time() raise UpdateFailed(REMOTE_FAILED.format(self.source, _http_resp.status_code)) + self.last_updated = time.time() + self.last_error = None return True def _parse_remote_response(self, response): diff --git a/tests/test_03_key_bundle.py b/tests/test_03_key_bundle.py index 7d25b392..7645af75 100755 --- a/tests/test_03_key_bundle.py +++ b/tests/test_03_key_bundle.py @@ -17,6 +17,7 @@ from cryptojwt.jwk.rsa import import_rsa_key_from_cert_file from cryptojwt.jwk.rsa import new_rsa_key from cryptojwt.key_bundle import KeyBundle +from cryptojwt.key_bundle import UpdateFailed from cryptojwt.key_bundle import build_key_bundle from cryptojwt.key_bundle import dump_jwks from cryptojwt.key_bundle import init_key @@ -1024,3 +1025,43 @@ def test_remote_not_modified(): assert kb2.httpc_params == {"timeout": (2, 2)} assert kb2.imp_jwks assert kb2.last_updated + + +def test_error_holddown(): + source_good = "https://example.com/keys.json" + source_bad = "https://example.com/keys-bad.json" + error_holddown = 1 + # Mock response + with responses.RequestsMock() as rsps: + rsps.add(method="GET", url=source_good, json=JWKS_DICT, status=200) + rsps.add(method="GET", url=source_bad, json=JWKS_DICT, status=500) + httpc_params = {"timeout": (2, 2)} # connect, read timeouts in seconds + kb = KeyBundle( + source=source_good, + httpc=requests.request, + httpc_params=httpc_params, + error_holddown=error_holddown, + ) + res = kb.do_remote() + assert res == True + assert kb.last_error is None + + # refetch, but fail by using a bad source + kb.source = source_bad + try: + res = kb.do_remote() + except UpdateFailed: + pass + + # retry should fail silently as we're in holddown + res = kb.do_remote() + assert kb.last_error is not None + assert res == False + + # wait until holddown + time.sleep(error_holddown + 1) + + # try again + kb.source = source_good + res = kb.do_remote() + assert res == True From a8506578490e8873d7a9602c4aacd6af9e0c2232 Mon Sep 17 00:00:00 2001 From: Jakob Schlyter Date: Mon, 7 Sep 2020 08:51:23 +0200 Subject: [PATCH 03/15] fix formatting --- src/cryptojwt/key_bundle.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cryptojwt/key_bundle.py b/src/cryptojwt/key_bundle.py index 6fd16cbd..f81a8b31 100755 --- a/src/cryptojwt/key_bundle.py +++ b/src/cryptojwt/key_bundle.py @@ -417,7 +417,9 @@ def do_remote(self): else: LOGGER.warning( - "HTTP status %d reading remote JWKS from %s", _http_resp.status_code, self.source, + "HTTP status %d reading remote JWKS from %s", + _http_resp.status_code, + self.source, ) self.last_error = time.time() raise UpdateFailed(REMOTE_FAILED.format(self.source, _http_resp.status_code)) From 6ac7c42cbcb59514a039fc73dd7abfa89183519f Mon Sep 17 00:00:00 2001 From: Jakob Schlyter Date: Thu, 10 Sep 2020 13:28:50 +0200 Subject: [PATCH 04/15] rework error holddown based on comments from @c00kiemon5ter --- src/cryptojwt/key_bundle.py | 32 ++++++++++++++------------------ tests/test_03_key_bundle.py | 12 ++++++------ 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/cryptojwt/key_bundle.py b/src/cryptojwt/key_bundle.py index f81a8b31..903bbcfa 100755 --- a/src/cryptojwt/key_bundle.py +++ b/src/cryptojwt/key_bundle.py @@ -157,7 +157,7 @@ def __init__( keys=None, source="", cache_time=300, - error_holddown=0, + ignore_errors_period=0, fileformat="jwks", keytype="RSA", keyusage=None, @@ -190,7 +190,8 @@ def __init__( self.remote = False self.local = False self.cache_time = cache_time - self.error_holddown = error_holddown + self.ignore_errors_period = ignore_errors_period + self.ignore_errors_until = None # UNIX timestamp of last error self.time_out = 0 self.etag = "" self.source = None @@ -201,7 +202,6 @@ def __init__( self.last_updated = 0 self.last_remote = None # HTTP Date of last remote update self.last_local = None # UNIX timestamp of last local update - self.last_error = None # UNIX timestamp of last error if httpc: self.httpc = httpc @@ -369,15 +369,13 @@ def do_remote(self): # if self.verify_ssl is not None: # self.httpc_params["verify"] = self.verify_ssl - if self.last_error: - t = self.last_error + self.error_holddown - if time.time() < t: - LOGGER.warning( - "Not reading remote JWKS from %s (in error holddown until %s)", - self.source, - datetime.fromtimestamp(t), - ) - return False + if self.ignore_errors_until and time.time() < self.ignore_errors_until: + LOGGER.warning( + "Not reading remote JWKS from %s (in error holddown until %s)", + self.source, + datetime.fromtimestamp(self.ignore_errors_until), + ) + return False LOGGER.info("Reading remote JWKS from %s", self.source) try: @@ -404,7 +402,7 @@ def do_remote(self): self.do_keys(self.imp_jwks["keys"]) except KeyError: LOGGER.error("No 'keys' keyword in JWKS") - self.last_error = time.time() + self.ignore_errors_until = time.time() + self.ignore_errors_period raise UpdateFailed(MALFORMED.format(self.source)) if hasattr(_http_resp, "headers"): @@ -417,15 +415,13 @@ def do_remote(self): else: LOGGER.warning( - "HTTP status %d reading remote JWKS from %s", - _http_resp.status_code, - self.source, + "HTTP status %d reading remote JWKS from %s", _http_resp.status_code, self.source, ) - self.last_error = time.time() + self.ignore_errors_until = time.time() + self.ignore_errors_period raise UpdateFailed(REMOTE_FAILED.format(self.source, _http_resp.status_code)) self.last_updated = time.time() - self.last_error = None + self.ignore_errors_until = None return True def _parse_remote_response(self, response): diff --git a/tests/test_03_key_bundle.py b/tests/test_03_key_bundle.py index 7645af75..35fbdff2 100755 --- a/tests/test_03_key_bundle.py +++ b/tests/test_03_key_bundle.py @@ -1027,10 +1027,10 @@ def test_remote_not_modified(): assert kb2.last_updated -def test_error_holddown(): +def test_ignore_errors_period(): source_good = "https://example.com/keys.json" source_bad = "https://example.com/keys-bad.json" - error_holddown = 1 + ignore_errors_period = 1 # Mock response with responses.RequestsMock() as rsps: rsps.add(method="GET", url=source_good, json=JWKS_DICT, status=200) @@ -1040,11 +1040,11 @@ def test_error_holddown(): source=source_good, httpc=requests.request, httpc_params=httpc_params, - error_holddown=error_holddown, + ignore_errors_period=ignore_errors_period, ) res = kb.do_remote() assert res == True - assert kb.last_error is None + assert kb.ignore_errors_until is None # refetch, but fail by using a bad source kb.source = source_bad @@ -1055,11 +1055,11 @@ def test_error_holddown(): # retry should fail silently as we're in holddown res = kb.do_remote() - assert kb.last_error is not None + assert kb.ignore_errors_until is not None assert res == False # wait until holddown - time.sleep(error_holddown + 1) + time.sleep(ignore_errors_period + 1) # try again kb.source = source_good From 09947d481ce99704284304a3e34de3c8aeffad90 Mon Sep 17 00:00:00 2001 From: Jakob Schlyter Date: Thu, 10 Sep 2020 13:37:08 +0200 Subject: [PATCH 05/15] format --- src/cryptojwt/key_bundle.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cryptojwt/key_bundle.py b/src/cryptojwt/key_bundle.py index 903bbcfa..3f8205d7 100755 --- a/src/cryptojwt/key_bundle.py +++ b/src/cryptojwt/key_bundle.py @@ -415,7 +415,9 @@ def do_remote(self): else: LOGGER.warning( - "HTTP status %d reading remote JWKS from %s", _http_resp.status_code, self.source, + "HTTP status %d reading remote JWKS from %s", + _http_resp.status_code, + self.source, ) self.ignore_errors_until = time.time() + self.ignore_errors_period raise UpdateFailed(REMOTE_FAILED.format(self.source, _http_resp.status_code)) From 759bd39352d535695c65a4775c2d067624041fc2 Mon Sep 17 00:00:00 2001 From: Jan Stein Date: Wed, 2 Sep 2020 18:57:51 +0200 Subject: [PATCH 06/15] Fix 2 inactive key problems. Fix that updated keys are marked as inactive if we get 304 from server. Make sure we return inactive keys when calling get_jwt_verify_keys(). --- src/cryptojwt/key_bundle.py | 27 ++++++++++++++++----------- src/cryptojwt/key_issuer.py | 2 +- src/cryptojwt/key_jar.py | 6 +++--- tests/test_03_key_bundle.py | 3 ++- tests/test_04_key_jar.py | 6 ++++++ 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/cryptojwt/key_bundle.py b/src/cryptojwt/key_bundle.py index 3f8205d7..5f915607 100755 --- a/src/cryptojwt/key_bundle.py +++ b/src/cryptojwt/key_bundle.py @@ -364,7 +364,7 @@ def do_remote(self): """ Load a JWKS from a webpage. - :return: True or False if load was successful + :return: True if load was successful or False if remote hasn't been modified """ # if self.verify_ssl is not None: # self.httpc_params["verify"] = self.verify_ssl @@ -408,10 +408,12 @@ def do_remote(self): if hasattr(_http_resp, "headers"): headers = getattr(_http_resp, "headers") self.last_remote = headers.get("last-modified") or headers.get("date") + res = True elif _http_resp.status_code == 304: # Not modified LOGGER.debug("%s not modified since %s", self.source, self.last_remote) self.time_out = time.time() + self.cache_time + res = False else: LOGGER.warning( @@ -424,7 +426,7 @@ def do_remote(self): self.last_updated = time.time() self.ignore_errors_until = None - return True + return res def _parse_remote_response(self, response): """ @@ -465,7 +467,6 @@ def update(self): This is a forced update, will happen even if cache time has not elapsed. Replaced keys will be marked as inactive and not removed. """ - res = True # An update was successful if self.source: _old_keys = self._keys # just in case @@ -478,21 +479,25 @@ def update(self): self.do_local_jwk(self.source) elif self.fileformat == "der": self.do_local_der(self.source, self.keytype, self.keyusage) + updated = True elif self.remote: - res = self.do_remote() + updated = self.do_remote() except Exception as err: LOGGER.error("Key bundle update failed: %s", err) self._keys = _old_keys # restore return False - 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) + 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 res + return True def get(self, typ="", only_active=True): """ diff --git a/src/cryptojwt/key_issuer.py b/src/cryptojwt/key_issuer.py index 0e02e1ac..fed55db7 100755 --- a/src/cryptojwt/key_issuer.py +++ b/src/cryptojwt/key_issuer.py @@ -297,7 +297,7 @@ def get(self, key_use, key_type="", kid=None, alg="", **kwargs): else: _bkeys = bundle.keys() for key in _bkeys: - if key.inactive_since and key_use != "sig": + if key.inactive_since and key_use != "ver": # Skip inactive keys unless for signature verification continue if not key.use or use == key.use: diff --git a/src/cryptojwt/key_jar.py b/src/cryptojwt/key_jar.py index 4b58bfe7..c0f1ab8e 100755 --- a/src/cryptojwt/key_jar.py +++ b/src/cryptojwt/key_jar.py @@ -601,13 +601,13 @@ def get_jwt_verify_keys(self, jwt, **kwargs): except KeyError: pass - keys = self._add_key([], _iss, "sig", _key_type, _kid, nki, allow_missing_kid) + keys = self._add_key([], _iss, "ver", _key_type, _kid, nki, allow_missing_kid) if _key_type == "oct": - keys.extend(self.get(key_use="sig", issuer_id="", key_type=_key_type)) + keys.extend(self.get(key_use="ver", issuer_id="", key_type=_key_type)) else: # No issuer, just use all keys I have - keys = self.get(key_use="sig", issuer_id="", key_type=_key_type) + keys = self.get(key_use="ver", issuer_id="", key_type=_key_type) # Only want the appropriate keys. keys = [k for k in keys if k.appropriate_for("verify")] diff --git a/tests/test_03_key_bundle.py b/tests/test_03_key_bundle.py index 35fbdff2..f4fdac46 100755 --- a/tests/test_03_key_bundle.py +++ b/tests/test_03_key_bundle.py @@ -1009,7 +1009,7 @@ def test_remote_not_modified(): with responses.RequestsMock() as rsps: rsps.add(method="GET", url=source, status=304, headers=headers) - assert kb.do_remote() + assert not kb.do_remote() assert kb.last_remote == headers.get("Last-Modified") timeout2 = kb.time_out @@ -1019,6 +1019,7 @@ def test_remote_not_modified(): kb2 = KeyBundle().load(exp) assert kb2.source == source assert len(kb2.keys()) == 3 + assert len(kb2.active_keys()) == 3 assert len(kb2.get("rsa")) == 1 assert len(kb2.get("oct")) == 1 assert len(kb2.get("ec")) == 1 diff --git a/tests/test_04_key_jar.py b/tests/test_04_key_jar.py index b31e5ba8..3c141fb2 100755 --- a/tests/test_04_key_jar.py +++ b/tests/test_04_key_jar.py @@ -746,6 +746,12 @@ def test_aud(self): keys = self.bob_keyjar.get_jwt_verify_keys(_jwt.jwt, no_kid_issuer=no_kid_issuer) assert len(keys) == 1 + def test_inactive_verify_key(self): + _jwt = factory(self.sjwt_b) + self.alice_keyjar.return_issuer("Bob")[0].mark_all_as_inactive() + keys = self.alice_keyjar.get_jwt_verify_keys(_jwt.jwt) + assert len(keys) == 1 + def test_copy(): kj = KeyJar() From 4558d6e6ab966651d3d78796c2a4029c2559baac Mon Sep 17 00:00:00 2001 From: Jan Stein Date: Thu, 3 Sep 2020 17:04:15 +0200 Subject: [PATCH 07/15] review updates --- src/cryptojwt/__init__.py | 2 +- src/cryptojwt/key_bundle.py | 89 ++++++++++++++++++++----------------- 2 files changed, 50 insertions(+), 41 deletions(-) diff --git a/src/cryptojwt/__init__.py b/src/cryptojwt/__init__.py index 1ae167ce..6ccd4445 100644 --- a/src/cryptojwt/__init__.py +++ b/src/cryptojwt/__init__.py @@ -21,7 +21,7 @@ except ImportError: pass -__version__ = "1.2.0" +__version__ = "1.2.1a0" logger = logging.getLogger(__name__) diff --git a/src/cryptojwt/key_bundle.py b/src/cryptojwt/key_bundle.py index 5f915607..a2c169de 100755 --- a/src/cryptojwt/key_bundle.py +++ b/src/cryptojwt/key_bundle.py @@ -318,16 +318,21 @@ def do_local_jwk(self, filename): Load a JWKS from a local file :param filename: Name of the file from which the JWKS should be loaded + :return: True if load was successful or False if file hasn't been modified """ - LOGGER.info("Reading local JWKS from %s", filename) - with open(filename) as input_file: - _info = json.load(input_file) - if "keys" in _info: - self.do_keys(_info["keys"]) + if self._local_update_required(): + LOGGER.info("Reading local JWKS from %s", filename) + with open(filename) as input_file: + _info = json.load(input_file) + if "keys" in _info: + self.do_keys(_info["keys"]) + else: + self.do_keys([_info]) + self.last_local = time.time() + self.time_out = self.last_local + self.cache_time + return True else: - self.do_keys([_info]) - self.last_local = time.time() - self.time_out = self.last_local + self.cache_time + return False def do_local_der(self, filename, keytype, keyusage=None, kid=""): """ @@ -336,29 +341,34 @@ def do_local_der(self, filename, keytype, keyusage=None, kid=""): :param filename: Name of the file :param keytype: Presently 'rsa' and 'ec' supported :param keyusage: encryption ('enc') or signing ('sig') or both + :return: True if load was successful or False if file hasn't been modified """ - LOGGER.info("Reading local DER from %s", filename) - key_args = {} - _kty = keytype.lower() - if _kty in ["rsa", "ec"]: - key_args["kty"] = _kty - _key = import_private_key_from_pem_file(filename) - key_args["priv_key"] = _key - key_args["pub_key"] = _key.public_key() - else: - raise NotImplementedError("No support for DER decoding of key type {}".format(_kty)) + if self._local_update_required(): + LOGGER.info("Reading local DER from %s", filename) + key_args = {} + _kty = keytype.lower() + if _kty in ["rsa", "ec"]: + key_args["kty"] = _kty + _key = import_private_key_from_pem_file(filename) + key_args["priv_key"] = _key + key_args["pub_key"] = _key.public_key() + else: + raise NotImplementedError("No support for DER decoding of key type {}".format(_kty)) - if not keyusage: - key_args["use"] = ["enc", "sig"] - else: - key_args["use"] = harmonize_usage(keyusage) + if not keyusage: + key_args["use"] = ["enc", "sig"] + else: + key_args["use"] = harmonize_usage(keyusage) - if kid: - key_args["kid"] = kid + if kid: + key_args["kid"] = kid - self.do_keys([key_args]) - self.last_local = time.time() - self.time_out = self.last_local + self.cache_time + self.do_keys([key_args]) + self.last_local = time.time() + self.time_out = self.last_local + self.cache_time + return True + else: + return False def do_remote(self): """ @@ -390,7 +400,10 @@ def do_remote(self): LOGGER.error(err) raise UpdateFailed(REMOTE_FAILED.format(self.source, str(err))) - if _http_resp.status_code == 200: # New content + load_successful = _http_resp.status_code == 200 + not_modified = _http_resp.status_code == 304 + + if load_successful: self.time_out = time.time() + self.cache_time self.imp_jwks = self._parse_remote_response(_http_resp) @@ -408,9 +421,8 @@ def do_remote(self): if hasattr(_http_resp, "headers"): headers = getattr(_http_resp, "headers") self.last_remote = headers.get("last-modified") or headers.get("date") - res = True - elif _http_resp.status_code == 304: # Not modified + elif not_modified: LOGGER.debug("%s not modified since %s", self.source, self.last_remote) self.time_out = time.time() + self.cache_time res = False @@ -426,7 +438,7 @@ def do_remote(self): self.last_updated = time.time() self.ignore_errors_until = None - return res + return load_successful def _parse_remote_response(self, response): """ @@ -451,14 +463,10 @@ def _parse_remote_response(self, response): return None def _uptodate(self): - res = False if self.remote or self.local: if time.time() > self.time_out: - if self.local and not self._local_update_required(): - res = True - elif self.update(): - res = True - return res + return self.update() + return False def update(self): """ @@ -466,6 +474,8 @@ def update(self): This is a forced update, will happen even if cache time has not elapsed. Replaced keys will be marked as inactive and not removed. + + :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 @@ -476,10 +486,9 @@ def update(self): try: if self.local: if self.fileformat in ["jwks", "jwk"]: - self.do_local_jwk(self.source) + updated = self.do_local_jwk(self.source) elif self.fileformat == "der": - self.do_local_der(self.source, self.keytype, self.keyusage) - updated = True + updated = self.do_local_der(self.source, self.keytype, self.keyusage) elif self.remote: updated = self.do_remote() except Exception as err: From bbe6a05f50afd4026f5217589e58ad2bda906f0c Mon Sep 17 00:00:00 2001 From: Jan Stein Date: Fri, 4 Sep 2020 13:06:17 +0200 Subject: [PATCH 08/15] revert version bump --- src/cryptojwt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptojwt/__init__.py b/src/cryptojwt/__init__.py index 6ccd4445..1ae167ce 100644 --- a/src/cryptojwt/__init__.py +++ b/src/cryptojwt/__init__.py @@ -21,7 +21,7 @@ except ImportError: pass -__version__ = "1.2.1a0" +__version__ = "1.2.0" logger = logging.getLogger(__name__) From b0bea754a45fca32fd755b3a94a572260aa76bf8 Mon Sep 17 00:00:00 2001 From: Ivan Kanakarakis Date: Thu, 10 Sep 2020 17:33:30 +0300 Subject: [PATCH 09/15] Minimize the diff Signed-off-by: Ivan Kanakarakis --- src/cryptojwt/key_bundle.py | 70 ++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/cryptojwt/key_bundle.py b/src/cryptojwt/key_bundle.py index a2c169de..db12600e 100755 --- a/src/cryptojwt/key_bundle.py +++ b/src/cryptojwt/key_bundle.py @@ -320,20 +320,20 @@ def do_local_jwk(self, filename): :param filename: Name of the file from which the JWKS should be loaded :return: True if load was successful or False if file hasn't been modified """ - if self._local_update_required(): - LOGGER.info("Reading local JWKS from %s", filename) - with open(filename) as input_file: - _info = json.load(input_file) - if "keys" in _info: - self.do_keys(_info["keys"]) - else: - self.do_keys([_info]) - self.last_local = time.time() - self.time_out = self.last_local + self.cache_time - return True - else: + if not self._local_update_required(): return False + LOGGER.info("Reading local JWKS from %s", filename) + with open(filename) as input_file: + _info = json.load(input_file) + if "keys" in _info: + self.do_keys(_info["keys"]) + else: + self.do_keys([_info]) + self.last_local = time.time() + self.time_out = self.last_local + self.cache_time + return True + def do_local_der(self, filename, keytype, keyusage=None, kid=""): """ Load a DER encoded file amd create a key from it. @@ -343,32 +343,32 @@ def do_local_der(self, filename, keytype, keyusage=None, kid=""): :param keyusage: encryption ('enc') or signing ('sig') or both :return: True if load was successful or False if file hasn't been modified """ - if self._local_update_required(): - LOGGER.info("Reading local DER from %s", filename) - key_args = {} - _kty = keytype.lower() - if _kty in ["rsa", "ec"]: - key_args["kty"] = _kty - _key = import_private_key_from_pem_file(filename) - key_args["priv_key"] = _key - key_args["pub_key"] = _key.public_key() - else: - raise NotImplementedError("No support for DER decoding of key type {}".format(_kty)) - - if not keyusage: - key_args["use"] = ["enc", "sig"] - else: - key_args["use"] = harmonize_usage(keyusage) + if not self._local_update_required(): + return False - if kid: - key_args["kid"] = kid + LOGGER.info("Reading local DER from %s", filename) + key_args = {} + _kty = keytype.lower() + if _kty in ["rsa", "ec"]: + key_args["kty"] = _kty + _key = import_private_key_from_pem_file(filename) + key_args["priv_key"] = _key + key_args["pub_key"] = _key.public_key() + else: + raise NotImplementedError("No support for DER decoding of key type {}".format(_kty)) - self.do_keys([key_args]) - self.last_local = time.time() - self.time_out = self.last_local + self.cache_time - return True + if not keyusage: + key_args["use"] = ["enc", "sig"] else: - return False + key_args["use"] = harmonize_usage(keyusage) + + if kid: + key_args["kid"] = kid + + self.do_keys([key_args]) + self.last_local = time.time() + self.time_out = self.last_local + self.cache_time + return True def do_remote(self): """ From 603bb355634b27da275c4fd14088895be733964e Mon Sep 17 00:00:00 2001 From: Ivan Kanakarakis Date: Thu, 10 Sep 2020 17:33:46 +0300 Subject: [PATCH 10/15] Remove leftover assignment Signed-off-by: Ivan Kanakarakis --- src/cryptojwt/key_bundle.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cryptojwt/key_bundle.py b/src/cryptojwt/key_bundle.py index db12600e..4a30faf6 100755 --- a/src/cryptojwt/key_bundle.py +++ b/src/cryptojwt/key_bundle.py @@ -421,12 +421,9 @@ def do_remote(self): if hasattr(_http_resp, "headers"): headers = getattr(_http_resp, "headers") self.last_remote = headers.get("last-modified") or headers.get("date") - elif not_modified: LOGGER.debug("%s not modified since %s", self.source, self.last_remote) self.time_out = time.time() + self.cache_time - res = False - else: LOGGER.warning( "HTTP status %d reading remote JWKS from %s", From 7b618b8cd8aa9d6dd36e25496cde39b87626a8b5 Mon Sep 17 00:00:00 2001 From: Ivan Kanakarakis Date: Thu, 10 Sep 2020 17:48:56 +0300 Subject: [PATCH 11/15] Set verbose output for tests Signed-off-by: Ivan Kanakarakis --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index def7f92e..272756a9 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ envlist = py{36,37,38},quality [testenv] passenv = CI TRAVIS TRAVIS_* commands = - py.test --cov=cryptojwt --isort --black {posargs} + pytest -vvv -ra --cov=cryptojwt --isort --black {posargs} codecov extras = testing deps = From aa0f2bd104a4c752ef7fa0ca20d5278cfb2e318c Mon Sep 17 00:00:00 2001 From: Ivan Kanakarakis Date: Fri, 11 Sep 2020 00:59:41 +0300 Subject: [PATCH 12/15] Revert key_use ver back to sig Signed-off-by: Ivan Kanakarakis --- src/cryptojwt/key_issuer.py | 2 +- src/cryptojwt/key_jar.py | 6 +++--- tests/test_04_key_jar.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cryptojwt/key_issuer.py b/src/cryptojwt/key_issuer.py index fed55db7..0e02e1ac 100755 --- a/src/cryptojwt/key_issuer.py +++ b/src/cryptojwt/key_issuer.py @@ -297,7 +297,7 @@ def get(self, key_use, key_type="", kid=None, alg="", **kwargs): else: _bkeys = bundle.keys() for key in _bkeys: - if key.inactive_since and key_use != "ver": + if key.inactive_since and key_use != "sig": # Skip inactive keys unless for signature verification continue if not key.use or use == key.use: diff --git a/src/cryptojwt/key_jar.py b/src/cryptojwt/key_jar.py index c0f1ab8e..4b58bfe7 100755 --- a/src/cryptojwt/key_jar.py +++ b/src/cryptojwt/key_jar.py @@ -601,13 +601,13 @@ def get_jwt_verify_keys(self, jwt, **kwargs): except KeyError: pass - keys = self._add_key([], _iss, "ver", _key_type, _kid, nki, allow_missing_kid) + keys = self._add_key([], _iss, "sig", _key_type, _kid, nki, allow_missing_kid) if _key_type == "oct": - keys.extend(self.get(key_use="ver", issuer_id="", key_type=_key_type)) + keys.extend(self.get(key_use="sig", issuer_id="", key_type=_key_type)) else: # No issuer, just use all keys I have - keys = self.get(key_use="ver", issuer_id="", key_type=_key_type) + keys = self.get(key_use="sig", issuer_id="", key_type=_key_type) # Only want the appropriate keys. keys = [k for k in keys if k.appropriate_for("verify")] diff --git a/tests/test_04_key_jar.py b/tests/test_04_key_jar.py index 3c141fb2..53cb5ef5 100755 --- a/tests/test_04_key_jar.py +++ b/tests/test_04_key_jar.py @@ -750,7 +750,7 @@ def test_inactive_verify_key(self): _jwt = factory(self.sjwt_b) self.alice_keyjar.return_issuer("Bob")[0].mark_all_as_inactive() keys = self.alice_keyjar.get_jwt_verify_keys(_jwt.jwt) - assert len(keys) == 1 + assert len(keys) == 0 def test_copy(): From 54e59e19dcfca8e3ec6a069b0c6b7e6538f5d889 Mon Sep 17 00:00:00 2001 From: Ivan Kanakarakis Date: Fri, 11 Sep 2020 11:02:09 +0300 Subject: [PATCH 13/15] Idle some time before modifying the temp-file Signed-off-by: Ivan Kanakarakis --- tests/test_03_key_bundle.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_03_key_bundle.py b/tests/test_03_key_bundle.py index f4fdac46..7d120269 100755 --- a/tests/test_03_key_bundle.py +++ b/tests/test_03_key_bundle.py @@ -567,6 +567,7 @@ def test_update_2(): ec_key = new_ec_key(crv="P-256", key_ops=["sign"]) _jwks = {"keys": [rsa_key.serialize(), ec_key.serialize()]} + time.sleep(0.5) with open(fname, "w") as fp: fp.write(json.dumps(_jwks)) From df6599d4b1917746312d89e2d3170f786fd8c6bc Mon Sep 17 00:00:00 2001 From: Jakob Schlyter Date: Fri, 11 Sep 2020 17:18:46 +0200 Subject: [PATCH 14/15] prepare for 1.2.1 --- src/cryptojwt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptojwt/__init__.py b/src/cryptojwt/__init__.py index 1ae167ce..522d61fe 100644 --- a/src/cryptojwt/__init__.py +++ b/src/cryptojwt/__init__.py @@ -21,7 +21,7 @@ except ImportError: pass -__version__ = "1.2.0" +__version__ = "1.2.1" logger = logging.getLogger(__name__) From 160e42bc6abdf8a2b6e88eac10b0ce0318b0e4e2 Mon Sep 17 00:00:00 2001 From: Jakob Schlyter Date: Fri, 11 Sep 2020 17:20:06 +0200 Subject: [PATCH 15/15] next release will be 1.3.0 (new options for KeyBundle) --- src/cryptojwt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptojwt/__init__.py b/src/cryptojwt/__init__.py index 522d61fe..44444781 100644 --- a/src/cryptojwt/__init__.py +++ b/src/cryptojwt/__init__.py @@ -21,7 +21,7 @@ except ImportError: pass -__version__ = "1.2.1" +__version__ = "1.3.0" logger = logging.getLogger(__name__)