anna/IM-server-new
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Anticipated credentials check before dropping an already connected clientID

Browse Source
This commit is contained in:
andrea
2015-06-28 15:31:21 +02:00
parent 7dece38b57
commit 5416a87149
3 changed files with 87 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ChangeLog.txt
+1
View File
@@ -19,6 +19,7 @@ Version 0.7:
Fixed duplicated subscription with different QoS same client and topic (fix issue #49)
Fixed UNSUBSCRIBE that missed to remove the subscription from the persistent store (fix issue #50)
Fixed issue on multiple publish for overlapping subscription by the same client (fix issue #52).
Moved the authorizaion check before dropping an already connected client with same clientID (fix issue #44).
Version 0.6:
Fix issue #42, bad password file path composition
broker/src/main/java/org/eclipse/moquette/spi/impl/ProtocolProcessor.java
+38 -18
View File
@@ -164,6 +164,25 @@ class ProtocolProcessor implements EventHandler<ValueEvent> {
return;
}
//handle user authentication
if (msg.isUserFlag()) {
String pwd = null;
if (msg.isPasswordFlag()) {
pwd = msg.getPassword();
} else if (!this.allowAnonymous) {
failedCredentials(session);
return;
}
if (!m_authenticator.checkValid(msg.getUsername(), pwd)) {
failedCredentials(session);
return;
}
session.setAttribute(NettyChannel.ATTR_KEY_USERNAME, msg.getUsername());
} else if (!this.allowAnonymous) {
failedCredentials(session);
return;
}
//if an old client with the same ID already exists close its session.
if (m_clientIDs.containsKey(msg.getClientID())) {
LOG.info("Found an existing connection with same client ID <{}>, forcing to close", msg.getClientID());
@@ -202,24 +221,24 @@ class ProtocolProcessor implements EventHandler<ValueEvent> {
m_willStore.put(msg.getClientID(), will);
}
//handle user authentication
if (msg.isUserFlag()) {
String pwd = null;
if (msg.isPasswordFlag()) {
pwd = msg.getPassword();
} else if (!this.allowAnonymous) {
failedCredentials(session);
return;
}
if (!m_authenticator.checkValid(msg.getUsername(), pwd)) {
failedCredentials(session);
return;
}
session.setAttribute(NettyChannel.ATTR_KEY_USERNAME, msg.getUsername());
} else if (!this.allowAnonymous) {
failedCredentials(session);
return;
}
// //handle user authentication
// if (msg.isUserFlag()) {
// String pwd = null;
// if (msg.isPasswordFlag()) {
// pwd = msg.getPassword();
// } else if (!this.allowAnonymous) {
// failedCredentials(session);
// return;
// }
// if (!m_authenticator.checkValid(msg.getUsername(), pwd)) {
// failedCredentials(session);
// return;
// }
// session.setAttribute(NettyChannel.ATTR_KEY_USERNAME, msg.getUsername());
// } else if (!this.allowAnonymous) {
// failedCredentials(session);
// return;
// }
subscriptions.activate(msg.getClientID());
@@ -250,6 +269,7 @@ class ProtocolProcessor implements EventHandler<ValueEvent> {
ConnAckMessage okResp = new ConnAckMessage();
okResp.setReturnCode(ConnAckMessage.BAD_USERNAME_OR_PASSWORD);
session.write(okResp);
session.close(false);
}
/**
broker/src/test/java/org/eclipse/moquette/spi/impl/ProtocolProcessorTest.java
+48 -9
View File
@@ -25,7 +25,6 @@ import org.eclipse.moquette.server.netty.NettyChannel;
import org.eclipse.moquette.spi.IMatchingCondition;
import org.eclipse.moquette.spi.IMessagesStore;
import org.eclipse.moquette.spi.ISessionsStore;
import org.eclipse.moquette.spi.impl.events.LostConnectionEvent;
import org.eclipse.moquette.spi.impl.events.PublishEvent;
import org.eclipse.moquette.spi.impl.security.PermitAllAuthorizator;
import org.eclipse.moquette.spi.impl.subscriptions.Subscription;
@@ -58,8 +57,10 @@ public class ProtocolProcessorTest {
final static String TEST_USER = "fakeuser";
final static String TEST_PWD = "fakepwd";
final static String EVIL_TEST_USER = "eviluser";
final static String EVIL_TEST_PWD = "unsecret";
ServerChannel m_session;
DummyChannel m_session;
byte m_returnCode;
ConnectMessage connMsg;
ProtocolProcessor m_processor;
@@ -71,8 +72,10 @@ public class ProtocolProcessorTest {
MockAuthenticator m_mockAuthenticator;
class DummyChannel implements ServerChannel {
private boolean m_channelClosed = false;
private Map<Object, Object> m_attributes = new HashMap<Object, Object>();
private Map<Object, Object> m_attributes = new HashMap<>();
public Object getAttribute(AttributeKey<Object> key) {
return m_attributes.get(key);
@@ -86,7 +89,11 @@ public class ProtocolProcessorTest {
}
public void close(boolean immediately) {
m_channelClosed = true;
}
boolean isClosed() {
return this.m_channelClosed;
}
public void write(Object value) {
@@ -108,7 +115,7 @@ public class ProtocolProcessorTest {
class MockReceiverChannel implements ServerChannel {
// byte m_returnCode;
AbstractMessage m_receivedMessage;
private Map<Object, Object> m_attributes = new HashMap<Object, Object>();
private Map<Object, Object> m_attributes = new HashMap<>();
public Object getAttribute(AttributeKey<Object> key) {
return m_attributes.get(key);
@@ -158,7 +165,7 @@ public class ProtocolProcessorTest {
m_sessionStore = memStorage;
//m_storageService.initStore();
Map<String, String> users = new HashMap<String, String>();
Map<String, String> users = new HashMap<>();
users.put(TEST_USER, TEST_PWD);
m_mockAuthenticator = new MockAuthenticator(users);
@@ -208,7 +215,7 @@ public class ProtocolProcessorTest {
/*verify(mockedMessaging).publish(eq("topic"), eq("Topic message".getBytes()),
any(AbstractMessage.QOSType.class), anyBoolean(), eq("123"), any(IoSession.class));*/
}
@Test
public void validAuthentication() {
connMsg.setClientID("123");
@@ -290,8 +297,40 @@ public class ProtocolProcessorTest {
//Verify
assertEquals(ConnAckMessage.CONNECTION_ACCEPTED, m_returnCode);
}
@Test
public void connectWithSameClientIDBadCredentialsDoesntDropExistingClient() {
//Connect a client1
connMsg.setClientID("Client1");
connMsg.setUserFlag(true);
connMsg.setPasswordFlag(true);
connMsg.setUsername(TEST_USER);
connMsg.setPassword(TEST_PWD);
m_processor.processConnect(m_session, connMsg);
assertEquals(ConnAckMessage.CONNECTION_ACCEPTED, m_returnCode);
//create another connect same clientID but with bad credentials
ConnectMessage evilClientConnMsg = new ConnectMessage();
evilClientConnMsg.setProcotolVersion((byte) 0x03);
evilClientConnMsg.setClientID("Client1");
evilClientConnMsg.setUserFlag(true);
evilClientConnMsg.setPasswordFlag(true);
evilClientConnMsg.setUsername(EVIL_TEST_USER);
evilClientConnMsg.setPassword(EVIL_TEST_PWD);
DummyChannel evilSession = new DummyChannel();
//Exercise
m_processor.processConnect(evilSession, evilClientConnMsg);
//Verify
//the evil client gets a not auth notification
assertEquals(ConnAckMessage.BAD_USERNAME_OR_PASSWORD, m_returnCode);
//the good client remains connected
assertFalse(m_session.isClosed());
assertTrue(evilSession.isClosed());
}
@Test
public void testConnAckContainsSessionPresentFlag() throws InterruptedException {
connMsg = new ConnectMessage();