Skip to content

Commit f1994bd

Browse files
FliegenKLATSCHFliegenKLATSCH
authored
[SSHD-1259] Match key type in known_hosts lookup (apache#368)
Consider key types, and consider all keys for a host. Also handle the revoked flag: do not accept server keys that are marked as revoked in the known_hosts files.
1 parent 18f4346 commit f1994bd

File tree

4 files changed

+186
-113
lines changed

4 files changed

+186
-113
lines changed

‎sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java

+53-53
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@
3636
import java.util.Collections;
3737
import java.util.List;
3838
import java.util.Objects;
39+
import java.util.Optional;
3940
import java.util.TreeSet;
4041
import java.util.concurrent.atomic.AtomicReference;
4142
import java.util.function.Supplier;
43+
import java.util.stream.Collectors;
4244

4345
import org.apache.sshd.client.config.hosts.KnownHostEntry;
4446
import org.apache.sshd.client.config.hosts.KnownHostHashValue;
@@ -267,36 +269,63 @@ protected PublicKeyEntryResolver getFallbackPublicKeyEntryResolver() {
267269
protected boolean acceptKnownHostEntries(
268270
ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey,
269271
Collection<HostEntryPair> knownHosts) {
270-
// TODO allow for several candidates and check if ANY of them matches the key and has 'revoked' marker
271-
HostEntryPair match = findKnownHostEntry(clientSession, remoteAddress, knownHosts);
272-
if (match == null) {
272+
273+
List<HostEntryPair> hostMatches = findKnownHostEntries(clientSession, remoteAddress, knownHosts);
274+
if (hostMatches.isEmpty()) {
273275
return acceptUnknownHostKey(clientSession, remoteAddress, serverKey);
274276
}
275277

276-
KnownHostEntry entry = match.getHostEntry();
277-
PublicKey expected = match.getServerKey();
278-
if (KeyUtils.compareKeys(expected, serverKey)) {
279-
return acceptKnownHostEntry(clientSession, remoteAddress, serverKey, entry);
278+
String serverKeyType = KeyUtils.getKeyType(serverKey);
279+
280+
List<HostEntryPair> keyMatches = hostMatches.stream()
281+
.filter(entry -> serverKeyType.equals(entry.getHostEntry().getKeyEntry().getKeyType()))
282+
.filter(k -> KeyUtils.compareKeys(k.getServerKey(), serverKey))
283+
.collect(Collectors.toList());
284+
285+
if (keyMatches.stream()
286+
.anyMatch(k -> "revoked".equals(k.getHostEntry().getMarker()))) {
287+
log.debug("acceptKnownHostEntry({})[{}] key={}-{} marked as revoked",
288+
clientSession, remoteAddress, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey));
289+
return false;
280290
}
281291

292+
if (!keyMatches.isEmpty()) {
293+
return true;
294+
}
295+
296+
Optional<HostEntryPair> anyNonRevokedMatch = hostMatches.stream()
297+
.filter(k -> !"revoked".equals(k.getHostEntry().getMarker()))
298+
.findAny();
299+
300+
if (!anyNonRevokedMatch.isPresent()) {
301+
return acceptUnknownHostKey(clientSession, remoteAddress, serverKey);
302+
}
303+
304+
KnownHostEntry entry = anyNonRevokedMatch.get().getHostEntry();
305+
PublicKey expected = anyNonRevokedMatch.get().getServerKey();
306+
282307
try {
283-
if (!acceptModifiedServerKey(clientSession, remoteAddress, entry, expected, serverKey)) {
284-
return false;
308+
if (acceptModifiedServerKey(clientSession, remoteAddress, entry, expected, serverKey)) {
309+
updateModifiedServerKey(clientSession, remoteAddress, serverKey, knownHosts, anyNonRevokedMatch.get());
310+
return true;
285311
}
286312
} catch (Throwable t) {
287313
warn("acceptKnownHostEntries({})[{}] failed ({}) to accept modified server key: {}",
288314
clientSession, remoteAddress, t.getClass().getSimpleName(), t.getMessage(), t);
289-
return false;
290315
}
291316

317+
return false;
318+
}
319+
320+
protected void updateModifiedServerKey(
321+
ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey, Collection<HostEntryPair> knownHosts,
322+
HostEntryPair match) {
292323
Path file = getPath();
293324
try {
294325
updateModifiedServerKey(clientSession, remoteAddress, match, serverKey, file, knownHosts);
295326
} catch (Throwable t) {
296327
handleModifiedServerKeyUpdateFailure(clientSession, remoteAddress, match, serverKey, file, knownHosts, t);
297328
}
298-
299-
return true;
300329
}
301330

302331
/**
@@ -460,74 +489,45 @@ protected void handleModifiedServerKeyUpdateFailure(
460489
clientSession, remoteAddress, reason.getClass().getSimpleName(), match, reason.getMessage(), reason);
461490
}
462491

463-
/**
464-
* Invoked <U>after</U> known host entry located and keys match - by default checks that entry has not been revoked
465-
*
466-
* @param clientSession The {@link ClientSession}
467-
* @param remoteAddress The remote host address
468-
* @param serverKey The presented server {@link PublicKey}
469-
* @param entry The {@link KnownHostEntry} value - if {@code null} then no known matching host entry was
470-
* found - default will call
471-
* {@link #acceptUnknownHostKey(ClientSession, SocketAddress, PublicKey)}
472-
* @return {@code true} if OK to accept the server
473-
*/
474-
protected boolean acceptKnownHostEntry(
475-
ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey, KnownHostEntry entry) {
476-
if (entry == null) { // not really expected, but manage it
477-
return acceptUnknownHostKey(clientSession, remoteAddress, serverKey);
478-
}
479-
480-
if ("revoked".equals(entry.getMarker())) {
481-
log.debug("acceptKnownHostEntry({})[{}] key={}-{} marked as {}",
482-
clientSession, remoteAddress, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey),
483-
entry.getMarker());
484-
return false;
485-
}
486-
487-
if (log.isDebugEnabled()) {
488-
log.debug("acceptKnownHostEntry({})[{}] matched key={}-{}",
489-
clientSession, remoteAddress, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey));
490-
}
491-
return true;
492-
}
493-
494-
protected HostEntryPair findKnownHostEntry(
492+
protected List<HostEntryPair> findKnownHostEntries(
495493
ClientSession clientSession, SocketAddress remoteAddress, Collection<HostEntryPair> knownHosts) {
496494
if (GenericUtils.isEmpty(knownHosts)) {
497-
return null;
495+
return Collections.emptyList();
498496
}
499497

500498
Collection<SshdSocketAddress> candidates = resolveHostNetworkIdentities(clientSession, remoteAddress);
501499
boolean debugEnabled = log.isDebugEnabled();
502500
if (debugEnabled) {
503-
log.debug("findKnownHostEntry({})[{}] host network identities: {}",
501+
log.debug("findKnownHostEntries({})[{}] host network identities: {}",
504502
clientSession, remoteAddress, candidates);
505503
}
506504

507505
if (GenericUtils.isEmpty(candidates)) {
508-
return null;
506+
return Collections.emptyList();
509507
}
510508

511-
for (HostEntryPair match : knownHosts) {
512-
KnownHostEntry entry = match.getHostEntry();
509+
List<HostEntryPair> matches = new ArrayList<>();
510+
for (HostEntryPair line : knownHosts) {
511+
KnownHostEntry entry = line.getHostEntry();
513512
for (SshdSocketAddress host : candidates) {
514513
try {
515514
if (entry.isHostMatch(host.getHostName(), host.getPort())) {
516515
if (debugEnabled) {
517-
log.debug("findKnownHostEntry({})[{}] matched host={} for entry={}",
516+
log.debug("findKnownHostEntries({})[{}] matched host={} for entry={}",
518517
clientSession, remoteAddress, host, entry);
519518
}
520-
return match;
519+
matches.add(line);
520+
break;
521521
}
522522
} catch (RuntimeException | Error e) {
523-
warn("findKnownHostEntry({})[{}] failed ({}) to check host={} for entry={}: {}",
523+
warn("findKnownHostEntries({})[{}] failed ({}) to check host={} for entry={}: {}",
524524
clientSession, remoteAddress, e.getClass().getSimpleName(),
525525
host, entry.getConfigLine(), e.getMessage(), e);
526526
}
527527
}
528528
}
529529

530-
return null; // no match found
530+
return matches;
531531
}
532532

533533
/**

‎sshd-core/src/main/java/org/apache/sshd/client/keyverifier/ModifiedServerKeyAcceptor.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ public interface ModifiedServerKeyAcceptor {
3535
*
3636
* @param clientSession The {@link ClientSession}
3737
* @param remoteAddress The remote host address
38-
* @param entry The original {@link KnownHostEntry} whose key did not match
39-
* @param expected The expected server {@link PublicKey}
38+
* @param entry Any original {@link KnownHostEntry} whose key did not match
39+
* @param expected Any expected server {@link PublicKey}
4040
* @param actual The presented server {@link PublicKey}
4141
* @return {@code true} if accept the server key anyway
4242
* @throws Exception if cannot process the request - equivalent to {@code false} return value

0 commit comments

Comments
 (0)