This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new e66985e993e7 CAMEL-24937: camel-pqc - confine
FileBasedKeyLifecycleManager key file resolution to the configured directory
e66985e993e7 is described below
commit e66985e993e74ce0d361be177fe45d7ddfb8bcdb
Author: Andrea Cosentino <[email protected]>
AuthorDate: Mon Sep 28 18:48:21 2026 +0200
CAMEL-24937: camel-pqc - confine FileBasedKeyLifecycleManager key file
resolution to the configured directory
FileBasedKeyLifecycleManager built key file paths by appending the keyId,
which can come from the CamelPQCKeyId / CamelPQCNewKeyId headers, to the
key directory. A keyId such as ../victim could therefore read, overwrite
or delete a file outside the key directory.
A keyId must now be a flat file name: one containing a forward slash, a
backslash or a NUL character is rejected with an IllegalArgumentException,
and the resolved path is checked to stay inside the (absolute, normalized)
key directory. Deployments that used a keyId like tenant-a/signing to place
keys in a subdirectory must switch to a flat keyId; the upgrade guide
describes it. The in-memory and cloud-backed managers are not affected.
Closes #26782
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Co-authored-by: Claude Opus 4.8 <[email protected]>
Co-authored-by: Guillaume Nodet - AI Bot <[email protected]>
---
.../lifecycle/FileBasedKeyLifecycleManager.java | 36 ++++++++--
.../FileBasedKeyLifecycleManagerPathTest.java | 77 ++++++++++++++++++++++
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 13 ++++
3 files changed, 121 insertions(+), 5 deletions(-)
diff --git
a/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/lifecycle/FileBasedKeyLifecycleManager.java
b/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/lifecycle/FileBasedKeyLifecycleManager.java
index 33901921c08f..8d4b43f67e2b 100644
---
a/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/lifecycle/FileBasedKeyLifecycleManager.java
+++
b/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/lifecycle/FileBasedKeyLifecycleManager.java
@@ -67,7 +67,7 @@ public class FileBasedKeyLifecycleManager implements
KeyLifecycleManager {
private final ConcurrentHashMap<String, KeyMetadata> metadataCache = new
ConcurrentHashMap<>();
public FileBasedKeyLifecycleManager(String keyDirectoryPath) throws
IOException {
- this.keyDirectory = Paths.get(keyDirectoryPath);
+ this.keyDirectory =
Paths.get(keyDirectoryPath).toAbsolutePath().normalize();
this.objectMapper = new ObjectMapper();
this.objectMapper.enable(SerializationFeature.INDENT_OUTPUT);
Files.createDirectories(keyDirectory);
@@ -442,19 +442,45 @@ public class FileBasedKeyLifecycleManager implements
KeyLifecycleManager {
}
private Path getPrivateKeyFile(String keyId) {
- return keyDirectory.resolve(keyId + ".private.json");
+ return resolveKeyFile(keyId, ".private.json");
}
private Path getPublicKeyFile(String keyId) {
- return keyDirectory.resolve(keyId + ".public.json");
+ return resolveKeyFile(keyId, ".public.json");
}
private Path getMetadataFile(String keyId) {
- return keyDirectory.resolve(keyId + ".metadata");
+ return resolveKeyFile(keyId, ".metadata");
}
private Path getLegacyKeyFile(String keyId) {
- return keyDirectory.resolve(keyId + ".key");
+ return resolveKeyFile(keyId, ".key");
+ }
+
+ /**
+ * Resolves a key file inside the configured key directory, confining it
to that directory. keyId values arrive from
+ * message headers (CamelPQCKeyId / CamelPQCNewKeyId), so a value
containing path separators, parent references or
+ * an absolute path could otherwise resolve to a location outside the key
directory. keyId is therefore constrained
+ * to a single flat file-name segment, and the resolved path is checked to
lie textually under the key directory
+ * (which is stored absolute and normalized in the constructor).
+ * <p/>
+ * The {@code startsWith} check is textual and does not follow symbolic
links, so a symlink placed inside the key
+ * directory could still point elsewhere. Creating such a link requires
write access to the operator-controlled key
+ * directory, which is outside the header-supplied keyId threat this
guards against.
+ */
+ Path resolveKeyFile(String keyId, String suffix) {
+ if (keyId == null || keyId.isBlank()) {
+ throw new IllegalArgumentException("keyId must not be null or
empty");
+ }
+ if (keyId.indexOf('/') >= 0 || keyId.indexOf('\\') >= 0 ||
keyId.indexOf('\0') >= 0) {
+ throw new IllegalArgumentException("keyId must not contain path
separators (length: " + keyId.length() + ")");
+ }
+ Path resolved = keyDirectory.resolve(keyId + suffix).normalize();
+ if (!resolved.startsWith(keyDirectory)) {
+ throw new IllegalArgumentException(
+ "keyId must resolve inside the key directory (length: " +
keyId.length() + ")");
+ }
+ return resolved;
}
/**
diff --git
a/components/camel-pqc/src/test/java/org/apache/camel/component/pqc/lifecycle/FileBasedKeyLifecycleManagerPathTest.java
b/components/camel-pqc/src/test/java/org/apache/camel/component/pqc/lifecycle/FileBasedKeyLifecycleManagerPathTest.java
new file mode 100644
index 000000000000..6d54536b5877
--- /dev/null
+++
b/components/camel-pqc/src/test/java/org/apache/camel/component/pqc/lifecycle/FileBasedKeyLifecycleManagerPathTest.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.pqc.lifecycle;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * keyId values reach the file-based key store from message headers
(CamelPQCKeyId / CamelPQCNewKeyId). The manager must
+ * keep every key file inside its configured directory regardless of the
supplied keyId.
+ */
+class FileBasedKeyLifecycleManagerPathTest {
+
+ @Test
+ void rejectsKeyIdWithParentTraversal(@TempDir Path tempDir) throws
Exception {
+ Path keyDir = tempDir.resolve("keys");
+ FileBasedKeyLifecycleManager manager = new
FileBasedKeyLifecycleManager(keyDir.toString());
+
+ // A sentinel file one level above the key directory that an
unconfined "../victim" keyId would target
+ Path sentinel = tempDir.resolve("victim.private.json");
+ Files.writeString(sentinel, "{}");
+
+ assertThrows(IllegalArgumentException.class, () ->
manager.getKey("../victim"));
+ assertThrows(IllegalArgumentException.class, () ->
manager.deleteKey("../victim"));
+
+ assertTrue(Files.exists(sentinel), "a file outside the key directory
must not be reachable via keyId");
+ }
+
+ @Test
+ void rejectsAbsoluteSeparatorAndBlankKeyIds(@TempDir Path tempDir) throws
Exception {
+ Path keyDir = tempDir.resolve("keys");
+ FileBasedKeyLifecycleManager manager = new
FileBasedKeyLifecycleManager(keyDir.toString());
+
+ assertThrows(IllegalArgumentException.class, () ->
manager.getKeyMetadata("sub/evil"));
+ assertThrows(IllegalArgumentException.class, () ->
manager.getKeyMetadata("/etc/evil"));
+ assertThrows(IllegalArgumentException.class,
+ () ->
manager.getKeyMetadata(tempDir.resolve("abs").toString()));
+ assertThrows(IllegalArgumentException.class, () ->
manager.getKeyMetadata(""));
+ // NUL character — caught by character guard
+ assertThrows(IllegalArgumentException.class, () ->
manager.getKeyMetadata("evil\0inject"));
+ }
+
+ @Test
+ void allowsPlainKeyId(@TempDir Path tempDir) throws Exception {
+ Path keyDir = tempDir.resolve("keys");
+ FileBasedKeyLifecycleManager manager = new
FileBasedKeyLifecycleManager(keyDir.toString());
+
+ // A normal flat keyId is accepted: metadata for an absent key returns
null rather than being rejected
+ assertNull(manager.getKeyMetadata("tenant-a-signing-key"));
+ // and its key file resolves directly inside the key directory
+ Path resolved = manager.resolveKeyFile("tenant-a-signing-key",
".private.json");
+ assertEquals(keyDir.toAbsolutePath().normalize(),
resolved.getParent());
+ assertEquals("tenant-a-signing-key.private.json",
resolved.getFileName().toString());
+ }
+}
diff --git
a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
index 22e3d3ae84a2..96ade35bd1bf 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
@@ -3147,6 +3147,18 @@ are unaffected. Routes that set the header by its
literal string name, or that u
`allowTemplateFromHeader=true` with the old header names, must switch to the
new `Camel`-prefixed
names.
+=== camel-pqc - FileBasedKeyLifecycleManager restricts keyId to a flat file
name
+
+`FileBasedKeyLifecycleManager` now confines every key file to its configured
key directory. A `keyId`
+(supplied through the `CamelPQCKeyId` / `CamelPQCNewKeyId` headers) that
contains a forward slash, a
+backslash, a null character, or that would otherwise resolve outside the key
directory is now rejected
+with an `IllegalArgumentException`.
+
+Previously a `keyId` such as `tenant-a/signing` resolved into a subdirectory
of the key directory.
+Deployments that organised keys that way must switch to a flat `keyId` (for
example `tenant-a-signing`).
+Only the file-based manager is affected; the in-memory and cloud-backed
managers were never file-path
+based.
+
=== camel-crypto
Three changes to `CryptoDataFormat`, none of which affects the format of data
already written.
@@ -3172,6 +3184,7 @@ allocation; a declared length outside 0–1024 is now
rejected instead of attemp
Not changed: the HMAC key is still derived from the same key material as the
cipher. Separating them
would change the MAC written into the message and so could not be read by
earlier versions; that is
tracked separately.
+
=== camel-debezium - a failed embedded engine is now reported
The Debezium consumers now register a `CompletionCallback` on the embedded
engine. When the engine stops
