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:59:02 UTC

[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

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