FliegenKLATSCH commented on code in PR #368:
URL: https://github.com/apache/mina-sshd/pull/368#discussion_r1186893876
##########
sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java:
##########
@@ -267,36 +268,55 @@ protected PublicKeyEntryResolver
getFallbackPublicKeyEntryResolver() {
protected boolean acceptKnownHostEntries(
ClientSession clientSession, SocketAddress remoteAddress,
PublicKey serverKey,
Collection<HostEntryPair> knownHosts) {
- // TODO allow for several candidates and check if ANY of them matches
the key and has 'revoked' marker
- HostEntryPair match = findKnownHostEntry(clientSession, remoteAddress,
knownHosts);
- if (match == null) {
+
+ List<HostEntryPair> hostMatches = findKnownHostEntries(clientSession,
remoteAddress, knownHosts);
+ if (hostMatches.isEmpty()) {
return acceptUnknownHostKey(clientSession, remoteAddress,
serverKey);
}
- KnownHostEntry entry = match.getHostEntry();
- PublicKey expected = match.getServerKey();
- if (KeyUtils.compareKeys(expected, serverKey)) {
- return acceptKnownHostEntry(clientSession, remoteAddress,
serverKey, entry);
+ String serverKeyType = KeyUtils.getKeyType(serverKey);
+
+ List<HostEntryPair> keyMatches = hostMatches.stream()
+ .filter(entry ->
serverKeyType.equals(entry.getHostEntry().getKeyEntry().getKeyType()))
+ .filter(k -> KeyUtils.compareKeys(k.getServerKey(), serverKey))
+ .collect(Collectors.toList());
+
+ if (keyMatches.stream()
+ .anyMatch(k -> !acceptKnownHostEntry(clientSession,
remoteAddress, serverKey, k.getHostEntry()))) {
+ return false;
}
- try {
- if (!acceptModifiedServerKey(clientSession, remoteAddress, entry,
expected, serverKey)) {
- return false;
+ if (!keyMatches.isEmpty()) {
+ return true;
+ }
+
+ for (HostEntryPair match : hostMatches) {
+ KnownHostEntry entry = match.getHostEntry();
+ PublicKey expected = match.getServerKey();
+
+ try {
+ if (acceptModifiedServerKey(clientSession, remoteAddress,
entry, expected, serverKey)) {
+ updateModifiedServerKey(clientSession, remoteAddress,
serverKey, knownHosts, match);
+ return true;
+ }
Review Comment:
My first thought was to change the signature and pass a list (and adding
another list of all names under which the real key is known, similar to
openssh, which shows that with the warning), but I assume that "big" change is
not ideal - assuming a lot of implementations overwrite this function.
In the end, it may be (too) special cases where someone relies on an
specific existing entry, so you are probably right - but on the other end, if
someone accepts it right away, it will also not ask again, so it doesn't really
hurt?
I am wondering if some implementations would do an external lookup with the
old key..
--
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: dev-unsubscr...@mina.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org
