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 2015/08/11 17:00:49 UTC

qpid-proton git commit: PROTON-975: Fix to ensure that we switch to decrypting incoming frames early enough

Repository: qpid-proton
Updated Branches:
  refs/heads/master be4e0f0be -> a1345a278


PROTON-975: Fix to ensure that we switch to decrypting incoming frames early enough


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/a1345a27
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/a1345a27
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/a1345a27

Branch: refs/heads/master
Commit: a1345a2788475b432ab34b209ccccf1e92d0d2f6
Parents: be4e0f0
Author: Andrew Stitcher <as...@apache.org>
Authored: Tue Aug 11 02:48:59 2015 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Tue Aug 11 02:57:33 2015 -0400

----------------------------------------------------------------------
 proton-c/src/sasl/cyrus_sasl.c    |  2 +-
 proton-c/src/sasl/sasl-internal.h |  3 +-
 proton-c/src/sasl/sasl.c          | 51 +++++++++++++++++++++-------------
 tests/python/proton_tests/sasl.py | 16 +++++------
 4 files changed, 42 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a1345a27/proton-c/src/sasl/cyrus_sasl.c
----------------------------------------------------------------------
diff --git a/proton-c/src/sasl/cyrus_sasl.c b/proton-c/src/sasl/cyrus_sasl.c
index 802275e..809bad5 100644
--- a/proton-c/src/sasl/cyrus_sasl.c
+++ b/proton-c/src/sasl/cyrus_sasl.c
@@ -234,7 +234,7 @@ void pni_process_challenge(pn_transport_t *transport, const pn_bytes_t *recv)
 
             // Failed somehow - equivalent to failing authentication
             sasl->outcome = PN_SASL_AUTH;
-            pni_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME);
+            pni_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME_FAIL);
             break;
     }
 }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a1345a27/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 0073e5e..b3f4c7f 100644
--- a/proton-c/src/sasl/sasl-internal.h
+++ b/proton-c/src/sasl/sasl-internal.h
@@ -60,7 +60,8 @@ enum pni_sasl_state {
   SASL_POSTED_RESPONSE,
   SASL_POSTED_CHALLENGE,
   SASL_PRETEND_OUTCOME,
-  SASL_RECVED_OUTCOME,
+  SASL_RECVED_OUTCOME_SUCCEED,
+  SASL_RECVED_OUTCOME_FAIL,
   SASL_POSTED_OUTCOME,
   SASL_ERROR
 };

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a1345a27/proton-c/src/sasl/sasl.c
----------------------------------------------------------------------
diff --git a/proton-c/src/sasl/sasl.c b/proton-c/src/sasl/sasl.c
index 83543ec..f300f91 100644
--- a/proton-c/src/sasl/sasl.c
+++ b/proton-c/src/sasl/sasl.c
@@ -109,7 +109,8 @@ static bool pni_sasl_is_client_state(enum pni_sasl_state state)
       || state==SASL_POSTED_INIT
       || state==SASL_POSTED_RESPONSE
       || state==SASL_PRETEND_OUTCOME
-      || state==SASL_RECVED_OUTCOME
+      || state==SASL_RECVED_OUTCOME_SUCCEED
+      || state==SASL_RECVED_OUTCOME_FAIL
       || state==SASL_ERROR;
 }
 
@@ -117,16 +118,19 @@ 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
+  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_POSTED_OUTCOME
+      || ( desired_state==SASL_RECVED_OUTCOME_SUCCEED && last_state>=SASL_POSTED_INIT);
 }
 
 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
+      || last_state==SASL_RECVED_OUTCOME_SUCCEED
+      || last_state==SASL_RECVED_OUTCOME_FAIL
       || last_state==SASL_ERROR
       || last_state==SASL_POSTED_OUTCOME;
 }
@@ -161,9 +165,9 @@ void pni_sasl_set_desired_state(pn_transport_t *transport, enum pni_sasl_state d
       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'vwe already stoped outputting from this layer
-    if (sasl->last_state==SASL_PRETEND_OUTCOME && desired_state==SASL_RECVED_OUTCOME ) {
-        sasl->last_state = SASL_RECVED_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
@@ -229,15 +233,15 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
         desired_state = SASL_ERROR;
       }
       break;
-    case SASL_RECVED_OUTCOME:
-      if (sasl->last_state < SASL_POSTED_INIT && sasl->outcome==PN_SASL_OK) {
+    case SASL_RECVED_OUTCOME_SUCCEED:
+      if (sasl->last_state < SASL_POSTED_INIT) {
         desired_state = SASL_POSTED_INIT;
         continue;
       }
-      if (sasl->outcome!=PN_SASL_OK) {
-        pn_do_error(transport, "amqp:unauthorized-access", "Authentication failed [mech=%s]", transport->sasl->selected_mechanism);
-        desired_state = SASL_ERROR;
-      }
+      break;
+    case SASL_RECVED_OUTCOME_FAIL:
+      pn_do_error(transport, "amqp:unauthorized-access", "Authentication failed [mech=%s]", transport->sasl->selected_mechanism);
+      desired_state = SASL_ERROR;
       break;
     case SASL_ERROR:
       break;
@@ -298,6 +302,8 @@ static void pni_sasl_start_server_if_needed(pn_transport_t *transport)
 
 static ssize_t pn_input_read_sasl(pn_transport_t* transport, unsigned int layer, const char* bytes, size_t available)
 {
+  pni_sasl_t *sasl = transport->sasl;
+
   bool eos = pn_transport_capacity(transport)==PN_EOS;
   if (eos) {
     pn_do_error(transport, "amqp:connection:framing-error", "connection aborted");
@@ -309,11 +315,14 @@ static ssize_t pn_input_read_sasl(pn_transport_t* transport, unsigned int layer,
 
   ssize_t n = pn_dispatcher_input(transport, bytes, available, false, &transport->halt);
 
-  if (n!=0 || !pni_sasl_is_final_input_state(transport->sasl)) {
+  if (!pni_sasl_is_final_input_state(sasl)) {
+    return n;
+  }
+
+  if (!sasl->client && n!=0) {
     return n;
   }
 
-  pni_sasl_t *sasl = transport->sasl;
   if (pni_sasl_impl_can_encrypt(transport)) {
     sasl->max_encrypt_size = pni_sasl_impl_max_encrypt_size(transport);
     if (transport->trace & PN_TRACE_DRV)
@@ -322,9 +331,10 @@ static ssize_t pn_input_read_sasl(pn_transport_t* transport, unsigned int layer,
   } else if (sasl->client) {
     transport->io_layers[layer] = &pni_passthru_layer;
   } else {
+    assert(n==0);
     return pni_passthru_layer.process_input(transport, layer, bytes, available );
   }
-  return transport->io_layers[layer]->process_input(transport, layer, bytes, available);
+  return n+transport->io_layers[layer]->process_input(transport, layer, bytes+n, available-n);
 }
 
 static ssize_t pn_input_read_sasl_encrypt(pn_transport_t* transport, unsigned int layer, const char* bytes, size_t available)
@@ -551,7 +561,7 @@ static void pni_sasl_force_anonymous(pn_transport_t *transport)
       pni_sasl_set_desired_state(transport, SASL_PRETEND_OUTCOME);
     } else {
       sasl->outcome = PN_SASL_PERM;
-      pni_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME);
+      pni_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME_FAIL);
     }
   }
 }
@@ -688,7 +698,7 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t cha
     pni_sasl_set_desired_state(transport, SASL_POSTED_INIT);
   } else {
     sasl->outcome = PN_SASL_PERM;
-    pni_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME);
+    pni_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME_FAIL);
   }
 
   pn_free(mechs);
@@ -728,8 +738,9 @@ int pn_do_outcome(pn_transport_t *transport, uint8_t frame_type, uint16_t channe
 
   pni_sasl_t *sasl = transport->sasl;
   sasl->outcome = (pn_sasl_outcome_t) outcome;
-  transport->authenticated = sasl->outcome==PN_SASL_OK;
-  pni_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME);
+  bool authenticated = sasl->outcome==PN_SASL_OK;
+  transport->authenticated = authenticated;
+  pni_sasl_set_desired_state(transport, authenticated ? SASL_RECVED_OUTCOME_SUCCEED : SASL_RECVED_OUTCOME_FAIL);
 
   return 0;
 }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a1345a27/tests/python/proton_tests/sasl.py
----------------------------------------------------------------------
diff --git a/tests/python/proton_tests/sasl.py b/tests/python/proton_tests/sasl.py
index df2910a..72fa76c 100644
--- a/tests/python/proton_tests/sasl.py
+++ b/tests/python/proton_tests/sasl.py
@@ -40,35 +40,35 @@ def _testSaslMech(self, mech, clientUser='user@proton', authUser='user@proton',
   pump(self.t1, self.t2, 1024)
 
   if encrypted is not None:
-    assert self.t2.encrypted == encrypted
-    assert self.t1.encrypted == encrypted
+    assert self.t2.encrypted == encrypted, encrypted
+    assert self.t1.encrypted == encrypted, encrypted
 
-  assert self.t2.authenticated == authenticated
-  assert self.t1.authenticated == authenticated
+  assert self.t2.authenticated == authenticated, authenticated
+  assert self.t1.authenticated == authenticated, authenticated
   if authenticated:
     # Server
     assert self.t2.user == authUser
     assert self.s2.user == authUser
     assert self.s2.mech == mech.strip()
-    assert self.s2.outcome == SASL.OK
+    assert self.s2.outcome == SASL.OK, self.s2.outcome
     assert self.c2.state & Endpoint.LOCAL_ACTIVE and self.c2.state & Endpoint.REMOTE_ACTIVE,\
       "local_active=%s, remote_active=%s" % (self.c1.state & Endpoint.LOCAL_ACTIVE, self.c1.state & Endpoint.REMOTE_ACTIVE)
     # Client
     assert self.t1.user == clientUser
     assert self.s1.user == clientUser
     assert self.s1.mech == mech.strip()
-    assert self.s1.outcome == SASL.OK
+    assert self.s1.outcome == SASL.OK, self.s1.outcome
     assert self.c1.state & Endpoint.LOCAL_ACTIVE and self.c1.state & Endpoint.REMOTE_ACTIVE,\
       "local_active=%s, remote_active=%s" % (self.c1.state & Endpoint.LOCAL_ACTIVE, self.c1.state & Endpoint.REMOTE_ACTIVE)
   else:
     # Server
     assert self.t2.user == None
     assert self.s2.user == None
-    assert self.s2.outcome != SASL.OK
+    assert self.s2.outcome != SASL.OK, self.s2.outcome
     # Client
     assert self.t1.user == clientUser
     assert self.s1.user == clientUser
-    assert self.s1.outcome != SASL.OK
+    assert self.s1.outcome != SASL.OK, self.s1.outcome
 
 class Test(common.Test):
   pass


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org