qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From acon...@apache.org
Subject [32/41] qpid-proton git commit: PROTON-1865: Improve the split between general SASL code and the specific implementations
Date Wed, 04 Jul 2018 15:58:04 GMT
PROTON-1865: Improve the split between general SASL code and the specific implementations


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

Branch: refs/heads/go1
Commit: 885d68aeaf522021a35b7b5cecb7c7c53663929b
Parents: 21eb6d8
Author: Andrew Stitcher <astitcher@apache.org>
Authored: Tue Jun 19 08:53:40 2018 -0400
Committer: Andrew Stitcher <astitcher@apache.org>
Committed: Tue Jun 19 08:53:40 2018 -0400

----------------------------------------------------------------------
 c/include/proton/sasl-plugin.h |  1 +
 c/src/sasl/cyrus_sasl.c        | 20 +-------------------
 c/src/sasl/default_sasl.c      | 20 ++++++++------------
 c/src/sasl/sasl.c              | 28 +++++++++++++++++++++++++---
 4 files changed, 35 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/885d68ae/c/include/proton/sasl-plugin.h
----------------------------------------------------------------------
diff --git a/c/include/proton/sasl-plugin.h b/c/include/proton/sasl-plugin.h
index d420eaf..ae6b0ec 100644
--- a/c/include/proton/sasl-plugin.h
+++ b/c/include/proton/sasl-plugin.h
@@ -108,6 +108,7 @@ enum pnx_sasl_state {
 
 /* APIs used by sasl implementations */
 PN_EXTERN void  pnx_sasl_logf(pn_transport_t *transport, const char *format, ...);
+PN_EXTERN void  pnx_sasl_error(pn_transport_t *transport, const char* err, const char* condition_name);
 
 PN_EXTERN void *pnx_sasl_get_context(pn_transport_t *transport);
 PN_EXTERN void  pnx_sasl_set_context(pn_transport_t *transport, void *context);

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/885d68ae/c/src/sasl/cyrus_sasl.c
----------------------------------------------------------------------
diff --git a/c/src/sasl/cyrus_sasl.c b/c/src/sasl/cyrus_sasl.c
index c589fab..a7387e6 100644
--- a/c/src/sasl/cyrus_sasl.c
+++ b/c/src/sasl/cyrus_sasl.c
@@ -97,10 +97,7 @@ static inline bool pni_check_result(sasl_conn_t *conn, int r, pn_transport_t
*lo
     if (r==SASL_OK) return true;
 
     const char* err = conn ? sasl_errdetail(conn) : sasl_errstring(r, NULL, NULL);
-    pnx_sasl_logf(logger, "sasl error: %s", err);
-    pn_condition_t* c = pn_transport_condition(logger);
-    pn_condition_set_name(c, condition_name);
-    pn_condition_set_description(c, err);
+    pnx_sasl_error(logger, err, condition_name);
     return false;
 }
 
@@ -120,9 +117,6 @@ static void pni_cyrus_interact(pn_transport_t *transport, sasl_interact_t
*inter
   for (sasl_interact_t *i = interact; i->id!=SASL_CB_LIST_END; i++) {
     switch (i->id) {
     case SASL_CB_USER:
-      i->result = 0;
-      i->len = 0;
-      break;
     case SASL_CB_AUTHNAME: {
       const char *username = pnx_sasl_get_username(transport);
       i->result = username;
@@ -473,8 +467,6 @@ static void pni_process_server_result(pn_transport_t *transport, int result)
             const void* value;
             sasl_getprop(cyrus_conn, SASL_USERNAME, &value);
             pnx_sasl_succeed_authentication(transport, (const char*) value);
-            pnx_sasl_logf(transport, "Authenticated user: %s with mechanism %s",
-                          pnx_sasl_get_username(transport), pnx_sasl_get_selected_mechanism(transport));
             pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
             break;
         }
@@ -495,16 +487,6 @@ static void pni_process_server_result(pn_transport_t *transport, int
result)
 void cyrus_sasl_process_init(pn_transport_t *transport, const char *mechanism, const pn_bytes_t
*recv)
 {
     int result = pni_wrap_server_start(transport, mechanism, recv);
-    if (result==SASL_OK) {
-        // We need to filter out a supplied mech in in the inclusion list
-        // as the client could have used a mech that we support, but that
-        // wasn't on the list we sent.
-        if (!pnx_sasl_is_included_mech(transport, pn_bytes(strlen(mechanism), mechanism)))
{
-            sasl_conn_t *cyrus_conn = (sasl_conn_t*)pnx_sasl_get_context(transport);
-            sasl_seterror(cyrus_conn, 0, "Client mechanism not in mechanism inclusion list.");
-            result = SASL_FAIL;
-        }
-    }
     pni_process_server_result(transport, result);
 }
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/885d68ae/c/src/sasl/default_sasl.c
----------------------------------------------------------------------
diff --git a/c/src/sasl/default_sasl.c b/c/src/sasl/default_sasl.c
index 66dd318..7299318 100644
--- a/c/src/sasl/default_sasl.c
+++ b/c/src/sasl/default_sasl.c
@@ -93,6 +93,7 @@ void default_sasl_impl_free(pn_transport_t *transport)
 }
 
 // Client handles ANONYMOUS or PLAIN mechanisms if offered
+// Offered mechanisms have already been filtered against the "allowed" list
 bool default_sasl_process_mechanisms(pn_transport_t *transport, const char *mechs)
 {
   const char *username = pnx_sasl_get_username(transport);
@@ -101,9 +102,8 @@ bool default_sasl_process_mechanisms(pn_transport_t *transport, const
char *mech
   // Check whether offered EXTERNAL, PLAIN or ANONYMOUS
   // Look for "EXTERNAL" in mechs
   const char *found = strstr(mechs, EXTERNAL);
-  // Make sure that string is separated and terminated and allowed
-  if (found && (found==mechs || found[-1]==' ') && (found[8]==0 || found[8]=='
') &&
-      pnx_sasl_is_included_mech(transport, pn_bytes(8, found))) {
+  // Make sure that string is separated and terminated
+  if (found && (found==mechs || found[-1]==' ') && (found[8]==0 || found[8]=='
')) {
     pnx_sasl_set_selected_mechanism(transport, EXTERNAL);
     if (username) {
       size_t size = strlen(username);
@@ -127,7 +127,6 @@ bool default_sasl_process_mechanisms(pn_transport_t *transport, const
char *mech
   // Make sure that string is separated and terminated, allowed
   // and we have a username and password and connection is encrypted or we allow insecure
   if (found && (found==mechs || found[-1]==' ') && (found[5]==0 || found[5]=='
') &&
-      pnx_sasl_is_included_mech(transport, pn_bytes(5, found)) &&
       (pnx_sasl_is_transport_encrypted(transport) || pnx_sasl_get_allow_insecure_mechs(transport))
&&
       username && password) {
     pnx_sasl_set_selected_mechanism(transport, PLAIN);
@@ -155,8 +154,7 @@ bool default_sasl_process_mechanisms(pn_transport_t *transport, const
char *mech
   // Look for "ANONYMOUS" in mechs
   found = strstr(mechs, ANONYMOUS);
   // Make sure that string is separated and terminated and allowed
-  if (found && (found==mechs || found[-1]==' ') && (found[9]==0 || found[9]=='
') &&
-      pnx_sasl_is_included_mech(transport, pn_bytes(9, found))) {
+  if (found && (found==mechs || found[-1]==' ') && (found[9]==0 || found[9]=='
')) {
     pnx_sasl_set_selected_mechanism(transport, ANONYMOUS);
     if (username) {
       size_t size = strlen(username);
@@ -188,11 +186,11 @@ const char *default_sasl_impl_list_mechs(pn_transport_t *transport)
   }
 }
 
+// The selected mechanism has already been verified against the "allowed" list
 void default_sasl_process_init(pn_transport_t *transport, const char *mechanism, const pn_bytes_t
*recv)
 {
-  // Check that mechanism is ANONYMOUS and it is allowed
-  if (strcmp(mechanism, ANONYMOUS)==0 &&
-      pnx_sasl_is_included_mech(transport, pn_bytes(sizeof(ANONYMOUS)-1, ANONYMOUS))) {
+  // Check that mechanism is ANONYMOUS
+  if (strcmp(mechanism, ANONYMOUS)==0) {
     pnx_sasl_succeed_authentication(transport, "anonymous");
     pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
     return;
@@ -200,9 +198,7 @@ void default_sasl_process_init(pn_transport_t *transport, const char *mechanism,
 
   // Or maybe EXTERNAL
   const char *ext_username = pnx_sasl_get_external_username(transport);
-  if (strcmp(mechanism, EXTERNAL)==0 &&
-      pnx_sasl_is_included_mech(transport, pn_bytes(sizeof(EXTERNAL)-1, EXTERNAL)) &&
-      ext_username) {
+  if (strcmp(mechanism, EXTERNAL)==0 && ext_username) {
     pnx_sasl_succeed_authentication(transport, ext_username);
     pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
     return;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/885d68ae/c/src/sasl/sasl.c
----------------------------------------------------------------------
diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c
index f015514..8648afa 100644
--- a/c/src/sasl/sasl.c
+++ b/c/src/sasl/sasl.c
@@ -48,6 +48,14 @@ void pnx_sasl_logf(pn_transport_t *logger, const char *fmt, ...)
     va_end(ap);
 }
 
+void pnx_sasl_error(pn_transport_t *logger, const char* err, const char* condition_name)
+{
+    pnx_sasl_logf(logger, "sasl error: %s", err);
+    pn_condition_t* c = pn_transport_condition(logger);
+    pn_condition_set_name(c, condition_name);
+    pn_condition_set_description(c, err);
+}
+
 void *pnx_sasl_get_context(pn_transport_t *transport)
 {
   return transport->sasl ? transport->sasl->impl_context : NULL;
@@ -137,6 +145,9 @@ void  pnx_sasl_succeed_authentication(pn_transport_t *transport, const
char *use
     transport->sasl->username = username;
     transport->sasl->outcome = PN_SASL_OK;
     transport->authenticated = true;
+
+    pnx_sasl_logf(transport, "Authenticated user: %s with mechanism %s",
+                  username, transport->sasl->selected_mechanism);
   }
 }
 
@@ -822,7 +833,16 @@ int pn_do_init(pn_transport_t *transport, uint8_t frame_type, uint16_t
channel,
   if (err) return err;
   sasl->selected_mechanism = pn_strndup(mech.start, mech.size);
 
-  pni_sasl_impl_process_init(transport, sasl->selected_mechanism, &recv);
+  // We need to filter out a supplied mech in in the inclusion list
+  // as the client could have used a mech that we support, but that
+  // wasn't on the list we sent.
+  if (!pni_sasl_included_mech(sasl->included_mechanisms, mech)) {
+    pnx_sasl_error(transport, "Client mechanism not in mechanism inclusion list.", "amqp:unauthorized-access");
+    sasl->outcome = PN_SASL_AUTH;
+    pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
+  } else {
+    pni_sasl_impl_process_init(transport, sasl->selected_mechanism, &recv);
+  }
 
   return 0;
 }
@@ -845,7 +865,7 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t
cha
     // Now keep checking for end of array and pull a symbol
     while(pn_data_next(args)) {
       pn_bytes_t s = pn_data_get_symbol(args);
-      if (pnx_sasl_is_included_mech(transport, s)) {
+      if (pni_sasl_included_mech(sasl->included_mechanisms, s)) {
         pn_string_addf(mechs, "%*s ", (int)s.size, s.start);
       }
     }
@@ -860,7 +880,9 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t
cha
     int err = pn_data_scan(args, "D.[s]", &symbol);
     if (err) return err;
 
-    pn_string_setn(mechs, symbol.start, symbol.size);
+    if (pni_sasl_included_mech(sasl->included_mechanisms, symbol)) {
+      pn_string_setn(mechs, symbol.start, symbol.size);
+    }
   }
 
   if (!(pni_sasl_impl_init_client(transport) &&


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


Mime
View raw message