Skip to content

Commit de65c12

Browse files
tomaswolftomaswolf
committed
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
1 parent b450b77 commit de65c12

6 files changed

Lines changed: 81 additions & 18 deletions

File tree

‎CHANGES.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,16 @@
3737

3838
## Potential Compatibility Issues
3939

40+
* [GH-892](https://github.com/apache/mina-sshd/issues/892) Align handling certificates without principals with OpenSSH 10.3
41+
42+
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).
43+
In Apache MINA SSHD, there is a new flag `CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS` (by default `false`)
44+
that can be set on an `SshClient` or `SshServer` or also on a `Session` directly. If the value is `false`, certificates
45+
without principals are rejected as in OpenSSH 10.3; if it is `true`, such certificates are considered to match any
46+
user or host name as in OpenSSH &lt; 10.3.
47+
48+
Set the flag on an `SshClient` or `ClientSession` to determine the handling of host certificates. Set it on an
49+
`SshServer` or `ServerSession` to govern the handling of user certificates.
50+
4051
## Major Code Re-factoring
4152

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,11 +269,15 @@ protected void verifyCertificate(Session session, OpenSshCertificate openSshKey)
269269
"KeyExchange CA signature verification failed for key type=" + keyAlg + " of key ID=" + keyId);
270270
}
271271

272+
// OpenSSH < 10.3:
273+
//
272274
// "As a special case, a zero-length "valid principals" field means the certificate is valid for
273275
// any principal of the specified type."
274276
// See https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
275277
//
276278
// Empty principals in a host certificate mean the certificate is valid for any host.
279+
//
280+
// OpenSSH >= 10.3: such certificates are never valid.
277281
Collection<String> principals = openSshKey.getPrincipals();
278282
if (!GenericUtils.isEmpty(principals)) {
279283
/*
@@ -296,6 +300,9 @@ protected void verifyCertificate(Session session, OpenSshCertificate openSshKey)
296300
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
297301
"KeyExchange signature verification failed, could not determine connect host for key ID=" + keyId);
298302
}
303+
} else if (!CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.getRequired(session)) {
304+
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
305+
"KeyExchange signature verification failed because the certificate has no principals; key ID=" + keyId);
299306
}
300307

301308
if (!GenericUtils.isEmpty(openSshKey.getCriticalOptions())) {

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,12 @@ public final class CoreModuleProperties {
799799
}
800800
});
801801

802+
/**
803+
* Whether to allow SSH user or host certificates without principals. OpenSSH < 10.3 considered such certificates to
804+
* be valid for any principal; since OpenSSH 10.3 such certificates are rejected.
805+
*/
806+
public static final Property<Boolean> ALLOW_EMPTY_CERTIFICATE_PRINCIPALS = Property.bool("empty-cert-principals", false);
807+
802808
private CoreModuleProperties() {
803809
throw new UnsupportedOperationException("No instance");
804810
}

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

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.sshd.common.util.GenericUtils;
3737
import org.apache.sshd.common.util.MapEntryUtils;
3838
import org.apache.sshd.common.util.logging.AbstractLoggingBean;
39+
import org.apache.sshd.core.CoreModuleProperties;
3940
import org.apache.sshd.server.session.ServerSession;
4041

4142
/**
@@ -125,14 +126,18 @@ protected boolean matchesPrincipals(
125126
AuthorizedKeyEntry entry, String username, OpenSshCertificate cert,
126127
ServerSession session) {
127128
Collection<String> certPrincipals = cert.getPrincipals();
129+
// OpenSSH < 10.3:
130+
//
131+
// "As a special case, a zero-length "valid principals" field means the certificate is valid for
132+
// any principal of the specified type."
133+
// See https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
134+
//
135+
// This is true for user certificates unless they are checked via a TrustedUserCAKeys file, but
136+
// that is not what we implement here.
137+
// See https://man.openbsd.org/sshd_config#TrustedUserCAKeys
138+
//
139+
// OpenSSH >= 10.3: certificates without principals never match
128140
if (!GenericUtils.isEmpty(certPrincipals)) {
129-
// "As a special case, a zero-length "valid principals" field means the certificate is valid for
130-
// any principal of the specified type."
131-
// See https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
132-
//
133-
// This is true for user certificates unless they are checked via a TrustedUserCAKeys file, but
134-
// that is not what we implement here.
135-
// See https://man.openbsd.org/sshd_config#TrustedUserCAKeys
136141
String allowedPrincipals = entry.getLoginOptions().get("principals");
137142
if (!GenericUtils.isEmpty(allowedPrincipals)) {
138143
if (Stream.of(allowedPrincipals.split(",")) //
@@ -146,12 +151,16 @@ protected boolean matchesPrincipals(
146151
} else {
147152
// We have a match for the certificate, but no principals from the entry: check that given
148153
// user name is in the certificate's principals.
149-
if (!GenericUtils.isEmpty(certPrincipals) && !certPrincipals.contains(username)) {
154+
if (!certPrincipals.contains(username)) {
150155
log.debug("authenticate({})[{}] certificate match rejected, user not in certificate principals: {}",
151-
username, session, username);
156+
username, session, certPrincipals);
152157
return false;
153158
}
154159
}
160+
} else if (!CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.getRequired(session)) {
161+
log.debug("authenticate({})[{}] certificate match rejected because the certificate has no principals", username,
162+
session);
163+
return false;
155164
}
156165
return true;
157166
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,13 @@
3737
import org.apache.sshd.common.config.keys.PublicKeyEntry;
3838
import org.apache.sshd.common.keyprovider.KeyPairProvider;
3939
import org.apache.sshd.common.util.GenericUtils;
40+
import org.apache.sshd.core.CoreModuleProperties;
4041
import org.apache.sshd.server.SshServer;
4142
import org.apache.sshd.util.test.BaseTestSupport;
4243
import org.apache.sshd.util.test.CommonTestSupportUtils;
4344
import org.apache.sshd.util.test.CoreTestSupportUtils;
4445
import org.junit.jupiter.api.AfterAll;
46+
import org.junit.jupiter.api.AfterEach;
4547
import org.junit.jupiter.api.BeforeAll;
4648
import org.junit.jupiter.api.Test;
4749
import org.junit.jupiter.api.io.TempDir;
@@ -92,6 +94,11 @@ static void tearDownClientAndServer() throws Exception {
9294
}
9395
}
9496

97+
@AfterEach
98+
void resetCertificateProperty() {
99+
CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(client, false);
100+
}
101+
95102
private static Stream<String> markers() {
96103
return Stream.of("rejected", "", null);
97104
}
@@ -155,8 +162,22 @@ void testHostCertificateSucceeds() throws Exception {
155162
}
156163
}
157164

165+
@Test
166+
void testHostCertificateWithoutPrincipalsFails() throws Exception {
167+
initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256", "cert-authority",
168+
new String[0]);
169+
assertThrows(SshException.class, () -> {
170+
try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
171+
.getSession()) {
172+
s.addPasswordIdentity(getCurrentTestName());
173+
s.auth().verify(AUTH_TIMEOUT);
174+
}
175+
});
176+
}
177+
158178
@Test
159179
void testHostCertificateWithoutPrincipalsSucceeds() throws Exception {
180+
CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(client, true);
160181
initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256", "cert-authority",
161182
new String[0]);
162183
try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)

‎sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.apache.sshd.common.config.keys.PublicKeyEntry;
4141
import org.apache.sshd.common.config.keys.PublicKeyEntryResolver;
4242
import org.apache.sshd.common.util.GenericUtils;
43+
import org.apache.sshd.core.CoreModuleProperties;
4344
import org.apache.sshd.server.SshServer;
4445
import org.apache.sshd.server.auth.keyboard.KeyboardInteractiveAuthenticator;
4546
import org.apache.sshd.server.auth.password.RejectAllPasswordAuthenticator;
@@ -48,6 +49,7 @@
4849
import org.apache.sshd.util.test.CommonTestSupportUtils;
4950
import org.apache.sshd.util.test.CoreTestSupportUtils;
5051
import org.junit.jupiter.api.AfterAll;
52+
import org.junit.jupiter.api.AfterEach;
5153
import org.junit.jupiter.api.BeforeAll;
5254
import org.junit.jupiter.api.io.TempDir;
5355
import org.junit.jupiter.params.ParameterizedTest;
@@ -99,14 +101,20 @@ static void tearDownClientAndServer() throws Exception {
99101
}
100102
}
101103

104+
@AfterEach
105+
void resetCertificateProperty() {
106+
CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(sshd, false);
107+
}
108+
102109
private static Stream<Arguments> options() {
103-
return Stream.of(Arguments.of("", "", false), // Not CA: fail
104-
Arguments.of("", "user", false), // Not CA: fail
105-
Arguments.of("cert-authority", "user", true), // CA, principal=user: success
106-
Arguments.of("cert-authority", "", true), // CA, no principal: success
107-
Arguments.of("cert-authority", "other", false), // CA, wrong principal: fail
108-
Arguments.of("cert-authority,principals=\"other\"", "user", false), // CA, principal overridden
109-
Arguments.of("cert-authority,principals=\"other\"", "other", true) // CA, principal overridden
110+
return Stream.of(Arguments.of("", "", true, false), // Not CA: fail
111+
Arguments.of("", "user", true, false), // Not CA: fail
112+
Arguments.of("cert-authority", "user", false, true), // CA, principal=user: success
113+
Arguments.of("cert-authority", "", true, true), // CA, no principal: success
114+
Arguments.of("cert-authority", "", false, false), // CA, no principal: fail
115+
Arguments.of("cert-authority", "other", false, false), // CA, wrong principal: fail
116+
Arguments.of("cert-authority,principals=\"other\"", "user", false, false), // CA, principal overridden
117+
Arguments.of("cert-authority,principals=\"other\"", "other", false, true) // CA, principal overridden
110118
);
111119
}
112120

@@ -139,9 +147,10 @@ private void initCert(String caMarker, String... principals) throws Exception {
139147
client.setUserAuthFactories(Collections.singletonList(new UserAuthPublicKeyFactory()));
140148
}
141149

142-
@ParameterizedTest(name = "test {0} CA key with {1}")
150+
@ParameterizedTest(name = "test {0} CA key with ''{1}'' (empty={2})")
143151
@MethodSource("options")
144-
void testCertificate(String marker, String principals, boolean expectSuccess) throws Exception {
152+
void testCertificate(String marker, String principals, boolean allowEmpty, boolean expectSuccess) throws Exception {
153+
CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(sshd, allowEmpty);
145154
String[] certPrincipals = GenericUtils.isEmpty(principals) ? new String[0] : principals.split(",");
146155
initCert(marker, certPrincipals);
147156
try (ClientSession s = client.connect("user", TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)

0 commit comments

Comments
 (0)