Repository: mina-sshd Updated Branches: refs/heads/master 88c0c819d -> 951e8dd53
[SSHD-624] Client side public key authentication should validate SSH_MSG_USERAUTH_PK_OK message data Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/951e8dd5 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/951e8dd5 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/951e8dd5 Branch: refs/heads/master Commit: 951e8dd53f339380c80a3feb53a47b542ea97a26 Parents: 88c0c81 Author: Lyor Goldstein <[email protected]> Authored: Thu Jan 21 13:58:33 2016 +0200 Committer: Lyor Goldstein <[email protected]> Committed: Thu Jan 21 13:58:33 2016 +0200 ---------------------------------------------------------------------- .../client/auth/pubkey/UserAuthPublicKey.java | 39 +++++++++---- .../server/auth/pubkey/UserAuthPublicKey.java | 57 ++++++++++-------- .../sshd/common/auth/AuthenticationTest.java | 61 ++++++++++++++++++++ 3 files changed, 123 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/951e8dd5/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java index d9aff93..49a203f 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java @@ -21,6 +21,7 @@ package org.apache.sshd.client.auth.pubkey; import java.io.Closeable; import java.io.IOException; import java.security.PublicKey; +import java.security.spec.InvalidKeySpecException; import java.util.Iterator; import java.util.List; @@ -107,18 +108,34 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact @Override protected boolean processAuthDataRequest(ClientSession session, String service, Buffer buffer) throws Exception { + String name = getName(); int cmd = buffer.getUByte(); if (cmd != SshConstants.SSH_MSG_USERAUTH_PK_OK) { - throw new IllegalStateException("processAuthDataRequest(" + session + ")[" + service + "]" + throw new IllegalStateException("processAuthDataRequest(" + session + ")[" + service + "][" + name + "]" + " received unknown packet: cmd=" + SshConstants.getCommandMessageName(cmd)); } + /* + * Make sure the server echo-ed the same key we sent as + * sanctioned by RFC4252 section 7 + */ PublicKey key = current.getPublicKey(); String algo = KeyUtils.getKeyType(key); - String name = getName(); + String rspKeyType = buffer.getString(); + if (!rspKeyType.equals(algo)) { + throw new InvalidKeySpecException("processAuthDataRequest(" + session + ")[" + service + "][" + name + "]" + + " mismatched key types: expected=" + algo + ", actual=" + rspKeyType); + } + + PublicKey rspKey = buffer.getPublicKey(); + if (!KeyUtils.compareKeys(rspKey, key)) { + throw new InvalidKeySpecException("processAuthDataRequest(" + session + ")[" + service + "][" + name + "]" + + " mismatched " + algo + " keys: expected=" + KeyUtils.getFingerPrint(key) + ", actual=" + KeyUtils.getFingerPrint(rspKey)); + } + if (log.isDebugEnabled()) { - log.debug("processAuthDataRequest({})[{}] send SSH_MSG_USERAUTH_PK_OK reply {} type={} - fingerprint={}", - session, service, name, algo, KeyUtils.getFingerPrint(key)); + log.debug("processAuthDataRequest({})[{}][{}] SSH_MSG_USERAUTH_PK_OK type={}, fingerprint={}", + session, service, name, rspKeyType, KeyUtils.getFingerPrint(rspKey)); } String username = session.getUsername(); @@ -129,7 +146,12 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact buffer.putBoolean(true); buffer.putString(algo); buffer.putPublicKey(key); + appendSignature(session, service, name, username, algo, key, buffer); + session.writePacket(buffer); + return true; + } + protected void appendSignature(ClientSession session, String service, String name, String username, String algo, PublicKey key, Buffer buffer) throws Exception { byte[] id = session.getSessionId(); Buffer bs = new ByteArrayBuffer(id.length + username.length() + service.length() + name.length() + algo.length() + ByteArrayBuffer.DEFAULT_SIZE + Long.SIZE, false); @@ -145,19 +167,16 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact byte[] contents = bs.getCompactData(); byte[] sig = current.sign(contents); if (log.isTraceEnabled()) { - log.trace("processAuthDataRequest({})[{}] name={}, key type={}, fingerprint={} - verification data={}", + log.trace("appendSignature({})[{}] name={}, key type={}, fingerprint={} - verification data={}", session, service, name, algo, KeyUtils.getFingerPrint(key), BufferUtils.printHex(contents)); - log.trace("processAuthDataRequest({})[{}] name={}, key type={}, fingerprint={} - generated signature={}", + log.trace("appendSignature({})[{}] name={}, key type={}, fingerprint={} - generated signature={}", session, service, name, algo, KeyUtils.getFingerPrint(key), BufferUtils.printHex(sig)); } - bs = new ByteArrayBuffer(algo.length() + sig.length + Long.SIZE, false); + bs.clear(); bs.putString(algo); bs.putBytes(sig); buffer.putBytes(bs.array(), bs.rpos(), bs.available()); - - session.writePacket(buffer); - return true; } @Override http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/951e8dd5/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java ---------------------------------------------------------------------- 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 7540747..e00960f 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 @@ -19,6 +19,7 @@ package org.apache.sshd.server.auth.pubkey; import java.security.PublicKey; +import java.security.SignatureException; import java.util.Collection; import java.util.List; @@ -110,27 +111,33 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact log.debug("doAuth({}@{}) key type={}, fingerprint={} - authentication result: {}", username, session, alg, KeyUtils.getFingerPrint(key), authed); } + if (!authed) { return Boolean.FALSE; } if (!hasSig) { - if (log.isDebugEnabled()) { - log.debug("doAuth({}@{}) send SSH_MSG_USERAUTH_PK_OK for key type={}, fingerprint={}", - username, session, alg, KeyUtils.getFingerPrint(key)); - } - - Buffer buf = session.prepareBuffer(SshConstants.SSH_MSG_USERAUTH_PK_OK, BufferUtils.clear(buffer)); - buf.putString(alg); - buf.putRawBytes(buffer.array(), oldPos, 4 + len); - session.writePacket(buf); + sendPublicKeyResponse(session, username, alg, key, buffer.array(), oldPos, 4 + len, buffer); return null; } - // verify signature + buffer.rpos(oldPos); + buffer.wpos(oldPos + 4 + len); + if (!verifySignature(session, getService(), getName(), username, alg, key, buffer, verifier, sig)) { + throw new SignatureException("Key verification failed"); + } + + if (log.isDebugEnabled()) { + log.debug("doAuth({}@{}) key type={}, fingerprint={} - verified", + username, session, alg, KeyUtils.getFingerPrint(key)); + } + + return Boolean.TRUE; + } + + protected boolean verifySignature(ServerSession session, String service, String name, String username, + String alg, PublicKey key, Buffer buffer, Signature verifier, byte[] sig) throws Exception { byte[] id = session.getSessionId(); - String service = getService(); - String name = getName(); Buffer buf = new ByteArrayBuffer(id.length + username.length() + service.length() + name.length() + alg.length() + ByteArrayBuffer.DEFAULT_SIZE + Long.SIZE, false); buf.putBytes(id); @@ -140,27 +147,29 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact buf.putString(name); buf.putBoolean(true); buf.putString(alg); - buffer.rpos(oldPos); - buffer.wpos(oldPos + 4 + len); buf.putBuffer(buffer); if (log.isTraceEnabled()) { - log.trace("doAuth({}@{}) key type={}, fingerprint={} - verification data={}", - username, session, alg, KeyUtils.getFingerPrint(key), buf.printHex()); - log.trace("doAuth({}@{}) key type={}, fingerprint={} - expected signature={}", - username, session, alg, KeyUtils.getFingerPrint(key), BufferUtils.printHex(sig)); + log.trace("verifySignature({}@{})[{}][{}] key type={}, fingerprint={} - verification data={}", + username, session, service, name, alg, KeyUtils.getFingerPrint(key), buf.printHex()); + log.trace("verifySignature({}@{})[{}][{}] key type={}, fingerprint={} - expected signature={}", + username, session, service, name, alg, KeyUtils.getFingerPrint(key), BufferUtils.printHex(sig)); } verifier.update(buf.array(), buf.rpos(), buf.available()); - if (!verifier.verify(sig)) { - throw new Exception("Key verification failed"); - } + return verifier.verify(sig); + } + protected void sendPublicKeyResponse(ServerSession session, String username, String alg, PublicKey key, + byte[] keyBlob, int offset, int blobLen, Buffer buffer) throws Exception { if (log.isDebugEnabled()) { - log.debug("doAuth({}@{}) key type={}, fingerprint={} - verified", - username, session, alg, KeyUtils.getFingerPrint(key)); + log.debug("doAuth({}@{}) send SSH_MSG_USERAUTH_PK_OK for key type={}, fingerprint={}", + username, session, alg, KeyUtils.getFingerPrint(key)); } - return Boolean.TRUE; + Buffer buf = session.prepareBuffer(SshConstants.SSH_MSG_USERAUTH_PK_OK, BufferUtils.clear(buffer)); + buf.putString(alg); + buf.putRawBytes(keyBlob, offset, blobLen); + session.writePacket(buf); } } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/951e8dd5/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java b/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java index 41ed1b9..47a5934 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java @@ -23,6 +23,7 @@ import java.net.SocketAddress; import java.security.KeyPair; import java.security.PublicKey; import java.security.cert.X509Certificate; +import java.security.spec.InvalidKeySpecException; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -44,6 +45,8 @@ import org.apache.sshd.client.keyverifier.ServerKeyVerifier; import org.apache.sshd.client.session.ClientSession; import org.apache.sshd.common.NamedFactory; import org.apache.sshd.common.PropertyResolverUtils; +import org.apache.sshd.common.SshConstants; +import org.apache.sshd.common.SshException; import org.apache.sshd.common.config.keys.KeyUtils; import org.apache.sshd.common.io.IoSession; import org.apache.sshd.common.keyprovider.KeyPairProvider; @@ -54,6 +57,7 @@ import org.apache.sshd.common.signature.Signature; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.ValidateUtils; import org.apache.sshd.common.util.buffer.Buffer; +import org.apache.sshd.common.util.buffer.BufferUtils; import org.apache.sshd.common.util.net.SshdSocketAddress; import org.apache.sshd.server.ServerFactoryManager; import org.apache.sshd.server.SshServer; @@ -657,6 +661,63 @@ public class AuthenticationTest extends BaseTestSupport { } } + @Test // see SSHD-624 + public void testMismatchedUserAuthPkOkData() throws Exception { + final AtomicInteger challengeCounter = new AtomicInteger(0); + sshd.setUserAuthFactories(Collections.<NamedFactory<org.apache.sshd.server.auth.UserAuth>>singletonList( + new org.apache.sshd.server.auth.pubkey.UserAuthPublicKeyFactory() { + @Override + public org.apache.sshd.server.auth.pubkey.UserAuthPublicKey create() { + return new org.apache.sshd.server.auth.pubkey.UserAuthPublicKey() { + @Override + protected void sendPublicKeyResponse(ServerSession session, String username, String alg, PublicKey key, + byte[] keyBlob, int offset, int blobLen, Buffer buffer) throws Exception { + int count = challengeCounter.incrementAndGet(); + outputDebugMessage("sendPublicKeyChallenge(%s)[%s]: count=%d", session, alg, count); + if (count == 1) { + // send wrong key type + super.sendPublicKeyResponse(session, username, KeyPairProvider.SSH_DSS, key, keyBlob, offset, blobLen, buffer); + } else if (count == 2) { + // send another key + KeyPair otherPair = org.apache.sshd.util.test.Utils.generateKeyPair("RSA", 1024); + PublicKey otherKey = otherPair.getPublic(); + Buffer buf = session.prepareBuffer(SshConstants.SSH_MSG_USERAUTH_PK_OK, BufferUtils.clear(buffer)); + buf.putString(alg); + buf.putPublicKey(otherKey); + session.writePacket(buf); + } else { + super.sendPublicKeyResponse(session, username, alg, key, keyBlob, offset, blobLen, buffer); + } + } + }; + } + + })); + + try (SshClient client = setupTestClient()) { + KeyPair clientIdentity = Utils.generateKeyPair("RSA", 1024); + client.start(); + + try { + for (int index = 1; index <= 4; index++) { + try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) { + s.addPublicKeyIdentity(clientIdentity); + s.auth().verify(17L, TimeUnit.SECONDS); + assertEquals("Mismatched number of challenges", 3, challengeCounter.get()); + break; + } catch(SshException e) { // expected + outputDebugMessage("%s on retry #%d: %s", e.getClass().getSimpleName(), index, e.getMessage()); + + Throwable t = e.getCause(); + assertObjectInstanceOf("Unexpected failure cause at retry #" + index, InvalidKeySpecException.class, t); + } + } + } finally { + client.stop(); + } + } + } + @Test // see SSHD-620 public void testHostBasedAuthentication() throws Exception { final String CLIENT_USERNAME = getClass().getSimpleName();
