From 670aa54643f2a7baeb4c8cb3ab55e2c08e98741a Mon Sep 17 00:00:00 2001 From: iromli Date: Mon, 12 Feb 2024 04:00:53 +0700 Subject: [PATCH 1/5] fix: handle incorrect types of JWKS Signed-off-by: iromli --- .../scripts/auth_handler.py | 49 +++++++++++++++++-- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/docker-jans-certmanager/scripts/auth_handler.py b/docker-jans-certmanager/scripts/auth_handler.py index df0c348f346..600c48b5037 100644 --- a/docker-jans-certmanager/scripts/auth_handler.py +++ b/docker-jans-certmanager/scripts/auth_handler.py @@ -210,8 +210,7 @@ def __init__(self, manager, dry_run, **opts): @property def allowed_key_algs(self): - algs = self.sig_keys.split() + self.enc_keys.split() - return algs + return self.sig_keys.split() + self.enc_keys.split() def get_merged_keys(self, exp_hours): # get previous JWKS @@ -248,7 +247,7 @@ def get_merged_keys(self, exp_hours): if retcode != 0: logger.error(f"Unable to generate keys; reason={err.decode()}") - return + return "", "" new_jwks = deque(json.loads(out).get("keys", [])) @@ -263,6 +262,12 @@ def get_merged_keys(self, exp_hours): if key_ops_from_jwk(jwk) == "connect" ) + # counter for `ssa` keys + cnt_ssa = Counter( + jwk["alg"] for jwk in new_jwks + if key_ops_from_jwk(jwk) == "ssa" + ) + for jwk in old_jwks: # exclude alg if it's not allowed if jwk["alg"] not in self.allowed_key_algs: @@ -270,16 +275,27 @@ def get_merged_keys(self, exp_hours): ops_type = key_ops_from_jwk(jwk) + # exclude unsupported key_ops_type + if ops_type not in ["ssa", "connect"]: + continue + # cannot have more than 2 connect keys for same algorithm in new JWKS if ops_type == "connect" and cnt[jwk["alg"]] > 1: continue + # cannot have more than 1 connect keys for same algorithm in new JWKS + if ops_type == "ssa" and cnt_ssa[jwk["alg"]] > 0: + continue + # insert old key to new keys new_jwks.appendleft(jwk) if ops_type == "connect": cnt[jwk["alg"]] += 1 + if ops_type == "ssa": + cnt_ssa[jwk["alg"]] += 1 + # import key to new JKS keytool_import_key(old_jks_fn, jks_fn, jwk["kid"], jks_pass) @@ -523,9 +539,14 @@ def prune(self): if key_ops_from_jwk(jwk) == "connect" ) + cnt_ssa = Counter( + jwk["alg"] for jwk in new_jwks + if key_ops_from_jwk(jwk) == "ssa" + ) + for jwk in old_jwks: # exclude alg if it's not allowed - if jwk["alg"] not in self.allowed_key_algs: + if jwk.get("alg") not in self.allowed_key_algs: keytool_delete_key(jks_fn, jwk["kid"], jks_pass) continue @@ -536,12 +557,20 @@ def prune(self): keytool_delete_key(jks_fn, jwk["kid"], jks_pass) continue + # cannot have more than 1 key for same algorithm in new JWKS + if ops_type == "ssa" and cnt_ssa[jwk["alg"]]: + keytool_delete_key(jks_fn, jwk["kid"], jks_pass) + continue + # preserve the key new_jwks.append(jwk) if ops_type == "connect": cnt[jwk["alg"]] += 1 + if ops_type == "ssa": + cnt_ssa[jwk["alg"]] += 1 + web_keys["keys"] = new_jwks jwks_fn = "/etc/certs/auth-keys.json" @@ -665,7 +694,17 @@ def resolve_enc_keys(keys: str) -> str: def key_ops_from_jwk(jwk): """Resolve key_ops_type first value.""" - return (jwk.get("key_ops_type") or ["connect"])[0] + types = jwk.get("key_ops_type") or [] + try: + type_ = types[0] + except IndexError: + if jwk["kid"].startswith("connect_"): + type_ = "connect" + elif jwk["kid"].startswith("ssa_"): + type_ = "ssa" + else: + type_ = "" + return type_ def has_ext_jwks_uri(conf_dynamic, manager) -> bool: From dbb26018715f97ac6c07b04c71e1d3f10915d48c Mon Sep 17 00:00:00 2001 From: iromli Date: Mon, 12 Feb 2024 04:04:55 +0700 Subject: [PATCH 2/5] fix: handle missing alg from jwks Signed-off-by: iromli --- docker-jans-certmanager/scripts/auth_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-jans-certmanager/scripts/auth_handler.py b/docker-jans-certmanager/scripts/auth_handler.py index 600c48b5037..07c54e034e9 100644 --- a/docker-jans-certmanager/scripts/auth_handler.py +++ b/docker-jans-certmanager/scripts/auth_handler.py @@ -270,7 +270,7 @@ def get_merged_keys(self, exp_hours): for jwk in old_jwks: # exclude alg if it's not allowed - if jwk["alg"] not in self.allowed_key_algs: + if jwk.get("alg") not in self.allowed_key_algs: continue ops_type = key_ops_from_jwk(jwk) From 6407ebfdd04d0e88bb075628e994905e0c7a6bef Mon Sep 17 00:00:00 2001 From: iromli Date: Mon, 12 Feb 2024 23:32:25 +0700 Subject: [PATCH 3/5] fix: handle corrupted web keys Signed-off-by: iromli --- docker-jans-certmanager/scripts/auth_handler.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/docker-jans-certmanager/scripts/auth_handler.py b/docker-jans-certmanager/scripts/auth_handler.py index 07c54e034e9..5afcd4bbb8c 100644 --- a/docker-jans-certmanager/scripts/auth_handler.py +++ b/docker-jans-certmanager/scripts/auth_handler.py @@ -280,11 +280,11 @@ def get_merged_keys(self, exp_hours): continue # cannot have more than 2 connect keys for same algorithm in new JWKS - if ops_type == "connect" and cnt[jwk["alg"]] > 1: + if ops_type == "connect" and cnt[jwk["alg"]] >= 2: continue # cannot have more than 1 connect keys for same algorithm in new JWKS - if ops_type == "ssa" and cnt_ssa[jwk["alg"]] > 0: + if ops_type == "ssa" and cnt_ssa[jwk["alg"]] >= 1: continue # insert old key to new keys @@ -369,6 +369,10 @@ def patch(self): web_keys = json.loads(config["jansConfWebKeys"]) except TypeError: web_keys = config["jansConfWebKeys"] + except json.decoder.JSONDecodeError as exc: + # probably corrupted JSON + logger.warning(f"Unable to load jansConfWebKeys; reason={exc}. New JWKS will be created and overwrite existing value.") + web_keys = {"keys": []} with open("/etc/certs/auth-keys.old.json", "w") as f: f.write(json.dumps(web_keys, indent=2)) @@ -520,6 +524,10 @@ def prune(self): web_keys = json.loads(config["jansConfWebKeys"]) except TypeError: web_keys = config["jansConfWebKeys"] + except json.decoder.JSONDecodeError as exc: + # probably corrupted JSON + logger.warning(f"Unable to load jansConfWebKeys; reason={exc}. Existing JWKS will be deleted.") + web_keys = {"keys": []} logger.info("Cleaning up keys (if any)") From a08145e76630ec41cdc9dc7d08e23af3cd3b1fd9 Mon Sep 17 00:00:00 2001 From: iromli Date: Tue, 13 Feb 2024 05:04:38 +0700 Subject: [PATCH 4/5] fix: collect key_ops_type with lowercase value Signed-off-by: iromli --- docker-jans-certmanager/scripts/auth_handler.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docker-jans-certmanager/scripts/auth_handler.py b/docker-jans-certmanager/scripts/auth_handler.py index 5afcd4bbb8c..e22b754111c 100644 --- a/docker-jans-certmanager/scripts/auth_handler.py +++ b/docker-jans-certmanager/scripts/auth_handler.py @@ -230,6 +230,7 @@ def get_merged_keys(self, exp_hours): # a counter to determine value of `key_ops_type` to be passed to `KeyGenerator` ops_type_cnt = Counter(key_ops_from_jwk(jwk) for jwk in old_jwks) + logger.info(f"Detected key_ops_type={dict(ops_type_cnt)}") # if we have ssa key, use `connect` keys if ops_type_cnt["ssa"]: @@ -371,7 +372,7 @@ def patch(self): web_keys = config["jansConfWebKeys"] except json.decoder.JSONDecodeError as exc: # probably corrupted JSON - logger.warning(f"Unable to load jansConfWebKeys; reason={exc}. New JWKS will be created and overwrite existing value.") + logger.warning(f"Unable to load existing JWKS; reason={exc}. New JWKS will be created.") web_keys = {"keys": []} with open("/etc/certs/auth-keys.old.json", "w") as f: @@ -526,7 +527,7 @@ def prune(self): web_keys = config["jansConfWebKeys"] except json.decoder.JSONDecodeError as exc: # probably corrupted JSON - logger.warning(f"Unable to load jansConfWebKeys; reason={exc}. Existing JWKS will be deleted.") + logger.warning(f"Unable to load existing JWKS; reason={exc}. Existing JWKS will be deleted.") web_keys = {"keys": []} logger.info("Cleaning up keys (if any)") @@ -712,7 +713,7 @@ def key_ops_from_jwk(jwk): type_ = "ssa" else: type_ = "" - return type_ + return type_.lower() def has_ext_jwks_uri(conf_dynamic, manager) -> bool: From 40801393666929c3687476a3db357c4547ea3fa4 Mon Sep 17 00:00:00 2001 From: iromli Date: Tue, 13 Feb 2024 23:41:27 +0700 Subject: [PATCH 5/5] fix: check uppercased algorithm name in JWKS Signed-off-by: iromli --- .../scripts/auth_handler.py | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/docker-jans-certmanager/scripts/auth_handler.py b/docker-jans-certmanager/scripts/auth_handler.py index e22b754111c..2e4c1e9bae3 100644 --- a/docker-jans-certmanager/scripts/auth_handler.py +++ b/docker-jans-certmanager/scripts/auth_handler.py @@ -259,19 +259,21 @@ def get_merged_keys(self, exp_hours): # counter for `connect` keys cnt = Counter( - jwk["alg"] for jwk in new_jwks + jwk["alg"].upper() for jwk in new_jwks if key_ops_from_jwk(jwk) == "connect" ) # counter for `ssa` keys cnt_ssa = Counter( - jwk["alg"] for jwk in new_jwks + jwk["alg"].upper() for jwk in new_jwks if key_ops_from_jwk(jwk) == "ssa" ) for jwk in old_jwks: + alg = jwk.get("alg", "").upper() + # exclude alg if it's not allowed - if jwk.get("alg") not in self.allowed_key_algs: + if alg not in self.allowed_key_algs: continue ops_type = key_ops_from_jwk(jwk) @@ -281,21 +283,21 @@ def get_merged_keys(self, exp_hours): continue # cannot have more than 2 connect keys for same algorithm in new JWKS - if ops_type == "connect" and cnt[jwk["alg"]] >= 2: + if ops_type == "connect" and cnt[alg] >= 2: continue - # cannot have more than 1 connect keys for same algorithm in new JWKS - if ops_type == "ssa" and cnt_ssa[jwk["alg"]] >= 1: + # cannot have more than 1 ssa keys for same algorithm in new JWKS + if ops_type == "ssa" and cnt_ssa[alg] >= 1: continue # insert old key to new keys new_jwks.appendleft(jwk) if ops_type == "connect": - cnt[jwk["alg"]] += 1 + cnt[alg] += 1 if ops_type == "ssa": - cnt_ssa[jwk["alg"]] += 1 + cnt_ssa[alg] += 1 # import key to new JKS keytool_import_key(old_jks_fn, jks_fn, jwk["kid"], jks_pass) @@ -544,30 +546,32 @@ def prune(self): old_jwks = sorted(old_jwks, key=lambda k: k["exp"], reverse=True) cnt = Counter( - jwk["alg"] for jwk in new_jwks + jwk["alg"].upper() for jwk in new_jwks if key_ops_from_jwk(jwk) == "connect" ) cnt_ssa = Counter( - jwk["alg"] for jwk in new_jwks + jwk["alg"].upper() for jwk in new_jwks if key_ops_from_jwk(jwk) == "ssa" ) for jwk in old_jwks: + alg = jwk.get("alg", "").upper() + # exclude alg if it's not allowed - if jwk.get("alg") not in self.allowed_key_algs: + if alg not in self.allowed_key_algs: keytool_delete_key(jks_fn, jwk["kid"], jks_pass) continue ops_type = key_ops_from_jwk(jwk) - # cannot have more than 1 key for same algorithm in new JWKS - if ops_type == "connect" and cnt[jwk["alg"]]: + # cannot have more than 1 connect key for same algorithm in new JWKS + if ops_type == "connect" and cnt[alg] >= 1: keytool_delete_key(jks_fn, jwk["kid"], jks_pass) continue - # cannot have more than 1 key for same algorithm in new JWKS - if ops_type == "ssa" and cnt_ssa[jwk["alg"]]: + # cannot have more than 1 ssa key for same algorithm in new JWKS + if ops_type == "ssa" and cnt_ssa[alg] >= 1: keytool_delete_key(jks_fn, jwk["kid"], jks_pass) continue @@ -575,10 +579,10 @@ def prune(self): new_jwks.append(jwk) if ops_type == "connect": - cnt[jwk["alg"]] += 1 + cnt[alg] += 1 if ops_type == "ssa": - cnt_ssa[jwk["alg"]] += 1 + cnt_ssa[alg] += 1 web_keys["keys"] = new_jwks