You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by "Lyor Goldstein (Jira)" <ji...@apache.org> on 2020/08/04 06:31:00 UTC
[jira] [Comment Edited] (SSHD-1050) Race condition between early
exceptions and AuthFuture
[ https://issues.apache.org/jira/browse/SSHD-1050?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17170588#comment-17170588 ]
Lyor Goldstein edited comment on SSHD-1050 at 8/4/20, 6:30 AM:
---------------------------------------------------------------
{quote}
Perhaps the connect future should be fulfilled only once the initial KEX is done?
{quote}
It does not seem like a good idea - KEX occurs +after+ user has connected and provided the credentials - thus the need for auth future.
{code:java}
session = client.connect(...).verify(timeout);
session.addPassword/PublicKey(...);
session.auth().verify()
{code}
If we wait to "release" the connect future until after KEX then how would the user provide the credentials ? We would need to change the connect API in some way since we need a way for the user to provide the credentials after connect but before KEX (e.g., require some kind of callback for the credentials in the connect method). Such a change while possible would cause a bit of a revolution and require some extensive code re-factoring.
was (Author: lgoldstein):
{quote}
Perhaps the connect future should be fulfilled only once the initial KEX is done?
{quote}
It does not seem like a good idea - KEX occurs +after+ user has connected and provided the credentials - thus the need for auth future.
{code:java}
session = client.connect(...).verify(timeout);
session.addPassword/PublicKey(...);
session.auth().verify()
{code}
If we wait to "release" the connect future until after KEX then how would the user provide the credentials ? We would need to change the connect API in some way since we need way for the user to provide the credentials after connect but before KEX. Such a change while possible would cause a bit of a revolution and require some extensive code re-factoring.
> Race condition between early exceptions and AuthFuture
> ------------------------------------------------------
>
> Key: SSHD-1050
> URL: https://issues.apache.org/jira/browse/SSHD-1050
> Project: MINA SSHD
> Issue Type: Bug
> Affects Versions: 2.4.0, 2.6.0
> Reporter: Thomas Wolf
> Priority: Major
>
> It appears that sometimes exceptions that occur early in connection setup are not reported on the AuthFuture. When that happens, AuthFuture.verify(timeout) will spend the whole timeout waiting and then report a timeout. The earlier exception is lost and nowhere to be seen.
> I stumbled over this when analyzing [Eclipse bug 565394|https://bugs.eclipse.org/bugs/show_bug.cgi?id=565394].
> It's not easy to reproduce, but with the below client test I can manage to make the test fail from time to time if run repeatedly. (That test uses a large preamble before the server identification to provoke an early exception. Using publickey auth instead of password auth increases the chances to get a failure. Running in a debugger increases the chances even more. It's clearly timing-related.)
> {code:java}
> private String longPreamble() {
> StringBuilder b = new StringBuilder();
> for (int i = 0; i < 250; i++) {
> b.append('a');
> }
> String line = b.toString();
> b = new StringBuilder(line);
> int limit = CoreModuleProperties.MAX_IDENTIFICATION_SIZE.get(sshd)
> .orElse(Integer.valueOf(16 * 1024)).intValue();
> limit = limit / 250 + 1;
> for (int i = 0; i < limit; i++) {
> b.append(CoreModuleProperties.SERVER_EXTRA_IDENT_LINES_SEPARATOR)
> .append(line);
> }
> return b.toString();
> }
> @Test
> public void testAuthGetsNotifiedOnLongPreamble() throws Exception {
> CoreModuleProperties.SERVER_EXTRA_IDENTIFICATION_LINES.set(sshd,
> longPreamble());
> sshd.setPasswordAuthenticator(RejectAllPasswordAuthenticator.INSTANCE);
> sshd.setKeyboardInteractiveAuthenticator(KeyboardInteractiveAuthenticator.NONE);
> client.setUserAuthFactories(
> Collections.singletonList(UserAuthPublicKeyFactory.INSTANCE));
> client.start();
> try (ClientSession session =
> client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
> .verify(CONNECT_TIMEOUT).getSession()) {
> KeyPairProvider keys = createTestHostKeyProvider();
> session.addPublicKeyIdentity(
> keys.loadKey(session, CommonTestSupportUtils.DEFAULT_TEST_HOST_KEY_TYPE));
> // This auth should fail because the server sends too many lines before
> // the server identification. However, the auth must not time out! There's
> // an exception raised early on when the identification is read, but
> // frequently this exception gets not reported on the auth future that
> // we are waiting on here, and then we wait for the whole timeout.
> //
> // There's a race condition somewhere. The test succeeds frequently, and is
> // hard to make fail, but if run enough times it will usually fail. Running
> // this in a debugger increases the chances of it failing.
> Throwable e = assertThrows(Throwable.class,
> () -> session.auth().verify(AUTH_TIMEOUT));
> assertFalse(e.getMessage().contains("timeout"));
> assertFalse(session.isAuthenticated());
> } finally {
> client.stop();
> }
> }
> {code}
> Perhaps the connect future should be fulfilled only once the initial KEX is done? OpenSSH's ConnectTimeout does include the time until after the initial KEX.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org