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 2020/08/31 23:58:59 UTC

[qpid-proton] branch master updated (727fca0 -> 9a43b8b)

This is an automated email from the ASF dual-hosted git repository.

astitcher pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git.


    from 727fca0  PROTON-2270 remove explicit threaderciser test timeout; set a global one for Travis (#270)
     new 51158fa  PROTON-2234: Allow setting and retrieving SASL authzid on clients/servers
     new 1d29ac1  PROTON-2234: Python binding to SASL authzid setting - Python unit tests
     new 9a43b8b  PROTON-2257: Tidy up/finalize SASL plugin API - Make the API naming more consistent after review - Add extra argument to outcome procesing for the optional binary data

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 c/CMakeLists.txt                                  |  2 +-
 c/docs/user.doxygen.in                            |  2 +-
 c/include/proton/connection.h                     | 25 +++++++
 c/include/proton/sasl.h                           | 20 ++++++
 c/include/proton/{sasl-plugin.h => sasl_plugin.h} | 26 +++----
 c/src/core/engine-internal.h                      |  1 +
 c/src/core/engine.c                               | 14 ++++
 c/src/core/transport.c                            |  7 +-
 c/src/sasl/cyrus_sasl.c                           | 78 ++++++++++++++------
 c/src/sasl/cyrus_stub.c                           |  2 +-
 c/src/sasl/default_sasl.c                         | 47 +++++++++----
 c/src/sasl/sasl-internal.h                        |  5 +-
 c/src/sasl/sasl.c                                 | 86 +++++++++++++++--------
 python/proton/_endpoints.py                       | 25 ++++++-
 python/proton/_transport.py                       | 27 ++++++-
 python/tests/proton_tests/sasl.py                 | 53 +++++++++++++-
 16 files changed, 328 insertions(+), 92 deletions(-)
 rename c/include/proton/{sasl-plugin.h => sasl_plugin.h} (85%)


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


[qpid-proton] 01/03: PROTON-2234: Allow setting and retrieving SASL authzid on clients/servers

Posted by as...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

astitcher pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git

commit 51158fa5742b841f09ae5db5c517afc0c74356ec
Author: Andrew Stitcher <as...@apache.org>
AuthorDate: Tue Jul 14 18:15:14 2020 -0400

    PROTON-2234: Allow setting and retrieving SASL authzid on clients/servers
---
 c/include/proton/connection.h  | 25 +++++++++++++++++++++
 c/include/proton/sasl-plugin.h |  3 ++-
 c/include/proton/sasl.h        | 20 +++++++++++++++++
 c/src/core/engine-internal.h   |  1 +
 c/src/core/engine.c            | 14 ++++++++++++
 c/src/core/transport.c         |  7 ++++--
 c/src/sasl/cyrus_sasl.c        | 49 +++++++++++++++++++++++++++++++++++-------
 c/src/sasl/default_sasl.c      | 37 ++++++++++++++++++++++---------
 c/src/sasl/sasl-internal.h     |  3 ++-
 c/src/sasl/sasl.c              | 27 +++++++++++++++++++----
 10 files changed, 160 insertions(+), 26 deletions(-)

diff --git a/c/include/proton/connection.h b/c/include/proton/connection.h
index 9814828..73de72d 100644
--- a/c/include/proton/connection.h
+++ b/c/include/proton/connection.h
@@ -322,6 +322,23 @@ PN_EXTERN void pn_connection_set_user(pn_connection_t *connection, const char *u
 PN_EXTERN void pn_connection_set_password(pn_connection_t *connection, const char *password);
 
 /**
+ * Set the authorization id for a client connection
+ *
+ * It is necessary to set the authorization before binding the connection
+ * to a transport and it isn't allowed to change it after the binding.
+ *
+ * This is only useful if the negotiated sasl mechanism supports transporting an authorization id separate from
+ * the authentication user.
+ *
+ * By default all mechanisms will consider the requested authorization as identical to the authentication if it
+ * is not supplied.
+ *
+ * @param[in] connection the connection
+ * @param[in] authzid the authorization id
+ */
+PN_EXTERN void pn_connection_set_authorization(pn_connection_t *connection, const char *authzid);
+
+/**
  * Get the authentication username for a client connection
  *
  * @param[in] connection the connection
@@ -330,6 +347,14 @@ PN_EXTERN void pn_connection_set_password(pn_connection_t *connection, const cha
 PN_EXTERN const char *pn_connection_get_user(pn_connection_t *connection);
 
 /**
+ * Get the authorization id for a client connection
+ *
+ * @param[in] connection the connection
+ * @return the authorization passed into the connection
+ */
+PN_EXTERN const char *pn_connection_get_authorization(pn_connection_t *connection);
+
+/**
  * Get the value of the AMQP Hostname used by a connection object.
  *
  * The pointer returned by this operation is valid until
diff --git a/c/include/proton/sasl-plugin.h b/c/include/proton/sasl-plugin.h
index ae6b0ec..1269427 100644
--- a/c/include/proton/sasl-plugin.h
+++ b/c/include/proton/sasl-plugin.h
@@ -123,6 +123,7 @@ PN_EXTERN int   pnx_sasl_get_external_ssf(pn_transport_t *transport);
 
 PN_EXTERN const char *pnx_sasl_get_username(pn_transport_t *transport);
 PN_EXTERN const char *pnx_sasl_get_password(pn_transport_t *transport);
+PN_EXTERN const char *pnx_sasl_get_authorization(pn_transport_t *transport);
 PN_EXTERN void  pnx_sasl_clear_password(pn_transport_t *transport);
 PN_EXTERN const char *pnx_sasl_get_remote_fqdn(pn_transport_t *transport);
 PN_EXTERN const char *pnx_sasl_get_selected_mechanism(pn_transport_t *transport);
@@ -131,7 +132,7 @@ PN_EXTERN void  pnx_sasl_set_bytes_out(pn_transport_t *transport, pn_bytes_t byt
 PN_EXTERN void  pnx_sasl_set_desired_state(pn_transport_t *transport, enum pnx_sasl_state desired_state);
 PN_EXTERN void  pnx_sasl_set_selected_mechanism(pn_transport_t *transport, const char *mechanism);
 PN_EXTERN void  pnx_sasl_set_local_hostname(pn_transport_t * transport, const char * fqdn);
-PN_EXTERN void  pnx_sasl_succeed_authentication(pn_transport_t *transport, const char *username);
+PN_EXTERN void  pnx_sasl_succeed_authentication(pn_transport_t *transport, const char *username, const char *authzid);
 PN_EXTERN void  pnx_sasl_fail_authentication(pn_transport_t *transport);
 
 PN_EXTERN void  pnx_sasl_set_implementation(pn_transport_t *transport, const pnx_sasl_implementation *impl, void *context);
diff --git a/c/include/proton/sasl.h b/c/include/proton/sasl.h
index 251471e..906b0b3 100644
--- a/c/include/proton/sasl.h
+++ b/c/include/proton/sasl.h
@@ -125,6 +125,26 @@ PN_EXTERN pn_sasl_outcome_t pn_sasl_outcome(pn_sasl_t *sasl);
 PN_EXTERN const char *pn_sasl_get_user(pn_sasl_t *sasl);
 
 /**
+ * Retrieve the authorization id
+ *
+ * This is usually used at the the server end to find the name of the requested authorization id.
+ * On the client it will merely return whatever was passed in to the
+ * pn_transport_set_authorization() API.
+ *
+ * If pn_sasl_outcome() returns a value other than PN_SASL_OK, then there will be no user to return.
+ * The returned value is only reliable after the PN_TRANSPORT_AUTHENTICATED event has been received.
+ *
+ * @param[in] sasl the sasl layer
+ *
+ * @return
+ * If the SASL layer was not negotiated then 0 is returned
+ * If the ANONYMOUS mechanism is used then 0 is returned
+ * If no authorization id was requested then 0 is returned
+ * Otherwise a string containing the requested authorization id is returned.
+ */
+PN_EXTERN const char *pn_sasl_get_authorization(pn_sasl_t *sasl);
+
+/**
  * Return the selected SASL mechanism
  *
  * The returned value is only reliable after the PN_TRANSPORT_AUTHENTICATED event has been received.
diff --git a/c/src/core/engine-internal.h b/c/src/core/engine-internal.h
index 9dbc919..11718c9 100644
--- a/c/src/core/engine-internal.h
+++ b/c/src/core/engine-internal.h
@@ -243,6 +243,7 @@ struct pn_connection_t {
   pn_string_t *container;
   pn_string_t *hostname;
   pn_string_t *auth_user;
+  pn_string_t *authzid;
   pn_string_t *auth_password;
   pn_data_t *offered_capabilities;
   pn_data_t *desired_capabilities;
diff --git a/c/src/core/engine.c b/c/src/core/engine.c
index 8300e42..bfc8613 100644
--- a/c/src/core/engine.c
+++ b/c/src/core/engine.c
@@ -499,6 +499,7 @@ static void pn_connection_finalize(void *object)
   pn_free(conn->container);
   pn_free(conn->hostname);
   pn_free(conn->auth_user);
+  pn_free(conn->authzid);
   pn_free(conn->auth_password);
   pn_free(conn->offered_capabilities);
   pn_free(conn->desired_capabilities);
@@ -533,6 +534,7 @@ pn_connection_t *pn_connection()
   conn->container = pn_string(NULL);
   conn->hostname = pn_string(NULL);
   conn->auth_user = pn_string(NULL);
+  conn->authzid = pn_string(NULL);
   conn->auth_password = pn_string(NULL);
   conn->offered_capabilities = pn_data(0);
   conn->desired_capabilities = pn_data(0);
@@ -608,6 +610,18 @@ void pn_connection_set_user(pn_connection_t *connection, const char *user)
     pn_string_set(connection->auth_user, user);
 }
 
+const char *pn_connection_get_authorization(pn_connection_t *connection)
+{
+  assert(connection);
+  return pn_string_get(connection->authzid);
+}
+
+void pn_connection_set_authorization(pn_connection_t *connection, const char *authzid)
+{
+  assert(connection);
+  pn_string_set(connection->authzid, authzid);
+}
+
 void pn_connection_set_password(pn_connection_t *connection, const char *password)
 {
     assert(connection);
diff --git a/c/src/core/transport.c b/c/src/core/transport.c
index c2f1c05..fe6ebf1 100644
--- a/c/src/core/transport.c
+++ b/c/src/core/transport.c
@@ -695,9 +695,12 @@ int pn_transport_bind(pn_transport_t *transport, pn_connection_t *connection)
   pn_connection_bound(connection);
 
   // set the hostname/user/password
-  if (pn_string_size(connection->auth_user)) {
+  if (pn_string_size(connection->auth_user) || pn_string_size(connection->authzid)) {
     pn_sasl(transport);
-    pni_sasl_set_user_password(transport, pn_string_get(connection->auth_user), pn_string_get(connection->auth_password));
+    pni_sasl_set_user_password(transport,
+                               pn_string_get(connection->auth_user),
+                               pn_string_get(connection->authzid),
+                               pn_string_get(connection->auth_password));
   }
 
   if (pn_string_size(connection->hostname)) {
diff --git a/c/src/sasl/cyrus_sasl.c b/c/src/sasl/cyrus_sasl.c
index d98f3a3..fd189c0 100644
--- a/c/src/sasl/cyrus_sasl.c
+++ b/c/src/sasl/cyrus_sasl.c
@@ -118,7 +118,12 @@ 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:
+    case SASL_CB_USER: {
+      const char *authzid = pnx_sasl_get_authorization(transport);
+      i->result = authzid;
+      i->len = authzid ? strlen(authzid) : 0;
+      break;
+    }
     case SASL_CB_AUTHNAME: {
       const char *username = pnx_sasl_get_username(transport);
       i->result = username;
@@ -158,8 +163,32 @@ static const sasl_callback_t pni_user_password_callbacks[] = {
 };
 
 static const sasl_callback_t pni_user_callbacks[] = {
-    {SASL_CB_USER, NULL, NULL},
-    {SASL_CB_AUTHNAME, NULL, NULL},
+  {SASL_CB_USER, NULL, NULL},
+  {SASL_CB_AUTHNAME, NULL, NULL},
+  {SASL_CB_LIST_END, NULL, NULL},
+};
+
+static const sasl_callback_t pni_authzid_callbacks[] = {
+  {SASL_CB_USER, NULL, NULL},
+  {SASL_CB_LIST_END, NULL, NULL},
+};
+
+static int pni_authorize(sasl_conn_t *conn,
+    void *context,
+    const char *requested_user, unsigned rlen,
+    const char *auth_identity, unsigned alen,
+    const char *def_realm, unsigned urlen,
+    struct propctx *propctx)
+{
+  PN_LOG_DEFAULT(PN_SUBSYSTEM_SASL, PN_LEVEL_TRACE, "Authorized: userid=%*s by authuser=%*s @ %*s",
+    rlen, requested_user,
+    alen, auth_identity,
+    urlen, def_realm);
+  return SASL_OK;
+}
+
+static const sasl_callback_t pni_server_callbacks[] = {
+    {SASL_CB_PROXY_POLICY, (int(*)(void)) pni_authorize, NULL},
     {SASL_CB_LIST_END, NULL, NULL},
 };
 
@@ -234,7 +263,7 @@ static void pni_cyrus_server_once(void) {
     }
   }
   if (result==SASL_OK) {
-    result = sasl_server_init(NULL, pni_cyrus_config_name ? pni_cyrus_config_name : default_config_name);
+    result = sasl_server_init(pni_server_callbacks, pni_cyrus_config_name ? pni_cyrus_config_name : default_config_name);
   }
   pni_cyrus_server_started = true;
   pni_cyrus_server_init_rc = result;
@@ -263,7 +292,8 @@ bool cyrus_sasl_init_client(pn_transport_t* transport) {
     if (result!=SASL_OK) break;
 
     const sasl_callback_t *callbacks =
-      pnx_sasl_get_username(transport) ? pnx_sasl_get_password(transport) ? pni_user_password_callbacks : pni_user_callbacks : NULL;
+      pnx_sasl_get_username(transport) ? (pnx_sasl_get_password(transport) ? pni_user_password_callbacks : pni_user_callbacks) :
+      pnx_sasl_get_authorization(transport) ? pni_authzid_callbacks : NULL;
     result = sasl_client_new(amqp_service,
                              pnx_sasl_get_remote_fqdn(transport),
                              NULL, NULL,
@@ -466,9 +496,12 @@ static void pni_process_server_result(pn_transport_t *transport, int result)
         case SASL_OK: {
             // Authenticated
             // Get username from SASL
-            const void* value;
-            sasl_getprop(cyrus_conn, SASL_USERNAME, &value);
-            pnx_sasl_succeed_authentication(transport, (const char*) value);
+            const void* authcid;
+            sasl_getprop(cyrus_conn, SASL_AUTHUSER, &authcid);
+            // Get authzid from SASL
+            const void* authzid;
+            sasl_getprop(cyrus_conn, SASL_USERNAME, &authzid);
+            pnx_sasl_succeed_authentication(transport, (const char*) authcid, (const char*) authzid);
             pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
             break;
         }
diff --git a/c/src/sasl/default_sasl.c b/c/src/sasl/default_sasl.c
index 7299318..287bf06 100644
--- a/c/src/sasl/default_sasl.c
+++ b/c/src/sasl/default_sasl.c
@@ -98,6 +98,7 @@ bool default_sasl_process_mechanisms(pn_transport_t *transport, const char *mech
 {
   const char *username = pnx_sasl_get_username(transport);
   const char *password = pnx_sasl_get_password(transport);
+  const char *authzid  = pnx_sasl_get_authorization(transport);
 
   // Check whether offered EXTERNAL, PLAIN or ANONYMOUS
   // Look for "EXTERNAL" in mechs
@@ -105,14 +106,14 @@ bool default_sasl_process_mechanisms(pn_transport_t *transport, const char *mech
   // 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);
+    if (authzid) {
+      size_t size = strlen(authzid);
       char *iresp = (char *) malloc(size);
       if (!iresp) return false;
 
       pnx_sasl_set_context(transport, iresp);
 
-      memmove(iresp, username, size);
+      memmove(iresp, authzid, size);
       pnx_sasl_set_bytes_out(transport, pn_bytes(size, iresp));
     } else {
       static const char empty[] = "";
@@ -130,18 +131,20 @@ bool default_sasl_process_mechanisms(pn_transport_t *transport, const char *mech
       (pnx_sasl_is_transport_encrypted(transport) || pnx_sasl_get_allow_insecure_mechs(transport)) &&
       username && password) {
     pnx_sasl_set_selected_mechanism(transport, PLAIN);
+    size_t zsize = authzid ? strlen(authzid) : 0;
     size_t usize = strlen(username);
     size_t psize = strlen(password);
-    size_t size = usize + psize + 2;
+    size_t size = zsize + usize + psize + 2;
     char *iresp = (char *) malloc(size);
     if (!iresp) return false;
 
     pnx_sasl_set_context(transport, iresp);
 
-    iresp[0] = 0;
-    memmove(&iresp[1], username, usize);
-    iresp[usize + 1] = 0;
-    memmove(&iresp[usize + 2], password, psize);
+    if (authzid) memmove(&iresp[0], authzid, zsize);
+    iresp[zsize] = 0;
+    memmove(&iresp[zsize + 1], username, usize);
+    iresp[zsize + usize + 1] = 0;
+    memmove(&iresp[zsize + usize + 2], password, psize);
     pnx_sasl_set_bytes_out(transport, pn_bytes(size, iresp));
 
     // Zero out password and dealloc
@@ -191,7 +194,7 @@ void default_sasl_process_init(pn_transport_t *transport, const char *mechanism,
 {
   // Check that mechanism is ANONYMOUS
   if (strcmp(mechanism, ANONYMOUS)==0) {
-    pnx_sasl_succeed_authentication(transport, "anonymous");
+    pnx_sasl_succeed_authentication(transport, "anonymous", "anonymous");
     pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
     return;
   }
@@ -199,7 +202,21 @@ 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 && ext_username) {
-    pnx_sasl_succeed_authentication(transport, ext_username);
+
+    char *authzid = NULL;
+
+    if (recv->size) {
+      authzid = (char *) malloc(recv->size+1);
+      pnx_sasl_set_context(transport, authzid);
+
+      // If we didn't get memory pretend no authzid
+      if (authzid) {
+        memcpy(&authzid[0], recv->start, recv->size);
+        authzid[recv->size] = 0;
+      }
+    }
+
+    pnx_sasl_succeed_authentication(transport, ext_username, authzid ? authzid : ext_username);
     pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
     return;
   }
diff --git a/c/src/sasl/sasl-internal.h b/c/src/sasl/sasl-internal.h
index aade2e0..82ff85a 100644
--- a/c/src/sasl/sasl-internal.h
+++ b/c/src/sasl/sasl-internal.h
@@ -34,7 +34,7 @@ extern const pnx_sasl_implementation * const cyrus_sasl_impl;
 
 // SASL APIs used by transport code
 void pn_sasl_free(pn_transport_t *transport);
-void pni_sasl_set_user_password(pn_transport_t *transport, const char *user, const char *password);
+void pni_sasl_set_user_password(pn_transport_t *transport, const char *user, const char *authzid, const char *password);
 void pni_sasl_set_remote_hostname(pn_transport_t *transport, const char* fqdn);
 void pni_sasl_set_external_security(pn_transport_t *transport, int ssf, const char *authid);
 
@@ -44,6 +44,7 @@ struct pni_sasl_t {
   char *selected_mechanism;
   char *included_mechanisms;
   const char *username;
+  const char *authzid;
   char *password;
   const char *remote_fqdn;
   char *local_fqdn;
diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c
index b901e9f..500b286 100644
--- a/c/src/sasl/sasl.c
+++ b/c/src/sasl/sasl.c
@@ -95,6 +95,11 @@ const char *pnx_sasl_get_username(pn_transport_t *transport)
   return transport->sasl ? transport->sasl->username : NULL;
 }
 
+const char *pnx_sasl_get_authorization(pn_transport_t *transport)
+{
+  return transport->sasl ? transport->sasl->authzid : NULL;
+}
+
 const char *pnx_sasl_get_external_username(pn_transport_t *transport)
 {
   return transport->sasl ? transport->sasl->external_auth : NULL;
@@ -143,15 +148,21 @@ void  pnx_sasl_set_selected_mechanism(pn_transport_t *transport, const char *mec
   }
 }
 
-void  pnx_sasl_succeed_authentication(pn_transport_t *transport, const char *username)
+void  pnx_sasl_succeed_authentication(pn_transport_t *transport, const char *username, const char *authzid)
 {
   if (transport->sasl) {
     transport->sasl->username = username;
+    transport->sasl->authzid = authzid;
     transport->sasl->outcome = PN_SASL_OK;
     transport->authenticated = true;
 
-    PN_LOG(&transport->logger, PN_SUBSYSTEM_SASL, PN_LEVEL_INFO, "Authenticated user: %s with mechanism %s",
-                  username, transport->sasl->selected_mechanism);
+    if (authzid) {
+      PN_LOG(&transport->logger, PN_SUBSYSTEM_SASL, PN_LEVEL_INFO, "Authenticated user: %s for %s with mechanism %s",
+             username, authzid, transport->sasl->selected_mechanism);
+    } else {
+      PN_LOG(&transport->logger, PN_SUBSYSTEM_SASL, PN_LEVEL_INFO, "Authenticated user: %s with mechanism %s",
+             username, transport->sasl->selected_mechanism);
+    }
   }
 }
 
@@ -729,6 +740,7 @@ pn_sasl_t *pn_sasl(pn_transport_t *transport)
     sasl->selected_mechanism = NULL;
     sasl->included_mechanisms = NULL;
     sasl->username = NULL;
+    sasl->authzid = NULL;
     sasl->password = NULL;
     sasl->remote_fqdn = NULL;
     sasl->local_fqdn = NULL;
@@ -783,10 +795,11 @@ void pnx_sasl_set_local_hostname(pn_transport_t * transport, const char * fqdn)
   sasl->local_fqdn = pn_strdup(fqdn);
 }
 
-void pni_sasl_set_user_password(pn_transport_t *transport, const char *user, const char *password)
+void pni_sasl_set_user_password(pn_transport_t *transport, const char *user, const char *authzid, const char *password)
 {
   pni_sasl_t *sasl = transport->sasl;
   sasl->username = user;
+  sasl->authzid = authzid;
   free(sasl->password);
   sasl->password = password ? pn_strdup(password) : NULL;
 }
@@ -805,6 +818,12 @@ const char *pn_sasl_get_user(pn_sasl_t *sasl0)
     return sasl->username;
 }
 
+const char *pn_sasl_get_authorization(pn_sasl_t *sasl0)
+{
+    pni_sasl_t *sasl = get_sasl_internal(sasl0);
+    return sasl->authzid;
+}
+
 const char *pn_sasl_get_mech(pn_sasl_t *sasl0)
 {
     pni_sasl_t *sasl = get_sasl_internal(sasl0);


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


[qpid-proton] 02/03: PROTON-2234: Python binding to SASL authzid setting - Python unit tests

Posted by as...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

astitcher pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git

commit 1d29ac14e24981c530ae1f9ea1dbfeffc9727518
Author: Andrew Stitcher <as...@apache.org>
AuthorDate: Wed Aug 5 22:54:07 2020 -0400

    PROTON-2234: Python binding to SASL authzid setting
    - Python unit tests
---
 python/proton/_endpoints.py       | 25 ++++++++++++++++--
 python/proton/_transport.py       | 27 +++++++++++++++++++-
 python/tests/proton_tests/sasl.py | 53 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/python/proton/_endpoints.py b/python/proton/_endpoints.py
index e528de6..c5309ff 100644
--- a/python/proton/_endpoints.py
+++ b/python/proton/_endpoints.py
@@ -31,10 +31,12 @@ from cproton import PN_CONFIGURATION, PN_COORDINATOR, PN_DELIVERIES, PN_DIST_MOD
     PN_RCV_SECOND, PN_REMOTE_ACTIVE, PN_REMOTE_CLOSED, PN_REMOTE_UNINIT, PN_SND_MIXED, PN_SND_SETTLED, PN_SND_UNSETTLED, \
     PN_SOURCE, PN_TARGET, PN_UNSPECIFIED, pn_connection, pn_connection_attachments, pn_connection_close, \
     pn_connection_collect, pn_connection_condition, pn_connection_desired_capabilities, pn_connection_error, \
-    pn_connection_get_container, pn_connection_get_hostname, pn_connection_get_user, pn_connection_offered_capabilities, \
+    pn_connection_get_authorization, pn_connection_get_container, pn_connection_get_hostname, pn_connection_get_user, \
+    pn_connection_offered_capabilities, \
     pn_connection_open, pn_connection_properties, pn_connection_release, pn_connection_remote_condition, \
     pn_connection_remote_container, pn_connection_remote_desired_capabilities, pn_connection_remote_hostname, \
-    pn_connection_remote_offered_capabilities, pn_connection_remote_properties, pn_connection_set_container, \
+    pn_connection_remote_offered_capabilities, pn_connection_remote_properties, \
+    pn_connection_set_authorization, pn_connection_set_container, \
     pn_connection_set_hostname, pn_connection_set_password, pn_connection_set_user, pn_connection_state, \
     pn_connection_transport, pn_delivery, pn_error_code, pn_error_text, pn_link_advance, pn_link_attachments, \
     pn_link_available, pn_link_close, pn_link_condition, pn_link_credit, pn_link_current, pn_link_detach, pn_link_drain, \
@@ -266,6 +268,25 @@ class Connection(Wrapper, Endpoint):
         :type: ``str``
         """)
 
+    def _get_authorization(self):
+        return utf82unicode(pn_connection_get_authorization(self._impl))
+
+    def _set_authorization(self, name):
+        pn_connection_set_authorization(self._impl, unicode2utf8(name))
+
+    authorization = property(_get_authorization, _set_authorization, doc="""
+        The authorization username for a client connection.
+
+        It is necessary to set the authorization before binding
+        the connection to a transport and it isn't allowed to change
+        after the binding.
+
+        If not set then implicitly the requested authorization is the same as the
+        authentication user.
+
+        :type: ``str``
+        """)
+
     def _get_password(self):
         return None
 
diff --git a/python/proton/_transport.py b/python/proton/_transport.py
index 970ef2e..fa2fe89 100644
--- a/python/proton/_transport.py
+++ b/python/proton/_transport.py
@@ -26,7 +26,7 @@ from cproton import PN_EOS, PN_OK, PN_SASL_AUTH, PN_SASL_NONE, PN_SASL_OK, PN_SA
     PN_SSL_RESUME_REUSED, PN_SSL_RESUME_UNKNOWN, PN_SSL_SHA1, PN_SSL_SHA256, PN_SSL_SHA512, PN_SSL_VERIFY_PEER, \
     PN_SSL_VERIFY_PEER_NAME, PN_TRACE_DRV, PN_TRACE_FRM, PN_TRACE_OFF, PN_TRACE_RAW, pn_error_text, pn_sasl, \
     pn_sasl_allowed_mechs, pn_sasl_config_name, pn_sasl_config_path, pn_sasl_done, pn_sasl_extended, \
-    pn_sasl_get_allow_insecure_mechs, pn_sasl_get_mech, pn_sasl_get_user, pn_sasl_outcome, \
+    pn_sasl_get_allow_insecure_mechs, pn_sasl_get_mech, pn_sasl_get_user, pn_sasl_get_authorization, pn_sasl_outcome, \
     pn_sasl_set_allow_insecure_mechs, pn_ssl, pn_ssl_domain, pn_ssl_domain_allow_unsecured_client, pn_ssl_domain_free, \
     pn_ssl_domain_set_credentials, pn_ssl_domain_set_peer_authentication, pn_ssl_domain_set_trusted_ca_db, \
     pn_ssl_get_cert_fingerprint, pn_ssl_get_cipher_name, pn_ssl_get_peer_hostname, pn_ssl_get_protocol_name, \
@@ -608,6 +608,31 @@ class SASL(Wrapper):
         return pn_sasl_get_user(self._sasl)
 
     @property
+    def authorization(self):
+        """
+        Retrieve the requested authorization user. This is usually used at the the
+        server end to find the name of any requested authorization user.
+
+        If the peer has not requested an authorization user or the SASL mechanism has
+        no capability to transport an authorization id this will be the same as the
+        authenticated user.
+
+        Note that it is the role of the server to ensure that the authenticated user is
+        actually allowed to act as the requested authorization user.
+
+        If :meth:`outcome` returns a value other than :const:`OK`, then
+        there will be no user to return. The returned value is only reliable
+        after the ``PN_TRANSPORT_AUTHENTICATED`` event has been received.
+
+        :rtype: * If the SASL layer was not negotiated then ``0`` is returned.
+                * If the ``ANONYMOUS`` mechanism is used then the user will be
+                  ``"anonymous"``.
+                * Otherwise a string containing the user is
+                  returned.
+        """
+        return pn_sasl_get_authorization(self._sasl)
+
+    @property
     def mech(self):
         """
         Return the selected SASL mechanism.
diff --git a/python/tests/proton_tests/sasl.py b/python/tests/proton_tests/sasl.py
index a288536..7125c17 100644
--- a/python/tests/proton_tests/sasl.py
+++ b/python/tests/proton_tests/sasl.py
@@ -40,7 +40,7 @@ def _sslCertpath(file):
     return os.path.join(os.path.dirname(__file__),
                         "ssl_db/%s" % file)
 
-def _testSaslMech(self, mech, clientUser='user@proton', authUser='user@proton', encrypted=False, authenticated=True):
+def _testSaslMech(self, mech, clientUser='user@proton', authUser='user@proton', authzid=None, encrypted=False, authenticated=True):
   self.s1.allowed_mechs(mech)
   self.c1.open()
   self.c2.open()
@@ -51,12 +51,16 @@ def _testSaslMech(self, mech, clientUser='user@proton', authUser='user@proton',
     assert self.t2.encrypted == encrypted, encrypted
     assert self.t1.encrypted == encrypted, encrypted
 
+  if authzid is None:
+    authzid = authUser
+
   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.authorization == authzid, self.s2.authorization
     assert self.s2.mech == mech.strip()
     assert self.s2.outcome == SASL.OK, self.s2.outcome
     assert self.c2.state & Endpoint.LOCAL_ACTIVE and self.c2.state & Endpoint.REMOTE_ACTIVE,\
@@ -72,6 +76,7 @@ def _testSaslMech(self, mech, clientUser='user@proton', authUser='user@proton',
     # Server
     assert self.t2.user == None
     assert self.s2.user == None
+    assert self.s2.authorization == None, self.s2.authorization
     assert self.s2.outcome != SASL.OK, self.s2.outcome
     # Client
     assert self.t1.user == clientUser
@@ -401,6 +406,25 @@ class SSLSASLTest(Test):
 
     _testSaslMech(self, mech, encrypted=True)
 
+  def testSSLPlainAuthzid(self):
+    if not SASL.extended():
+      raise Skipped("Simple SASL server does not support PLAIN")
+    common.ensureCanTestExtendedSASL()
+
+    clientUser = 'user@proton'
+    authzid = 'user2'
+    mech = 'PLAIN'
+
+    self.c1.user = clientUser
+    self.c1.authorization = authzid
+    self.c1.password = 'password'
+    self.c1.hostname = 'localhost'
+
+    ssl1 = _sslConnection(self.client_domain, self.t1, self.c1)
+    ssl2 = _sslConnection(self.server_domain, self.t2, self.c2)
+
+    _testSaslMech(self, mech, authzid=authzid, encrypted=True)
+
   def testSSLPlainSimpleFail(self):
     if not SASL.extended():
       raise Skipped("Simple SASL server does not support PLAIN")
@@ -442,6 +466,33 @@ class SSLSASLTest(Test):
 
     _testSaslMech(self, mech, clientUser=None, authUser=extUser, encrypted=True)
 
+  def testSSLExternalAuthzid(self):
+    if os.name=="nt":
+      extUser = 'O=Client, CN=127.0.0.1'
+    else:
+      extUser = 'O=Client,CN=127.0.0.1'
+    mech = 'EXTERNAL'
+    authzid = 'user_foo'
+
+    self.c1.authorization = authzid
+
+    self.server_domain.set_credentials(_sslCertpath("server-certificate.pem"),
+                                       _sslCertpath("server-private-key.pem"),
+                                       "server-password")
+    self.server_domain.set_trusted_ca_db(_sslCertpath("ca-certificate.pem"))
+    self.server_domain.set_peer_authentication(SSLDomain.VERIFY_PEER,
+                                               _sslCertpath("ca-certificate.pem") )
+    self.client_domain.set_credentials(_sslCertpath("client-certificate.pem"),
+                                       _sslCertpath("client-private-key.pem"),
+                                       "client-password")
+    self.client_domain.set_trusted_ca_db(_sslCertpath("ca-certificate.pem"))
+    self.client_domain.set_peer_authentication(SSLDomain.VERIFY_PEER)
+
+    ssl1 = _sslConnection(self.client_domain, self.t1, self.c1)
+    ssl2 = _sslConnection(self.server_domain, self.t2, self.c2)
+
+    _testSaslMech(self, mech, clientUser=None, authUser=extUser, authzid=authzid, encrypted=True)
+
   def testSSLExternalSimpleFail(self):
     mech = 'EXTERNAL'
 


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


[qpid-proton] 03/03: PROTON-2257: Tidy up/finalize SASL plugin API - Make the API naming more consistent after review - Add extra argument to outcome procesing for the optional binary data

Posted by as...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

astitcher pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git

commit 9a43b8b78be81c3c24aaf4ef445e5b64eb78fd01
Author: Andrew Stitcher <as...@apache.org>
AuthorDate: Fri Jul 31 18:54:34 2020 -0400

    PROTON-2257: Tidy up/finalize SASL plugin API
    - Make the API naming more consistent after review
    - Add extra argument to outcome procesing for the optional binary data
---
 c/CMakeLists.txt                                  |  2 +-
 c/docs/user.doxygen.in                            |  2 +-
 c/include/proton/{sasl-plugin.h => sasl_plugin.h} | 25 +++++-----
 c/src/sasl/cyrus_sasl.c                           | 31 ++++++------
 c/src/sasl/cyrus_stub.c                           |  2 +-
 c/src/sasl/default_sasl.c                         | 14 +++---
 c/src/sasl/sasl-internal.h                        |  2 +-
 c/src/sasl/sasl.c                                 | 61 ++++++++++++-----------
 8 files changed, 72 insertions(+), 67 deletions(-)

diff --git a/c/CMakeLists.txt b/c/CMakeLists.txt
index 17d9423..d86eab4 100644
--- a/c/CMakeLists.txt
+++ b/c/CMakeLists.txt
@@ -304,7 +304,7 @@ set (qpid-proton-include
   include/proton/proactor.h
   include/proton/raw_connection.h
   include/proton/sasl.h
-  include/proton/sasl-plugin.h
+  include/proton/sasl_plugin.h
   include/proton/session.h
   include/proton/ssl.h
   include/proton/terminus.h
diff --git a/c/docs/user.doxygen.in b/c/docs/user.doxygen.in
index 8034e6c..62f114a 100644
--- a/c/docs/user.doxygen.in
+++ b/c/docs/user.doxygen.in
@@ -66,7 +66,7 @@ EXCLUDE_PATTERNS        = @CMAKE_SOURCE_DIR@/c/examples/*.c \
                           @CMAKE_SOURCE_DIR@/c/include/proton/log.h \
                           @CMAKE_SOURCE_DIR@/c/include/proton/object.h \
                           @CMAKE_SOURCE_DIR@/c/include/proton/reactor.h \
-                          @CMAKE_SOURCE_DIR@/c/include/proton/sasl-plugin.h \
+                          @CMAKE_SOURCE_DIR@/c/include/proton/sasl_plugin.h \
                           @CMAKE_SOURCE_DIR@/c/include/proton/selectable.h \
                           @CMAKE_SOURCE_DIR@/c/include/proton/type_compat.h
 FULL_PATH_NAMES         = YES
diff --git a/c/include/proton/sasl-plugin.h b/c/include/proton/sasl_plugin.h
similarity index 86%
rename from c/include/proton/sasl-plugin.h
rename to c/include/proton/sasl_plugin.h
index 1269427..c6b6015 100644
--- a/c/include/proton/sasl-plugin.h
+++ b/c/include/proton/sasl_plugin.h
@@ -23,6 +23,7 @@
  */
 
 #include <proton/import_export.h>
+#include <proton/logger.h>
 #include <proton/type_compat.h>
 #include <proton/types.h>
 
@@ -41,7 +42,7 @@ extern "C" {
   Return space separated list of supported mechanisms (client and server)
   If the returned string is dynamically allocated by the SASL implemetation
   it must stay valid until the free entry point is called.
-  const char *list_mechs(pn_transport_t *transport);
+  const char *list_mechanisms(pn_transport_t *transport);
 
   Initialise for either client or server (can't call both for a
   given transport/connection):
@@ -72,7 +73,7 @@ typedef struct pnx_sasl_implementation
 {
     void (*free)(pn_transport_t *transport);
 
-    const char*  (*list_mechs)(pn_transport_t *transport);
+    const char*  (*list_mechanisms)(pn_transport_t *transport);
 
     bool (*init_server)(pn_transport_t *transport);
     bool (*init_client)(pn_transport_t *transport);
@@ -84,7 +85,7 @@ typedef struct pnx_sasl_implementation
 
     bool (*process_mechanisms)(pn_transport_t *transport, const char *mechs);
     void (*process_challenge)(pn_transport_t *transport, const pn_bytes_t *recv);
-    void (*process_outcome)(pn_transport_t *transport);
+    void (*process_outcome)(pn_transport_t *transport, const pn_bytes_t *recv);
 
     bool    (*can_encrypt)(pn_transport_t *transport);
     ssize_t (*max_encrypt_size)(pn_transport_t *transport);
@@ -100,24 +101,24 @@ enum pnx_sasl_state {
   SASL_POSTED_MECHANISMS,
   SASL_POSTED_RESPONSE,
   SASL_POSTED_CHALLENGE,
-  SASL_RECVED_OUTCOME_SUCCEED,
-  SASL_RECVED_OUTCOME_FAIL,
+  SASL_RECVED_SUCCESS,
+  SASL_RECVED_FAILURE,
   SASL_POSTED_OUTCOME,
   SASL_ERROR
 };
 
 /* APIs used by sasl implementations */
-PN_EXTERN void  pnx_sasl_logf(pn_transport_t *transport, const char *format, ...);
+PN_EXTERN void  pnx_sasl_logf(pn_transport_t *transport, pn_log_level_t level, 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);
 
 PN_EXTERN bool  pnx_sasl_is_client(pn_transport_t *transport);
-PN_EXTERN bool  pnx_sasl_is_included_mech(pn_transport_t *transport, pn_bytes_t s);
+PN_EXTERN bool  pnx_sasl_is_mechanism_included(pn_transport_t *transport, pn_bytes_t s);
 PN_EXTERN bool  pnx_sasl_is_transport_encrypted(pn_transport_t *transport);
-PN_EXTERN bool  pnx_sasl_get_allow_insecure_mechs(pn_transport_t *transport);
-PN_EXTERN bool  pnx_sasl_get_auth_required(pn_transport_t *transport);
+PN_EXTERN bool  pnx_sasl_get_allow_insecure_mechanisms(pn_transport_t *transport);
+PN_EXTERN bool  pnx_sasl_get_authentication_required(pn_transport_t *transport);
 PN_EXTERN const char *pnx_sasl_get_external_username(pn_transport_t *transport);
 PN_EXTERN int   pnx_sasl_get_external_ssf(pn_transport_t *transport);
 
@@ -132,8 +133,8 @@ PN_EXTERN void  pnx_sasl_set_bytes_out(pn_transport_t *transport, pn_bytes_t byt
 PN_EXTERN void  pnx_sasl_set_desired_state(pn_transport_t *transport, enum pnx_sasl_state desired_state);
 PN_EXTERN void  pnx_sasl_set_selected_mechanism(pn_transport_t *transport, const char *mechanism);
 PN_EXTERN void  pnx_sasl_set_local_hostname(pn_transport_t * transport, const char * fqdn);
-PN_EXTERN void  pnx_sasl_succeed_authentication(pn_transport_t *transport, const char *username, const char *authzid);
-PN_EXTERN void  pnx_sasl_fail_authentication(pn_transport_t *transport);
+PN_EXTERN void  pnx_sasl_set_succeeded(pn_transport_t *transport, const char *username, const char *authzid);
+PN_EXTERN void  pnx_sasl_set_failed(pn_transport_t *transport);
 
 PN_EXTERN void  pnx_sasl_set_implementation(pn_transport_t *transport, const pnx_sasl_implementation *impl, void *context);
 PN_EXTERN void  pnx_sasl_set_default_implementation(const pnx_sasl_implementation *impl);
@@ -144,4 +145,4 @@ PN_EXTERN void  pnx_sasl_set_default_implementation(const pnx_sasl_implementatio
 }
 #endif
 
-#endif /* sasl-plugin.h */
+#endif /* sasl_plugin.h */
diff --git a/c/src/sasl/cyrus_sasl.c b/c/src/sasl/cyrus_sasl.c
index fd189c0..6b34eaa 100644
--- a/c/src/sasl/cyrus_sasl.c
+++ b/c/src/sasl/cyrus_sasl.c
@@ -25,7 +25,7 @@
 #include "core/logger_private.h"
 
 #include "proton/sasl.h"
-#include "proton/sasl-plugin.h"
+#include "proton/sasl_plugin.h"
 #include "proton/transport.h"
 
 #include <sasl/sasl.h>
@@ -50,7 +50,7 @@ static void cyrus_sasl_process_response(pn_transport_t *transport, const pn_byte
 static bool cyrus_sasl_init_client(pn_transport_t *transport);
 static bool cyrus_sasl_process_mechanisms(pn_transport_t *transport, const char *mechs);
 static void cyrus_sasl_process_challenge(pn_transport_t *transport, const pn_bytes_t *recv);
-static void cyrus_sasl_process_outcome(pn_transport_t *transport);
+static void cyrus_sasl_process_outcome(pn_transport_t *transport, const pn_bytes_t *recv);
 
 static bool cyrus_sasl_can_encrypt(pn_transport_t *transport);
 static ssize_t cyrus_sasl_max_encrypt_size(pn_transport_t *transport);
@@ -137,7 +137,7 @@ static void pni_cyrus_interact(pn_transport_t *transport, sasl_interact_t *inter
       break;
     }
     default:
-      pnx_sasl_logf(transport, "(%s): %s - %s", i->challenge, i->prompt, i->defresult);
+      pnx_sasl_logf(transport, PN_LEVEL_ERROR, "(%s): %s - %s", i->challenge, i->prompt, i->defresult);
     }
   }
 }
@@ -304,8 +304,8 @@ bool cyrus_sasl_init_client(pn_transport_t* transport) {
 
     sasl_security_properties_t secprops = {0};
     secprops.security_flags =
-      ( pnx_sasl_get_allow_insecure_mechs(transport) ? 0 : SASL_SEC_NOPLAINTEXT ) |
-      ( pnx_sasl_get_auth_required(transport) ? SASL_SEC_NOANONYMOUS : 0 ) ;
+      ( pnx_sasl_get_allow_insecure_mechanisms(transport) ? 0 : SASL_SEC_NOPLAINTEXT ) |
+      ( pnx_sasl_get_authentication_required(transport) ? SASL_SEC_NOANONYMOUS : 0 ) ;
     secprops.min_ssf = 0;
     secprops.max_ssf = 2048;
     secprops.maxbufsize = CYRUS_SASL_MAX_BUFFSIZE;
@@ -398,8 +398,7 @@ void cyrus_sasl_process_challenge(pn_transport_t *transport, const pn_bytes_t *r
     int result = pni_wrap_client_step(transport, recv);
     switch (result) {
         case SASL_OK:
-            // Authenticated
-            // TODO: Documented that we need to call sasl_client_step() again to be sure!;
+            // Potentially authenticated
         case SASL_CONTINUE:
             // Need to send a response
             pnx_sasl_set_desired_state(transport, SASL_POSTED_RESPONSE);
@@ -408,13 +407,13 @@ void cyrus_sasl_process_challenge(pn_transport_t *transport, const pn_bytes_t *r
             pni_check_sasl_result(cyrus_conn, result, transport);
 
             // Failed somehow - equivalent to failing authentication
-            pnx_sasl_fail_authentication(transport);
-            pnx_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME_FAIL);
+            pnx_sasl_set_failed(transport);
+            pnx_sasl_set_desired_state(transport, SASL_RECVED_FAILURE);
             break;
     }
 }
 
-void cyrus_sasl_process_outcome(pn_transport_t* transport)
+void cyrus_sasl_process_outcome(pn_transport_t* transport, const pn_bytes_t *recv)
 {
 }
 
@@ -433,8 +432,8 @@ bool cyrus_sasl_init_server(pn_transport_t* transport)
 
     sasl_security_properties_t secprops = {0};
     secprops.security_flags =
-      ( pnx_sasl_get_allow_insecure_mechs(transport) ? 0 : SASL_SEC_NOPLAINTEXT ) |
-      ( pnx_sasl_get_auth_required(transport) ? SASL_SEC_NOANONYMOUS : 0 ) ;
+      ( pnx_sasl_get_allow_insecure_mechanisms(transport) ? 0 : SASL_SEC_NOPLAINTEXT ) |
+      ( pnx_sasl_get_authentication_required(transport) ? SASL_SEC_NOANONYMOUS : 0 ) ;
     secprops.min_ssf = 0;
     secprops.max_ssf = 2048;
     secprops.maxbufsize = CYRUS_SASL_MAX_BUFFSIZE;
@@ -501,7 +500,7 @@ static void pni_process_server_result(pn_transport_t *transport, int result)
             // Get authzid from SASL
             const void* authzid;
             sasl_getprop(cyrus_conn, SASL_USERNAME, &authzid);
-            pnx_sasl_succeed_authentication(transport, (const char*) authcid, (const char*) authzid);
+            pnx_sasl_set_succeeded(transport, (const char*) authcid, (const char*) authzid);
             pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
             break;
         }
@@ -513,7 +512,7 @@ static void pni_process_server_result(pn_transport_t *transport, int result)
             pni_check_sasl_result(cyrus_conn, result, transport);
 
             // Failed to authenticate
-            pnx_sasl_fail_authentication(transport);
+            pnx_sasl_set_failed(transport);
             pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
             break;
     }
@@ -529,13 +528,13 @@ static int pni_wrap_server_step(pn_transport_t *transport, const pn_bytes_t *in)
 {
     int result;
     const char *out;
-    unsigned outlen;
+    unsigned outlen = 0; // Initialise to defend against buggy cyrus mech plugins
     sasl_conn_t *cyrus_conn = (sasl_conn_t*)pnx_sasl_get_context(transport);
     result = sasl_server_step(cyrus_conn,
                               in->start, in->size,
                               &out, &outlen);
 
-    pnx_sasl_set_bytes_out(transport, pn_bytes(outlen, out));
+    pnx_sasl_set_bytes_out(transport, pn_bytes(outlen, outlen ? out : NULL));
     return result;
 }
 
diff --git a/c/src/sasl/cyrus_stub.c b/c/src/sasl/cyrus_stub.c
index 983ace8..37905e6 100644
--- a/c/src/sasl/cyrus_stub.c
+++ b/c/src/sasl/cyrus_stub.c
@@ -20,7 +20,7 @@
  */
 
 #include "proton/sasl.h"
-#include "proton/sasl-plugin.h"
+#include "proton/sasl_plugin.h"
 
 extern const pnx_sasl_implementation * const cyrus_sasl_impl;
 const pnx_sasl_implementation * const cyrus_sasl_impl = NULL;
diff --git a/c/src/sasl/default_sasl.c b/c/src/sasl/default_sasl.c
index 287bf06..72a3d1e 100644
--- a/c/src/sasl/default_sasl.c
+++ b/c/src/sasl/default_sasl.c
@@ -20,7 +20,7 @@
  */
 
 #include "proton/sasl.h"
-#include "proton/sasl-plugin.h"
+#include "proton/sasl_plugin.h"
 
 #include <stdlib.h>
 #include <string.h>
@@ -37,7 +37,7 @@ static void default_sasl_process_response(pn_transport_t *transport, const pn_by
 static bool default_sasl_init_client(pn_transport_t *transport);
 static bool default_sasl_process_mechanisms(pn_transport_t *transport, const char *mechs);
 static void default_sasl_process_challenge(pn_transport_t *transport, const pn_bytes_t *recv);
-static void default_sasl_process_outcome(pn_transport_t *transport);
+static void default_sasl_process_outcome(pn_transport_t *transport, const pn_bytes_t *recv);
 
 static bool default_sasl_impl_can_encrypt(pn_transport_t *transport);
 static ssize_t default_sasl_impl_max_encrypt_size(pn_transport_t *transport);
@@ -128,7 +128,7 @@ 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_transport_encrypted(transport) || pnx_sasl_get_allow_insecure_mechs(transport)) &&
+      (pnx_sasl_is_transport_encrypted(transport) || pnx_sasl_get_allow_insecure_mechanisms(transport)) &&
       username && password) {
     pnx_sasl_set_selected_mechanism(transport, PLAIN);
     size_t zsize = authzid ? strlen(authzid) : 0;
@@ -194,7 +194,7 @@ void default_sasl_process_init(pn_transport_t *transport, const char *mechanism,
 {
   // Check that mechanism is ANONYMOUS
   if (strcmp(mechanism, ANONYMOUS)==0) {
-    pnx_sasl_succeed_authentication(transport, "anonymous", "anonymous");
+    pnx_sasl_set_succeeded(transport, "anonymous", "anonymous");
     pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
     return;
   }
@@ -216,13 +216,13 @@ void default_sasl_process_init(pn_transport_t *transport, const char *mechanism,
       }
     }
 
-    pnx_sasl_succeed_authentication(transport, ext_username, authzid ? authzid : ext_username);
+    pnx_sasl_set_succeeded(transport, ext_username, authzid ? authzid : ext_username);
     pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
     return;
   }
 
   // Otherwise authentication failed
-  pnx_sasl_fail_authentication(transport);
+  pnx_sasl_set_failed(transport);
   pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
 
 }
@@ -236,7 +236,7 @@ void default_sasl_process_response(pn_transport_t *transport, const pn_bytes_t *
 {
 }
 
-void default_sasl_process_outcome(pn_transport_t* transport)
+void default_sasl_process_outcome(pn_transport_t* transport, const pn_bytes_t *recv)
 {
 }
 
diff --git a/c/src/sasl/sasl-internal.h b/c/src/sasl/sasl-internal.h
index 82ff85a..376854e 100644
--- a/c/src/sasl/sasl-internal.h
+++ b/c/src/sasl/sasl-internal.h
@@ -27,7 +27,7 @@
 
 #include "proton/types.h"
 #include "proton/sasl.h"
-#include "proton/sasl-plugin.h"
+#include "proton/sasl_plugin.h"
 
 extern const pnx_sasl_implementation default_sasl_impl;
 extern const pnx_sasl_implementation * const cyrus_sasl_impl;
diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c
index 500b286..e239e0c 100644
--- a/c/src/sasl/sasl.c
+++ b/c/src/sasl/sasl.c
@@ -43,18 +43,18 @@ static const char pni_excluded_mechs[] = "GSSAPI GSS-SPNEGO GS2-KRB5 GS2-IAKERB"
 //-----------------------------------------------------------------------------
 // pnx_sasl: API for SASL implementations to use
 
-void pnx_sasl_logf(pn_transport_t *logger, const char *fmt, ...)
+void pnx_sasl_logf(pn_transport_t *logger, pn_log_level_t level, const char *fmt, ...)
 {
     va_list ap;
     va_start(ap, fmt);
-    if (PN_SHOULD_LOG(&logger->logger, PN_SUBSYSTEM_SASL, PN_LEVEL_ERROR))
-        pni_logger_vlogf(&logger->logger, PN_SUBSYSTEM_SASL, PN_LEVEL_ERROR, fmt, ap);
+    if (PN_SHOULD_LOG(&logger->logger, PN_SUBSYSTEM_SASL, level))
+        pni_logger_vlogf(&logger->logger, PN_SUBSYSTEM_SASL, level, fmt, ap);
     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);
+    pnx_sasl_logf(logger, PN_LEVEL_ERROR, "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);
@@ -80,12 +80,12 @@ bool  pnx_sasl_is_transport_encrypted(pn_transport_t *transport)
   return transport->sasl ? transport->sasl->external_ssf>0 : false;
 }
 
-bool  pnx_sasl_get_allow_insecure_mechs(pn_transport_t *transport)
+bool  pnx_sasl_get_allow_insecure_mechanisms(pn_transport_t *transport)
 {
   return transport->sasl ? transport->sasl->allow_insecure_mechs : false;
 }
 
-bool  pnx_sasl_get_auth_required(pn_transport_t *transport)
+bool  pnx_sasl_get_authentication_required(pn_transport_t *transport)
 {
   return transport->auth_required;
 }
@@ -148,7 +148,7 @@ void  pnx_sasl_set_selected_mechanism(pn_transport_t *transport, const char *mec
   }
 }
 
-void  pnx_sasl_succeed_authentication(pn_transport_t *transport, const char *username, const char *authzid)
+void  pnx_sasl_set_succeeded(pn_transport_t *transport, const char *username, const char *authzid)
 {
   if (transport->sasl) {
     transport->sasl->username = username;
@@ -166,7 +166,7 @@ void  pnx_sasl_succeed_authentication(pn_transport_t *transport, const char *use
   }
 }
 
-void  pnx_sasl_fail_authentication(pn_transport_t *transport)
+void  pnx_sasl_set_failed(pn_transport_t *transport)
 {
   if (transport->sasl) {
     transport->sasl->outcome = PN_SASL_AUTH;
@@ -195,7 +195,7 @@ static inline void pni_sasl_impl_free(pn_transport_t *transport)
 
 static inline const char *pni_sasl_impl_list_mechs(pn_transport_t *transport)
 {
-  return transport->sasl->impl->list_mechs(transport);
+  return transport->sasl->impl->list_mechanisms(transport);
 }
 
 static inline bool pni_sasl_impl_init_server(pn_transport_t *transport)
@@ -233,9 +233,9 @@ static inline void pni_sasl_impl_process_challenge(pn_transport_t *transport, co
   transport->sasl->impl->process_challenge(transport, recv);
 }
 
-static inline void pni_sasl_impl_process_outcome(pn_transport_t *transport)
+static inline void pni_sasl_impl_process_outcome(pn_transport_t *transport, const pn_bytes_t *recv)
 {
-  transport->sasl->impl->process_outcome(transport);
+  transport->sasl->impl->process_outcome(transport, recv);
 }
 
 static inline bool pni_sasl_impl_can_encrypt(pn_transport_t *transport)
@@ -332,16 +332,16 @@ static bool pni_sasl_is_client_state(enum pnx_sasl_state state)
   return state==SASL_NONE
       || state==SASL_POSTED_INIT
       || state==SASL_POSTED_RESPONSE
-      || state==SASL_RECVED_OUTCOME_SUCCEED
-      || state==SASL_RECVED_OUTCOME_FAIL
+      || state==SASL_RECVED_SUCCESS
+      || state==SASL_RECVED_FAILURE
       || state==SASL_ERROR;
 }
 
 static bool pni_sasl_is_final_input_state(pni_sasl_t *sasl)
 {
   enum pnx_sasl_state desired_state = sasl->desired_state;
-  return desired_state==SASL_RECVED_OUTCOME_SUCCEED
-      || desired_state==SASL_RECVED_OUTCOME_FAIL
+  return desired_state==SASL_RECVED_SUCCESS
+      || desired_state==SASL_RECVED_FAILURE
       || desired_state==SASL_ERROR
       || desired_state==SASL_POSTED_OUTCOME;
 }
@@ -350,9 +350,9 @@ static bool pni_sasl_is_final_output_state(pni_sasl_t *sasl)
 {
   enum pnx_sasl_state last_state = sasl->last_state;
   enum pnx_sasl_state desired_state = sasl->desired_state;
-  return (desired_state==SASL_RECVED_OUTCOME_SUCCEED && last_state>=SASL_POSTED_INIT)
-      || last_state==SASL_RECVED_OUTCOME_SUCCEED
-      || last_state==SASL_RECVED_OUTCOME_FAIL
+  return (desired_state==SASL_RECVED_SUCCESS && last_state>=SASL_POSTED_INIT)
+      || last_state==SASL_RECVED_SUCCESS
+      || last_state==SASL_RECVED_FAILURE
       || last_state==SASL_ERROR
       || last_state==SASL_POSTED_OUTCOME;
 }
@@ -429,7 +429,7 @@ static bool pni_sasl_client_included_mech(const char *included_mech_list, pn_byt
 // 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)
+bool pnx_sasl_is_mechanism_included(pn_transport_t* transport, pn_bytes_t s)
 {
   return pni_sasl_server_included_mech(transport->sasl->included_mechanisms, s);
 }
@@ -513,7 +513,7 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
         desired_state = SASL_POSTED_MECHANISMS;
         continue;
       }
-      pn_post_frame(transport, SASL_FRAME_TYPE, 0, "DL[B]", SASL_OUTCOME, sasl->outcome);
+      pn_post_frame(transport, SASL_FRAME_TYPE, 0, "DL[Bz]", SASL_OUTCOME, sasl->outcome, out.size, out.start);
       pni_emit(transport);
       if (sasl->outcome!=PN_SASL_OK) {
         pn_do_error(transport, "amqp:unauthorized-access", "Failed to authenticate client [mech=%s]",
@@ -521,13 +521,13 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
         desired_state = SASL_ERROR;
       }
       break;
-    case SASL_RECVED_OUTCOME_SUCCEED:
+    case SASL_RECVED_SUCCESS:
       if (sasl->last_state < SASL_POSTED_INIT) {
         desired_state = SASL_POSTED_INIT;
         continue;
       }
       break;
-    case SASL_RECVED_OUTCOME_FAIL:
+    case SASL_RECVED_FAILURE:
       pn_do_error(transport, "amqp:unauthorized-access", "Authentication failed [mech=%s]",
                   transport->sasl->selected_mechanism ? transport->sasl->selected_mechanism : "none");
       desired_state = SASL_ERROR;
@@ -944,7 +944,7 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t cha
         pn_string_size(mechs) &&
         pni_sasl_impl_process_mechanisms(transport, pn_string_get(mechs)))) {
     sasl->outcome = PN_SASL_PERM;
-    pnx_sasl_set_desired_state(transport, SASL_RECVED_OUTCOME_FAIL);
+    pnx_sasl_set_desired_state(transport, SASL_RECVED_FAILURE);
   }
 
   pn_free(mechs);
@@ -1006,16 +1006,21 @@ int pn_do_outcome(pn_transport_t *transport, uint8_t frame_type, uint16_t channe
   if (!sasl->client) return PN_ERR;
 
   uint8_t outcome;
-  int err = pn_data_scan(args, "D.[B]", &outcome);
+  pn_bytes_t recv;
+  int err = pn_data_scan(args, "D.[Bz]", &outcome, &recv);
   if (err) return err;
 
+  // Preset the outcome to what the server sent us - the plugin can alter this.
+  // In practise the plugin processing here should only fail because it fails
+  // to authenticate the server id after the server authenticates our user.
+  // It should never succeed after the server outcome was failure.
   sasl->outcome = (pn_sasl_outcome_t) outcome;
-  bool authenticated = sasl->outcome==PN_SASL_OK;
-  transport->authenticated = authenticated;
-  pnx_sasl_set_desired_state(transport, authenticated ? SASL_RECVED_OUTCOME_SUCCEED : SASL_RECVED_OUTCOME_FAIL);
 
-  pni_sasl_impl_process_outcome(transport);
+  pni_sasl_impl_process_outcome(transport, &recv);
 
+  bool authenticated = sasl->outcome==PN_SASL_OK;
+  transport->authenticated = authenticated;
+  pnx_sasl_set_desired_state(transport, authenticated ? SASL_RECVED_SUCCESS : SASL_RECVED_FAILURE);
   return 0;
 }
 


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