[bugfix]: resolve "Too many authentication failures" concurrent problem #3926

LL-LIN · 2025-12-23T07:41:10Z

The global singleton sshClient was shared across all connections, causing identity (passwords/keys) to accumulate globally. When with ProxyJump - authentication attempts would mix identities from different sessions, exceeding the server's MaxAuthTries and triggering "Too many authentication failures".
#3925

What's changed?

For ProxyJump connections, create a dedicated, short-lived SshClient instance per connection to avoid global identity contamination.
Ensure proper lifecycle management: scoped clients are stopped when the session closes or on failure.

Checklist

I have read the Contributing Guide
I have written the necessary doc or comment.
I have added the necessary unit tests and all cases have passed.

Add or update API

I have added the necessary e2e tests and all cases have passed.

The global singleton sshClient was shared across all connections, causing identity (passwords/keys) to accumulate globally. When with ProxyJump - authentication attempts would mix identities from different sessions, exceeding the server's MaxAuthTries and triggering "Too many authentication failures".

LL-LIN · 2025-12-23T07:42:50Z

#3925

Copilot

Pull request overview

This PR fixes the "Too many authentication failures" issue that occurred when using ProxyJump connections. The root cause was that the global singleton SSH client accumulated authentication identities from multiple concurrent sessions, causing authentication attempts to exceed the server's MaxAuthTries limit.

Key Changes:

Creates dedicated, short-lived SSH client instances for each proxy connection instead of using the shared global singleton
Implements proper lifecycle management with automatic cleanup when sessions close or authentication fails
Wraps connection logic in try-catch to ensure scoped clients are stopped even on failure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T12:47:52Z

...lector-common/src/main/java/org/apache/hertzbeat/collector/collect/common/ssh/SshHelper.java

+                    // If the session has not been established yet, manually stop
+                    if (clientSession == null || clientSession.isClosed()) {


The condition clientSession.isClosed() at line 256 may cause the scoped SSH client to be stopped twice in certain scenarios.

If the session was successfully created and authenticated (lines 198-235), but an exception occurs afterward, the session closing will trigger the addCloseFutureListener callback at lines 237-244, which stops the client. However, if the session is already closed when we reach this catch block, the condition at line 256 will also call sshClient.stop().

This creates a potential double-stop situation. While sshClient.stop() may be idempotent, it's better to avoid the redundancy. Consider changing the condition to check if the session was never established or if it's open (not closed), as the listener will handle the closed case.

Suggested change

// If the session has not been established yet, manually stop

if (clientSession == null || clientSession.isClosed()) {

// If the session has not been established yet, or is still open, manually stop

if (clientSession == null || clientSession.isOpen()) {

Copilot · 2025-12-23T12:47:52Z

...lector-common/src/main/java/org/apache/hertzbeat/collector/collect/common/ssh/SshHelper.java

        if (useProxy && StringUtils.hasText(sshProtocol.getProxyHost())) {
-            String proxySpec = String.format("%s@%s:%d", sshProtocol.getProxyUsername(), sshProtocol.getProxyHost(), Integer.parseInt(sshProtocol.getProxyPort()));
-            proxyConfig.setHostName(sshProtocol.getHost());
-            proxyConfig.setHost(sshProtocol.getHost());
-            proxyConfig.setPort(Integer.parseInt(sshProtocol.getPort()));
-            proxyConfig.setUsername(sshProtocol.getUsername());
-            proxyConfig.setProxyJump(proxySpec);
+            // create a dedicated client instance to avoid contaminating the global singleton
+            sshClient = SshClient.setUpDefaultClient();
+            // accept all server key verifier, will print warn log : Server at {} presented unverified {} key: {}
+            AcceptAllServerKeyVerifier verifier = AcceptAllServerKeyVerifier.INSTANCE;
+            sshClient.setServerKeyVerifier(verifier);
+            // set connection heartbeat interval time 2000ms, wait for heartbeat response timeout 300_000ms
+            PropertyResolverUtils.updateProperty(
+                sshClient, CoreModuleProperties.HEARTBEAT_INTERVAL.getName(), 2000);
+            PropertyResolverUtils.updateProperty(
+                sshClient, CoreModuleProperties.HEARTBEAT_NO_REPLY_MAX.getName(), 30);
+            PropertyResolverUtils.updateProperty(
+                sshClient, CoreModuleProperties.SOCKET_KEEPALIVE.getName(), true);
+            // set support all KeyExchange
+            sshClient.setKeyExchangeFactories(NamedFactory.setUpTransformedFactories(
+                false,
+                BuiltinDHFactories.VALUES,
+                ClientBuilder.DH2KEX
+            ));
+            sshClient.start();
+            isScopedClient = true;
+            log.debug("Created scoped SshClient for proxy connection to avoid race condition.");
+        } else {
+            sshClient = CommonSshClient.getSshClient();
+        }

-            // Apache SSHD requires the password for the proxy to be preloaded into the sshClient instance before connecting
-            if (StringUtils.hasText(sshProtocol.getProxyPassword())) {
-                sshClient.addPasswordIdentity(sshProtocol.getProxyPassword());
-                log.debug("Loaded proxy server password authentication: {}@{}", sshProtocol.getProxyUsername(), sshProtocol.getProxyHost());
-            }
-            if (StringUtils.hasText(sshProtocol.getProxyPrivateKey())) {
-                proxyConfig.setIdentities(List.of(sshProtocol.getProxyPrivateKey()));
-                log.debug("Proxy private key loaded into HostConfigEntry");
+        try {
+            HostConfigEntry proxyConfig = new HostConfigEntry();
+            if (useProxy && StringUtils.hasText(sshProtocol.getProxyHost())) {
+                String proxySpec = String.format("%s@%s:%d", sshProtocol.getProxyUsername(), sshProtocol.getProxyHost(),
+                                                 Integer.parseInt(sshProtocol.getProxyPort()));
+                proxyConfig.setHostName(sshProtocol.getHost());
+                proxyConfig.setHost(sshProtocol.getHost());
+                proxyConfig.setPort(Integer.parseInt(sshProtocol.getPort()));
+                proxyConfig.setUsername(sshProtocol.getUsername());
+                proxyConfig.setProxyJump(proxySpec);
+
+                // Apache SSHD requires the password for the proxy to be preloaded into the sshClient instance before connecting
+                if (StringUtils.hasText(sshProtocol.getProxyPassword())) {
+                    sshClient.addPasswordIdentity(sshProtocol.getProxyPassword());
+                    log.debug("Loaded proxy server password authentication: {}@{}", sshProtocol.getProxyUsername(),
+                              sshProtocol.getProxyHost());
+                }
+                if (StringUtils.hasText(sshProtocol.getProxyPrivateKey())) {
+                    proxyConfig.setIdentities(List.of(sshProtocol.getProxyPrivateKey()));
+                    log.debug("Proxy private key loaded into HostConfigEntry");
+                }
            }
-        }

-        if (useProxy && StringUtils.hasText(sshProtocol.getProxyHost())) {
-            try {
+            if (useProxy && StringUtils.hasText(sshProtocol.getProxyHost())) {
                clientSession = sshClient.connect(proxyConfig)
                                         .verify(timeout, TimeUnit.MILLISECONDS).getSession();
+            } else {
+                clientSession = sshClient.connect(sshProtocol.getUsername(), sshProtocol.getHost(), Integer.parseInt(sshProtocol.getPort()))
+                                         .verify(timeout, TimeUnit.MILLISECONDS).getSession();
            }
-            finally {
-                sshClient.removePasswordIdentity(sshProtocol.getProxyPassword());
-            }
-        } else {
-            clientSession = sshClient.connect(sshProtocol.getUsername(), sshProtocol.getHost(), Integer.parseInt(sshProtocol.getPort()))
-                                     .verify(timeout, TimeUnit.MILLISECONDS).getSession();
-        }

-        if (StringUtils.hasText(sshProtocol.getPassword())) {
-            clientSession.addPasswordIdentity(sshProtocol.getPassword());
-        } else if (StringUtils.hasText(sshProtocol.getPrivateKey())) {
-            var resourceKey = PrivateKeyUtils.writePrivateKey(sshProtocol.getHost(), sshProtocol.getPrivateKey());
-            try (InputStream keyStream = new FileInputStream(resourceKey)) {
-                FilePasswordProvider passwordProvider = (session, resource, index) -> {
-                    if (StringUtils.hasText(sshProtocol.getPrivateKeyPassphrase())) {
-                        return sshProtocol.getPrivateKeyPassphrase();
+            if (StringUtils.hasText(sshProtocol.getPassword())) {
+                clientSession.addPasswordIdentity(sshProtocol.getPassword());
+            } else if (StringUtils.hasText(sshProtocol.getPrivateKey())) {
+                var resourceKey = PrivateKeyUtils.writePrivateKey(sshProtocol.getHost(), sshProtocol.getPrivateKey());
+                try (InputStream keyStream = new FileInputStream(resourceKey)) {
+                    FilePasswordProvider passwordProvider = (session, resource, index) -> {
+                        if (StringUtils.hasText(sshProtocol.getPrivateKeyPassphrase())) {
+                            return sshProtocol.getPrivateKeyPassphrase();
+                        }
+                        return null;
+                    };
+                    Iterable<KeyPair> keyPairs = SecurityUtils.loadKeyPairIdentities(null, () -> resourceKey, keyStream,
+                                                                                     passwordProvider);
+                    if (keyPairs != null) {
+                        keyPairs.forEach(clientSession::addPublicKeyIdentity);
+                    } else {
+                        log.error("Failed to load private key pairs from: {}", resourceKey);
                    }
-                    return null;
-                };
-                Iterable<KeyPair> keyPairs = SecurityUtils.loadKeyPairIdentities(null, () -> resourceKey, keyStream, passwordProvider);
-                if (keyPairs != null) {
-                    keyPairs.forEach(clientSession::addPublicKeyIdentity);
-                } else {
-                    log.error("Failed to load private key pairs from: {}", resourceKey);
+                } catch (IOException e) {
+                    log.error("Error reading private key file: {}", e.getMessage());
                }
-            } catch (IOException e) {
-                log.error("Error reading private key file: {}", e.getMessage());
-            }
-        }  // else auth with localhost private public key certificates
+            }  // else auth with localhost private public key certificates

-        // auth
-        if (!clientSession.auth().verify(timeout, TimeUnit.MILLISECONDS).isSuccess()) {
-            clientSession.close();
-            throw new IllegalArgumentException("ssh auth failed.");
-        }
-        if (reuseConnection || useProxy) {
-            SshConnect sshConnect = new SshConnect(clientSession);
-            CONNECTION_COMMON_CACHE.addCache(identifier, sshConnect);
+            // auth
+            if (!clientSession.auth().verify(timeout, TimeUnit.MILLISECONDS).isSuccess()) {
+                clientSession.close();
+                if (isScopedClient) {
+                    sshClient.stop();
+                }
+                throw new IllegalArgumentException("ssh auth failed.");
+            }
+            if (isScopedClient) {
+                clientSession.addCloseFutureListener(future -> {
+                    try {
+                        log.debug("Session closed, stopping scoped SshClient.");
+                        sshClient.stop();
+                    } catch (Exception e) {
+                        log.error("Failed to stop scoped SshClient", e);
+                    }
+                });
+            }
+            if (reuseConnection || useProxy) {
+                SshConnect sshConnect = new SshConnect(clientSession);
+                CONNECTION_COMMON_CACHE.addCache(identifier, sshConnect);
+            }
+            return clientSession;
+        } catch (Exception e) {
+            if (isScopedClient && sshClient.isStarted()) {
+                try {
+                    // If the session has been established but an error occurs afterward, session.close() will trigger the above listener.
+                    // If the session has not been established yet, manually stop
+                    if (clientSession == null || clientSession.isClosed()) {
+                        sshClient.stop();
+                    }
+                } catch (Exception ex) {
+                    log.warn("Error stopping scoped client on failure", ex);
+                }
+            }


The scoped SSH client lifecycle management logic introduces significant new behavior that is not covered by tests. The new code handles:

Creating a dedicated SSH client for proxy connections (lines 150-169)

Starting and stopping the scoped client (lines 167, 232, 240, 257)

Adding a close listener to clean up resources (lines 237-244)

Complex error handling with conditional cleanup (lines 252-262)

This functionality is critical for preventing the "Too many authentication failures" issue. Consider adding unit tests to verify:

Scoped client is created only for proxy connections

Client is properly stopped when session closes

Client is stopped on authentication failure

Client is stopped when exceptions occur before session establishment

No resource leaks occur in various failure scenarios

Copilot · 2025-12-23T12:47:53Z

...lector-common/src/main/java/org/apache/hertzbeat/collector/collect/common/ssh/SshHelper.java

+            sshClient = SshClient.setUpDefaultClient();
+            // accept all server key verifier, will print warn log : Server at {} presented unverified {} key: {}
+            AcceptAllServerKeyVerifier verifier = AcceptAllServerKeyVerifier.INSTANCE;
+            sshClient.setServerKeyVerifier(verifier);


The scoped SSH client created for proxy connections is missing the forwarding filter configuration that is present in the global singleton client. The CommonSshClient sets setForwardingFilter(new AcceptAllForwardingFilter()) to handle port forwarding, which is essential for ProxyJump functionality.

Without this configuration, the scoped client may not properly handle the port forwarding required for proxy jump connections. Consider adding this line after the server key verifier setup to ensure feature parity with the global client.

Copilot · 2025-12-23T12:47:53Z

...lector-common/src/main/java/org/apache/hertzbeat/collector/collect/common/ssh/SshHelper.java

+            if (isScopedClient) {
+                clientSession.addCloseFutureListener(future -> {
+                    try {
+                        log.debug("Session closed, stopping scoped SshClient.");
+                        sshClient.stop();
+                    } catch (Exception e) {
+                        log.error("Failed to stop scoped SshClient", e);
+                    }
+                });


The close listener is registered after successful authentication, but if the session is added to the cache and later closed by other code (e.g., from cache cleanup at lines 131-138), the listener will still attempt to stop the scoped client. This could cause the scoped client to be stopped while other sessions might still be using it if there's connection reuse.

However, looking at line 236, the listener is only added when isScopedClient is true, and scoped clients are specifically created for proxy connections where useProxy is true. At line 246-248, when using a proxy, the connection is always added to cache. This means a single scoped client instance is tied to a single cached session.

The current design appears safe because:

Each scoped client is dedicated to one proxy connection

When the cached session closes, the client should also stop

The cache uses the same identifier for the same connection parameters

This is working as intended, but it would be clearer with a comment explaining that scoped clients have 1:1 lifecycle with their sessions.

LL-LIN · 2025-12-24T02:58:14Z

observed that there is a situation where the sshclient thread is not being properly recycled.

github-actions bot added collector backend labels Dec 23, 2025

LL-LIN mentioned this pull request Dec 23, 2025

[BUG] ssh monitor with proxyjump, triggering "Too many authentication failures" exception #3925

Open

1 task

zqr10159 requested a review from Copilot December 23, 2025 12:42

Copilot started reviewing on behalf of zqr10159 December 23, 2025 12:43 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

Merge branch 'master' into bugfix-apache#3925

c440bd1

Aias00 and others added 4 commits December 28, 2025 18:39

Merge branch 'master' into bugfix-apache#3925

c8de73d

Merge branch 'master' into bugfix-apache#3925

de3dc71

Merge branch 'master' into bugfix-apache#3925

5b6299d

Merge branch 'master' into bugfix-apache#3925

a69be6f

love12yadav mentioned this pull request Jan 4, 2026

SSH ProxyJump connections fail due to shared sshClient authentication state #3942

Closed

Merge branch 'master' into bugfix-apache#3925

5a18999

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[bugfix]: resolve "Too many authentication failures" concurrent problem #3926

[bugfix]: resolve "Too many authentication failures" concurrent problem #3926

Uh oh!

LL-LIN commented Dec 23, 2025 •

edited

Loading

Uh oh!

LL-LIN commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

LL-LIN commented Dec 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		// If the session has not been established yet, manually stop
		if (clientSession == null \|\| clientSession.isClosed()) {

[bugfix]: resolve "Too many authentication failures" concurrent problem #3926

Are you sure you want to change the base?

[bugfix]: resolve "Too many authentication failures" concurrent problem #3926

Uh oh!

Conversation

LL-LIN commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's changed?

Checklist

Add or update API

Uh oh!

LL-LIN commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

LL-LIN commented Dec 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LL-LIN commented Dec 23, 2025 •

edited

Loading