You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tw...@apache.org on 2025/02/27 20:55:23 UTC
(mina-sshd) 01/04: [SSHD-1161] Pubkey auth: check certificate signature in server user auth
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 0ab624e3a064904b904769b90990de6fb2f5b811
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Mon Feb 3 21:59:28 2025 +0100
[SSHD-1161] Pubkey auth: check certificate signature in server user auth
The certificate must be validated. The code did not check the
certificate signature at all.
Add the missing signature verification, and add a test.
---
.../sshd/server/auth/pubkey/UserAuthPublicKey.java | 39 ++++++--
.../common/auth/PublicKeyAuthenticationTest.java | 110 ++++++++++++++++-----
2 files changed, 112 insertions(+), 37 deletions(-)
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 8e1e57173..bc2ed93bb 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
@@ -91,6 +91,7 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact
buffer.wpos(buffer.rpos() + len);
PublicKey key = buffer.getRawPublicKey();
+ PublicKey verifyKey = key;
if (key instanceof OpenSshCertificate) {
OpenSshCertificate cert = (OpenSshCertificate) key;
@@ -101,18 +102,19 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact
if (!OpenSshCertificate.isValidNow(cert)) {
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");
}
- // TODO: cert.getCaKey() must be either in authorized_keys, marked as a CA key
- // and not revoked, or in TrustedUserCAKeys and then also match
- // AuthorizedPricipalsFile, if present.
- } catch (CertificateException e) {
+ } catch (Exception e) {
warn("doAuth({}@{}): public key certificate (id={}) is not valid: {}", username, session, cert.getId(),
e.getMessage(), e);
- throw e;
+ return Boolean.FALSE;
}
+ // Need to use the certified public key for signature verification, not the certificate itself
+ verifyKey = cert.getCertPubKey();
}
Collection<NamedFactory<Signature>> factories = ValidateUtils.checkNotNullAndNotEmpty(
@@ -124,11 +126,6 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact
log.debug("doAuth({}@{}) verify key type={}, factories={}, fingerprint={}",
username, session, alg, NamedResource.getNames(factories), KeyUtils.getFingerPrint(key));
}
- /*
- * When users employ cert authentication, need to use the public key in the cert for signing
- * and cannot use the cert itself directly for signing
- */
- PublicKey verifyKey = key instanceof OpenSshCertificate ? ((OpenSshCertificate) key).getCertPubKey() : key;
Signature verifier = ValidateUtils.checkNotNull(
NamedFactory.create(factories, alg),
"No verifier located for algorithm=%s",
@@ -149,6 +146,7 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact
boolean authed;
try {
authed = authenticator.authenticate(username, key, session);
+ // Also checks username against the certificate's principals, if key is a certificate.
} catch (Error e) {
warn("doAuth({}@{}) failed ({}) to consult delegate for {} key={}: {}",
username, session, e.getClass().getSimpleName(), alg, KeyUtils.getFingerPrint(key), e.getMessage(), e);
@@ -183,6 +181,27 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact
return Boolean.TRUE;
}
+ protected void verifyCertificateSignature(ServerSession session, OpenSshCertificate cert) throws Exception {
+ PublicKey signatureKey = cert.getCaPubKey();
+ String keyAlg = KeyUtils.getKeyType(signatureKey);
+ String keyId = cert.getId();
+
+ String sigAlg = cert.getSignatureAlgorithm();
+ if (!keyAlg.equals(KeyUtils.getCanonicalKeyType(sigAlg))) {
+ throw new CertificateException(
+ "Found invalid signature alg " + sigAlg + " for key ID=" + keyId + " using a " + keyAlg + " CA key");
+ }
+
+ Signature verif = ValidateUtils.checkNotNull(NamedFactory.create(session.getSignatureFactories(), sigAlg),
+ "No CA verifier located for algorithm=%s of key ID=%s", sigAlg, keyId);
+ verif.initVerifier(session, signatureKey);
+ verif.update(session, cert.getMessage());
+
+ if (!verif.verify(session, cert.getSignature())) {
+ throw new CertificateException("CA signature verification failed for key type=" + keyAlg + " of key ID=" + keyId);
+ }
+ }
+
protected boolean verifySignature(
ServerSession session, String username, String alg, PublicKey key, Buffer buffer, Signature verifier, byte[] sig)
throws Exception {
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java b/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
index 134ab37e6..be0359fe0 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
@@ -49,6 +49,7 @@ import org.apache.sshd.common.SshException;
import org.apache.sshd.common.config.keys.FilePasswordProvider;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.config.keys.OpenSshCertificate;
+import org.apache.sshd.common.config.keys.OpenSshCertificateImpl;
import org.apache.sshd.common.keyprovider.KeyIdentityProvider;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.session.SessionContext;
@@ -460,25 +461,23 @@ public class PublicKeyAuthenticationTest extends AuthenticationTestSupport {
}
}
- @ParameterizedTest(name = "test certificates issued using the {0} algorithm")
+ @ParameterizedTest(name = "test certificate {0} signed with {2}")
@MethodSource("certificateAlgorithms")
- void testCertificateWithDifferentAlgorithms(String keyAlgorithm, int keySize, String signatureAlgorithm) throws Exception {
+ void testCertificateWithDifferentAlgorithms(
+ String keyAlgorithm, int keySize, String sigKeyAlgorithm, int sigKeySize,
+ String signatureAlgorithm) throws Exception {
// 1. Generating a user key pair
KeyPair userkey = CommonTestSupportUtils.generateKeyPair(keyAlgorithm, keySize);
// 2. Generating CA key pair
- KeyPair caKeypair = CommonTestSupportUtils.generateKeyPair(keyAlgorithm, keySize);
+ KeyPair caKeypair = CommonTestSupportUtils.generateKeyPair(sigKeyAlgorithm, sigKeySize);
// 3. Building openSshCertificate
- OpenSshCertificate signedCert = OpenSshCertificateBuilder.userCertificate()
- .serial(System.currentTimeMillis())
- .publicKey(userkey.getPublic())
- .id("test-cert-" + keyAlgorithm)
- .validBefore(System.currentTimeMillis() + TimeUnit.HOURS.toMillis(1))
- .principals(Collections.singletonList("user01"))
- .criticalOptions(Collections.emptyList())
- .extensions(Arrays.asList(
- new OpenSshCertificate.CertificateOption("permit-X11-forwarding"),
- new OpenSshCertificate.CertificateOption("permit-agent-forwarding")))
+ OpenSshCertificate signedCert = OpenSshCertificateBuilder.userCertificate() //
+ .serial(System.currentTimeMillis()) //
+ .publicKey(userkey.getPublic()) //
+ .id("test-cert-" + keyAlgorithm) //
+ .validBefore(System.currentTimeMillis() + TimeUnit.HOURS.toMillis(1)) //
+ .principals(Collections.singletonList("user01")) //
.sign(caKeypair, signatureAlgorithm);
// 4. Configuring the ssh server
@@ -486,8 +485,7 @@ public class PublicKeyAuthenticationTest extends AuthenticationTestSupport {
sshd.setKeyboardInteractiveAuthenticator(KeyboardInteractiveAuthenticator.NONE);
CoreTestSupportUtils.setupFullSignaturesSupport(sshd);
- sshd.setUserAuthFactories(Collections.singletonList(
- new org.apache.sshd.server.auth.pubkey.UserAuthPublicKeyFactory()));
+ sshd.setUserAuthFactories(Collections.singletonList(new org.apache.sshd.server.auth.pubkey.UserAuthPublicKeyFactory()));
AtomicInteger authAttempts = new AtomicInteger(0);
sshd.setPublickeyAuthenticator((username, key, session) -> {
@@ -502,14 +500,12 @@ public class PublicKeyAuthenticationTest extends AuthenticationTestSupport {
// 5. Testing Client Authentication
try (SshClient client = setupTestClient()) {
CoreTestSupportUtils.setupFullSignaturesSupport(client);
- client.setUserAuthFactories(Collections.singletonList(
- new org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyFactory()));
+ client.setUserAuthFactories(
+ Collections.singletonList(new org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyFactory()));
client.start();
- try (ClientSession session = client.connect("user01", TEST_LOCALHOST, port)
- .verify(CONNECT_TIMEOUT)
- .getSession()) {
+ try (ClientSession session = client.connect("user01", TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT).getSession()) {
KeyPair certKeyPair = new KeyPair(signedCert, userkey.getPrivate());
session.addPublicKeyIdentity(certKeyPair);
@@ -524,12 +520,72 @@ public class PublicKeyAuthenticationTest extends AuthenticationTestSupport {
}
private static Stream<Arguments> certificateAlgorithms() {
- return Stream.of(
- // key size, signature algorithm, algorithm name
- Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-512"),
- Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-256"),
- Arguments.of(KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256"),
- Arguments.of(KeyUtils.EC_ALGORITHM, 384, "ecdsa-sha2-nistp384"),
- Arguments.of(KeyUtils.EC_ALGORITHM, 521, "ecdsa-sha2-nistp521"));
+ return Stream.of( //
+ Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-512"),
+ Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-256"),
+ Arguments.of(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256"),
+ Arguments.of(KeyUtils.EC_ALGORITHM, 384, KeyUtils.EC_ALGORITHM, 384, "ecdsa-sha2-nistp384"),
+ Arguments.of(KeyUtils.EC_ALGORITHM, 521, KeyUtils.EC_ALGORITHM, 521, "ecdsa-sha2-nistp521"),
+ Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, KeyUtils.EC_ALGORITHM, 384, "ecdsa-sha2-nistp384"),
+ Arguments.of(KeyUtils.EC_ALGORITHM, 384, KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-512"));
+ }
+
+ @Test
+ void testCertificateWithBrokenSignature() throws Exception {
+ KeyPair userkey = CommonTestSupportUtils.generateKeyPair(KeyUtils.EC_ALGORITHM, 256);
+ KeyPair caKeypair = CommonTestSupportUtils.generateKeyPair(KeyUtils.EC_ALGORITHM, 256);
+
+ OpenSshCertificate signedCert = OpenSshCertificateBuilder.userCertificate() //
+ .serial(System.currentTimeMillis()) //
+ .publicKey(userkey.getPublic()) //
+ .id("test-cert-ecdsa-sha2-nistp256") //
+ .validBefore(System.currentTimeMillis() + TimeUnit.HOURS.toMillis(1)) //
+ .principals(Collections.singletonList("user01")) //
+ .sign(caKeypair, "ecdsa-sha2-nistp256");
+
+ // Break the certificate
+ assertTrue(signedCert instanceof OpenSshCertificateImpl);
+ OpenSshCertificateImpl certImpl = (OpenSshCertificateImpl) signedCert;
+ byte[] certSig = certImpl.getSignature();
+ certSig[certSig.length - 1] ^= 0xAA;
+ certImpl.setSignature(certSig);
+
+ // Configure the ssh server
+ sshd.setPasswordAuthenticator(RejectAllPasswordAuthenticator.INSTANCE);
+ sshd.setKeyboardInteractiveAuthenticator(KeyboardInteractiveAuthenticator.NONE);
+ CoreTestSupportUtils.setupFullSignaturesSupport(sshd);
+
+ sshd.setUserAuthFactories(Collections.singletonList(new org.apache.sshd.server.auth.pubkey.UserAuthPublicKeyFactory()));
+
+ AtomicInteger authAttempts = new AtomicInteger(0);
+ sshd.setPublickeyAuthenticator((username, key, session) -> {
+ authAttempts.incrementAndGet();
+ if (key instanceof OpenSshCertificate) {
+ OpenSshCertificate cert = (OpenSshCertificate) key;
+ return KeyUtils.compareKeys(cert.getCaPubKey(), caKeypair.getPublic());
+ }
+ return false;
+ });
+
+ // Client authentication should fail
+ try (SshClient client = setupTestClient()) {
+ CoreTestSupportUtils.setupFullSignaturesSupport(client);
+ client.setUserAuthFactories(
+ Collections.singletonList(new org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyFactory()));
+
+ client.start();
+
+ try (ClientSession session = client.connect("user01", TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT).getSession()) {
+
+ KeyPair certKeyPair = new KeyPair(signedCert, userkey.getPrivate());
+ session.addPublicKeyIdentity(certKeyPair);
+
+ AuthFuture auth = session.auth();
+ assertThrows(SshException.class, () -> auth.verify(AUTH_TIMEOUT));
+ } finally {
+ client.stop();
+ }
+ }
}
+
}