SASL Code Quality and Feature Improvements

include/authcookie.h

@@ -20,10 +20,10 @@ struct authcookie_ {
 
 E void authcookie_init(void);
 E authcookie_t *authcookie_create(myuser_t *mu);
-E authcookie_t *authcookie_find(char *ticket, myuser_t *myuser);
+E authcookie_t *authcookie_find(const char *ticket, myuser_t *myuser);
 E void authcookie_destroy(authcookie_t *ac);
 E void authcookie_destroy_all(myuser_t *mu);
-E bool authcookie_validate(char *ticket, myuser_t *myuser);
+E bool authcookie_validate(const char *ticket, myuser_t *myuser);
 E void authcookie_expire(void *arg);
 
 #endif

include/pbkdf2v2.h

@@ -78,16 +78,10 @@ static const unsigned char ClientKeyStr[] = {
 	0x43, 0x6C, 0x69, 0x65, 0x6E, 0x74, 0x20, 0x4B, 0x65, 0x79
 };
 
-/*
- * For modules/saslserv/scram-sha to make an inter-module function call to
- * modules/crypto/pbkdf2v2:atheme_pbkdf2v2_scram_dbextract()
- */
-typedef bool (*atheme_pbkdf2v2_scram_dbextract_fn)(const char *restrict, struct pbkdf2v2_dbentry *restrict);
-
-/*
- * For modules/saslserv/scram-sha to make an inter-module function call to
- * modules/crypto/pbkdf2v2:atheme_pbkdf2v2_scram_normalize()
- */
-typedef const char *(*atheme_pbkdf2v2_scram_normalize_fn)(const char *restrict);
+struct pbkdf2v2_scram_functions
+{
+	bool             (*dbextract)(const char *restrict, struct pbkdf2v2_dbentry *restrict);
+	const char *     (*normalize)(const char *restrict);
+};
 
 #endif /* !PBKDF2V2_H */

include/sasl.h

@@ -9,57 +9,68 @@
 #ifndef SASL_H
 #define SASL_H
 
-#define SASL_MESSAGE_MAXPARA	8	/* arbitrary, increment if needed in future */
-
-typedef struct sasl_session_ sasl_session_t;
-typedef struct sasl_message_ sasl_message_t;
-typedef struct sasl_mechanism_ sasl_mechanism_t;
-
-struct sasl_session_ {
-  char *uid;
-  char *buf, *p;
-  int len, flags;
-
-  server_t *server;
-
-  struct sasl_mechanism_ *mechptr;
-  void *mechdata;
-
-  char *username;
-  char *certfp;
-  char *authzid;
-
-  char *host;
-  char *ip;
-  bool tls;
+#define SASL_MESSAGE_MAXPARA    8       /* arbitrary, increment if needed in future */
+#define SASL_MECHANISM_MAXLEN   60
+#define SASL_S2S_MAXLEN         400
+
+struct sasl_session;
+struct sasl_message;
+struct sasl_mechanism;
+struct sasl_core_functions;
+
+struct sasl_session
+{
+	struct sasl_mechanism   *mechptr;
+	server_t                *server;
+	sourceinfo_t            *si;
+	char                    *uid;
+	char                    *buf;
+	char                    *p;
+	void                    *mechdata;
+	char                    *authceid;
+	char                    *authzeid;
+	char                    *certfp;
+	char                    *host;
+	char                    *ip;
+	size_t                   len;
+	int                      flags;
+	bool                     tls;
 };
 
-struct sasl_message_ {
-  char *uid;
-  char mode;
-  char *parv[SASL_MESSAGE_MAXPARA];
-  int parc;
+struct sasl_message
+{
+	server_t  *server;
+	char      *uid;
+	char      *parv[SASL_MESSAGE_MAXPARA];
+	int        parc;
+	char       mode;
+};
 
-  server_t *server;
+struct sasl_mechanism
+{
+	char       name[SASL_MECHANISM_MAXLEN];
+	int      (*mech_start)(struct sasl_session *, char **, size_t *);
+	int      (*mech_step)(struct sasl_session *, char *, size_t, char **, size_t *);
+	void     (*mech_finish)(struct sasl_session *);
 };
 
-struct sasl_mechanism_ {
-  char name[60];
-  int (*mech_start) (struct sasl_session_ *sptr, char **buffer, size_t *buflen);
-  int (*mech_step) (struct sasl_session_ *sptr, char *message, size_t length, char **buffer, size_t *buflen);
-  void (*mech_finish) (struct sasl_session_ *sptr);
+struct sasl_core_functions
+{
+	void     (*mech_register)(struct sasl_mechanism *);
+	void     (*mech_unregister)(struct sasl_mechanism *);
+	bool     (*authcid_can_login)(struct sasl_session *, const char *, myuser_t **);
+	bool     (*authzid_can_login)(struct sasl_session *, const char *, myuser_t **);
 };
 
 typedef struct {
-  myuser_t *source_mu;
-  myuser_t *target_mu;
-  bool allowed;
+
+	myuser_t  *source_mu;
+	myuser_t  *target_mu;
+	bool       allowed;
+
 } hook_sasl_may_impersonate_t;
 
-typedef struct {
-	void (*mech_register) (struct sasl_mechanism_ *mech);
-	void (*mech_unregister) (struct sasl_mechanism_ *mech);
-} sasl_mech_register_func_t;
+typedef struct sasl_message sasl_message_t;
 
 #define ASASL_FAIL 0 /* client supplied invalid credentials / screwed up their formatting */
 #define ASASL_MORE 1 /* everything looks good so far, but we're not done yet */
@@ -69,9 +80,3 @@ typedef struct {
 #define ASASL_NEED_LOG              2 /* user auth success needs to be logged still */
 
 #endif
-
-/* vim:cinoptions=>s,e0,n0,f0,{0,}0,^0,=s,ps,t0,c3,+s,(2s,us,)20,*30,gs,hs
- * vim:ts=8
- * vim:sw=8
- * vim:noexpandtab
- */

libathemecore/authcookie.c

@@ -75,7 +75,7 @@ authcookie_t *authcookie_create(myuser_t *mu)
  * Side Effects:
  *       none
  */
-authcookie_t *authcookie_find(char *ticket, myuser_t *myuser)
+authcookie_t *authcookie_find(const char *ticket, myuser_t *myuser)
 {
 	mowgli_node_t *n;
 	authcookie_t *ac;
@@ -205,7 +205,7 @@ void authcookie_expire(void *arg)
  * Side Effects:
  *       expired authcookies are destroyed here
  */
-bool authcookie_validate(char *ticket, myuser_t *myuser)
+bool authcookie_validate(const char *ticket, myuser_t *myuser)
 {
 	authcookie_t *ac = authcookie_find(ticket, myuser);
 

modules/crypto/pbkdf2v2.c

@@ -218,17 +218,11 @@ atheme_pbkdf2v2_scram_derive(const struct pbkdf2v2_dbentry *const dbe, const uns
  * These 2 functions are provided for modules/saslserv/scram-sha (RFC 5802, RFC 7677, RFC 4013) *
  * The second function is also used by *this* module for password normalization (in SCRAM mode) *
  *                                                                                              *
- * Prototypes for them appear first, to avoid `-Wmissing-prototypes' diagnostics (under Clang)  *
- *                                                                                              *
- * Constant-but-unused function pointers for them appear last, so that the compiler can verify  *
- * their signatures match the typedefs in include/pbkdf2v2.h (necessary for bug-free inter-     *
- * module function calls)                                                                       *
+ * A structure containing pointers to them appears last, so that it can be imported by the      *
+ * SCRAM-SHA module.                                                                            *
  ********************************************************************************************** */
 
-bool atheme_pbkdf2v2_scram_dbextract(const char *restrict, struct pbkdf2v2_dbentry *restrict);
-const char *atheme_pbkdf2v2_scram_normalize(const char *restrict);
-
-bool
+static bool
 atheme_pbkdf2v2_scram_dbextract(const char *const restrict parameters, struct pbkdf2v2_dbentry *const restrict dbe)
 {
 	if (! atheme_pbkdf2v2_parse_dbentry(dbe, parameters, true))
@@ -285,7 +279,7 @@ atheme_pbkdf2v2_scram_dbextract(const char *const restrict parameters, struct pb
 	return true;
 }
 
-const char *
+static const char *
 atheme_pbkdf2v2_scram_normalize(const char *const restrict input)
 {
 	static char buf[ATHEME_SASLPREP_MAXLEN];
@@ -309,11 +303,14 @@ atheme_pbkdf2v2_scram_normalize(const char *const restrict input)
 	return buf;
 }
 
-static const atheme_pbkdf2v2_scram_dbextract_fn __attribute__((unused)) ex_fn_ptr = &atheme_pbkdf2v2_scram_dbextract;
-static const atheme_pbkdf2v2_scram_normalize_fn __attribute__((unused)) nm_fn_ptr = &atheme_pbkdf2v2_scram_normalize;
+const struct pbkdf2v2_scram_functions pbkdf2v2_scram_functions = {
+
+	.dbextract      = &atheme_pbkdf2v2_scram_dbextract,
+	.normalize      = &atheme_pbkdf2v2_scram_normalize,
+};
 
 /* **********************************************************************************************
- * End external functions                                                                       *
+ * End SCRAM-SHA functions                                                                      *
  ********************************************************************************************** */
 
 #endif /* HAVE_LIBIDN */

modules/saslserv/authcookie.c

@@ -8,72 +8,70 @@
 #include "atheme.h"
 #include "authcookie.h"
 
-sasl_mech_register_func_t *regfuncs;
-static int mech_start(sasl_session_t *p, char **out, size_t *out_len);
-static int mech_step(sasl_session_t *p, char *message, size_t len, char **out, size_t *out_len);
-static void mech_finish(sasl_session_t *p);
-sasl_mechanism_t mech = {"AUTHCOOKIE", &mech_start, &mech_step, &mech_finish};
+static const struct sasl_core_functions *sasl_core_functions = NULL;
 
-static void
-mod_init(module_t *const restrict m)
-{
-	MODULE_TRY_REQUEST_SYMBOL(m, regfuncs, "saslserv/main", "sasl_mech_register_funcs");
-	regfuncs->mech_register(&mech);
-}
-
-static void
-mod_deinit(const module_unload_intent_t intent)
+static int
+mech_step(struct sasl_session *const restrict p, char *const restrict message, const size_t len,
+          char __attribute__((unused)) **const restrict out, size_t __attribute__((unused)) *const restrict out_len)
 {
-	regfuncs->mech_unregister(&mech);
-}
+	if (! (message && len))
+		return ASASL_FAIL;
 
-static int mech_start(sasl_session_t *p, char **out, size_t *out_len)
-{
-	return ASASL_MORE;
-}
+	char data[768];
+	if (len >= sizeof data)
+		return ASASL_FAIL;
 
-static int mech_step(sasl_session_t *p, char *message, size_t len, char **out, size_t *out_len)
-{
-	char authz[256];
-	char authc[256];
-	char cookie[256];
-	myuser_t *mu;
+	(void) memset(data, 0x00, sizeof data);
+	(void) memcpy(data, message, len);
 
-	/* Skip the authzid entirely */
+	const char *ptr = data;
+	const char *const end = data + len;
 
-	if(strlen(message) > 255)
+	const char *const authzid = ptr;
+	if (! *ptr || (ptr += strlen(ptr) + 1) >= end)
 		return ASASL_FAIL;
-	len -= strlen(message) + 1;
-	if(len <= 0)
+
+	const char *const authcid = ptr;
+	if (! *ptr || (ptr += strlen(ptr) + 1) >= end)
 		return ASASL_FAIL;
-	strcpy(authz, message);
-	message += strlen(message) + 1;
 
-	/* Copy the authcid */
-	if(strlen(message) > 255)
+	const char *const secret = ptr;
+	if (! *secret)
 		return ASASL_FAIL;
-	len -= strlen(message) + 1;
-	if(len <= 0)
+
+	if (! sasl_core_functions->authzid_can_login(p, authzid, NULL))
 		return ASASL_FAIL;
-	strcpy(authc, message);
-	message += strlen(message) + 1;
 
-	/* Copy the authcookie */
-	if(strlen(message) > 255)
+	myuser_t *mu = NULL;
+	if (! sasl_core_functions->authcid_can_login(p, authcid, &mu))
 		return ASASL_FAIL;
-	mowgli_strlcpy(cookie, message, len + 1);
 
-	/* Done dissecting, now check. */
-	if(!(mu = myuser_find_by_nick(authc)))
+	if (! authcookie_find(secret, mu))
 		return ASASL_FAIL;
 
-	p->username = sstrdup(authc);
-	p->authzid = sstrdup(authz);
-	return authcookie_find(cookie, mu) != NULL ? ASASL_DONE : ASASL_FAIL;
+	return ASASL_DONE;
 }
 
-static void mech_finish(sasl_session_t *p)
+static struct sasl_mechanism mech = {
+
+	.name           = "AUTHCOOKIE",
+	.mech_start     = NULL,
+	.mech_step      = &mech_step,
+	.mech_finish    = NULL,
+};
+
+static void
+mod_init(module_t *const restrict m)
+{
+	MODULE_TRY_REQUEST_SYMBOL(m, sasl_core_functions, "saslserv/main", "sasl_core_functions");
+
+	(void) sasl_core_functions->mech_register(&mech);
+}
+
+static void
+mod_deinit(const module_unload_intent_t __attribute__((unused)) intent)
 {
+	(void) sasl_core_functions->mech_unregister(&mech);
 }
 
 SIMPLE_DECLARE_MODULE_V1("saslserv/authcookie", MODULE_UNLOAD_CAPABILITY_OK)