qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From acon...@apache.org
Subject [33/41] qpid-proton git commit: PROTON-1354: Don't allow SASL mechanisms GSSAPI or GSS-SPNEGO client-side by default - If you want to use these mechanisms they must be explicitly set in the client allowed mechanisms list. - The mechanisms offered by th
Date Wed, 04 Jul 2018 15:58:05 GMT
PROTON-1354: Don't allow SASL mechanisms GSSAPI or GSS-SPNEGO client-side by default
- If you want to use these mechanisms they must be explicitly set in the
  client allowed mechanisms list.
- The mechanisms offered by the server side are unchanged.


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

Branch: refs/heads/go1
Commit: 170be2e68f0824376b4641dfc2b65193bdaac317
Parents: 885d68a
Author: Andrew Stitcher <astitcher@apache.org>
Authored: Tue Jun 12 16:17:42 2018 -0400
Committer: Andrew Stitcher <astitcher@apache.org>
Committed: Tue Jun 19 16:25:40 2018 -0400

----------------------------------------------------------------------
 c/include/proton/sasl.h | 10 ++++++++++
 c/src/sasl/sasl.c       | 31 +++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/170be2e6/c/include/proton/sasl.h
----------------------------------------------------------------------
diff --git a/c/include/proton/sasl.h b/c/include/proton/sasl.h
index 03d784e..3cd00d9 100644
--- a/c/include/proton/sasl.h
+++ b/c/include/proton/sasl.h
@@ -91,6 +91,8 @@ PN_EXTERN bool pn_sasl_extended(void);
  * Set the outcome of SASL negotiation
  *
  * Used by the server to set the result of the negotiation process.
+ *
+ * @deprecated Do not use - there is no correct way to use this API
  */
 PN_EXTERN void pn_sasl_done(pn_sasl_t *sasl, pn_sasl_outcome_t outcome);
 
@@ -135,6 +137,14 @@ PN_EXTERN const char *pn_sasl_get_mech(pn_sasl_t *sasl);
  * This can be used on either the client or the server to restrict the SASL
  * mechanisms that may be used to the mechanisms on the list.
  *
+ * @note By default the GSSAPI and GSS-SPNEGO mechanisms are not enabled for clients. This
is because
+ * these mechanisms have the problematic behaviour of 'capturing' the client whenever they
are installed
+ * so that they will be used by the client if offered by the server even if the client can't
successfully
+ * authenticate this way. This can lead to some very hard to debug failures.
+ *
+ * @note The GSSAPI or GSS-SPNEGO mechanisms need to be explicitly enabled if they are required
(together
+ * with any other required mechanisms).
+ *
  * @param[in] sasl the SASL layer
  * @param[in] mechs space separated list of mechanisms that are allowed for authentication
  */

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/170be2e6/c/src/sasl/sasl.c
----------------------------------------------------------------------
diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c
index 8648afa..125c06d 100644
--- a/c/src/sasl/sasl.c
+++ b/c/src/sasl/sasl.c
@@ -36,6 +36,9 @@
 // Change this to &default_sasl_impl when we change cyrus to opt in
 static const pnx_sasl_implementation *global_sasl_impl = NULL;
 
+// List of SASL mechansms to exclude by default as they cause user pain
+static const char* pni_excluded_mechs = "GSSAPI GSS-SPNEGO";
+
 //-----------------------------------------------------------------------------
 // pnx_sasl: API for SASL implementations to use
 
@@ -384,8 +387,6 @@ void pnx_sasl_set_desired_state(pn_transport_t *transport, enum pnx_sasl_state
d
 // Note that if there is no inclusion list then every mech is implicitly included.
 static bool pni_sasl_included_mech(const char *included_mech_list, pn_bytes_t s)
 {
-  if (!included_mech_list) return true;
-
   const char * end_list = included_mech_list+strlen(included_mech_list);
   size_t len = s.size;
   const char *c = included_mech_list;
@@ -402,12 +403,26 @@ static bool pni_sasl_included_mech(const char *included_mech_list, pn_bytes_t
s)
   return false;
 }
 
+static bool pni_sasl_server_included_mech(const char *included_mech_list, pn_bytes_t s)
+{
+  if (!included_mech_list) return true;
+
+  return pni_sasl_included_mech(included_mech_list, s);
+}
+
+static bool pni_sasl_client_included_mech(const char *included_mech_list, pn_bytes_t s)
+{
+  if (!included_mech_list && pni_excluded_mechs) return !pni_sasl_included_mech(pni_excluded_mechs,
s);
+
+  return pni_sasl_included_mech(included_mech_list, s);
+}
+
 // Look for symbol in the mech include list - plugin API version
 //
 // Note that if there is no inclusion list then every mech is implicitly included.
 bool pnx_sasl_is_included_mech(pn_transport_t* transport, pn_bytes_t s)
 {
-  return pni_sasl_included_mech(transport->sasl->included_mechanisms, s);
+  return pni_sasl_server_included_mech(transport->sasl->included_mechanisms, s);
 }
 
 // This takes a space separated list and zero terminates it in place
@@ -423,7 +438,7 @@ static void pni_split_mechs(char *mechlist, const char* included_mechs,
char *me
     if (*end == ' ') {
       if (start != end) {
         *end = '\0';
-        if (pni_sasl_included_mech(included_mechs, pn_bytes(end-start, start))) {
+        if (pni_sasl_server_included_mech(included_mechs, pn_bytes(end-start, start))) {
           mechs[(*count)++] = start;
         }
       }
@@ -435,7 +450,7 @@ static void pni_split_mechs(char *mechlist, const char* included_mechs,
char *me
   }
 
   if (start != end) {
-    if (pni_sasl_included_mech(included_mechs, pn_bytes(end-start, start))) {
+    if (pni_sasl_server_included_mech(included_mechs, pn_bytes(end-start, start))) {
       mechs[(*count)++] = start;
     }
   }
@@ -836,7 +851,7 @@ int pn_do_init(pn_transport_t *transport, uint8_t frame_type, uint16_t
channel,
   // 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)) {
+  if (!pni_sasl_server_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);
@@ -865,7 +880,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 (pni_sasl_included_mech(sasl->included_mechanisms, s)) {
+      if (pni_sasl_client_included_mech(sasl->included_mechanisms, s)) {
         pn_string_addf(mechs, "%*s ", (int)s.size, s.start);
       }
     }
@@ -880,7 +895,7 @@ 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;
 
-    if (pni_sasl_included_mech(sasl->included_mechanisms, symbol)) {
+    if (pni_sasl_client_included_mech(sasl->included_mechanisms, symbol)) {
       pn_string_setn(mechs, symbol.start, symbol.size);
     }
   }


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


Mime
View raw message