This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 05311cac82b05ebcf30b580eeac1e3e7f343d72c
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sun Feb 16 22:40:44 2025 +0100
[SSHD-1161] Pubkey auth: handle certificates in PublickeyAuthenticator
Move the responsibility of checking the user name against the
certificate's principals into the PublickeyAuthenticator.
First, if not, an "accept all" authenticator wouldn't work since the
session might have rejected the certificate already.
Second, the OpenSSH format allows the user to explicitly specify
principals in the authorized_keys file. If so, these principals override
the log-in user name.
---
...AuthorizedKeyEntriesPublickeyAuthenticator.java | 60 ++++++-
.../auth/pubkey/CachingPublicKeyAuthenticator.java | 5 +-
.../server/auth/pubkey/PublickeyAuthenticator.java | 9 +-
.../sshd/server/auth/pubkey/UserAuthPublicKey.java | 5 -
.../server/auth/AuthorizedKeysCertificateTest.java | 198 +++++++++++++++++++++
5 files changed, 266 insertions(+), 11 deletions(-)
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 8650aa8f9..3feafc83c 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
@@ -26,10 +26,12 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
+import java.util.stream.Stream;
import org.apache.sshd.common.AttributeRepository;
import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.OpenSshCertificate;
import org.apache.sshd.common.config.keys.PublicKeyEntryResolver;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.MapEntryUtils;
@@ -85,24 +87,76 @@ public class AuthorizedKeyEntriesPublickeyAuthenticator
extends AbstractLoggingB
return false;
}
+ PublicKey keyToCheck = key;
+ boolean isCert = false;
+ if (key instanceof OpenSshCertificate) {
+ keyToCheck = ((OpenSshCertificate) key).getCaPubKey();
+ isCert = true;
+ }
for (Map.Entry<AuthorizedKeyEntry, PublicKey> e :
resolvedKeys.entrySet()) {
- if (KeyUtils.compareKeys(key, e.getValue())) {
+ AuthorizedKeyEntry entry = e.getKey();
+ if (isCert == entry.getLoginOptions().containsKey("cert-authority")
+ && KeyUtils.compareKeys(keyToCheck, e.getValue())) {
if (log.isDebugEnabled()) {
log.debug("authenticate({})[{}] match found", username,
session);
}
+ // TODO: the entry might have an "expiry-time" option.
+ // See https://man.openbsd.org/sshd.8#expiry-time=_timespec_
+ // (Certificate expiration as stored in the certificate itself
has been checked already.)
+ // TODO: the entry might have a "from" option limiting
possible source addresses by IP, hostnames,
+ // patterns, or CIDRs.
+ // See https://man.openbsd.org/sshd.8#from=_pattern-list_
+ if (isCert && !matchesPrincipals(entry, username,
(OpenSshCertificate) key, session)) {
+ continue;
+ }
if (session != null) {
- session.setAttribute(AUTHORIZED_KEY, e.getKey());
+ session.setAttribute(AUTHORIZED_KEY, entry);
}
return true;
}
}
if (log.isDebugEnabled()) {
- log.debug("authenticate({})[{}] match not found", username,
session);
+ log.debug("authenticate({})[{}] no match found", username,
session);
}
return false;
}
+ protected boolean matchesPrincipals(
+ AuthorizedKeyEntry entry, String username, OpenSshCertificate cert,
+ ServerSession session) {
+ Collection<String> certPrincipals = cert.getPrincipals();
+ 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(",")) //
+ .map(String::trim) //
+ .filter(s -> !GenericUtils.isEmpty(s)) //
+ .noneMatch(certPrincipals::contains)) {
+ log.debug("authenticate({})[{}] certificate match ignored,
none of the allowed principals matched: {}",
+ username, session, allowedPrincipals);
+ return false;
+ }
+ } 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)) {
+ log.debug("authenticate({})[{}] certificate match
rejected, user not in certificate principals: {}",
+ username, session, username);
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
@Override
public String toString() {
return Objects.toString(getId());
diff --git
a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
index fb5e0f730..6fe798d55 100644
---
a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
+++
b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
@@ -26,6 +26,7 @@ import java.util.concurrent.ConcurrentHashMap;
import org.apache.sshd.common.AttributeRepository;
import org.apache.sshd.common.RuntimeSshException;
import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.OpenSshCertificate;
import org.apache.sshd.common.util.logging.AbstractLoggingBean;
import org.apache.sshd.server.session.ServerSession;
@@ -66,7 +67,9 @@ public class CachingPublicKeyAuthenticator extends
AbstractLoggingBean implement
log.debug("authenticate({}@{}) cache result={} for {} key={}",
username, session, result, KeyUtils.getKeyType(key),
KeyUtils.getFingerPrint(key));
}
- map.put(key, result);
+ if (!(key instanceof OpenSshCertificate)) {
+ map.put(key, result);
+ }
} else {
if (log.isDebugEnabled()) {
log.debug("authenticate({}@{}) use cached result={} for {}
key={}",
diff --git
a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/PublickeyAuthenticator.java
b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/PublickeyAuthenticator.java
index 45767f08c..703c5887d 100644
---
a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/PublickeyAuthenticator.java
+++
b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/PublickeyAuthenticator.java
@@ -38,12 +38,17 @@ import org.apache.sshd.server.session.ServerSession;
public interface PublickeyAuthenticator {
/**
- * Check the validity of a public key.
+ * Checks whether the given {@link PublicKey} is allowed to be used for
authenticating user "username" in a session.
+ * <p>
+ * Note that the {@code key} may be a {@link
org.apache.sshd.common.config.keys.OpenSshCertificate}. A typical
+ * implementation for a certificate would check that the certificate's CA
key is known to be trusted as a
+ * certificate authority, and that the given user name is listed in the
certificate's principals.
+ * </p>
*
* @param username the username
* @param key the key
* @param session the server session
- * @return a boolean indicating if authentication
succeeded or not
+ * @return {@code true} if the key may be used; {@code
false} otherwise
* @throws AsyncAuthException If the authentication is performed
asynchronously
*/
boolean authenticate(String username, PublicKey key, ServerSession
session) throws AsyncAuthException;
diff --git
a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java
b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java
index bc2ed93bb..04c0e8d28 100644
---
a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java
+++
b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java
@@ -103,11 +103,6 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
throw new CertificateException("expired");
}
verifyCertificateSignature(session, cert);
- // TODO: move this into the authenticator
- Collection<String> principals = cert.getPrincipals();
- if (!GenericUtils.isEmpty(principals) &&
!principals.contains(username)) {
- throw new CertificateException("not valid for the given
username");
- }
} catch (Exception e) {
warn("doAuth({}@{}): public key certificate (id={}) is not
valid: {}", username, session, cert.getId(),
e.getMessage(), e);
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
new file mode 100644
index 000000000..549e1f6ea
--- /dev/null
+++
b/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sshd.server.auth;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyPair;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
+
+import org.apache.sshd.certificate.OpenSshCertificateBuilder;
+import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyFactory;
+import org.apache.sshd.client.future.AuthFuture;
+import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.common.SshException;
+import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
+import org.apache.sshd.common.config.keys.KeyUtils;
+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.server.SshServer;
+import org.apache.sshd.server.auth.keyboard.KeyboardInteractiveAuthenticator;
+import org.apache.sshd.server.auth.password.RejectAllPasswordAuthenticator;
+import
org.apache.sshd.server.auth.pubkey.AuthorizedKeyEntriesPublickeyAuthenticator;
+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.BeforeAll;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for user certificate authentication with checking an OpenSSH-style
authorized_keys file.
+ */
+class AuthorizedKeysCertificateTest extends BaseTestSupport {
+
+ private static SshServer sshd;
+ private static SshClient client;
+ private static int port;
+
+ @TempDir
+ private Path tmp;
+
+ private KeyPair caKey;
+ private KeyPair userKey;
+ private OpenSshCertificate userCert;
+
+ @BeforeAll
+ private static void setupClientAndServer() throws Exception {
+ sshd =
CoreTestSupportUtils.setupTestFullSupportServer(AuthorizedKeysCertificateTest.class);
+ sshd.start();
+ port = sshd.getPort();
+
+ client =
CoreTestSupportUtils.setupTestFullSupportClient(AuthorizedKeysCertificateTest.class);
+ client.start();
+ }
+
+ @AfterAll
+ static void tearDownClientAndServer() throws Exception {
+ if (sshd != null) {
+ try {
+ sshd.stop(true);
+ } finally {
+ sshd = null;
+ }
+ }
+
+ if (client != null) {
+ try {
+ client.stop();
+ } finally {
+ client = null;
+ }
+ }
+ }
+
+ 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
+ );
+ }
+
+ private void initCert(String caMarker, String... principals) throws
Exception {
+ caKey = CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM,
2048);
+ userKey =
CommonTestSupportUtils.generateKeyPair(KeyUtils.EC_ALGORITHM, 256);
+ // Generate a user certificate.
+ userCert = OpenSshCertificateBuilder.userCertificate() //
+ .serial(System.currentTimeMillis()) //
+ .publicKey(userKey.getPublic()) //
+ .id("test-cert") //
+ .validBefore(System.currentTimeMillis() +
TimeUnit.HOURS.toMillis(1)) //
+ .principals(Arrays.asList(principals)) //
+ .sign(caKey, "rsa-sha2-256");
+ Path authorizedKeys = tmp.resolve("authorized_keys");
+ List<String> lines = new ArrayList<>();
+ StringBuilder line = new StringBuilder();
+ if (!GenericUtils.isEmpty(caMarker)) {
+ line.append(caMarker).append(' ');
+ }
+ line.append(PublicKeyEntry.toString(caKey.getPublic()));
+ lines.add(line.toString());
+ lines.add(PublicKeyEntry.toString(userKey.getPublic()));
+ Files.write(authorizedKeys, lines);
+
+ sshd.setPasswordAuthenticator(RejectAllPasswordAuthenticator.INSTANCE);
+
sshd.setKeyboardInteractiveAuthenticator(KeyboardInteractiveAuthenticator.NONE);
+ sshd.setPublickeyAuthenticator(new
AuthorizedKeyEntriesPublickeyAuthenticator(authorizedKeys, null,
+ AuthorizedKeyEntry.readAuthorizedKeys(authorizedKeys),
PublicKeyEntryResolver.FAILING));
+ client.setUserAuthFactories(Collections.singletonList(new
UserAuthPublicKeyFactory()));
+ }
+
+ @ParameterizedTest(name = "test {0} CA key with {1}")
+ @MethodSource("options")
+ void testCertificate(String marker, String principals, boolean
expectSuccess) throws Exception {
+ 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)
+ .getSession()) {
+ KeyPair certKeyPair = new KeyPair(userCert, userKey.getPrivate());
+ s.addPublicKeyIdentity(certKeyPair);
+ AuthFuture auth = s.auth();
+ if (expectSuccess) {
+ auth.verify(AUTH_TIMEOUT);
+ } else {
+ assertThrows(SshException.class, () ->
auth.verify(AUTH_TIMEOUT));
+ }
+ }
+ }
+
+ private static Stream<Arguments> plainOptions() {
+ return Stream.of(Arguments.of("", true), // Not CA: success
+ Arguments.of("cert-authority", false) // CA: fail
+ );
+ }
+
+ private void initKeys(String marker) throws Exception {
+ caKey = CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM,
2048);
+ userKey =
CommonTestSupportUtils.generateKeyPair(KeyUtils.EC_ALGORITHM, 256);
+ Path authorizedKeys = tmp.resolve("authorized_keys");
+ StringBuilder line = new StringBuilder();
+ if (!GenericUtils.isEmpty(marker)) {
+ line.append(marker).append(' ');
+ }
+ line.append(PublicKeyEntry.toString(userKey.getPublic()));
+ Files.write(authorizedKeys,
Collections.singletonList(line.toString()));
+
+ sshd.setPasswordAuthenticator(RejectAllPasswordAuthenticator.INSTANCE);
+
sshd.setKeyboardInteractiveAuthenticator(KeyboardInteractiveAuthenticator.NONE);
+ sshd.setPublickeyAuthenticator(new
AuthorizedKeyEntriesPublickeyAuthenticator(authorizedKeys, null,
+ AuthorizedKeyEntry.readAuthorizedKeys(authorizedKeys),
PublicKeyEntryResolver.FAILING));
+ client.setUserAuthFactories(Collections.singletonList(new
UserAuthPublicKeyFactory()));
+ }
+
+ @ParameterizedTest(name = "test plain key with {0}")
+ @MethodSource("plainOptions")
+ void testPlainKey(String marker, boolean expectSuccess) throws Exception {
+ initKeys(marker);
+ try (ClientSession s = client.connect("user", TEST_LOCALHOST,
port).verify(CONNECT_TIMEOUT).getSession()) {
+ s.addPublicKeyIdentity(userKey);
+ AuthFuture auth = s.auth();
+ if (expectSuccess) {
+ auth.verify(AUTH_TIMEOUT);
+ } else {
+ assertThrows(SshException.class, () ->
auth.verify(AUTH_TIMEOUT));
+ }
+ }
+ }
+}
