You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ta...@apache.org on 2018/01/16 16:11:43 UTC
qpid-jms git commit: QPIDJMS-355 Use the new SaslListener events to
do authentication
Repository: qpid-jms
Updated Branches:
refs/heads/master ec506c6c4 -> a297163ea
QPIDJMS-355 Use the new SaslListener events to do authentication
Leverage the SaslListener to reduce checking of SASL state on each
read from the Transport layer.
Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/a297163e
Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/a297163e
Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/a297163e
Branch: refs/heads/master
Commit: a297163ea23462af134ab8dd9637d142f487df8a
Parents: ec506c6
Author: Timothy Bish <ta...@gmail.com>
Authored: Wed Jan 10 13:08:42 2018 -0500
Committer: Timothy Bish <ta...@gmail.com>
Committed: Tue Jan 16 10:57:17 2018 -0500
----------------------------------------------------------------------
.../qpid/jms/provider/amqp/AmqpProvider.java | 99 ++++++++++++--------
.../provider/amqp/AmqpSaslAuthenticator.java | 63 +++++--------
.../amqp/AmqpSaslAuthenticatorTest.java | 38 ++++----
3 files changed, 106 insertions(+), 94 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/a297163e/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpProvider.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpProvider.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpProvider.java
index b68e161..3432bd2 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpProvider.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpProvider.java
@@ -76,6 +76,7 @@ import org.apache.qpid.proton.engine.EndpointState;
import org.apache.qpid.proton.engine.Event;
import org.apache.qpid.proton.engine.Event.Type;
import org.apache.qpid.proton.engine.Sasl;
+import org.apache.qpid.proton.engine.SaslListener;
import org.apache.qpid.proton.engine.impl.CollectorImpl;
import org.apache.qpid.proton.engine.impl.ProtocolTracer;
import org.apache.qpid.proton.engine.impl.TransportImpl;
@@ -99,7 +100,7 @@ import io.netty.util.ReferenceCountUtil;
* All work within this Provider is serialized to a single Thread. Any asynchronous exceptions
* will be dispatched from that Thread and all in-bound requests are handled there as well.
*/
-public class AmqpProvider implements Provider, TransportListener , AmqpResourceParent {
+public class AmqpProvider implements Provider, TransportListener , AmqpResourceParent, SaslListener {
private static final Logger LOG = LoggerFactory.getLogger(AmqpProvider.class);
@@ -205,8 +206,9 @@ public class AmqpProvider implements Provider, TransportListener , AmqpResourceP
}
sasl.setRemoteHostname(hostname);
+ sasl.setListener(AmqpProvider.this);
- authenticator = new AmqpSaslAuthenticator(sasl, (remoteMechanisms) -> findSaslMechanism(remoteMechanisms));
+ authenticator = new AmqpSaslAuthenticator((remoteMechanisms) -> findSaslMechanism(remoteMechanisms));
pumpToProtonTransport();
} else {
@@ -866,6 +868,62 @@ public class AmqpProvider implements Provider, TransportListener , AmqpResourceP
}
}
+ @Override
+ public void onSaslMechanisms(Sasl sasl, org.apache.qpid.proton.engine.Transport transport) {
+ authenticator.handleSaslMechanisms(sasl, transport);
+ checkSaslAuthenticationState();
+ }
+
+ @Override
+ public void onSaslChallenge(Sasl sasl, org.apache.qpid.proton.engine.Transport transport) {
+ authenticator.handleSaslChallenge(sasl, transport);
+ checkSaslAuthenticationState();
+ }
+
+ @Override
+ public void onSaslOutcome(Sasl sasl, org.apache.qpid.proton.engine.Transport transport) {
+ authenticator.handleSaslOutcome(sasl, transport);
+ checkSaslAuthenticationState();
+ }
+
+ @Override
+ public void onSaslInit(Sasl sasl, org.apache.qpid.proton.engine.Transport transport) {
+ // Server only event
+ }
+
+ @Override
+ public void onSaslResponse(Sasl sasl, org.apache.qpid.proton.engine.Transport transport) {
+ // Server only event
+ }
+
+ private void checkSaslAuthenticationState() {
+ try {
+ if (authenticator.isComplete()) {
+ if (!authenticator.wasSuccessful()) {
+ // Close the transport to avoid emitting any additional frames if the
+ // authentication process was unsuccessful, then signal the completion
+ // to avoid any race with the caller triggering any other traffic.
+ // Don't release the authenticator as we need it on close to know what
+ // the state of authentication was.
+ org.apache.qpid.proton.engine.Transport t = protonConnection.getTransport();
+ t.close_head();
+ connectionRequest.onFailure(authenticator.getFailureCause());
+ } else {
+ // Signal completion and release the authenticator we won't use it again.
+ connectionRequest.onSuccess();
+ authenticator = null;
+ }
+ }
+ } catch (Throwable ex) {
+ try {
+ org.apache.qpid.proton.engine.Transport t = protonConnection.getTransport();
+ t.close_head();
+ } finally {
+ fireProviderException(ex);
+ }
+ }
+ }
+
private void processUpdates() {
try {
Event protonEvent = null;
@@ -936,9 +994,6 @@ public class AmqpProvider implements Provider, TransportListener , AmqpResourceP
protonCollector.pop();
}
-
- // We have to do this to pump SASL bytes in as SASL is not event driven yet.
- processSaslAuthentication();
} catch (Throwable t) {
try {
LOG.warn("Caught problem during update processing: {}", t.getMessage(), t);
@@ -948,40 +1003,6 @@ public class AmqpProvider implements Provider, TransportListener , AmqpResourceP
}
}
- private void processSaslAuthentication() {
- if (authenticator == null) {
- return;
- }
-
- try {
- authenticator.tryAuthenticate();
-
- if (authenticator.isComplete()) {
- if (!authenticator.wasSuccessful()) {
- // Close the transport to avoid emitting any additional frames if the
- // authentication process was unsuccessful, then signal the completion
- // to avoid any race with the caller triggering any other traffic.
- // Don't release the authenticator as we need it on close to know what
- // the state of authentication was.
- org.apache.qpid.proton.engine.Transport t = protonConnection.getTransport();
- t.close_head();
- connectionRequest.onFailure(authenticator.getFailureCause());
- } else {
- // Signal completion and release the authenticator we won't use it again.
- connectionRequest.onSuccess();
- authenticator = null;
- }
- }
- } catch (Throwable ex) {
- try {
- org.apache.qpid.proton.engine.Transport t = protonConnection.getTransport();
- t.close_head();
- } finally {
- fireProviderException(ex);
- }
- }
- }
-
protected boolean pumpToProtonTransport() {
return pumpToProtonTransport(NOOP_REQUEST);
}
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/a297163e/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java
index d3bd50c..25dc617 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java
@@ -23,13 +23,13 @@ import javax.jms.JMSSecurityRuntimeException;
import org.apache.qpid.jms.sasl.Mechanism;
import org.apache.qpid.proton.engine.Sasl;
+import org.apache.qpid.proton.engine.Transport;
/**
* Manage the SASL authentication process
*/
public class AmqpSaslAuthenticator {
- private final Sasl sasl;
private final Function<String[], Mechanism> mechanismFinder;
private Mechanism mechanism;
@@ -39,45 +39,13 @@ public class AmqpSaslAuthenticator {
/**
* Create the authenticator and initialize it.
*
- * @param sasl
- * The Proton SASL entry point this class will use to manage the authentication.
* @param mechanismFinder
* An object that is used to locate the most correct SASL Mechanism to perform the authentication.
*/
- public AmqpSaslAuthenticator(Sasl sasl, Function<String[], Mechanism> mechanismFinder) {
- this.sasl = sasl;
+ public AmqpSaslAuthenticator(Function<String[], Mechanism> mechanismFinder) {
this.mechanismFinder = mechanismFinder;
}
- /**
- * Process the SASL authentication cycle until such time as an outcome is determine. This
- * method must be called by the managing entity until the return value is true indicating a
- * successful authentication or a JMSSecurityException is thrown indicating that the
- * handshake failed.
- */
- public void tryAuthenticate() {
- try {
- switch (sasl.getState()) {
- case PN_SASL_IDLE:
- handleSaslInit();
- break;
- case PN_SASL_STEP:
- handleSaslStep();
- break;
- case PN_SASL_FAIL:
- handleSaslFail();
- break;
- case PN_SASL_PASS:
- handleSaslCompletion();
- break;
- default:
- break;
- }
- } catch (Throwable error) {
- recordFailure(error.getMessage(), error);
- }
- }
-
public boolean isComplete() {
return complete;
}
@@ -94,7 +62,9 @@ public class AmqpSaslAuthenticator {
}
}
- private void handleSaslInit() {
+ //----- SaslListener implementation --------------------------------------//
+
+ public void handleSaslMechanisms(Sasl sasl, Transport transport) {
try {
String[] remoteMechanisms = sasl.getRemoteMechanisms();
if (remoteMechanisms != null && remoteMechanisms.length != 0) {
@@ -116,7 +86,7 @@ public class AmqpSaslAuthenticator {
}
}
- private void handleSaslStep() {
+ public void handleSaslChallenge(Sasl sasl, Transport transport) {
try {
if (sasl.pending() != 0) {
byte[] challenge = new byte[sasl.pending()];
@@ -131,6 +101,25 @@ public class AmqpSaslAuthenticator {
}
}
+ public void handleSaslOutcome(Sasl sasl, Transport transport) {
+ try {
+ switch (sasl.getState()) {
+ case PN_SASL_FAIL:
+ handleSaslFail();
+ break;
+ case PN_SASL_PASS:
+ handleSaslCompletion(sasl);
+ break;
+ default:
+ break;
+ }
+ } catch (Throwable error) {
+ recordFailure(error.getMessage(), error);
+ }
+ }
+
+ //----- Internal support methods -----------------------------------------//
+
private void handleSaslFail() {
if (mechanism != null) {
recordFailure("Client failed to authenticate using SASL: " + mechanism.getName(), null);
@@ -139,7 +128,7 @@ public class AmqpSaslAuthenticator {
}
}
- private void handleSaslCompletion() {
+ private void handleSaslCompletion(Sasl sasl) {
try {
if (sasl.pending() != 0) {
byte[] additionalData = new byte[sasl.pending()];
http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/a297163e/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticatorTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticatorTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticatorTest.java
index 4f4038d..b492a59 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticatorTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticatorTest.java
@@ -39,6 +39,7 @@ import javax.security.sasl.SaslException;
import org.apache.qpid.jms.sasl.Mechanism;
import org.apache.qpid.proton.engine.Sasl;
import org.apache.qpid.proton.engine.Sasl.SaslState;
+import org.apache.qpid.proton.engine.Transport;
import org.junit.Before;
import org.junit.Test;
import org.mockito.stubbing.Answer;
@@ -52,6 +53,7 @@ public class AmqpSaslAuthenticatorTest {
private static final byte[] EMPTY_BYTES = {};
private final Sasl sasl = mock(Sasl.class);
+ private final Transport transport = mock(Transport.class);
@Before
public void setUp() {
@@ -63,8 +65,8 @@ public class AmqpSaslAuthenticatorTest {
public void testNullSaslMechanismReturnedErroneously() throws Exception {
Function<String[], Mechanism> mechanismFunction = mechanismName -> null;
- AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(sasl, mechanismFunction);
- authenticator.tryAuthenticate();
+ AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(mechanismFunction);
+ authenticator.handleSaslMechanisms(sasl, transport);
assertTrue(authenticator.isComplete());
assertFalse(authenticator.wasSuccessful());
@@ -78,8 +80,8 @@ public class AmqpSaslAuthenticatorTest {
throw new JMSSecurityRuntimeException("reasons");
};
- AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(sasl, mechanismFunction);
- authenticator.tryAuthenticate();
+ AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(mechanismFunction);
+ authenticator.handleSaslMechanisms(sasl, transport);
assertTrue(authenticator.isComplete());
assertFalse(authenticator.wasSuccessful());
@@ -90,20 +92,20 @@ public class AmqpSaslAuthenticatorTest {
@Test
public void testAuthenticationSuccess() throws Exception {
Mechanism mechanism = new TestSaslMechanism(INITIAL_RESPONSE, EXPECTED_CHALLENGE, RESPONSE);
- AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(sasl, mechanismName -> mechanism);
+ AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(mechanismName -> mechanism);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslMechanisms(sasl, transport);
verify(sasl).setMechanisms(mechanism.getName());
verifySaslMockReceived(sasl, INITIAL_RESPONSE);
when(sasl.getState()).thenReturn(SaslState.PN_SASL_STEP);
configureSaslMockToProduce(sasl, EXPECTED_CHALLENGE);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslChallenge(sasl, transport);
verifySaslMockReceived(sasl, RESPONSE);
when(sasl.getState()).thenReturn(SaslState.PN_SASL_PASS);
configureSaslMockToProduce(sasl, EMPTY_BYTES);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslOutcome(sasl, transport);
assertTrue(authenticator.isComplete());
assertTrue(authenticator.wasSuccessful());
@@ -113,14 +115,14 @@ public class AmqpSaslAuthenticatorTest {
@Test
public void testAuthenticationSuccessWithoutChallengeStep() throws Exception {
Mechanism mechanism = new TestSaslMechanism(INITIAL_RESPONSE);
- AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(sasl, mechanismName -> mechanism);
+ AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(mechanismName -> mechanism);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslMechanisms(sasl, transport);
verifySaslMockReceived(sasl, INITIAL_RESPONSE);
when(sasl.getState()).thenReturn(SaslState.PN_SASL_PASS);
configureSaslMockToProduce(sasl, EMPTY_BYTES);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslOutcome(sasl, transport);
assertTrue(authenticator.isComplete());
assertTrue(authenticator.wasSuccessful());
@@ -129,13 +131,13 @@ public class AmqpSaslAuthenticatorTest {
@Test
public void testPeerSignalsAuthenticationFail() throws Exception {
Mechanism mechanism = new TestSaslMechanism(INITIAL_RESPONSE);
- AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(sasl, mechanismName -> mechanism);
+ AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(mechanismName -> mechanism);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslMechanisms(sasl, transport);
verifySaslMockReceived(sasl, INITIAL_RESPONSE);
when(sasl.getState()).thenReturn(SaslState.PN_SASL_FAIL);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslOutcome(sasl, transport);
assertTrue(authenticator.isComplete());
assertFalse(authenticator.wasSuccessful());
@@ -148,20 +150,20 @@ public class AmqpSaslAuthenticatorTest {
Mechanism mechanism = new TestSaslMechanism(INITIAL_RESPONSE,
EXPECTED_CHALLENGE, RESPONSE,
EMPTY_BYTES, EMPTY_BYTES);
- AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(sasl, mechanismName -> mechanism);
+ AmqpSaslAuthenticator authenticator = new AmqpSaslAuthenticator(mechanismName -> mechanism);
when(sasl.getState()).thenReturn(SaslState.PN_SASL_IDLE);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslMechanisms(sasl, transport);
verifySaslMockReceived(sasl, INITIAL_RESPONSE);
when(sasl.getState()).thenReturn(SaslState.PN_SASL_STEP);
configureSaslMockToProduce(sasl, EXPECTED_CHALLENGE);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslChallenge(sasl, transport);
verifySaslMockReceived(sasl, RESPONSE);
when(sasl.getState()).thenReturn(SaslState.PN_SASL_PASS);
configureSaslMockToProduce(sasl, EMPTY_BYTES);
- authenticator.tryAuthenticate();
+ authenticator.handleSaslOutcome(sasl, transport);
assertTrue(authenticator.isComplete());
assertFalse(authenticator.wasSuccessful());
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org