This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push:
new cb38574 [SSHD-1141] Tolerate SSH_MSG_USERAUTH_PK_OK with wrong
algorithm
new 87d5fc4 Merge pull request #185 from tomaswolf/SSHD-1141
cb38574 is described below
commit cb385742bf0962cbfcda011d7c4a89aa6ab7716a
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Tue Mar 23 19:51:41 2021 +0100
[SSHD-1141] Tolerate SSH_MSG_USERAUTH_PK_OK with wrong algorithm
SSH_MSG_USERAUTH_PK_OK is supposed to say "the server will accept the
key and signature algorithm the client sent in the previous
SSH_MSG_USERAUTH_REQUEST message". It is _not_ intended as a mechanism
for the server to override the client's choice of algorithm.
See RFC 4252.[1]
Just log a warning if a server sends back an unexpected "algorithm".
(Github's SSH-2.0-babeld-383743ad was found to reply "ssh-rsa" even
if the client requested "rsa-sha2-512" and the server had previously
announced via server-sig-algs that it supports rsa-sha2-512. Subsequent
authentication with rsa-sha2-512 then still works.)
Of course the returned public key must still match exactly.
The client always uses the signature algorithm that it had announced
it would use.
[1] https://tools.ietf.org/html/rfc4252#page-9
---
.../sshd/client/auth/pubkey/UserAuthPublicKey.java | 53 ++++++-------
.../common/auth/PublicKeyAuthenticationTest.java | 90 +++++++++++++---------
2 files changed, 76 insertions(+), 67 deletions(-)
diff --git
a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java
b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java
index d34a3df..eec73ec 100644
---
a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java
+++
b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java
@@ -23,7 +23,6 @@ import java.io.IOException;
import java.security.KeyPair;
import java.security.PublicKey;
import java.security.spec.InvalidKeySpecException;
-import java.util.Collection;
import java.util.Deque;
import java.util.Iterator;
import java.util.LinkedList;
@@ -61,6 +60,7 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
protected Iterator<PublicKeyIdentity> keys;
protected PublicKeyIdentity current;
protected List<NamedFactory<Signature>> factories;
+ protected String chosenAlgorithm;
public UserAuthPublicKey() {
this(null);
@@ -102,6 +102,7 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
if (current == null) {
// Just to be safe. (Paranoia)
currentAlgorithms.clear();
+ chosenAlgorithm = null;
} else if (!currentAlgorithms.isEmpty()) {
currentAlgorithm = currentAlgorithms.poll();
}
@@ -114,7 +115,7 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
session, service, e.getClass().getSimpleName(),
e.getMessage(), e);
throw new RuntimeSshException(e);
}
-
+ chosenAlgorithm = null;
if (current == null) {
if (debugEnabled) {
log.debug("resolveAttemptedPublicKeyIdentity({})[{}] no
more keys to send", session, service);
@@ -175,6 +176,7 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
reporter.signalAuthenticationAttempt(session, service, keyPair,
currentAlgorithm);
}
+ chosenAlgorithm = currentAlgorithm;
Buffer buffer =
session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST);
buffer.putString(session.getUsername());
buffer.putString(service);
@@ -228,46 +230,31 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
}
PublicKey pubKey = keyPair.getPublic();
- String curKeyType = KeyUtils.getKeyType(pubKey);
- String rspKeyType = buffer.getString();
- Collection<String> aliases =
KeyUtils.getAllEquivalentKeyTypes(curKeyType);
- String algo;
- // SSHD-1104 see if key aliases used
- if (GenericUtils.isEmpty(aliases)) {
- algo = curKeyType;
- if (!rspKeyType.equals(algo)) {
- throw new InvalidKeySpecException(
- "processAuthDataRequest(" + session + ")[" + service +
"][" + name + "]"
- + " mismatched key types:
expected=" + algo + ", actual=" + rspKeyType);
- }
- } else {
- if (GenericUtils.findFirstMatchingMember(n ->
n.equalsIgnoreCase(rspKeyType), aliases) == null) {
- throw new InvalidKeySpecException(
- "processAuthDataRequest(" + session + ")[" + service +
"][" + name + "]"
- + " unsupported key type:
expected=" + aliases + ", actual=" + rspKeyType);
- }
- algo = rspKeyType;
- }
+ String rspKeyType = buffer.getString();
PublicKey rspKey = buffer.getPublicKey();
+
+ if (debugEnabled) {
+ log.debug("processAuthDataRequest({})[{}][{}]
SSH_MSG_USERAUTH_PK_OK type={}, fingerprint={}",
+ session, service, name, rspKeyType,
KeyUtils.getFingerPrint(rspKey));
+ }
if (!KeyUtils.compareKeys(rspKey, pubKey)) {
throw new InvalidKeySpecException(
"processAuthDataRequest(" + session + ")[" + service +
"][" + name + "]"
- + " mismatched " + algo + "
keys: expected=" + KeyUtils.getFingerPrint(pubKey)
+ + " mismatched " + rspKeyType +
" keys: expected=" + KeyUtils.getFingerPrint(pubKey)
+ ", actual=" +
KeyUtils.getFingerPrint(rspKey));
}
- if (debugEnabled) {
- log.debug("processAuthDataRequest({})[{}][{}]
SSH_MSG_USERAUTH_PK_OK type={}, fingerprint={}",
- session, service, name, rspKeyType,
KeyUtils.getFingerPrint(rspKey));
+ if (!chosenAlgorithm.equalsIgnoreCase(rspKeyType)) {
+ log.warn("processAuthDataRequest({})[{}][{}] sent algorithm {} but
got back {} from {}",
+ session, service, name, chosenAlgorithm, rspKeyType,
session.getServerVersion());
}
String username = session.getUsername();
+ String algo = chosenAlgorithm;
buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST,
- GenericUtils.length(username) + GenericUtils.length(service)
- +
GenericUtils.length(name)
- +
GenericUtils.length(algo)
- +
ByteArrayBuffer.DEFAULT_SIZE + Long.SIZE);
+ GenericUtils.length(username) + GenericUtils.length(service) +
GenericUtils.length(name)
+ + GenericUtils.length(algo) + ByteArrayBuffer.DEFAULT_SIZE +
Long.SIZE);
buffer.putString(username);
buffer.putString(service);
buffer.putString(name);
@@ -275,6 +262,11 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
buffer.putString(algo);
buffer.putPublicKey(pubKey);
+ if (debugEnabled) {
+ log.debug(
+ "processAuthDataRequest({})[{}][{}]: signing with
algorithm {}", //$NON-NLS-1$
+ session, service, name, algo);
+ }
byte[] sig = appendSignature(session, service, name, username, algo,
pubKey, buffer);
PublicKeyAuthenticationReporter reporter =
session.getPublicKeyAuthenticationReporter();
if (reporter != null) {
@@ -363,6 +355,7 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
protected void releaseKeys() throws IOException {
currentAlgorithms.clear();
current = null;
+ chosenAlgorithm = null;
try {
if (keys instanceof Closeable) {
if (log.isTraceEnabled()) {
diff --git
a/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
b/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
index 2f23152..aa0c2cf 100644
---
a/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
+++
b/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
@@ -138,8 +138,54 @@ public class PublicKeyAuthenticationTest extends
AuthenticationTestSupport {
}
@Test // see SSHD-624
- public void testMismatchedUserAuthPkOkData() throws Exception {
- AtomicInteger challengeCounter = new AtomicInteger(0);
+ public void testUserAuthPkOkWrongKey() throws Exception {
+ sshd.setUserAuthFactories(Collections.singletonList(
+ new
org.apache.sshd.server.auth.pubkey.UserAuthPublicKeyFactory() {
+ @Override
+ public
org.apache.sshd.server.auth.pubkey.UserAuthPublicKey
createUserAuth(ServerSession session)
+ throws IOException {
+ return new
org.apache.sshd.server.auth.pubkey.UserAuthPublicKey() {
+ @Override
+ protected void sendPublicKeyResponse(
+ ServerSession session, String username, String
alg, PublicKey key,
+ byte[] keyBlob, int offset, int blobLen,
Buffer buffer)
+ throws Exception {
+ // send another key
+ KeyPair otherPair =
org.apache.sshd.util.test.CommonTestSupportUtils
+
.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024);
+ PublicKey otherKey = otherPair.getPublic();
+ Buffer buf =
session.createBuffer(SshConstants.SSH_MSG_USERAUTH_PK_OK,
+ blobLen + alg.length() + Long.SIZE);
+ buf.putString(alg);
+ buf.putPublicKey(otherKey);
+ session.writePacket(buf);
+ }
+ };
+ }
+
+ }));
+
+ try (SshClient client = setupTestClient()) {
+ KeyPair clientIdentity = CommonTestSupportUtils.generateKeyPair(
+
CommonTestSupportUtils.DEFAULT_TEST_HOST_KEY_PROVIDER_ALGORITHM,
+ CommonTestSupportUtils.DEFAULT_TEST_HOST_KEY_SIZE);
+ client.start();
+
+ try (ClientSession s = client.connect(getCurrentTestName(),
TEST_LOCALHOST, port)
+ .verify(CONNECT_TIMEOUT)
+ .getSession()) {
+ s.addPublicKeyIdentity(clientIdentity);
+ SshException e = assertThrows(SshException.class, () ->
s.auth().verify(AUTH_TIMEOUT));
+ Throwable t = e.getCause();
+ assertObjectInstanceOf("Unexpected failure cause",
InvalidKeySpecException.class, t);
+ } finally {
+ client.stop();
+ }
+ }
+ }
+
+ @Test // see SSHD-1141
+ public void testUserAuthPkOkWrongAlgorithm() throws Exception {
sshd.setUserAuthFactories(Collections.singletonList(
new
org.apache.sshd.server.auth.pubkey.UserAuthPublicKeyFactory() {
@Override
@@ -151,25 +197,7 @@ public class PublicKeyAuthenticationTest extends
AuthenticationTestSupport {
ServerSession session, String username,
String alg, PublicKey key,
byte[] keyBlob, int offset, int blobLen,
Buffer buffer)
throws Exception {
- int count = challengeCounter.incrementAndGet();
-
outputDebugMessage("sendPublicKeyChallenge(%s)[%s]: count=%d", session, alg,
count);
- if (count == 1) {
- // send wrong key type
- super.sendPublicKeyResponse(session,
username,
- KeyPairProvider.SSH_DSS, key,
keyBlob, offset, blobLen, buffer);
- } else if (count == 2) {
- // send another key
- KeyPair otherPair =
org.apache.sshd.util.test.CommonTestSupportUtils
-
.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024);
- PublicKey otherKey = otherPair.getPublic();
- Buffer buf =
session.createBuffer(SshConstants.SSH_MSG_USERAUTH_PK_OK,
- blobLen + alg.length() +
Long.SIZE);
- buf.putString(alg);
- buf.putPublicKey(otherKey);
- session.writePacket(buf);
- } else {
- super.sendPublicKeyResponse(session,
username, alg, key, keyBlob, offset, blobLen, buffer);
- }
+ super.sendPublicKeyResponse(session, username,
KeyPairProvider.SSH_DSS, key, keyBlob, offset, blobLen, buffer);
}
};
}
@@ -182,22 +210,10 @@ public class PublicKeyAuthenticationTest extends
AuthenticationTestSupport {
CommonTestSupportUtils.DEFAULT_TEST_HOST_KEY_SIZE);
client.start();
- try {
- for (int index = 1; index <= 4; index++) {
- try (ClientSession s =
client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
- .verify(CONNECT_TIMEOUT)
- .getSession()) {
- s.addPublicKeyIdentity(clientIdentity);
- s.auth().verify(AUTH_TIMEOUT);
- assertEquals("Mismatched number of challenges", 3,
challengeCounter.get());
- break;
- } catch (SshException e) { // expected
- outputDebugMessage("%s on retry #%d: %s",
e.getClass().getSimpleName(), index, e.getMessage());
-
- Throwable t = e.getCause();
- assertObjectInstanceOf("Unexpected failure cause at
retry #" + index, InvalidKeySpecException.class, t);
- }
- }
+ try (ClientSession s = client.connect(getCurrentTestName(),
TEST_LOCALHOST, port)
+ .verify(CONNECT_TIMEOUT).getSession()) {
+ s.addPublicKeyIdentity(clientIdentity);
+ assertTrue("Successful authentication expected",
s.auth().verify(AUTH_TIMEOUT).isSuccess());
} finally {
client.stop();
}
