luigidemasi commented on code in PR #27821:
URL: https://github.com/apache/camel/pull/27821#discussion_r4270491447
##########
components/camel-ssh/src/main/java/org/apache/camel/component/ssh/ResourceBasedSSHKeyVerifier.java:
##########
@@ -88,105 +147,143 @@ public boolean verifyServerKey(ClientSession
sshClientSession, SocketAddress rem
}
}
- private PublicKey findKeyForServerToken(InputStream knownHostsInputStream,
List<String> possibleTokens) {
- String knowHostsLines =
camelContext.getTypeConverter().convertTo(String.class, knownHostsInputStream);
- if (knowHostsLines == null) {
- log.warn("Could not read from the known_hosts file input stream");
- return null;
+ private boolean isSameKey(KnownHostsEntry entry, PublicKey serverKey) {
+ if (entry.key == null) {
+ log.warn("Unsupported key type {} in the known_hosts file {}",
entry.keyType, knownHostsResource);
+ return false;
}
+ return KeyUtils.compareKeys(entry.key, serverKey);
+ }
- for (String s : knowHostsLines.split("\n")) {
- String[] parts = s.split(" ");
- if (parts.length != 3) {
- log.warn("Found malformed entry in known_hosts file");
- continue;
- }
- String entry = parts[0];
- String key = parts[2];
- for (String serverToken : possibleTokens) {
- if (entry.contains(serverToken)) {
- try {
- return loadKey(key);
- } catch (NoSuchAlgorithmException |
InvalidKeySpecException e) {
- log.warn(String.format("Could not load key for server
token %s", entry), e);
- }
- }
- }
+ /*
+ * The name OpenSSH looks the server up by: the host name or address the
client connected to, as configured on the
+ * endpoint and never resolved, written host for port 22 and [host]:port
for another port.
+ */
+ private static String toKnownHostsName(SocketAddress remoteAddress) {
+ if (!(remoteAddress instanceof InetSocketAddress) && !(remoteAddress
instanceof SshdSocketAddress)) {
+ return null;
}
- return null;
+ SshdSocketAddress address =
SshdSocketAddress.toSshdSocketAddress(remoteAddress);
+ String host = address.getHostName().toLowerCase(Locale.ROOT);
Review Comment:
**Major / P1: Preserve the configured IPv6 spelling for known-host lookup
(pre-existing issue).**
`SshdSocketAddress.toSshdSocketAddress(remoteAddress)` delegates to
`InetSocketAddress.getHostString()`, which does not necessarily retain a
numeric IPv6 literal as configured. Connecting to `::1` gives this verifier
`0:0:0:0:0:0:0:1`.
With a valid known_hosts entry for `[::1]:<port>`, the code instead looks
for `[0:0:0:0:0:0:0:1]:<port>`. The configured server is consequently
classified as unknown:
- with `failOnUnknownHost=true`, even the correct listed key is rejected;
- with the default `false`, an arbitrary different key is accepted.
I reproduced both cases using a real IPv6 loopback SSH server. In the second
case the server received the synthetic password. `ssh-keygen -F '[::1]:<port>'`
recognizes the same fixture.
This behavior also occurs before this PR; I am flagging an existing gap in
the lookup path being changed/documented, not claiming a new regression. Please
carry the configured host spelling through to verification rather than relying
on the resolved socket address to preserve it, and add connection-level IPv6
tests for the legitimate and mismatching keys with both values of
`failOnUnknownHost`.
_AI-generated review by Codex on behalf of @luigidemasi._
##########
components/camel-ssh/src/main/java/org/apache/camel/component/ssh/ResourceBasedSSHKeyVerifier.java:
##########
@@ -88,105 +147,143 @@ public boolean verifyServerKey(ClientSession
sshClientSession, SocketAddress rem
}
}
- private PublicKey findKeyForServerToken(InputStream knownHostsInputStream,
List<String> possibleTokens) {
- String knowHostsLines =
camelContext.getTypeConverter().convertTo(String.class, knownHostsInputStream);
- if (knowHostsLines == null) {
- log.warn("Could not read from the known_hosts file input stream");
- return null;
+ private boolean isSameKey(KnownHostsEntry entry, PublicKey serverKey) {
+ if (entry.key == null) {
+ log.warn("Unsupported key type {} in the known_hosts file {}",
entry.keyType, knownHostsResource);
+ return false;
}
+ return KeyUtils.compareKeys(entry.key, serverKey);
+ }
- for (String s : knowHostsLines.split("\n")) {
- String[] parts = s.split(" ");
- if (parts.length != 3) {
- log.warn("Found malformed entry in known_hosts file");
- continue;
- }
- String entry = parts[0];
- String key = parts[2];
- for (String serverToken : possibleTokens) {
- if (entry.contains(serverToken)) {
- try {
- return loadKey(key);
- } catch (NoSuchAlgorithmException |
InvalidKeySpecException e) {
- log.warn(String.format("Could not load key for server
token %s", entry), e);
- }
- }
- }
+ /*
+ * The name OpenSSH looks the server up by: the host name or address the
client connected to, as configured on the
+ * endpoint and never resolved, written host for port 22 and [host]:port
for another port.
+ */
+ private static String toKnownHostsName(SocketAddress remoteAddress) {
+ if (!(remoteAddress instanceof InetSocketAddress) && !(remoteAddress
instanceof SshdSocketAddress)) {
+ return null;
}
- return null;
+ SshdSocketAddress address =
SshdSocketAddress.toSshdSocketAddress(remoteAddress);
+ String host = address.getHostName().toLowerCase(Locale.ROOT);
+ return address.getPort() == DEFAULT_PORT ? host : "[" + host + "]:" +
address.getPort();
}
- private List<String> getKnownHostsFileTokensForSocketAddress(SocketAddress
remoteAddress) {
- List<String> returnList = new LinkedList<>();
- if (remoteAddress instanceof InetSocketAddress) {
- InetSocketAddress inetSocketAddress = (InetSocketAddress)
remoteAddress;
-
- String hostName = inetSocketAddress.getHostName();
- String ipAddress = inetSocketAddress.getAddress().getHostAddress();
- String remotePort = String.valueOf(inetSocketAddress.getPort());
-
- returnList.add(hostName);
- returnList.add("[" + hostName + "]:" + remotePort);
- returnList.add(ipAddress);
- returnList.add("[" + ipAddress + "]:" + remotePort);
+ /*
+ * Matches a pattern where * stands for any sequence of characters and ?
for any single character.
+ */
+ static boolean matchesPattern(String pattern, String name) {
+ int p = 0;
+ int n = 0;
+ int star = -1;
+ int resume = 0;
+ while (n < name.length()) {
+ if (p < pattern.length() && (pattern.charAt(p) == '?' ||
pattern.charAt(p) == name.charAt(n))) {
+ p++;
+ n++;
+ } else if (p < pattern.length() && pattern.charAt(p) == '*') {
+ star = p++;
+ resume = n;
+ } else if (star >= 0) {
+ p = star + 1;
+ n = ++resume;
+ } else {
+ return false;
+ }
}
-
- return returnList;
+ while (p < pattern.length() && pattern.charAt(p) == '*') {
+ p++;
+ }
+ return p == pattern.length();
}
/*
- * Decode the public key string, which is a base64 encoded string that
consists
- * of multiple parts: 1. public key type (ssh-rsa, ssh-dss, ...) 2. binary
key
- * data (May consists of multiple parts)
- *
- * Each part is composed by two sub-parts 1. Length of the part (4 bytes)
2.
- * Binary part (length as defined by 1.)
- *
- * Uses SSHPublicKeyHolder to construct the actual PublicKey Object
- *
- * Note: Currently only supports RSA and DSA Public keys as required by
- * https://tools.ietf.org/html/rfc4253#section-6.6
- *
+ * A line of the known_hosts file: an optional marker, the host patterns
or a hashed host, and the key. Anything
+ * after the key is a comment.
*/
- PublicKey loadKey(String key) throws NoSuchAlgorithmException,
InvalidKeySpecException {
- SSHPublicKeyHolder sshPublicKeyHolder = new SSHPublicKeyHolder();
-
- byte[] keyByteArray = Base64.getDecoder().decode(key);
- int keyByteArrayCursor = 0;
-
- byte[] tmpData = new byte[4];
- int tmpCursor = 0;
-
- boolean getLengthMode = true;
- while (keyByteArrayCursor < keyByteArray.length) {
- if (getLengthMode) {
- if (tmpCursor < 4) {
- tmpData[tmpCursor] = keyByteArray[keyByteArrayCursor];
- tmpCursor++;
- keyByteArrayCursor++;
- continue;
- } else {
- tmpCursor = 0;
- getLengthMode = false;
- tmpData = new byte[byteArrayToInt(tmpData)];
+ private static final class KnownHostsEntry {
+ private final String marker;
+ private final String hosts;
+ private final byte[] salt;
+ private final byte[] hash;
+ private final String keyType;
+ private final PublicKey key;
+
+ private KnownHostsEntry(String marker, String hosts, byte[] salt,
byte[] hash, String keyType, PublicKey key) {
+ this.marker = marker;
+ this.hosts = hosts;
+ this.salt = salt;
+ this.hash = hash;
+ this.keyType = keyType;
+ this.key = key;
+ }
+
+ /*
+ * Returns null when the line cannot be read. A key of a type that is
not supported is kept as a null key.
+ */
+ static KnownHostsEntry parse(String line, SessionContext session) {
+ String[] fields = line.split("\\s+");
+ int index = 0;
+ String marker = null;
+ if (fields[0].startsWith("@")) {
+ marker = fields[0];
+ if (!MARKER_REVOKED.equals(marker) &&
!MARKER_CERT_AUTHORITY.equals(marker)) {
+ return null;
}
+ index = 1;
}
- tmpData[tmpCursor] = keyByteArray[keyByteArrayCursor];
- tmpCursor++;
- keyByteArrayCursor++;
- if (tmpCursor == tmpData.length) {
- sshPublicKeyHolder.push(tmpData);
- getLengthMode = true;
- tmpData = new byte[4];
- tmpCursor = 0;
+ if (fields.length < index + 3) {
+ return null;
+ }
+ String hosts = fields[index];
+ try {
+ byte[] salt = null;
+ byte[] hash = null;
+ if (hosts.startsWith(HASHED_HOST_PREFIX)) {
+ String[] parts =
hosts.substring(HASHED_HOST_PREFIX.length()).split("\\|", -1);
+ if (parts.length != 2) {
+ return null;
+ }
+ salt = Base64.getDecoder().decode(parts[0]);
+ hash = Base64.getDecoder().decode(parts[1]);
+ if (salt.length != HASH_SALT_LENGTH) {
Review Comment:
**Required / P2: Validate the hashed-host digest as well as the salt.**
Only the salt length is checked. A `|1|` entry with a valid 20-byte salt but
an empty or truncated digest is accepted by the parser. It then never matches a
host, so `failOnUnknownHost=false` permits an arbitrary key instead of
rejecting the malformed resource.
I reproduced this with a real loopback SSH connection: a valid hashed entry
rejected the different server key with no password delivered; truncating its
decoded digest from 20 bytes to 19 made the same different key authenticate and
receive the synthetic password. Verifier probes also reproduce acceptance with
an empty digest. A malformed hashed line after a valid trusted entry is ignored
even in strict mode, contradicting the new promise that an unreadable line
rejects every connection.
Additionally, a host field beginning with an unsupported hash prefix such as
`|2|` currently falls through to ordinary glob matching, with the same
unknown-host fallback.
Please require exactly 20 decoded bytes for the HMAC-SHA1 digest, and treat
a leading `|` that does not form a supported, valid hashed entry as a parse
failure. Extend `rejectsEveryServerWhenALineCannotBeRead` with valid-length
salt plus empty/short/long digest and unsupported-prefix cases, both alone and
after a valid entry, with both values of `failOnUnknownHost`.
This requires malformed local configuration; it is a gap in the fail-closed
guarantee, not a claim that an external peer can modify known_hosts.
_AI-generated review by Codex on behalf of @luigidemasi._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
