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/07/28 16:59:48 UTC

qpid-proton git commit: PROTON-963: Raise Transport Error event with correct condition on authentication failure

Repository: qpid-proton
Updated Branches:
  refs/heads/master 0773d9b19 -> 0dea44515


PROTON-963: Raise Transport Error event with correct condition on authentication failure


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

Branch: refs/heads/master
Commit: 0dea44515b152752f04d46aecc6243ebff101cef
Parents: 0773d9b
Author: Andrew Stitcher <as...@apache.org>
Authored: Tue Jul 28 10:48:58 2015 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Tue Jul 28 10:48:58 2015 -0400

----------------------------------------------------------------------
 proton-c/src/sasl/sasl.c          | 37 ++++++++++++++++++++--------------
 tests/python/proton_tests/sasl.py | 18 +++++++++--------
 2 files changed, 32 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/0dea4451/proton-c/src/sasl/sasl.c
----------------------------------------------------------------------
diff --git a/proton-c/src/sasl/sasl.c b/proton-c/src/sasl/sasl.c
index ccddda9..fb028cc 100644
--- a/proton-c/src/sasl/sasl.c
+++ b/proton-c/src/sasl/sasl.c
@@ -99,7 +99,8 @@ static bool pni_sasl_is_server_state(enum pni_sasl_state state)
   return state==SASL_NONE
       || state==SASL_POSTED_MECHANISMS
       || state==SASL_POSTED_CHALLENGE
-      || state==SASL_POSTED_OUTCOME;
+      || state==SASL_POSTED_OUTCOME
+      || state==SASL_ERROR;
 }
 
 static bool pni_sasl_is_client_state(enum pni_sasl_state state)
@@ -108,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
+      || state==SASL_ERROR;
 }
 
 static bool pni_sasl_is_final_input_state(pni_sasl_t *sasl)
@@ -161,7 +163,8 @@ void pni_sasl_set_desired_state(pn_transport_t *transport, enum pni_sasl_state d
         sasl->last_state = SASL_RECVED_OUTCOME;
     }
     sasl->desired_state = desired_state;
-    pni_emit(transport);
+    // Don't emit transport event on error as there will be a TRANSPORT_ERROR event
+    if (desired_state != SASL_ERROR) pni_emit(transport);
   }
 }
 
@@ -218,15 +221,24 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
       }
       pn_post_frame(transport, SASL_FRAME_TYPE, 0, "DL[B]", SASL_OUTCOME, sasl->outcome);
       pni_emit(transport);
+      if (sasl->outcome!=PN_SASL_OK) {
+        pn_do_error(transport, "amqp:unauthorized-access", "Failed to authenticate client");
+        desired_state = SASL_ERROR;
+      }
       break;
     case SASL_RECVED_OUTCOME:
       if (sasl->last_state < SASL_POSTED_INIT && sasl->outcome==PN_SASL_OK) {
         desired_state = SASL_POSTED_INIT;
         continue;
       }
+      if (sasl->outcome!=PN_SASL_OK) {
+        pn_do_error(transport, "amqp:unauthorized-access", "Authentication failed");
+        desired_state = SASL_ERROR;
+      }
       break;
-    case SASL_NONE:
     case SASL_ERROR:
+      break;
+    case SASL_NONE:
       return;
     }
     sasl->last_state = desired_state;
@@ -237,11 +249,7 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
 static void pn_error_sasl(pn_transport_t* transport)
 {
   transport->close_sent = true;
-  pni_sasl_t *sasl = transport->sasl;
-  if (!pni_sasl_is_final_input_state(sasl)) {
-    sasl->desired_state = SASL_ERROR;
-    sasl->last_state = SASL_ERROR;
-  }
+  pni_sasl_set_desired_state(transport, SASL_ERROR);
 }
 
 static ssize_t pn_input_read_sasl_header(pn_transport_t* transport, unsigned int layer, const char* bytes, size_t available)
@@ -368,11 +376,10 @@ static ssize_t pn_output_write_sasl(pn_transport_t* transport, unsigned int laye
 
   if (transport->available != 0 || !pni_sasl_is_final_output_state(sasl)) {
     return pn_dispatcher_output(transport, bytes, available);
-  } else {
-    if (sasl->outcome != PN_SASL_OK && pni_sasl_is_final_input_state(sasl)) {
-      pn_transport_close_tail(transport);
-      return PN_EOS;
-    }
+  }
+
+  if (sasl->outcome != PN_SASL_OK && pni_sasl_is_final_input_state(sasl)) {
+    return PN_EOS;
   }
 
   if (pni_sasl_impl_can_encrypt(transport)) {
@@ -661,7 +668,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_ERROR);
   }
 
   pn_free(mechs);

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/0dea4451/tests/python/proton_tests/sasl.py
----------------------------------------------------------------------
diff --git a/tests/python/proton_tests/sasl.py b/tests/python/proton_tests/sasl.py
index f93eef9..29c5c43 100644
--- a/tests/python/proton_tests/sasl.py
+++ b/tests/python/proton_tests/sasl.py
@@ -488,8 +488,9 @@ class SASLEventTest(engine.CollectorTest):
     _testSaslMech(self, 'DIGEST-MD5', clientUser=clientUser, authenticated=False)
     self.expect(Event.CONNECTION_INIT, Event.CONNECTION_BOUND,
                 Event.CONNECTION_LOCAL_OPEN, Event.TRANSPORT,
+                Event.TRANSPORT_ERROR,
                 Event.TRANSPORT_TAIL_CLOSED,
-                Event.TRANSPORT_ERROR, Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_CLOSED)
+                Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_CLOSED)
 
   def testFailedAuthenticationServer(self):
     common.ensureCanTestExtendedSASL()
@@ -501,8 +502,9 @@ class SASLEventTest(engine.CollectorTest):
     _testSaslMech(self, 'DIGEST-MD5', clientUser=clientUser, authenticated=False)
     self.expect(Event.CONNECTION_INIT, Event.CONNECTION_BOUND,
                 Event.CONNECTION_LOCAL_OPEN, Event.TRANSPORT,
+                Event.TRANSPORT_ERROR,
                 Event.TRANSPORT_TAIL_CLOSED,
-                Event.TRANSPORT_ERROR, Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_CLOSED)
+                Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_CLOSED)
 
   def testNoMechClient(self):
     common.ensureCanTestExtendedSASL()
@@ -513,8 +515,8 @@ class SASLEventTest(engine.CollectorTest):
     _testSaslMech(self, 'DIGEST-MD5', authenticated=False)
     self.expect(Event.CONNECTION_INIT, Event.CONNECTION_BOUND,
                 Event.CONNECTION_LOCAL_OPEN, Event.TRANSPORT,
-                Event.TRANSPORT_TAIL_CLOSED,
-                Event.TRANSPORT_ERROR, Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_CLOSED)
+                Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_TAIL_CLOSED, Event.TRANSPORT_CLOSED,
+                Event.TRANSPORT_ERROR)
 
   def testNoMechServer(self):
     common.ensureCanTestExtendedSASL()
@@ -535,8 +537,8 @@ class SASLEventTest(engine.CollectorTest):
     _testSaslMech(self, 'IMPOSSIBLE', authenticated=False)
     self.expect(Event.CONNECTION_INIT, Event.CONNECTION_BOUND,
                 Event.CONNECTION_LOCAL_OPEN, Event.TRANSPORT,
-                Event.TRANSPORT_TAIL_CLOSED,
-                Event.TRANSPORT_ERROR, Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_CLOSED)
+                Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_TAIL_CLOSED, Event.TRANSPORT_CLOSED,
+                Event.TRANSPORT_ERROR)
 
   def testDisallowedMechServer(self):
     self.c2.collect(self.collector)
@@ -555,8 +557,8 @@ class SASLEventTest(engine.CollectorTest):
     _testSaslMech(self, 'PLAIN', authenticated=False)
     self.expect(Event.CONNECTION_INIT, Event.CONNECTION_BOUND,
                 Event.CONNECTION_LOCAL_OPEN, Event.TRANSPORT,
-                Event.TRANSPORT_TAIL_CLOSED,
-                Event.TRANSPORT_ERROR, Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_CLOSED)
+                Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_TAIL_CLOSED, Event.TRANSPORT_CLOSED,
+                Event.TRANSPORT_ERROR)
 
   def testDisallowedPlainServer(self):
     self.c2.collect(self.collector)


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