You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by as...@apache.org on 2016/04/06 16:43:31 UTC
qpid-proton git commit: PROTON-1135: Do not pipeline SASL frames into
AMQP frames from client side - Test that we will accept pipelined frames
though.
Repository: qpid-proton
Updated Branches:
refs/heads/master c7dff22d4 -> 6738c16ff
PROTON-1135: Do not pipeline SASL frames into AMQP frames from client side
- Test that we will accept pipelined frames though.
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/6738c16f
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/6738c16f
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/6738c16f
Branch: refs/heads/master
Commit: 6738c16ffcb93981f1bb936b21e86515b1787f54
Parents: c7dff22
Author: Andrew Stitcher <as...@apache.org>
Authored: Thu Mar 31 02:05:17 2016 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Wed Apr 6 00:45:07 2016 -0400
----------------------------------------------------------------------
proton-c/src/sasl/sasl-internal.h | 1 -
proton-c/src/sasl/sasl.c | 70 ++++-------------
tests/python/proton_tests/engine.py | 32 +++++---
tests/python/proton_tests/sasl.py | 128 ++++++++++++-------------------
4 files changed, 89 insertions(+), 142 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6738c16f/proton-c/src/sasl/sasl-internal.h
----------------------------------------------------------------------
diff --git a/proton-c/src/sasl/sasl-internal.h b/proton-c/src/sasl/sasl-internal.h
index b3f4c7f..063cd0d 100644
--- a/proton-c/src/sasl/sasl-internal.h
+++ b/proton-c/src/sasl/sasl-internal.h
@@ -59,7 +59,6 @@ enum pni_sasl_state {
SASL_POSTED_MECHANISMS,
SASL_POSTED_RESPONSE,
SASL_POSTED_CHALLENGE,
- SASL_PRETEND_OUTCOME,
SASL_RECVED_OUTCOME_SUCCEED,
SASL_RECVED_OUTCOME_FAIL,
SASL_POSTED_OUTCOME,
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6738c16f/proton-c/src/sasl/sasl.c
----------------------------------------------------------------------
diff --git a/proton-c/src/sasl/sasl.c b/proton-c/src/sasl/sasl.c
index 29d377e..47dc76c 100644
--- a/proton-c/src/sasl/sasl.c
+++ b/proton-c/src/sasl/sasl.c
@@ -108,7 +108,6 @@ static bool pni_sasl_is_client_state(enum pni_sasl_state state)
return state==SASL_NONE
|| state==SASL_POSTED_INIT
|| state==SASL_POSTED_RESPONSE
- || state==SASL_PRETEND_OUTCOME
|| state==SASL_RECVED_OUTCOME_SUCCEED
|| state==SASL_RECVED_OUTCOME_FAIL
|| state==SASL_ERROR;
@@ -116,20 +115,17 @@ static bool pni_sasl_is_client_state(enum pni_sasl_state state)
static bool pni_sasl_is_final_input_state(pni_sasl_t *sasl)
{
- enum pni_sasl_state last_state = sasl->last_state;
enum pni_sasl_state desired_state = sasl->desired_state;
- return last_state==SASL_RECVED_OUTCOME_SUCCEED
- || last_state==SASL_RECVED_OUTCOME_FAIL
- || last_state==SASL_ERROR
- || desired_state==SASL_POSTED_OUTCOME
- || ( desired_state==SASL_RECVED_OUTCOME_SUCCEED && last_state>=SASL_POSTED_INIT);
+ return desired_state==SASL_RECVED_OUTCOME_SUCCEED
+ || desired_state==SASL_RECVED_OUTCOME_FAIL
+ || desired_state==SASL_ERROR
+ || desired_state==SASL_POSTED_OUTCOME;
}
static bool pni_sasl_is_final_output_state(pni_sasl_t *sasl)
{
enum pni_sasl_state last_state = sasl->last_state;
- return last_state==SASL_PRETEND_OUTCOME
- || last_state==SASL_RECVED_OUTCOME_SUCCEED
+ return last_state==SASL_RECVED_OUTCOME_SUCCEED
|| last_state==SASL_RECVED_OUTCOME_FAIL
|| last_state==SASL_ERROR
|| last_state==SASL_POSTED_OUTCOME;
@@ -164,11 +160,6 @@ void pni_sasl_set_desired_state(pn_transport_t *transport, enum pni_sasl_state d
if (sasl->last_state==desired_state && desired_state==SASL_POSTED_CHALLENGE) {
sasl->last_state = SASL_POSTED_MECHANISMS;
}
- // If we already pretended to receive outcome and we actually received outcome
- // we must set last_state here as we've already stoped outputting from this layer
- if (sasl->last_state==SASL_PRETEND_OUTCOME && (desired_state==SASL_RECVED_OUTCOME_SUCCEED || desired_state==SASL_RECVED_OUTCOME_FAIL) ) {
- sasl->last_state = desired_state;
- }
sasl->desired_state = desired_state;
// Don't emit transport event on error as there will be a TRANSPORT_ERROR event
if (desired_state != SASL_ERROR) pni_emit(transport);
@@ -188,12 +179,6 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
out.size, out.start);
pni_emit(transport);
break;
- case SASL_PRETEND_OUTCOME:
- if (sasl->last_state < SASL_POSTED_INIT) {
- desired_state = SASL_POSTED_INIT;
- continue;
- }
- break;
case SASL_POSTED_MECHANISMS: {
// TODO: Hardcoded limit of 16 mechanisms
char *mechs[16];
@@ -317,15 +302,17 @@ static ssize_t pn_input_read_sasl(pn_transport_t* transport, unsigned int layer,
return pn_dispatcher_input(transport, bytes, available, false, &transport->halt);
}
+ if (!pni_sasl_is_final_output_state(sasl)) {
+ return pni_passthru_layer.process_input(transport, layer, bytes, available);
+ }
+
if (pni_sasl_impl_can_encrypt(transport)) {
sasl->max_encrypt_size = pni_sasl_impl_max_encrypt_size(transport);
if (transport->trace & PN_TRACE_DRV)
pn_transport_logf(transport, "SASL Encryption enabled: buffer=%d", sasl->max_encrypt_size);
transport->io_layers[layer] = &sasl_encrypt_layer;
- } else if (sasl->client) {
- transport->io_layers[layer] = &pni_passthru_layer;
} else {
- return pni_passthru_layer.process_input(transport, layer, bytes, available);
+ transport->io_layers[layer] = &pni_passthru_layer;
}
return transport->io_layers[layer]->process_input(transport, layer, bytes, available);
}
@@ -386,8 +373,12 @@ static ssize_t pn_output_write_sasl(pn_transport_t* transport, unsigned int laye
return pn_dispatcher_output(transport, bytes, available);
}
- // We only get here if there is nothing to output and we're a final output state
- if (sasl->outcome != PN_SASL_OK && pni_sasl_is_final_input_state(sasl)) {
+ if (!pni_sasl_is_final_input_state(sasl)) {
+ return pni_passthru_layer.process_output(transport, layer, bytes, available );
+ }
+
+ // We only get here if there is nothing to output and we're in a final state
+ if (sasl->outcome != PN_SASL_OK) {
return PN_EOS;
}
@@ -397,8 +388,6 @@ static ssize_t pn_output_write_sasl(pn_transport_t* transport, unsigned int laye
if (transport->trace & PN_TRACE_DRV)
pn_transport_logf(transport, "SASL Encryption enabled: buffer=%d", sasl->max_encrypt_size);
transport->io_layers[layer] = &sasl_encrypt_layer;
- } else if (sasl->client) {
- return pni_passthru_layer.process_output(transport, layer, bytes, available );
} else {
transport->io_layers[layer] = &pni_passthru_layer;
}
@@ -541,26 +530,6 @@ void pn_sasl_free(pn_transport_t *transport)
}
}
-// This is a hack to tell us that
-// no actual negotiation is going to happen and we can go
-// straight to the AMQP layer; it can only work on the client side
-// As the server doesn't know if SASL is even active until it sees
-// the SASL header from the client first.
-static void pni_sasl_force_anonymous(pn_transport_t *transport)
-{
- pni_sasl_t *sasl = transport->sasl;
- if (sasl->client) {
- // Pretend we got sasl mechanisms frame with just ANONYMOUS
- if (pni_init_client(transport) &&
- pni_process_mechanisms(transport, "ANONYMOUS")) {
- pni_sasl_set_desired_state(transport, SASL_PRETEND_OUTCOME);
- } else {
- sasl->outcome = PN_SASL_PERM;
- pni_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME_FAIL);
- }
- }
-}
-
void pni_sasl_set_remote_hostname(pn_transport_t * transport, const char * fqdn)
{
pni_sasl_t *sasl = transport->sasl;
@@ -600,10 +569,6 @@ void pn_sasl_allowed_mechs(pn_sasl_t *sasl0, const char *mechs)
pni_sasl_t *sasl = get_sasl_internal(sasl0);
free(sasl->included_mechanisms);
sasl->included_mechanisms = mechs ? pn_strdup(mechs) : NULL;
- if (mechs && strcmp(mechs, "ANONYMOUS")==0 ) {
- pn_transport_t *transport = get_transport_internal(sasl0);
- pni_sasl_force_anonymous(transport);
- }
}
void pn_sasl_set_allow_insecure_mechs(pn_sasl_t *sasl0, bool insecure)
@@ -666,9 +631,6 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t cha
{
pni_sasl_t *sasl = transport->sasl;
- // If we already pretended we got the ANONYMOUS mech then ignore
- if (sasl->last_state==SASL_PRETEND_OUTCOME) return 0;
-
// This scanning relies on pn_data_scan leaving the pn_data_t cursors
// where they are after finishing the scan
pn_string_t *mechs = pn_string("");
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6738c16f/tests/python/proton_tests/engine.py
----------------------------------------------------------------------
diff --git a/tests/python/proton_tests/engine.py b/tests/python/proton_tests/engine.py
index 0a6eb8d..e7708da 100644
--- a/tests/python/proton_tests/engine.py
+++ b/tests/python/proton_tests/engine.py
@@ -2665,10 +2665,16 @@ class SaslEventTest(CollectorTest):
s.allowed_mechs("ANONYMOUS PLAIN")
transport.bind(conn)
self.expect(Event.CONNECTION_INIT, Event.CONNECTION_BOUND)
- transport.push(str2bin('AMQP\x03\x01\x00\x00\x00\x00\x00\x1c\x02\x01\x00\x00\x00S@'
- '\xc0\x0f\x01\xe0\x0c\x01\xa3\tANONYMOUS\x00\x00\x00\x10'
- '\x02\x01\x00\x00\x00SD\xc0\x03\x01P\x00AMQP\x00\x01\x00'
- '\x00'))
+ transport.push(str2bin(
+ # SASL
+ 'AMQP\x03\x01\x00\x00'
+ # @sasl-mechanisms(64) [sasl-server-mechanisms=@PN_SYMBOL[:ANONYMOUS]]
+ '\x00\x00\x00\x1c\x02\x01\x00\x00\x00S@\xc0\x0f\x01\xe0\x0c\x01\xa3\tANONYMOUS'
+ # @sasl-outcome(68) [code=0]
+ '\x00\x00\x00\x10\x02\x01\x00\x00\x00SD\xc0\x03\x01P\x00'
+ # AMQP
+ 'AMQP\x00\x01\x00\x00'
+ ))
self.expect(Event.TRANSPORT)
p = transport.pending()
bytes = transport.peek(p)
@@ -2676,6 +2682,7 @@ class SaslEventTest(CollectorTest):
server = Transport(Transport.SERVER)
server.push(bytes)
+ assert s.outcome == SASL.OK
assert server.sasl().outcome == SASL.OK
def testPipelinedServerWriteFirst(self):
@@ -2690,14 +2697,21 @@ class SaslEventTest(CollectorTest):
p = transport.pending()
bytes = transport.peek(p)
transport.pop(p)
- self.expect(Event.CONNECTION_INIT, Event.CONNECTION_BOUND, Event.TRANSPORT)
- transport.push(str2bin('AMQP\x03\x01\x00\x00\x00\x00\x00\x1c\x02\x01\x00\x00\x00S@'
- '\xc0\x0f\x01\xe0\x0c\x01\xa3\tANONYMOUS\x00\x00\x00\x10'
- '\x02\x01\x00\x00\x00SD\xc0\x03\x01P\x00AMQP\x00\x01\x00'
- '\x00'))
+ self.expect(Event.CONNECTION_INIT, Event.CONNECTION_BOUND)
+ transport.push(str2bin(
+ # SASL
+ 'AMQP\x03\x01\x00\x00'
+ # @sasl-mechanisms(64) [sasl-server-mechanisms=@PN_SYMBOL[:ANONYMOUS]]
+ '\x00\x00\x00\x1c\x02\x01\x00\x00\x00S@\xc0\x0f\x01\xe0\x0c\x01\xa3\tANONYMOUS'
+ # @sasl-outcome(68) [code=0]
+ '\x00\x00\x00\x10\x02\x01\x00\x00\x00SD\xc0\x03\x01P\x00'
+ # AMQP
+ 'AMQP\x00\x01\x00\x00'
+ ))
self.expect(Event.TRANSPORT)
p = transport.pending()
bytes = transport.peek(p)
transport.pop(p)
+ assert s.outcome == SASL.OK
# XXX: the bytes above appear to be correct, but we don't get any
# sort of event indicating that the transport is authenticated
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6738c16f/tests/python/proton_tests/sasl.py
----------------------------------------------------------------------
diff --git a/tests/python/proton_tests/sasl.py b/tests/python/proton_tests/sasl.py
index 6adb77d..29b578d 100644
--- a/tests/python/proton_tests/sasl.py
+++ b/tests/python/proton_tests/sasl.py
@@ -80,6 +80,15 @@ def _testSaslMech(self, mech, clientUser='user@proton', authUser='user@proton',
class Test(common.Test):
pass
+def consumeAllOuput(t):
+ stops = 0
+ while stops<1:
+ out = t.peek(1024)
+ l = len(out)
+ t.pop(l)
+ if l <= 0:
+ stops += 1
+
class SaslTest(Test):
def setUp(self):
@@ -92,104 +101,67 @@ class SaslTest(Test):
def pump(self):
pump(self.t1, self.t2, 1024)
- # Note that due to server protocol autodetect, there can be no "pipelining"
- # of protocol frames from the server end only from the client end.
- #
- # This is because the server cannot know which protocol layers are active
- # and therefore which headers need to be sent,
- # until it sees the respective protocol headers from the client.
+ # We have to generate the client frames manually because proton does not
+ # generate pipelined SASL and AMQP frames together
def testPipelinedClient(self):
+ # TODO: When PROTON-1136 is fixed then remove this test
if "java" in sys.platform:
- raise Skipped("Proton-J does not support client pipelining")
+ raise Skipped("Proton-J does not support pipelined client input")
- # Client
- self.s1.allowed_mechs('ANONYMOUS')
# Server
self.s2.allowed_mechs('ANONYMOUS')
- assert self.s1.outcome is None
+ c2 = Connection()
+ self.t2.bind(c2)
+
assert self.s2.outcome is None
# Push client bytes into server
- out1 = self.t1.peek(1024)
- self.t1.pop(len(out1))
- self.t2.push(out1)
-
- out2 = self.t2.peek(1024)
- self.t2.pop(len(out2))
+ self.t2.push(str2bin(
+ # SASL
+ 'AMQP\x03\x01\x00\x00'
+ # @sasl-init(65) [mechanism=:ANONYMOUS, initial-response=b"anonymous@fuschia"]
+ '\x00\x00\x002\x02\x01\x00\x00\x00SA\xd0\x00\x00\x00"\x00\x00\x00\x02\xa3\x09ANONYMOUS\xa0\x11anonymous@fuschia'
+ # AMQP
+ 'AMQP\x00\x01\x00\x00'
+ # @open(16) [container-id="", channel-max=1234]
+ '\x00\x00\x00!\x02\x00\x00\x00\x00S\x10\xd0\x00\x00\x00\x11\x00\x00\x00\x0a\xa1\x00@@`\x04\xd2@@@@@@'
+ ))
+
+ consumeAllOuput(self.t2)
- assert self.s1.outcome is None
-
- self.t1.push(out2)
-
- assert self.s1.outcome == SASL.OK
assert self.s2.outcome == SASL.OK
+ assert c2.state & Endpoint.REMOTE_ACTIVE
- def testPipelinedClientFail(self):
- if "java" in sys.platform:
- raise Skipped("Proton-J does not support client pipelining")
-
+ def testPipelinedServer(self):
# Client
self.s1.allowed_mechs('ANONYMOUS')
- # Server
- self.s2.allowed_mechs('PLAIN DIGEST-MD5 SCRAM-SHA-1')
-
- assert self.s1.outcome is None
- assert self.s2.outcome is None
-
- # Push client bytes into server
- out1 = self.t1.peek(1024)
- self.t1.pop(len(out1))
- self.t2.push(out1)
-
- out2 = self.t2.peek(1024)
- self.t2.pop(len(out2))
-
- assert self.s1.outcome is None
-
- self.t1.push(out2)
-
- assert self.s1.outcome == SASL.AUTH
- assert self.s2.outcome == SASL.AUTH
-
- def testSaslAndAmqpInSingleChunk(self):
- if "java" in sys.platform:
- raise Skipped("Proton-J does not support client pipelining")
-
- self.s1.allowed_mechs('ANONYMOUS')
- self.s2.allowed_mechs('ANONYMOUS')
- # do some work to generate AMQP data
c1 = Connection()
- c2 = Connection()
self.t1.bind(c1)
- c1._transport = self.t1
- self.t2.bind(c2)
- c2._transport = self.t2
- c1.open()
-
- # get all t1's output in one buffer then pass it all to t2
- out1_sasl_and_amqp = str2bin("")
- t1_still_producing = True
- while t1_still_producing:
- out1 = self.t1.peek(1024)
- self.t1.pop(len(out1))
- out1_sasl_and_amqp += out1
- t1_still_producing = out1
-
- t2_still_consuming = True
- while t2_still_consuming:
- num = min(self.t2.capacity(), len(out1_sasl_and_amqp))
- self.t2.push(out1_sasl_and_amqp[:num])
- out1_sasl_and_amqp = out1_sasl_and_amqp[num:]
- t2_still_consuming = num > 0 and len(out1_sasl_and_amqp) > 0
+ assert self.s1.outcome is None
- assert len(out1_sasl_and_amqp) == 0, (len(out1_sasl_and_amqp), out1_sasl_and_amqp)
+ # Push server bytes into client
+ # Commented out lines in this test are where the client input processing doesn't
+ # run after output processing even though there is input waiting
+ self.t1.push(str2bin(
+ # SASL
+ 'AMQP\x03\x01\x00\x00'
+ # @sasl-mechanisms(64) [sasl-server-mechanisms=@PN_SYMBOL[:ANONYMOUS]]
+ '\x00\x00\x00\x1c\x02\x01\x00\x00\x00S@\xc0\x0f\x01\xe0\x0c\x01\xa3\tANONYMOUS'
+ # @sasl-outcome(68) [code=0]
+ '\x00\x00\x00\x10\x02\x01\x00\x00\x00SD\xc0\x03\x01P\x00'
+ # AMQP
+ 'AMQP\x00\x01\x00\x00'
+ # @open(16) [container-id="", channel-max=1234]
+ '\x00\x00\x00!\x02\x00\x00\x00\x00S\x10\xd0\x00\x00\x00\x11\x00\x00\x00\x0a\xa1\x00@@`\x04\xd2@@@@@@'
+ ))
+
+ consumeAllOuput(self.t1)
- # check that t2 processed both the SASL data and the AMQP data
- assert self.s2.outcome == SASL.OK
- assert c2.state & Endpoint.REMOTE_ACTIVE
+ assert self.s1.outcome == SASL.OK
+ assert c1.state & Endpoint.REMOTE_ACTIVE
def testPipelined2(self):
if "java" in sys.platform:
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org