Merge 2.18.0 into dev_3.0.0

Also prepare release documentation.

Author: tomaswolf

CHANGES.md

@@ -34,6 +34,12 @@ there are only two bug fixes from 3.0.0-M2.
 
 * [Change notes for 3.0.0-M3](./docs/changes/3.0.0-M3.md)
 
+## Milestone 3: Pre-Release 3.0.0-M4
+
+Merges 2.18.0 into the 3.0.0 stream.
+
+* [Change notes for 3.0.0-M4](./docs/changes/3.0.0-M4.md)
+
 # Planned for the Next Milestone Release
 
 ## Bug Fixes

docs/changes/2.18.0.md

@@ -0,0 +1,37 @@
+# Introduced in 2.18.0
+
+## Bug Fixes
+
+* [GH-743](https://github.com/apache/mina-sshd/issues/743) Ensure the Java `ServiceLoader` use a singleton `SftpFileSystemProvider`
+* [GH-879](https://github.com/apache/mina-sshd/issues/879) Close SSH channel gracefully on exception in port forwarding
+* Improve handling of repository paths in `sshd-git`.
+
+## New Features
+
+* [GH-892](https://github.com/apache/mina-sshd/issues/892) Align handling certificates without principals with OpenSSH 10.3
+
+Wildcard principals in host certificates are handled now.
+
+* Putty keys with non-ASCII passphrases
+
+The passphrase needs to be converted to a byte sequence to compute a decryption key for an encrypted private key. This
+conversion depends on the character encoding. Putty on Windows uses the ANSI codepage set when the key was generated.
+Apache MINA SSHD now tries multiple encodings in sequence: UTF-8, then the OS encoding, and finally ISO-8859-1 as a
+last-chance fallback.
+
+## Potential Compatibility Issues
+
+* [GH-892](https://github.com/apache/mina-sshd/issues/892) Align handling certificates without principals with OpenSSH 10.3
+
+OpenSSH 10.3 changed the way such certificates are handled; see the [OpenSSH 10.3 release notes](https://www.openssh.org/txt/release-10.3).
+In Apache MINA SSHD, there is a new flag `CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS` (by default `false`)
+that can be set on an `SshClient` or `SshServer` or also on a `Session` directly. If the value is `false`, certificates
+without principals are rejected as in OpenSSH 10.3; if it is `true`, such certificates are considered to match any
+user or host name as in OpenSSH < 10.3.
+
+Set the flag on an `SshClient` or `ClientSession` to determine the handling of host certificates. Set it on an
+`SshServer` or `ServerSession` to govern the handling of user certificates.
+
+## Major Code Re-factoring
+
+None.

docs/changes/3.0.0-M3.md

@@ -2,7 +2,7 @@
 
 This milestone release fixes a minor bug in 3.0.0-M3 and brings in all improvements from the 2.X branch.
 
-It includes all the features and bug fixes of [version 2.17.1](./docs/changes/2.17.1.md) and up to
+It includes all the features and bug fixes of [version 2.17.1](./2.17.1.md) and up to
 [commit a3d22510](https://github.com/apache/mina-sshd/blob/a3d22510/CHANGES.md#planned-for-next-version).
 
 * [Change notes for 3.0.0-M1](./3.0.0-M1.md)

docs/changes/3.0.0-M4.md

@@ -0,0 +1,24 @@
+# Milestone Pre-Release 3.0.0-M4
+
+This milestone release just merges the 2.18.0 branch.
+
+It includes all the features and bug fixes of [version 2.18.0](./2.18.0.md).
+
+* [Change notes for 3.0.0-M1](./3.0.0-M1.md)
+* [Change notes for 3.0.0-M2](./3.0.0-M2.md)
+* [Change notes for 3.0.0-M3](./3.0.0-M3.md)
+
+* For building Apache MINA SSHD 3.0, **Java >= 24** and Apache **Maven >= 3.9.12** are required. Generated artifacts
+  still use Java 8 as minimum runtime requirement.
+
+## Bug Fixes
+
+None.
+
+## Major Code Re-factoring
+
+None.
+
+## New Features
+
+None.

pom.xml

@@ -92,7 +92,7 @@
         <required.java.version>[${java.sdk.version},)</required.java.version>
 
         <groovy.version>4.0.17</groovy.version>
-        <bouncycastle.version>1.83</bouncycastle.version>
+        <bouncycastle.version>1.84</bouncycastle.version>
         <!-- BC FIPS has version numbers 2.x.y. -->
         <bouncycastle.upper.bound>3</bouncycastle.upper.bound>
             <!-- NOTE: upgrading slf4j beyond this version causes
@@ -109,9 +109,9 @@
             <!-- mockito 5.0 requires Java 11. -->
         <mockito.version>4.11.0</mockito.version>
         <testcontainers.version>2.0.4</testcontainers.version>
-        <grpc.version>1.78.0</grpc.version> <!-- Used only in tests -->
+        <grpc.version>1.79.0</grpc.version> <!-- Used only in tests -->
 
-        <maven.archiver.version>3.6.5</maven.archiver.version>
+        <maven.archiver.version>3.6.6</maven.archiver.version>
         <plexus.archiver.version>4.11.0</plexus.archiver.version>
         <!-- See https://pmd.github.io/ for available latest version -->
         <pmd.version>7.22.0</pmd.version>
@@ -803,7 +803,7 @@
                         <dependency>
                             <groupId>commons-io</groupId>
                             <artifactId>commons-io</artifactId>
-                            <version>2.20.0</version>
+                            <version>2.21.0</version>
                         </dependency>
                     </dependencies>
                 </plugin>
@@ -1017,7 +1017,7 @@
                 <plugin>
                     <groupId>net.revelc.code</groupId>
                     <artifactId>impsort-maven-plugin</artifactId>
-                    <version>1.12.0</version>
+                    <version>1.13.0</version>
                     <configuration>
                         <lineEnding>LF</lineEnding>
                         <groups>java.,javax.,org.w3c.,org.xml.,junit.</groups>

sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java

@@ -24,6 +24,8 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.sshd.client.session.AbstractClientSession;
 import org.apache.sshd.common.NamedFactory;
@@ -257,11 +259,15 @@ protected void verifyCertificate(Session session, OpenSshCertificate openSshKey)
                     "KeyExchange CA signature verification failed for key type=" + keyAlg + " of key ID=" + keyId);
         }
 
+        // OpenSSH < 10.3:
+        //
         // "As a special case, a zero-length "valid principals" field means the certificate is valid for
         // any principal of the specified type."
         // See https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
         //
         // Empty principals in a host certificate mean the certificate is valid for any host.
+        //
+        // OpenSSH >= 10.3: such certificates are never valid.
         Collection<String> principals = openSshKey.getPrincipals();
         if (!GenericUtils.isEmpty(principals)) {
             /*
@@ -275,7 +281,7 @@ protected void verifyCertificate(Session session, OpenSshCertificate openSshKey)
 
             if (connectSocketAddress instanceof InetSocketAddress) {
                 String hostName = ((InetSocketAddress) connectSocketAddress).getHostString();
-                if (GenericUtils.isEmpty(principals) || (!principals.contains(hostName))) {
+                if (!hostMatches(principals, hostName)) {
                     throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
                             "KeyExchange signature verification failed, invalid principal " + hostName + " for key ID=" + keyId
                                                                                              + " - allowed=" + principals);
@@ -284,6 +290,9 @@ protected void verifyCertificate(Session session, OpenSshCertificate openSshKey)
                 throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
                         "KeyExchange signature verification failed, could not determine connect host for key ID=" + keyId);
             }
+        } else if (!CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.getRequired(session)) {
+            throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
+                    "KeyExchange signature verification failed because the certificate has no principals; key ID=" + keyId);
         }
 
         if (!openSshKey.getCriticalOptions().isEmpty()) {
@@ -295,4 +304,18 @@ protected void verifyCertificate(Session session, OpenSshCertificate openSshKey)
                                                                                      + keyId);
         }
     }
+
+    private Pattern wildcardToRegex(String wildcardPattern) {
+        String re = "^\\Q" + wildcardPattern + "\\E$";
+        re = re.replace("?", "\\E.\\Q").replaceAll("\\*+", Matcher.quoteReplacement("\\E.*?\\Q")).replace("\\Q\\E", "");
+        return Pattern.compile(re);
+    }
+
+    private boolean hostMatches(Collection<String> principals, String hostName) {
+        return principals.stream() //
+                .filter(s -> s != null && !s.isEmpty()) //
+                .anyMatch(principal -> (principal.contains("?") || principal.contains("*"))
+                        ? wildcardToRegex(principal).matcher(hostName).matches()
+                        : principal.equals(hostName));
+    }
 }

sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java

@@ -298,6 +298,18 @@ protected boolean acceptKnownHostEntries(
                 .filter(e -> isCert == "cert-authority".equals(e.getHostEntry().getMarker()))
                 .collect(Collectors.toList());
         if (!keyMatches.isEmpty()) {
+            if (serverKey instanceof OpenSshCertificate) {
+                // Also check whether the certified key has been revoked.
+                PublicKey certifiedKey = ((OpenSshCertificate) serverKey).getCertPubKey();
+                String certKeyType = KeyUtils.getKeyType(certifiedKey);
+                if (hostMatches.stream() //
+                        .filter(entry -> certKeyType.equals(entry.getHostEntry().getKeyEntry().getKeyType()))
+                        .filter(k -> KeyUtils.compareKeys(k.getServerKey(), certifiedKey))
+                        .anyMatch(entry -> "revoked".equals(entry.getHostEntry().getMarker()))) {
+                    handleRevokedKey(clientSession, remoteAddress, certifiedKey);
+                    return false;
+                }
+            }
             return true;
         }
 
@@ -306,7 +318,8 @@ protected boolean acceptKnownHostEntries(
         }
 
         Optional<HostEntryPair> anyNonRevokedMatch = hostMatches.stream()
-                .filter(k -> !"revoked".equals(k.getHostEntry().getMarker()))
+                .filter(k -> !"revoked".equals(k.getHostEntry().getMarker())
+                        && !"cert-authority".equals(k.getHostEntry().getMarker()))
                 .findAny();
 
         if (!anyNonRevokedMatch.isPresent()) {

sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java

@@ -780,6 +780,12 @@ public final class CoreModuleProperties {
         }
     });
 
+    /**
+     * Whether to allow SSH user or host certificates without principals. OpenSSH < 10.3 considered such certificates to
+     * be valid for any principal; since OpenSSH 10.3 such certificates are rejected.
+     */
+    public static final Property<Boolean> ALLOW_EMPTY_CERTIFICATE_PRINCIPALS = Property.bool("empty-cert-principals", false);
+
     public static final int DEFAULT_MAX_MSGS_BEFORE_KEX_INIT = 1_000;
 
     /**

sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/AuthorizedKeyEntriesPublickeyAuthenticator.java

@@ -36,6 +36,7 @@
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.MapEntryUtils;
 import org.apache.sshd.common.util.logging.AbstractLoggingBean;
+import org.apache.sshd.core.CoreModuleProperties;
 import org.apache.sshd.server.session.ServerSession;
 
 /**
@@ -125,14 +126,18 @@ protected boolean matchesPrincipals(
             AuthorizedKeyEntry entry, String username, OpenSshCertificate cert,
             ServerSession session) {
         Collection<String> certPrincipals = cert.getPrincipals();
+        // OpenSSH < 10.3:
+        //
+        // "As a special case, a zero-length "valid principals" field means the certificate is valid for
+        // any principal of the specified type."
+        // See https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
+        //
+        // This is true for user certificates unless they are checked via a TrustedUserCAKeys file, but
+        // that is not what we implement here.
+        // See https://man.openbsd.org/sshd_config#TrustedUserCAKeys
+        //
+        // OpenSSH >= 10.3: certificates without principals never match
         if (!GenericUtils.isEmpty(certPrincipals)) {
-            // "As a special case, a zero-length "valid principals" field means the certificate is valid for
-            // any principal of the specified type."
-            // See https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
-            //
-            // This is true for user certificates unless they are checked via a TrustedUserCAKeys file, but
-            // that is not what we implement here.
-            // See https://man.openbsd.org/sshd_config#TrustedUserCAKeys
             String allowedPrincipals = entry.getLoginOptions().get("principals");
             if (!GenericUtils.isEmpty(allowedPrincipals)) {
                 if (Stream.of(allowedPrincipals.split(",")) //
@@ -146,12 +151,16 @@ protected boolean matchesPrincipals(
             } else {
                 // We have a match for the certificate, but no principals from the entry: check that given
                 // user name is in the certificate's principals.
-                if (!GenericUtils.isEmpty(certPrincipals) && !certPrincipals.contains(username)) {
+                if (!certPrincipals.contains(username)) {
                     log.debug("authenticate({})[{}] certificate match rejected, user not in certificate principals: {}",
-                            username, session, username);
+                            username, session, certPrincipals);
                     return false;
                 }
             }
+        } else if (!CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.getRequired(session)) {
+            log.debug("authenticate({})[{}] certificate match rejected because the certificate has no principals", username,
+                    session);
+            return false;
         }
         return true;
     }

sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java

@@ -20,6 +20,7 @@
 
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 import java.security.KeyPair;
 import java.util.Arrays;
 import java.util.Collections;
@@ -37,16 +38,19 @@
 import org.apache.sshd.common.config.keys.PublicKeyEntry;
 import org.apache.sshd.common.keyprovider.KeyPairProvider;
 import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.core.CoreModuleProperties;
 import org.apache.sshd.server.SshServer;
 import org.apache.sshd.util.test.BaseTestSupport;
 import org.apache.sshd.util.test.CommonTestSupportUtils;
 import org.apache.sshd.util.test.CoreTestSupportUtils;
 import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * Tests for KEX with host certificates with host key validation through a {@link KnownHostsServerKeyVerifier}.
@@ -92,6 +96,11 @@ static void tearDownClientAndServer() throws Exception {
         }
     }
 
+    @AfterEach
+    void resetCertificateProperty() {
+        CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(client, false);
+    }
+
     private static Stream<String> markers() {
         return Stream.of("rejected", "", null);
     }
@@ -155,8 +164,22 @@ void testHostCertificateSucceeds() throws Exception {
         }
     }
 
+    @Test
+    void testHostCertificateWithoutPrincipalsFails() throws Exception {
+        initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256", "cert-authority",
+                new String[0]);
+        assertThrows(SshException.class, () -> {
+            try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
+                    .getSession()) {
+                s.addPasswordIdentity(getCurrentTestName());
+                s.auth().verify(AUTH_TIMEOUT);
+            }
+        });
+    }
+
     @Test
     void testHostCertificateWithoutPrincipalsSucceeds() throws Exception {
+        CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(client, true);
         initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256", "cert-authority",
                 new String[0]);
         try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
@@ -165,4 +188,36 @@ void testHostCertificateWithoutPrincipalsSucceeds() throws Exception {
             s.auth().verify(AUTH_TIMEOUT);
         }
     }
+
+    @ParameterizedTest(name = "test CA key with {0}")
+    @ValueSource(strings = { "loca?host,127.0.0.?", "loca*ost,127.?.?.*" })
+    void testHostCertificateWithWildcardSucceeds(String principals) throws Exception {
+        initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256", "cert-authority",
+                principals.split(","));
+        try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
+                .getSession()) {
+            s.addPasswordIdentity(getCurrentTestName());
+            s.auth().verify(AUTH_TIMEOUT);
+        }
+    }
+
+    @Test
+    void testHostCertificateWithRejectedHostKeyFails() throws Exception {
+        initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256", "cert-authority");
+        Path knownHosts = tmp.resolve("known_hosts");
+        StringBuilder line = new StringBuilder();
+        line.append("@revoked ");
+        line.append("[localhost]:").append(port).append(",[127.0.0.1]:").append(port).append(' ');
+        line.append(PublicKeyEntry.toString(hostKey.getPublic()));
+        line.append('\n');
+        Files.write(knownHosts, Collections.singletonList(line.toString()), StandardOpenOption.APPEND);
+        client.setServerKeyVerifier(new KnownHostsServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE, knownHosts));
+        assertThrows(SshException.class, () -> {
+            try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
+                    .getSession()) {
+                s.addPasswordIdentity(getCurrentTestName());
+                s.auth().verify(AUTH_TIMEOUT);
+            }
+        });
+    }
 }