Hi there guys, I am building a XMPP service around jabberd2 and I am very impressed with the current architecture. However, as I am getting further into our implementation, a couple of problems have become apparent.
Problems:
- We need to maintain per user session data in c2s. For example:
authentication token and unique user ID that is system and session
specific. authreg_t has a private member to hold authreg related data
but this is module wide and not tied to an individual user.
- For CRAM-MD5, to generate the proper challenge, we need more than the
username. Here, realm would be good to have and for verifying the
response, the associated resource would be needed to generate the
unique login ID in our system.
For us, the two are intertwined and the current APIs and data structures
do not provide a solution to the above problems, to the best of my
knowledge. If I am mistaken, please do let me know :)
Solutions:
There needs to be a way for a 3rd party to plug in and provide the
necessary information to respond to the initial auth request and also
have all the relevant information to properly authenticate the user
and be able to retain user & resource specific tokens throughout the
lifetime of the user's session.
- Build a hash table of relevant data and store it in the authreg_t
private data member.
Pros: Fits nicely into the existing architecture
Cons: Its up to the authreg provider to maintain proper state and
moves session related data away from the session and into a component
which outlives the span of an individual session. Doesn¹t help much
With problem (1).
- Retrofit existing interfaces with the necessary data.
a. Introduce void *sess_private in sess_t.
b. APIs to add pointer to sess_private along with realm and
resource arguments when available.
struct sess_st {
...
void* sess_private; /* Data per session per user */
}
int (*create_challenge)(authreg_t ar, sess_private *data,
const char *username, const char *realm, const char *challenge,
int maxlen);
int (*check_response)(authreg_t ar, sess_private *data,
const char *username, const char *realm, const char *resource,
const char *challenge, const char *response);
Pros: Maintain same methods but new parameters, faster approach.
Cons: (BIG)breaks everyone out there. In some cases, other 3rd parties
may want similar mechanism for plain text login as well and this
approach wouldn't work for them.
- Introduce the private data in sess_t as above and also new functions in
authreg_t that allows for generic authentication handling by third
parties.
(see sess_private above)
/* Extension for custom authentication providers */
int (*custom_auth_get)(authreg_t ar, authdata_t data);
int (*custom_auth_set)(authreg_t ar, authdata_t data);
where
/* Custom authentication data object */
struct authdata_st {
void **sess_private;
const char *username;
const char *resource;
const char *realm;
const char *password;
const char *digest;
char *challenge;
int challenge_maxlen;
const char *response;
};
The authdata_st can be expanded as needed without changing the APIs.
The APIs mimic authreg_auth_[get/set] functions and for get, in the
case of CRAM-MD5, returns a challenge based on the contextual data. In
the case of set, it has all the relevant pieces of information to
serve the request as necessary and return true or false for success
or failure.
Pros: A very, very generic way to leave it to the authreg implementor
to authenticate the user, as long as the right results are returned.
Cons: New APIs, potentially very generic and open.
I chose the third route, because others could implement their own
authentication schemes now that all the relevant and required pieces of
information is available. I put together a quick implementation and the
patch is added below. I coded it up in a way that if the custom
authentication approach fails, we fall back on the existing methods so
the impact would be inconsequential if an auth provider doesn't
implement the change. This applies to both get & set functionality. See
the patch for more details.
I am trying to come up with a solution that would be accepted
by the current maintainers and that I can submit back
to the community. This patch by no means is the final version and I open
to all feedback, be it naming, implementation, or the approach in
general, as long as it solves the two problems stated above.
Looking forward to the feedback, thanks.
Shawn
---
diff --git a/c2s/authreg.c b/c2s/authreg.c
index 627d22e..ace320c 100644
--- a/c2s/authreg.c
+++ b/c2s/authreg.c
@@ -135,6 +135,8 @@ static void _authreg_auth_get(c2s_t c2s, sess_t sess,
nad_t nad) {
int ns, elem, attr, err;
char username[1024], id[128];
int ar_mechs;
+ authdata_t ad;
+ int processed = 0;
/* can't auth if they're active */
if(sess->active) {
@@ -211,16 +213,50 @@ static void _authreg_auth_get(c2s_t c2s, sess_t
sess, nad_t nad) {
nad_append_elem(nad, ns, "digest", 2);
if (ar_mechs & AR_MECH_TRAD_CRAMMD5 && c2s->ar->create_challenge !=
NULL) {
- err = (c2s->ar->create_challenge)(c2s->ar, (char *) username,
(char *) sess->auth_challenge, sizeof(sess->auth_challenge));
- if (0 == err) { /* operation failed */
- sx_nad_write(sess->s, stanza_tofrom(stanza_error(nad, 0,
stanza_err_INTERNAL_SERVER_ERROR), 0));
- return;
+ /*
+ * Check if the authreg provider has the custom auth get/set
+ * available, if so, call the custom_auth_get function.
+ *
+ * If any errors are encountered, fall back to traditional
+ * functions, otherwise bypass the call to create_challenge.
+ */
+ if (c2s->ar->custom_auth_get != NULL) {
+ ad = (authdata_t)calloc(1, sizeof(struct authdata_st));
+ if (ad != NULL) {
+ ad->sess_private = &(sess->sess_private);
+ ad->username = username;
+ ad->resource = NULL; /* not reqd for get */
+ ad->realm = sess->host->realm;
+ ad->password = NULL; /* not reqd for get */
+ ad->digest = NULL; /* not reqd for get */
+ ad->challenge = sess->auth_challenge;
+ ad->challenge_maxlen = sizeof(sess->auth_challenge);
+ ad->response = NULL; /* not reqd for get */
+
+ err = (c2s->ar->custom_auth_get)(c2s->ar, ad);
+ if (1 == err) /* operation succeeded */
+ processed = 1;
+
+ free(ad);
+ }
}
- else if (1 == err) { /* operation succeeded */
- nad_append_elem(nad, ns, "crammd5", 2);
- nad_append_attr(nad, -1, "challenge", sess->auth_challenge);
+
+ /*
+ * Call create_challenge only if custom_auth_get is not
+ * available or the call to custom_auth_get failed.
+ */
+ if (!processed) {
+ err = (c2s->ar->create_challenge)(c2s->ar, (char *) username,
(char *) sess->auth_challenge, sizeof(sess->auth_challenge));
+ if (0 == err) { /* operation failed */
+ sx_nad_write(sess->s, stanza_tofrom(stanza_error(nad, 0,
stanza_err_INTERNAL_SERVER_ERROR), 0));
+ return;
+ }
+ else if (1 == err) { /* operation succeeded */
+ nad_append_elem(nad, ns, "crammd5", 2);
+ nad_append_attr(nad, -1, "challenge",
sess->auth_challenge);
+ }
+ else ; /* auth method unsupported for user */
}
- else ; /* auth method unsupported for user */
}
/* give it back to the client */
@@ -233,7 +269,9 @@ static void _authreg_auth_get(c2s_t c2s, sess_t sess,
nad_t nad) {
static void _authreg_auth_set(c2s_t c2s, sess_t sess, nad_t nad) {
int ns, elem, attr, authd = 0;
char username[1024], resource[1024], str[1024], hash[280];
+ char response[1024];
int ar_mechs;
+ authdata_t ad;
/* can't auth if they're active */
if(sess->active) {
@@ -294,15 +332,56 @@ static void _authreg_auth_set(c2s_t c2s, sess_t
sess, nad_t nad) {
sx_nad_write(sess->s, stanza_tofrom(stanza_error(nad, 0,
stanza_err_OLD_UNAUTH), 0));
return;
}
-
+
+ /* handle custom authentication */
+ if (!authd && c2s->ar->custom_auth_set != NULL) {
+ ad = (authdata_t)calloc(1, sizeof(struct authdata_st));
+ if (ad != NULL) {
+ ad->sess_private = &(sess->sess_private);
+ ad->username = username;
+ ad->resource = resource;
+ ad->realm = sess->host->realm;
+ ad->challenge = sess->auth_challenge;
+ ad->challenge_maxlen = sizeof(sess->auth_challenge);
+
+ elem = nad_find_elem(nad, 1, ns, "crammd5", 1);
+ if(elem >= 0) {
+ snprintf(response, 1024, "%.*s", NAD_CDATA_L(nad, elem),
NAD_CDATA(nad, elem));
+ ad->response = response;
+ }
+
+ elem = nad_find_elem(nad, 1, ns, "digest", 1);
+ if(elem >= 0) {
+ snprintf(hash, 280, "%.*s", NAD_CDATA_L(nad, elem),
NAD_CDATA(nad, elem));
+ ad->digest = hash;
+ }
+
+ elem = nad_find_elem(nad, 1, ns, "password", 1);
+ if(elem >= 0) {
+ snprintf(str, 1024, "%.*s", NAD_CDATA_L(nad, elem),
NAD_CDATA(nad, elem));
+ ad->password = str;
+ }
+
+ if((c2s->ar->custom_auth_set)(c2s->ar, ad) == 0) {
+ authd = 1;
+ log_debug(ZONE, "custom auth (check) succeded");
+ _authreg_auth_log(c2s, sess, "custom auth", username,
resource, TRUE);
+ } else {
+ _authreg_auth_log(c2s, sess, "custom auth", username,
resource, FALSE);
+ }
+
+ free(ad);
+ }
+ }
+
/* handle CRAM-MD5 response */
if(!authd && ar_mechs & AR_MECH_TRAD_CRAMMD5 &&
c2s->ar->check_response != NULL)
{
elem = nad_find_elem(nad, 1, ns, "crammd5", 1);
if(elem >= 0)
{
- snprintf(str, 1024, "%.*s", NAD_CDATA_L(nad, elem),
NAD_CDATA(nad, elem));
- if((c2s->ar->check_response)(c2s->ar, username,
sess->host->realm, sess->auth_challenge, str) == 0)
+ snprintf(response, 1024, "%.*s", NAD_CDATA_L(nad, elem),
NAD_CDATA(nad, elem));
+ if((c2s->ar->check_response)(c2s->ar, username,
sess->host->realm, sess->auth_challenge, response) == 0)
{
log_debug(ZONE, "crammd5 auth (check) succeded");
authd = 1;
diff --git a/c2s/c2s.h b/c2s/c2s.h
index f1cfc79..55efbfd 100644
--- a/c2s/c2s.h
+++ b/c2s/c2s.h
@@ -54,6 +54,7 @@ typedef struct c2s_st *c2s_t;
typedef struct bres_st *bres_t;
typedef struct sess_st *sess_t;
typedef struct authreg_st *authreg_t;
+typedef struct authdata_st *authdata_t;
/** list of resources bound to session */
struct bres_st {
@@ -111,6 +112,9 @@ struct sess_st {
/** Apple: session challenge for challenge-response authentication */
char auth_challenge[65];
+
+ /* Storage of per user session authreg data */
+ void *sess_private;
};
/* allowed mechanisms */
@@ -347,6 +351,10 @@ struct authreg_st
/** Apple extensions for challenge/response authentication methods */
int (*create_challenge)(authreg_t ar, const char *username,
const char *challenge, int maxlen);
int (*check_response)(authreg_t ar, const char *username,
const char *realm, const char *challenge, const char *response);
+
+ /* Extension for custom authentication providers */
+ int (*custom_auth_get)(authreg_t ar, authdata_t data);
+ int (*custom_auth_set)(authreg_t ar, authdata_t data);
};
/** get a handle for a single module */
@@ -386,3 +394,15 @@ typedef struct stream_redirect_st
const char *to_port;
} *stream_redirect_t;
+/* Custom authentication data object */
+struct authdata_st {
+ void **sess_private;
+ const char *username;
+ const char *resource;
+ const char *realm;
+ const char *password;
+ const char *digest;
+ char *challenge;
+ int challenge_maxlen;
+ const char *response;
+};
c2s.patch.gz
Description: c2s.patch.gz
