[
https://issues.apache.org/jira/browse/CAMEL-25801?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen resolved CAMEL-25801.
---------------------------------
Resolution: Fixed
Fixed by https://github.com/apache/camel/pull/28105 (merged to main for 4.24.0).
_Claude Code on behalf of davsclaus_
> 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
> Assignee: Andrea Cosentino
> Priority: Major
> Fix For: 4.24.0
>
>
> 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)
