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();
+ }
+ }
}
+
}
