[
https://issues.apache.org/jira/browse/SSHD-1197?focusedWorklogId=622075&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-622075
]
ASF GitHub Bot logged work on SSHD-1197:
----------------------------------------
Author: ASF GitHub Bot
Created on: 13/Jul/21 15:52
Start Date: 13/Jul/21 15:52
Worklog Time Spent: 10m
Work Description: lgoldstein commented on a change in pull request #201:
URL: https://github.com/apache/mina-sshd/pull/201#discussion_r668893454
##########
File path:
sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
##########
@@ -696,7 +702,18 @@ protected void handleKexInit(Buffer buffer) throws
Exception {
protected void doKexNegotiation() throws Exception {
if (kexState.compareAndSet(KexState.DONE, KexState.RUN)) {
sendKexInit();
- } else if (!kexState.compareAndSet(KexState.INIT, KexState.RUN)) {
+ } else if (kexState.compareAndSet(KexState.INIT, KexState.RUN)) {
+ DefaultKeyExchangeFuture initFuture;
+ synchronized (kexState) {
+ initFuture = kexInitializedFuture;
+ if (initFuture == null) {
+ initFuture = new DefaultKeyExchangeFuture(toString(),
kexLock);
+ kexInitializedFuture = initFuture;
+ }
+ }
+
+ initFuture.await();
Review comment:
I am not comfortable with an infinite wait - just in case some
deadlock/bug occurs. Let's define a
`CoreModuleProperties#MAX_KEX_INIT_WAIT_TIMEOUT` and use it here (I think ~30
seconds as default value should do, but you are welcome to use some other
value). If it expires then throw an exception (which will close the session).
--
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 622075)
Time Spent: 40m (was: 0.5h)
> Race condition in KEX
> ---------------------
>
> Key: SSHD-1197
> URL: https://issues.apache.org/jira/browse/SSHD-1197
> Project: MINA SSHD
> Issue Type: Bug
> Affects Versions: 2.7.0
> Reporter: Thomas Wolf
> Assignee: Thomas Wolf
> Priority: Critical
> Time Spent: 40m
> Remaining Estimate: 0h
>
> There is a race condition in the KEX implementation. A simple reproducer can
> be obtained by modifying {{SftpTransferTest}} by inserting the following in
> method {{doTestTransferIntegrity()}} just before the main {{try-finally}}:
> {code:java}
> CoreModuleProperties.REKEY_BLOCKS_LIMIT.set(client, Long.valueOf(65536));
> CoreModuleProperties.REKEY_BLOCKS_LIMIT.set(sshd, Long.valueOf(65536));
> try (ClientSession session = createAuthenticatedClientSession();
> ...
> {code}
> This forces rekeying every 512kB; the test roundtrips a 10Mb file twice,
> which gives ample opportunity to run into this race condition. Typically the
> test fails very quickly and hangs.
> The hang is always caused by an async write not being cancelled when the
> session gets disconnected. The session disconnects during KEX because KEX
> state is corrupted because of the race condition.
> Most of the time the race condition causes a signature verification failure
> during KEX, but I also got
> "Disconnecting(ClientSessionImpl[testTransferIntegrity@/127.0.0.1:62186]):
> SSH2_DISCONNECT_KEY_EXCHANGE_FAILED - Unable to negotiate key exchange for
> mac algorithms (server to client) (client: null / server:
> [email protected],[email protected],[email protected],hmac-sha2-256,hmac-sha2-512,hmac-sha1)",
> which is easier to understand than the signature verification failure.
> The precise sequence of events in that case is
> {code:java}
> Server Client
> 1. thread-nio-4 requestNewKeyExchange DONE->INIT
> 2. thread-nio-4 sendKexInit
> 3. main
> requestNewKeyExchange DONE->INIT
> 4. thread-nio-3
> handleKexInit
> 5. thread-nio-3
> doKexNegotiation INIT->RUN
> 6. thread-nio-3
> negotiate -> Exception: client proposal null
> 7. main
> sendKexInit
> 8. thread-nio-2 receive KEX_INIT INIT->RUN
> 9. thread-nio-3
> Exception caught
> 10. thread-nio-3
> Disconnecting
> 11. thread-nio-5 process SSH_MSG_DISCONNECT (KexState RUN)
> {code}
> There is window between steps 3 and 7 in
> {{AbstractSession.requestNewKeyExchange()}} during which the KEX state is
> INIT, but the client proposal isn't initialized yet; it's initialized only
> after {{sendKexInit()}} has been done. However, the client already got the
> server's KEX_INIT message, and in {{doKexNegotiation()}} proceeds as if the
> client proposal was already set up.
> So, theres' two problems here:
> # KEX fails due to a race condition.
> # The client hangs after disconnecting because an async write future is not
> terminated.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
