Andrea Cosentino created CAMEL-25801:
----------------------------------------
Summary: camel-pqc - align the KeyLifecycleManager implementations
on metadata updates, listing and deletion
Key: CAMEL-25801
URL: https://issues.apache.org/jira/browse/CAMEL-25801
Project: Camel
Issue Type: Bug
Components: camel-pqc
Reporter: Andrea Cosentino
The five {{KeyLifecycleManager}} implementations all override all fifteen
interface methods, but they diverge behaviourally in ways the interface does
not document and callers cannot anticipate.
h3. 1. Three managers rewrite the private key on every metadata update
{{AwsSecretsManagerKeyLifecycleManager.java:370-376}} (identical at
{{AzureKeyVaultKeyLifecycleManager.java:297-303}} and
{{HashicorpVaultKeyLifecycleManager.java:410-416}}):
{code:java}
public void updateKeyMetadata(String keyId, KeyMetadata metadata) throws
Exception {
// Read existing key pair
KeyPair keyPair = getKey(keyId);
// Store updated metadata with existing key pair
storeKey(keyId, keyPair, metadata);
}
{code}
versus {{FileBasedKeyLifecycleManager.java:236-242}}, which writes only the
metadata file.
Consequences: every {{expireKey}}, {{revokeKey}} and rotation-deprecation
re-uploads the PKCS#8 private key and creates a new secret version — on Vault
KV v2 with the default {{max_versions = 10}}, ten metadata edits purge every
historical version of the private key. It also means {{revokeKey}} on a key
whose private secret is already gone throws from {{getKey}} instead of
recording the revocation, which is exactly when revocation matters most.
{{PQCProducer.persistStatefulKeyStateAfterSign}} compounds it at {{:1186-1189}}
by calling {{updateKeyMetadata}} and then {{storeKey}} — six remote writes per
signature where three suffice.
h3. 2. {{FileBasedKeyLifecycleManager.listKeys()}} reads a boot-time cache
{{FileBasedKeyLifecycleManager.java:257-259}}:
{code:java}
public List<KeyMetadata> listKeys() throws Exception {
return new ArrayList<>(metadataCache.values());
}
{code}
The other managers query the backing store (AWS {{:392}} {{listSecrets}}, Azure
{{:319}} {{listPropertiesOfSecrets}}, Vault {{:435}} {{vaultTemplate.list}}).
The directory is scanned exactly once, by {{loadExistingKeys()}} in the
constructor ({{:76}}, {{:384}}). Any key file written after start-up is
invisible to {{listKeys()}} forever, even though {{getKey}} would read it
happily. Since {{KeyRotationScheduler.checkAndRotate()}} drives entirely off
{{listKeys()}} ({{:123}}), a file-backed deployment silently stops rotating
anything it did not see at boot.
h3. 3. Opposite deletion semantics behind one interface
{{AwsSecretsManagerKeyLifecycleManager.java:525-529}}:
{code:java}
DeleteSecretRequest request = DeleteSecretRequest.builder()
.secretId(secretName)
.forceDeleteWithoutRecovery(true)
.build();
{code}
bypasses the 7–30 day recovery window entirely, while
{{AzureKeyVaultKeyLifecycleManager.java:405-408}}:
{code:java}
secretClient.beginDeleteSecret(secretName);
LOG.debug("Deletion initiated for secret: {}", secretName);
{code}
discards the returned {{SyncPoller}}, so {{deleteKey}} returns before the vault
has deleted anything and a subsequent {{storeKey}} with the same id can race
the in-flight delete. The whole contract of {{KeyLifecycleManager#deleteKey}}
({{:78-81}}) is {{/** Delete key */}}.
h3. 4. {{deleteKeyState}} silently succeeds with no manager configured
{{PQCProducer.java:1102-1105}}:
{code:java}
KeyLifecycleManager klm = getConfiguration().getKeyLifecycleManager();
if (klm != null) {
klm.deleteKey(keyId);
}
{code}
Every other lifecycle operation uses {{getRequiredKeyLifecycleManager()}}
({{:929-937}}) and fails loudly. Reporting success for a stateful key state
that was never deleted is the worst available failure mode here.
h3. 5. {{exportKey(PEM, includePrivate=true)}} cannot be re-imported
{{KeyFormatConverter.java:72-74}}:
{code:java}
if (format == KeyLifecycleManager.KeyFormat.PEM) {
return (new String(publicKeyBytes) + "\n" + new
String(privateKeyBytes)).getBytes();
{code}
{{KeyFormatConverter.java:168-173}}:
{code:java}
String pemString = new String(pemData);
pemString = pemString.replace("-----BEGIN PRIVATE KEY-----", "")
.replace("-----END PRIVATE KEY-----", "")
.replaceAll("\\s", "");
return Base64.getDecoder().decode(pemString);
{code}
The combined blob carries both armour pairs; {{parsePEMPrivateKey}} strips only
the PRIVATE markers, leaving {{-----BEGIN PUBLIC KEY-----}} in the string
passed to {{Base64.getDecoder()}}, which rejects {{-}}. The {{importKey}}
fallback to {{importPublicKey}} hits the mirror-image failure at {{:155-162}}.
So the only export format that carries a private key does not round-trip. The
DER branch ({{:76-77}}) silently drops the public key.
The same class uses the platform default charset at {{:74}}, {{:134}},
{{:151}}, {{:158}} and {{:169}}.
h3. 6. AWS and Vault still duplicate {{KeyAlgorithmSupport}}
{{AwsSecretsManagerKeyLifecycleManager.java:177-218}} and
{{HashicorpVaultKeyLifecycleManager.java:191-232}} inline their own key
generation, provider resolution and {{getDefaultKeySize}}, while InMemory,
FileBased and Azure call {{KeyAlgorithmSupport.generateKeyPair}} in one line.
The copies are already dead-ish: both {{getDefaultKeySize(String algorithm)}}
implementations ignore their parameter and return {{256}}.
h3. 7. AWS {{listKeys()}} has no server-side filter
{{AwsSecretsManagerKeyLifecycleManager.java:398-399}} paginates correctly via
{{nextToken}}, but applies no {{filters(...)}}, so a shared AWS account is
paged through in full on every rotation tick and filtered client-side at
{{:410}}.
h3. Suggested fix
Give {{KeyLifecycleManager}} a real contract per method, make
{{updateKeyMetadata}} metadata-only everywhere, make {{FileBased.listKeys()}}
re-scan the directory, settle on one deletion semantic (recoverable, awaited),
route {{deleteKeyState}} through {{getRequiredKeyLifecycleManager()}}, fix the
combined-PEM round-trip with explicit UTF-8, migrate AWS/Vault onto
{{KeyAlgorithmSupport}}, and add a name filter to the AWS list call.
h3. Test gaps
No test round-trips {{exportKey(PEM, includePrivate=true)}} through
{{importKey}} — {{PQCKeyLifecycleTest.java:121}} exports and asserts on bytes
only, and every import test uses {{exportPublicKey}}. No test writes a key file
out of band and then calls {{FileBased.listKeys()}}. AWS and HashiCorp have
only {{*IT}} tests needing live services, so none of the above has a unit-level
net; a mocked client test mirroring {{AzureKeyVaultKeyLifecycleManagerTest}}
would close that.
----
_Reported by Claude Code (AI agent) on behalf of [~oscerd]. Every code
reference was verified against commit {{7aae27315946}} on {{main}}._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
