You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tw...@apache.org on 2022/04/15 17:50:24 UTC
[mina-sshd] branch master updated (e74e20417 -> d7b7649f5)
This is an automated email from the ASF dual-hosted git repository.
twolf pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
from e74e20417 Clean up service support
new 7750c537c Simplify uses of the KEX future
new 1e4265c8c Fix ClientTest
new d7b7649f5 KEX: change output encoding atomically
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../sshd/client/session/AbstractClientSession.java | 7 +--
.../common/session/helpers/AbstractSession.java | 50 ++++++++--------------
.../java/org/apache/sshd/client/ClientTest.java | 3 +-
3 files changed, 20 insertions(+), 40 deletions(-)
[mina-sshd] 02/03: Fix ClientTest
Posted by tw...@apache.org.
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
commit 1e4265c8ca6d9d9530a8712a3fd6b0c9e34ae183
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Fri Apr 15 17:51:05 2022 +0200
Fix ClientTest
Don't use `PublicKey.equals()` to check two `PublicKey` instances for
equality. This may fail depending on the implementation class. Use our
own `KeyUtils.compareKeys()` instead.
---
sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
index ad40b463e..2f196eab1 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
@@ -75,6 +75,7 @@ import org.apache.sshd.common.channel.Channel;
import org.apache.sshd.common.channel.ChannelListener;
import org.apache.sshd.common.channel.StreamingChannel;
import org.apache.sshd.common.channel.exception.SshChannelClosedException;
+import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.future.CloseFuture;
import org.apache.sshd.common.future.SshFutureListener;
import org.apache.sshd.common.io.IoInputStream;
@@ -1063,7 +1064,7 @@ public class ClientTest extends BaseTestSupport {
KeyPairProvider keys = createTestHostKeyProvider();
KeyPair pair = keys.loadKey(null, CommonTestSupportUtils.DEFAULT_TEST_HOST_KEY_TYPE);
- sshd.setPublickeyAuthenticator((username, key, session) -> key.equals(pair.getPublic()));
+ sshd.setPublickeyAuthenticator((username, key, session) -> KeyUtils.compareKeys(key, pair.getPublic()));
client.setUserAuthFactories(Collections.singletonList(UserAuthPublicKeyFactory.INSTANCE));
client.start();
[mina-sshd] 03/03: KEX: change output encoding atomically
Posted by tw...@apache.org.
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
commit d7b7649f56983d470a79c0fdb720cfd548227b87
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sun Apr 10 18:53:34 2022 +0200
KEX: change output encoding atomically
Ensure the `encodeLock` is held for sending the `SSH_MSG_NEWKEYS`
message and changing the output encoding.
For the input encoding a similar change is not needed since the
`decodeLock` is held anyway.
Strictly speaking acquiring the `encodeLock` should not be necessary
since any other potentially concurrent writes should be queued because
the KEX state is not yet `DONE`.
---
.../common/session/helpers/AbstractSession.java | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index e4032b9aa..a556a2b4e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -608,11 +608,18 @@ public abstract class AbstractSession extends SessionHelper {
log.debug("sendNewKeys({}) Send SSH_MSG_NEWKEYS", this);
}
- Buffer buffer = createBuffer(SshConstants.SSH_MSG_NEWKEYS, Byte.SIZE);
- IoWriteFuture future = writePacket(buffer);
prepareNewKeys();
- // Use the new settings from now on for any outgoing packet
- setOutputEncoding();
+ Buffer buffer = createBuffer(SshConstants.SSH_MSG_NEWKEYS, Byte.SIZE);
+ IoWriteFuture future;
+ synchronized (encodeLock) {
+ // writePacket() would also work since it would never try to queue the packet, and would never try to
+ // initiate a new KEX, and thus would never try to get the kexLock monitor. If it did, we might get a
+ // deadlock due to lock inversion. It seems safer to push this out directly, though.
+ future = doWritePacket(buffer);
+ // Use the new settings from now on for any outgoing packet
+ setOutputEncoding();
+ }
+ resetIdleTimeout();
/*
* According to https://tools.ietf.org/html/rfc8308#section-2.4:
*
@@ -625,12 +632,9 @@ public abstract class AbstractSession extends SessionHelper {
* + As the next packet following the server's first SSH_MSG_NEWKEYS.
*/
KexExtensionHandler extHandler = getKexExtensionHandler();
- if ((extHandler == null)
- || (!extHandler.isKexExtensionsAvailable(this, AvailabilityPhase.NEWKEYS))) {
- return future;
+ if ((extHandler != null) && extHandler.isKexExtensionsAvailable(this, AvailabilityPhase.NEWKEYS)) {
+ extHandler.sendKexExtensions(this, KexPhase.NEWKEYS);
}
-
- extHandler.sendKexExtensions(this, KexPhase.NEWKEYS);
return future;
}
[mina-sshd] 01/03: Simplify uses of the KEX future
Posted by tw...@apache.org.
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
commit 7750c537cca9d42d4a6d9ee1a047d624f8a87c17
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sat Apr 9 15:25:51 2022 +0200
Simplify uses of the KEX future
`DefaultKeyExchangeFuture.setValue()` allows setting the value only
once. Subsequent invocations are no-ops. It is thus not necessary to
synchronize on that future and set the value only if not set yet.
---
.../sshd/client/session/AbstractClientSession.java | 7 +-----
.../common/session/helpers/AbstractSession.java | 28 ++++------------------
2 files changed, 5 insertions(+), 30 deletions(-)
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
index cf21a9e9c..2858c400a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
@@ -644,12 +644,7 @@ public abstract class AbstractClientSession extends AbstractSession implements C
DefaultKeyExchangeFuture kexFuture = new DefaultKeyExchangeFuture(toString(), null);
DefaultKeyExchangeFuture prev = kexFutureHolder.getAndSet(kexFuture);
if (prev != null) {
- synchronized (prev) {
- Object value = prev.getValue();
- if (value == null) {
- prev.setValue(new SshException("Switch to none cipher while previous KEX is ongoing"));
- }
- }
+ prev.setValue(new SshException("Switch to none cipher while previous KEX is ongoing"));
}
String c2sEncServer;
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index 1632c1ff6..e4032b9aa 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -469,12 +469,7 @@ public abstract class AbstractSession extends SessionHelper {
DefaultKeyExchangeFuture kexFuture = kexFutureHolder.get();
// if have any ongoing KEX notify it about the failure
if (kexFuture != null) {
- synchronized (kexFuture) {
- Object value = kexFuture.getValue();
- if (value == null) {
- kexFuture.setValue(e);
- }
- }
+ kexFuture.setValue(e);
}
if (e instanceof Exception) {
@@ -799,12 +794,7 @@ public abstract class AbstractSession extends SessionHelper {
DefaultKeyExchangeFuture kexFuture = kexFutureHolder.get();
if (kexFuture != null) {
- synchronized (kexFuture) {
- Object value = kexFuture.getValue();
- if (value == null) {
- kexFuture.setValue(Boolean.TRUE);
- }
- }
+ kexFuture.setValue(Boolean.TRUE);
}
signalSessionEvent(SessionListener.Event.KeyEstablished);
@@ -887,12 +877,7 @@ public abstract class AbstractSession extends SessionHelper {
DefaultKeyExchangeFuture kexFuture = kexFutureHolder.get();
if (kexFuture != null) {
// if have any pending KEX then notify it about the closing session
- synchronized (kexFuture) {
- Object value = kexFuture.getValue();
- if (value == null) {
- kexFuture.setValue(new SshException("Session closing while KEX in progress"));
- }
- }
+ kexFuture.setValue(new SshException("Session closing while KEX in progress"));
}
// if anyone waiting for global response notify them about the closing session
@@ -2288,12 +2273,7 @@ public abstract class AbstractSession extends SessionHelper {
DefaultKeyExchangeFuture newFuture = new DefaultKeyExchangeFuture(toString(), null);
DefaultKeyExchangeFuture kexFuture = kexFutureHolder.getAndSet(newFuture);
if (kexFuture != null) {
- synchronized (kexFuture) {
- Object value = kexFuture.getValue();
- if (value == null) {
- kexFuture.setValue(new SshException("New KEX started while previous one still ongoing"));
- }
- }
+ kexFuture.setValue(new SshException("New KEX started while previous one still ongoing"));
}
return newFuture;