diff options
4 files changed, 72 insertions, 9 deletions
diff --git a/releases.moxie b/releases.moxie index d64de1c9..a40560ec 100644 --- a/releases.moxie +++ b/releases.moxie @@ -6,6 +6,8 @@ r34: { id: ${project.version} date: ${project.buildDate} note: '' + This release fixes a vulnerability allowing an attacker to circumvent authentication on the SSH transport. Users are urged to update to this version. + Should you have disabled the Flash-based copy-to-clipboard function because it wasn't working anymore (`web.allowFlashCopyToClipboard = false`), you may want to rethink this and enable it again. The configuration property has the same name, but the mechanism was exchanged. Flash is gone, and a modern JavaScript solution is now used to copy text directly to the clipboard (via clipboard.js). The setting `server.requireClientCertificates` now has three values: `required`, `optional` and `none`. While `required` is synonymous to the old `true` value, and `optional` is synonymous to the old `false` value, the new `none` value results in the server never asking the client to present any client certificate at all. The old values `true` and `false` can still be used and keep their meaning. @@ -18,6 +20,7 @@ r34: { Highlights: * Support for ECDSA and Ed25519 SSH keys + * Fix vulnerability that allowed SSH authentication to be circumvented * Explicitly disable requesting optional client TLS certificates * Copy-to-clipboard button is back and working * Minimal required Java version is Java 8 @@ -30,6 +33,7 @@ r34: { '' security: - Fix path traversal vulnerability which allowed access to "/resources//../WEB-INF/". (CVE-2022-31268) This was fixed by updating Jetty. (issue-1409) + - Fix exploit circumventing SSH authentication. Many thanks to András Veres-Szentkirályi (silentsignal.eu) for the report. (CVE-2024-28080) fixes: - Fix crash in Gitblit Authority when users were deleted from Gitblit but still had entries (certificates) in the Authority. (issue-1359, pr-1435) - Fix tab-to-space conversion to work like tabs. (pr-1065 by @QuentinC) @@ -92,6 +96,7 @@ r34: { - Tino Desjardins - @xxl-cc - Egor Shchegolkov + - András Veres-Szentkirályi } # diff --git a/src/main/java/com/gitblit/transport/ssh/SshKrbAuthenticator.java b/src/main/java/com/gitblit/transport/ssh/SshKrbAuthenticator.java index b6d233cf..06444606 100644 --- a/src/main/java/com/gitblit/transport/ssh/SshKrbAuthenticator.java +++ b/src/main/java/com/gitblit/transport/ssh/SshKrbAuthenticator.java @@ -54,10 +54,7 @@ public class SshKrbAuthenticator extends GSSAuthenticator { public boolean validateIdentity(ServerSession session, String identity) { log.info("identify with kerberos {}", identity); SshDaemonClient client = session.getAttribute(SshDaemonClient.KEY); - if (client.getUser() != null) { - log.info("{} has already authenticated!", identity); - return true; - } + String username = identity.toLowerCase(Locale.US); if (stripDomain) { int p = username.indexOf('@'); @@ -67,6 +64,7 @@ public class SshKrbAuthenticator extends GSSAuthenticator { } UserModel user = authManager.authenticate(username); if (user != null) { +// TODO: Check if the user was set in the client and if it is the same as this user. Do not allow changing the user during the SSH auth process. client.setUser(user); return true; } diff --git a/src/main/java/com/gitblit/transport/ssh/UsernamePasswordAuthenticator.java b/src/main/java/com/gitblit/transport/ssh/UsernamePasswordAuthenticator.java index e9e2d7e1..fa56baa8 100644 --- a/src/main/java/com/gitblit/transport/ssh/UsernamePasswordAuthenticator.java +++ b/src/main/java/com/gitblit/transport/ssh/UsernamePasswordAuthenticator.java @@ -45,14 +45,11 @@ public class UsernamePasswordAuthenticator implements PasswordAuthenticator { @Override public boolean authenticate(String username, String password, ServerSession session) { SshDaemonClient client = session.getAttribute(SshDaemonClient.KEY); - if (client.getUser() != null) { - log.info("{} has already authenticated!", username); - return true; - } username = username.toLowerCase(Locale.US); UserModel user = authManager.authenticate(username, password.toCharArray(), null); if (user != null) { +// TODO: Check if the user was set in the client and if it is the same as this user. Do not allow changing the user during the SSH auth process. client.setUser(user); return true; } diff --git a/src/test/java/com/gitblit/tests/SshDaemonTest.java b/src/test/java/com/gitblit/tests/SshDaemonTest.java index c7d06198..e88dc9bb 100644 --- a/src/test/java/com/gitblit/tests/SshDaemonTest.java +++ b/src/test/java/com/gitblit/tests/SshDaemonTest.java @@ -16,10 +16,12 @@ package com.gitblit.tests; import java.io.File; +import java.security.KeyPair; import java.text.MessageFormat; import java.util.List; import org.apache.sshd.client.SshClient; +import org.apache.sshd.client.future.AuthFuture; import org.apache.sshd.client.session.ClientSession; import org.eclipse.jgit.api.CloneCommand; import org.eclipse.jgit.api.Git; @@ -42,11 +44,72 @@ public class SshDaemonTest extends SshUnitTest { String url = GitBlitSuite.sshDaemonUrl; @Test + public void testPasswordAuthentication() throws Exception { + SshClient client = getClient(); + ClientSession session = client.connect(username, "localhost", GitBlitSuite.sshPort).verify().getSession(); + + session.addPasswordIdentity(password); + AuthFuture authFuture = session.auth(); + assertTrue(authFuture.await()); + assertTrue(authFuture.isSuccess()); + } + + @Test public void testPublicKeyAuthentication() throws Exception { SshClient client = getClient(); ClientSession session = client.connect(username, "localhost", GitBlitSuite.sshPort).verify().getSession(); + session.addPublicKeyIdentity(rwKeyPair); - assertTrue(session.auth().await()); + AuthFuture authFuture = session.auth(); + assertTrue(authFuture.await()); + assertTrue(authFuture.isSuccess()); + } + + @Test + public void testWrongPublicKeyAuthentication() throws Exception { + SshClient client = getClient(); + ClientSession session = client.connect(username, "localhost", GitBlitSuite.sshPort).verify().getSession(); + KeyPair attackKeyPair = generator.generateKeyPair(); + + session.addPublicKeyIdentity(attackKeyPair); + AuthFuture authFuture = session.auth(); + assertTrue(authFuture.await()); + assertFalse(authFuture.isSuccess()); + } + + @Test + public void testWrongPublicKeyThenPasswordAuthentication() throws Exception { + SshClient client = getClient(); + ClientSession session = client.connect(username, "localhost", GitBlitSuite.sshPort).verify().getSession(); + KeyPair otherKeyPair = generator.generateKeyPair(); + + session.addPublicKeyIdentity(otherKeyPair); + AuthFuture authFuture = session.auth(); + assertTrue(authFuture.await()); + assertFalse(authFuture.isSuccess()); + + session.addPasswordIdentity(password); + authFuture = session.auth(); + assertTrue(authFuture.await()); + assertTrue(authFuture.isSuccess()); + } + + @Test + public void testWrongPublicKeyThenWrongPasswordAuthentication() throws Exception { + SshClient client = getClient(); + ClientSession session = client.connect(username, "localhost", GitBlitSuite.sshPort).verify().getSession(); + KeyPair otherKeyPair = generator.generateKeyPair(); + KeyPair attackKeyPair = new KeyPair(rwKeyPair.getPublic(), otherKeyPair.getPrivate()); + + session.addPublicKeyIdentity(attackKeyPair); + AuthFuture authFuture = session.auth(); + assertTrue(authFuture.await()); + assertFalse(authFuture.isSuccess()); + + session.addPasswordIdentity("nothing"); + authFuture = session.auth(); + assertTrue(authFuture.await()); + assertFalse(authFuture.isSuccess()); } @Test |