This is an automated email from the ASF dual-hosted git repository. tomaswolf pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit de65c121e8910dbe9a01ecb864eb147461a68b9f Author: Thomas Wolf <[email protected]> AuthorDate: Fri Apr 17 20:49:18 2026 +0200 GH-892: Adapt handling certificates without principals Align the handling of certificates without principals with OpenSSH 10.3. OpenSSH < 10.3 treated this as a wildcard (except for user certificates checked via the TrustedUserCAKeys mechanism) and let such certificates match any user or host name. OpenSSH 10.3 changed that and rejects such certificates always since they don't match any principal.[1] Implement this. Add a new flag in CoreModuleProperties through which client code can choose what to do with such certificates: allow them (and let them match always; as in OpenSSH < 10.3), or forbid them as in OpenSSH >= 10.3. By default, such certificates without principals are rejected, which is a change in behavior. [1] https://www.openssh.org/txt/release-10.3 --- CHANGES.md | 11 +++++++++ .../java/org/apache/sshd/client/kex/DHGClient.java | 7 ++++++ .../org/apache/sshd/core/CoreModuleProperties.java | 6 +++++ ...AuthorizedKeyEntriesPublickeyAuthenticator.java | 27 ++++++++++++++-------- .../signature/KnownHostsCertificateTest.java | 21 +++++++++++++++++ .../server/auth/AuthorizedKeysCertificateTest.java | 27 ++++++++++++++-------- 6 files changed, 81 insertions(+), 18 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d4f4dafbf..37edef7b6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -37,5 +37,16 @@ ## 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 diff --git a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java index bab1928f0..589676714 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java @@ -269,11 +269,15 @@ public class DHGClient extends AbstractDHClientKeyExchange { "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)) { /* @@ -296,6 +300,9 @@ public class DHGClient extends AbstractDHClientKeyExchange { 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 (!GenericUtils.isEmpty(openSshKey.getCriticalOptions())) { diff --git a/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java b/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java index 2bec5c3a3..ec33ae0ba 100644 --- a/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java +++ b/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java @@ -799,6 +799,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); + private CoreModuleProperties() { throw new UnsupportedOperationException("No instance"); } diff --git a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/AuthorizedKeyEntriesPublickeyAuthenticator.java b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/AuthorizedKeyEntriesPublickeyAuthenticator.java index a667af8be..d41fbe3b1 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/AuthorizedKeyEntriesPublickeyAuthenticator.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/AuthorizedKeyEntriesPublickeyAuthenticator.java @@ -36,6 +36,7 @@ import org.apache.sshd.common.config.keys.PublicKeyEntryResolver; 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 @@ public class AuthorizedKeyEntriesPublickeyAuthenticator extends AbstractLoggingB 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 @@ public class AuthorizedKeyEntriesPublickeyAuthenticator extends AbstractLoggingB } 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; } diff --git a/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java b/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java index ffb3c1cd5..be4472f72 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java @@ -37,11 +37,13 @@ import org.apache.sshd.common.config.keys.OpenSshCertificate; 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; @@ -92,6 +94,11 @@ class KnownHostsCertificateTest extends BaseTestSupport { } } + @AfterEach + void resetCertificateProperty() { + CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(client, false); + } + private static Stream<String> markers() { return Stream.of("rejected", "", null); } @@ -155,8 +162,22 @@ class KnownHostsCertificateTest extends BaseTestSupport { } } + @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) diff --git a/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java b/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java index 49b2b2052..d11404e2e 100644 --- a/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java @@ -40,6 +40,7 @@ import org.apache.sshd.common.config.keys.OpenSshCertificate; import org.apache.sshd.common.config.keys.PublicKeyEntry; import org.apache.sshd.common.config.keys.PublicKeyEntryResolver; import org.apache.sshd.common.util.GenericUtils; +import org.apache.sshd.core.CoreModuleProperties; import org.apache.sshd.server.SshServer; import org.apache.sshd.server.auth.keyboard.KeyboardInteractiveAuthenticator; import org.apache.sshd.server.auth.password.RejectAllPasswordAuthenticator; @@ -48,6 +49,7 @@ 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.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; @@ -99,14 +101,20 @@ class AuthorizedKeysCertificateTest extends BaseTestSupport { } } + @AfterEach + void resetCertificateProperty() { + CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(sshd, false); + } + private static Stream<Arguments> options() { - return Stream.of(Arguments.of("", "", false), // Not CA: fail - Arguments.of("", "user", false), // Not CA: fail - Arguments.of("cert-authority", "user", true), // CA, principal=user: success - Arguments.of("cert-authority", "", true), // CA, no principal: success - Arguments.of("cert-authority", "other", false), // CA, wrong principal: fail - Arguments.of("cert-authority,principals=\"other\"", "user", false), // CA, principal overridden - Arguments.of("cert-authority,principals=\"other\"", "other", true) // CA, principal overridden + return Stream.of(Arguments.of("", "", true, false), // Not CA: fail + Arguments.of("", "user", true, false), // Not CA: fail + Arguments.of("cert-authority", "user", false, true), // CA, principal=user: success + Arguments.of("cert-authority", "", true, true), // CA, no principal: success + Arguments.of("cert-authority", "", false, false), // CA, no principal: fail + Arguments.of("cert-authority", "other", false, false), // CA, wrong principal: fail + Arguments.of("cert-authority,principals=\"other\"", "user", false, false), // CA, principal overridden + Arguments.of("cert-authority,principals=\"other\"", "other", false, true) // CA, principal overridden ); } @@ -139,9 +147,10 @@ class AuthorizedKeysCertificateTest extends BaseTestSupport { client.setUserAuthFactories(Collections.singletonList(new UserAuthPublicKeyFactory())); } - @ParameterizedTest(name = "test {0} CA key with {1}") + @ParameterizedTest(name = "test {0} CA key with ''{1}'' (empty={2})") @MethodSource("options") - void testCertificate(String marker, String principals, boolean expectSuccess) throws Exception { + void testCertificate(String marker, String principals, boolean allowEmpty, boolean expectSuccess) throws Exception { + CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(sshd, allowEmpty); String[] certPrincipals = GenericUtils.isEmpty(principals) ? new String[0] : principals.split(","); initCert(marker, certPrincipals); try (ClientSession s = client.connect("user", TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
