oscerd commented on code in PR #27821:
URL: https://github.com/apache/camel/pull/27821#discussion_r4269939873
##########
components/camel-ssh/src/main/java/org/apache/camel/component/ssh/ResourceBasedSSHKeyVerifier.java:
##########
@@ -62,22 +71,53 @@ public ResourceBasedSSHKeyVerifier(CamelContext
camelContext, String knownHostsR
@Override
public boolean verifyServerKey(ClientSession sshClientSession,
SocketAddress remoteAddress, PublicKey serverKey) {
log.debug("Trying to find known_hosts file {}", knownHostsResource);
+ List<KnownHostEntry> entries = Collections.emptyList();
InputStream knownHostsInputStream = null;
try {
knownHostsInputStream =
ResourceHelper.resolveMandatoryResourceAsInputStream(camelContext,
knownHostsResource);
- List<String> possibleTokens =
getKnownHostsFileTokensForSocketAddress(remoteAddress);
- log.debug("Trying to match PublicKey against provided known_hosts
file");
- PublicKey matchingKey =
findKeyForServerToken(knownHostsInputStream, possibleTokens);
- if (matchingKey != null) {
- log.debug("Found PublicKey match for server");
- return Arrays.areEqual(matchingKey.getEncoded(),
serverKey.getEncoded());
- }
+ entries =
KnownHostEntry.readKnownHostEntries(knownHostsInputStream, false);
Review Comment:
Fixed in 370054641e57. The verifier no longer parses with MINA's
`KnownHostEntry`/`HostPatternsHolder`. It reads each line itself and matches
every host pattern as an OpenSSH glob against the name OpenSSH looks the server
up by: `host` for port 22, `[host]:port` otherwise. `*` and `?` are wildcards,
`!` applies per pattern, and case is ignored. So
`![excluded.example.com]:2222,[db.example.com]:2222` is read as OpenSSH reads
it.
A line that cannot be read now makes every connection fail, whatever
`failOnUnknownHost` is set to, and so does a resource that cannot be read. A
parse failure can no longer turn a listed host into an unknown one.
Tests, all three of which fail on aa8050b:
- `matchesNegatedPortQualifiedPatterns`: the listed key and a different key
with both values of `failOnUnknownHost`, and the excluded host;
- `rejectsEveryServerWhenALineCannotBeRead`;
- `rejectsEveryServerWhenTheResourceCannotBeRead`.
_Claude Code on behalf of Andrea Cosentino (@oscerd)_
##########
components/camel-ssh/src/main/java/org/apache/camel/component/ssh/ResourceBasedSSHKeyVerifier.java:
##########
@@ -62,22 +71,53 @@ public ResourceBasedSSHKeyVerifier(CamelContext
camelContext, String knownHostsR
@Override
public boolean verifyServerKey(ClientSession sshClientSession,
SocketAddress remoteAddress, PublicKey serverKey) {
log.debug("Trying to find known_hosts file {}", knownHostsResource);
+ List<KnownHostEntry> entries = Collections.emptyList();
InputStream knownHostsInputStream = null;
try {
knownHostsInputStream =
ResourceHelper.resolveMandatoryResourceAsInputStream(camelContext,
knownHostsResource);
- List<String> possibleTokens =
getKnownHostsFileTokensForSocketAddress(remoteAddress);
- log.debug("Trying to match PublicKey against provided known_hosts
file");
- PublicKey matchingKey =
findKeyForServerToken(knownHostsInputStream, possibleTokens);
- if (matchingKey != null) {
- log.debug("Found PublicKey match for server");
- return Arrays.areEqual(matchingKey.getEncoded(),
serverKey.getEncoded());
- }
+ entries =
KnownHostEntry.readKnownHostEntries(knownHostsInputStream, false);
} catch (IOException ioException) {
log.debug(String.format("Could not find known_hosts file %s",
knownHostsResource), ioException);
} finally {
IOHelper.close(knownHostsInputStream);
}
+
+ SshdSocketAddress server = toServerAddress(remoteAddress);
+ boolean hostListed = false;
+ boolean keyListed = false;
+ if (server != null) {
+ log.debug("Trying to match PublicKey of {} against provided
known_hosts file", server);
+ for (KnownHostEntry entry : entries) {
+ if (!entry.isHostMatch(server.getHostName(), server.getPort())
Review Comment:
Fixed in 370054641e57. Each pattern is now matched against `host` /
`[host]:port` as in OpenSSH, so `*` also matches `[db.example.com]:2222` and
`@revoked *` applies there.
`rejectsARevokedKeyOnEveryPortAndInEveryOrder` covers ports 22 and 2222,
both values of `failOnUnknownHost`, and the revocation before and after the
accepted entry.
The verifier only accepts after it has read every line. The early-return
mutation you describe now fails two tests:
- that one ("revocation after the entry, port 22");
- `rejectsEveryServerWhenALineCannotBeRead`.
_Claude Code on behalf of Andrea Cosentino (@oscerd)_
##########
components/camel-ssh/src/main/java/org/apache/camel/component/ssh/ResourceBasedSSHKeyVerifier.java:
##########
@@ -62,22 +71,53 @@ public ResourceBasedSSHKeyVerifier(CamelContext
camelContext, String knownHostsR
@Override
public boolean verifyServerKey(ClientSession sshClientSession,
SocketAddress remoteAddress, PublicKey serverKey) {
log.debug("Trying to find known_hosts file {}", knownHostsResource);
+ List<KnownHostEntry> entries = Collections.emptyList();
InputStream knownHostsInputStream = null;
try {
knownHostsInputStream =
ResourceHelper.resolveMandatoryResourceAsInputStream(camelContext,
knownHostsResource);
- List<String> possibleTokens =
getKnownHostsFileTokensForSocketAddress(remoteAddress);
- log.debug("Trying to match PublicKey against provided known_hosts
file");
- PublicKey matchingKey =
findKeyForServerToken(knownHostsInputStream, possibleTokens);
- if (matchingKey != null) {
- log.debug("Found PublicKey match for server");
- return Arrays.areEqual(matchingKey.getEncoded(),
serverKey.getEncoded());
- }
+ entries =
KnownHostEntry.readKnownHostEntries(knownHostsInputStream, false);
} catch (IOException ioException) {
log.debug(String.format("Could not find known_hosts file %s",
knownHostsResource), ioException);
} finally {
IOHelper.close(knownHostsInputStream);
}
+
+ SshdSocketAddress server = toServerAddress(remoteAddress);
+ boolean hostListed = false;
+ boolean keyListed = false;
+ if (server != null) {
+ log.debug("Trying to match PublicKey of {} against provided
known_hosts file", server);
+ for (KnownHostEntry entry : entries) {
+ if (!entry.isHostMatch(server.getHostName(), server.getPort())
+ || MARKER_CERT_AUTHORITY.equals(entry.getMarker())) {
+ continue;
+ }
+ boolean sameKey = isSameKey(sshClientSession, entry,
serverKey);
+ if (MARKER_REVOKED.equals(entry.getMarker())) {
+ if (sameKey) {
+ log.warn("The key presented by {} is marked as revoked
in the known_hosts file {}", server,
+ knownHostsResource);
+ return false;
+ }
+ } else {
Review Comment:
Fixed in 370054641e57. Only `@revoked` and `@cert-authority` are recognised.
Any other marker makes the line unreadable, and an unreadable line makes every
connection fail whatever `failOnUnknownHost` is set to. So such a line can
neither grant trust nor fall through to unknown-host acceptance.
`rejectsEveryServerWhenALineCannotBeRead` covers `@unknown db.example.com
<key>`:
- alone and after a valid entry;
- for the listed host and an unlisted one;
- with both values of `failOnUnknownHost`.
_Claude Code on behalf of Andrea Cosentino (@oscerd)_
##########
components/camel-ssh/src/main/java/org/apache/camel/component/ssh/ResourceBasedSSHKeyVerifier.java:
##########
@@ -62,22 +71,53 @@ public ResourceBasedSSHKeyVerifier(CamelContext
camelContext, String knownHostsR
@Override
public boolean verifyServerKey(ClientSession sshClientSession,
SocketAddress remoteAddress, PublicKey serverKey) {
log.debug("Trying to find known_hosts file {}", knownHostsResource);
+ List<KnownHostEntry> entries = Collections.emptyList();
InputStream knownHostsInputStream = null;
try {
knownHostsInputStream =
ResourceHelper.resolveMandatoryResourceAsInputStream(camelContext,
knownHostsResource);
- List<String> possibleTokens =
getKnownHostsFileTokensForSocketAddress(remoteAddress);
- log.debug("Trying to match PublicKey against provided known_hosts
file");
- PublicKey matchingKey =
findKeyForServerToken(knownHostsInputStream, possibleTokens);
- if (matchingKey != null) {
- log.debug("Found PublicKey match for server");
- return Arrays.areEqual(matchingKey.getEncoded(),
serverKey.getEncoded());
- }
+ entries =
KnownHostEntry.readKnownHostEntries(knownHostsInputStream, false);
} catch (IOException ioException) {
log.debug(String.format("Could not find known_hosts file %s",
knownHostsResource), ioException);
} finally {
IOHelper.close(knownHostsInputStream);
}
+
+ SshdSocketAddress server = toServerAddress(remoteAddress);
+ boolean hostListed = false;
+ boolean keyListed = false;
+ if (server != null) {
+ log.debug("Trying to match PublicKey of {} against provided
known_hosts file", server);
+ for (KnownHostEntry entry : entries) {
+ if (!entry.isHostMatch(server.getHostName(), server.getPort())
+ || MARKER_CERT_AUTHORITY.equals(entry.getMarker())) {
+ continue;
+ }
+ boolean sameKey = isSameKey(sshClientSession, entry,
serverKey);
+ if (MARKER_REVOKED.equals(entry.getMarker())) {
+ if (sameKey) {
+ log.warn("The key presented by {} is marked as revoked
in the known_hosts file {}", server,
+ knownHostsResource);
+ return false;
+ }
+ } else {
+ hostListed = true;
+ if (sameKey) {
+ keyListed = true;
+ }
+ }
+ }
Review Comment:
Thanks. The PR description over-claimed here. I corrected the wording rather
than the behaviour, in 370054641e57: the PR description, the
`failOnUnknownHost` description and javadoc, the docs and the upgrade notes now
say "entries without a marker".
The behaviour follows OpenSSH:
- In `hostfile.c`, `check_hostkeys_by_key_or_type()` skips every entry whose
marker is not the wanted one (`MRK_NONE` for a plain key) when it decides
between `HOST_NEW` and `HOST_CHANGED`.
- `check_key_not_revoked()` checks the `@revoked` entries separately.
- A host listed only in `@revoked` lines is therefore an unknown host, as
davsclaus also notes on the docs thread.
Counting `@revoked` entries as listing the host would make the usual
`@revoked * <key>` line list every host. With `failOnUnknownHost=false`, every
host without its own entry would then be rejected.
The revoked key itself is always rejected, whatever the flag
(`rejectsARevokedKeyOnEveryPortAndInEveryOrder`).
`treatsAHostWithOnlyRevokedEntriesAsUnknown` pins the behaviour for a different
key.
_Claude Code on behalf of Andrea Cosentino (@oscerd)_
##########
components/camel-ssh/src/main/java/org/apache/camel/component/ssh/ResourceBasedSSHKeyVerifier.java:
##########
@@ -18,30 +18,39 @@
import java.io.IOException;
import java.io.InputStream;
-import java.math.BigInteger;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
-import java.security.NoSuchAlgorithmException;
+import java.security.GeneralSecurityException;
import java.security.PublicKey;
-import java.security.spec.InvalidKeySpecException;
-import java.util.Base64;
-import java.util.LinkedList;
+import java.util.Collections;
import java.util.List;
import org.apache.camel.CamelContext;
import org.apache.camel.support.ResourceHelper;
import org.apache.camel.util.IOHelper;
+import org.apache.sshd.client.config.hosts.KnownHostEntry;
import org.apache.sshd.client.keyverifier.ServerKeyVerifier;
import org.apache.sshd.client.session.ClientSession;
-import org.bouncycastle.util.Arrays;
+import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.PublicKeyEntryResolver;
+import org.apache.sshd.common.util.net.SshdSocketAddress;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* ServerKeyVerifier that takes a camel resource as input file to validate the
server key against.
- *
+ * <p/>
Review Comment:
Changed to `<p>` in 370054641e57.
_Claude Code on behalf of Andrea Cosentino (@oscerd)_
--
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]
