Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
Add a registry for managing key types.
Review Request #15183 — Created July 21, 2026 and updated
This introduces
KeyTypesRegistry, which tracks all key types supported
by Cryptozoology. This provides access to keys by key type ID and by
encryption or keywrap algorithm.While this only includes support for AES and EC keys right now, it would
allow callers to provide or override key types as needed.The existing call sites that hard-code knowledge of key types now make
use of the registry to fetch keys by ID or algorithm, except where
appropriate (e.g., if explicitly an AES key needs to be created).
Unit tests pass.
| Summary | ID |
|---|---|
| 908e102286672e8dda1fcd967449e9c176c4e818 |
| Description | From | Last Updated |
|---|---|---|
|
Do we want to document somewhere that get_key_type('ec') resolves only to the private key class? It feels a little confusing … |
david
|
|
|
Do we not want to reexport KeyTypesRegistry as well? |
david
|
|
|
We should protect this with the registry's lock so we don't end up with issues across threads: with self._lock: for … |
david
|
|
|
This ends up caching None for invalid values, making it so a stream of Secret.decode() calls with random alg= values … |
david
|
|
|
Same here regarding self._lock and not caching invalid values. Perhaps we could extract this into a helper? |
david
|
|
|
Can we make this use the same typed signature as the base class? |
david
|
|
|
This reads a little odd. How about just "This clears the algorithm caches whenever an item is registered"? |
david
|
|
|
Can we make this use the same typed signature as the base class? |
david
|
|
|
Same here re: "This will be used" |
david
|
|
|
If we wrap the cache writes in with self._lock its less of an issue, but this is rebinding rather than … |
david
|
|
|
The actual implementation is named "KeyTypesRegistry" (with an s). The name here (and in all the test methods/docstrings) is incorrect. … |
david
|
|
|
Can we call registry.populate() here so that this matches more closely with test_get_for_keywrap_alg_uses_cache? |
david
|
-
-
Do we want to document somewhere that
get_key_type('ec')resolves only to the private key class? It feels a little confusing that we have separate public and private classes but only the private one is registered. -
-
We should protect this with the registry's lock so we don't end up with issues across threads:
with self._lock: for key_cls in self: ... self._enc_key_cache_map[alg] = key_cls -
This ends up caching
Nonefor invalid values, making it so a stream ofSecret.decode()calls with randomalg=values grows memory without bound.Can we only cache positive results?
Same for the keywrap algorithm.
-
Same here regarding
self._lockand not caching invalid values. Perhaps we could extract this into a helper? -
-
This reads a little odd. How about just "This clears the algorithm caches whenever an item is registered"?
-
-
-
If we wrap the cache writes in
with self._lockits less of an issue, but this is rebinding rather than clearing. I think it would be better to useself._*_map.clear()here. -
The actual implementation is named "KeyTypesRegistry" (with an s). The name here (and in all the test methods/docstrings) is incorrect.
That said, all of our other registries are named in the singular, so
KeyTypeRegistryis perhaps the better one to use. -
Can we call
registry.populate()here so that this matches more closely withtest_get_for_keywrap_alg_uses_cache?