Re: c2s per session user data & authreg auth API extension
>Changes almost ready to go, will submit a pull request within the hour. Pull request submitted: https://github.com/jabberd2/jabberd2/pull/72 Issue create to track (perhaps excessive): https://github.com/jabberd2/jabberd2/issues/71 Looking forward to comments and feedback! Thanks, Shawn
Re: c2s per session user data & authreg auth API extension
>>I have modified the >> APIs to pass sess_t and then the implementation can choose to pack it >> in their private authreg_private data if they so choose. > >WFM :-) Changes almost ready to go, will submit a pull request within the hour.
Re: c2s per session user data & authreg auth API extension
Dnia 2014-08-14, czw o godzinie 23:45 +, Shawn Debnath pisze:
> I have modified the
> APIs to pass sess_t and then the implementation can choose to pack it
> in their private authreg_private data if they so choose.
WFM :-)
--
Tomasz Sterna :(){ :|:&};:
Instant Messaging Consultant Open Source Developer
http://abadcafe.pl/ http://xiaoka.com/portfolio
signature.asc
Description: This is a digitally signed message part
Re: c2s per session user data & authreg auth API extension
>Dnia 2014-08-14, czw o godzinie 16:20 +, Shawn Debnath pisze:
>> I would change all the APIs and to pass in a pointer to the sess_t as
>> I also need it in check_passsword.
>
>I would advise to include sess_t* in authreg_private then.
>
>It's OK for authreg to dig around session data, but the API should be
>flexible enough to give option to pass anything as authreg_private, not
>only sess_t*.
authreg_private is opaque as far as c2s is concerned. I have modified the
APIs to pass sess_t and then the implementation can choose to pack it in
their private authreg_private data if they so choose. c2s shouldn¹t dictate
what¹s in authreg_private as its a void * member and implementation
dependent.
Once they have sess_t, they can initialize authreg_private as they wish or
leave it NULL. On session closure, c2s checks if the pointer is NULL, if
so,
calls free().
Here¹s what it looks like:
Struct sess_t {
[...
/* Per user session authreg private data */
void*authreg_private;
};
/** returns 1 if the user exists, 0 if not */
int (*user_exists)(authreg_t ar, sess_t sess, const char
*username,const char *realm);
/** return this users cleartext password in the array (digest auth,
password auth) */
int (*get_password)(authreg_t ar, sess_t sess, const char
*username, const char *realm, char password[257]);
/** check the given password against the stored password, 0 if equal,
!0 if not equal (password auth) */
int (*check_password)(authreg_t ar, sess_t sess, const char
*username, const char *realm, char password[257]);
/** store this password (register) */
int (*set_password)(authreg_t ar, sess_t sess, const char
*username, const char *realm, char password[257]);
/** make or break the user (register / register remove) */
int (*create_user)(authreg_t ar, sess_t sess, const char
*username, const char *realm);
int (*delete_user)(authreg_t ar, sess_t sess, const char
*username, const char *realm);
void(*free)(authreg_t ar);
/* Additions at the end - to preserve offsets for existing modules */
/** returns 1 if the user is permitted to authorize as the
requested_user, 0 if not. requested_user is a JID */
int (*user_authz_allowed)(authreg_t ar, sess_t sess, const
char *username, const char *realm, const char *requested_user);
/** Apple extensions for challenge/response authentication methods */
int (*create_challenge)(authreg_t ar, sess_t sess, const char
*username, const char *realm, const char *challenge, int maxlen);
int (*check_response)(authreg_t ar, sess_t sess, const char
*username, const char *realm, const char *challenge, const char *response);
};
Re: c2s per session user data & authreg auth API extension
Dnia 2014-08-14, czw o godzinie 16:20 +, Shawn Debnath pisze:
> I would change all the APIs and to pass in a pointer to the sess_t as
> I also need it in check_passsword.
I would advise to include sess_t* in authreg_private then.
It's OK for authreg to dig around session data, but the API should be
flexible enough to give option to pass anything as authreg_private, not
only sess_t*.
--
Tomasz Sterna :(){ :|:&};:
Instant Messaging Consultant Open Source Developer
http://abadcafe.pl/ http://xiaoka.com/portfolio
signature.asc
Description: This is a digitally signed message part
Re: c2s per session user data & authreg auth API extension
Great! Yes I was a bit iffy on the ³custom² approach myself given it really didn¹t align with any authentication method. But if API breakage is okay, this is definitely the cleanest way. >>- Retrofit existing interfaces with the necessary data. >> a. Introduce void *sess_private in sess_t. > >It's not really sess_private, but authreg_private, right? Yep agreed. Though what would be better is if we could somehow classify it as per user per session data. Thinking of user_st.module_data in sm.h. But authreg_private works if no other suggestions are found. >> 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. > >Agreed. >I think we should extend all authreg calls with a pointer to session >attached, authreg private data. >In the simplest case it could be even set to point to sess_st, for the >mechanizm to dig in session by itself. >This is how it is done all around jabberd2. Yep agreed, providing sess_st avoids another API change down the road and gives access to all the necessary data needed. >Also good point, that create_challenge misses realm parameter. > >If we go for it, we will just release 2.4.x line which hints API >breakage. ;-) This is perfect. Though, just a heads up, I may have some more suggestions on the storage_* side as well once I dig into it a bit more :) >Let's just implement CRAM-MD5 properly, with all needed features, even >if it means API changes. >We're open source - we are not afraid to change things. :-) I would change all the APIs and to pass in a pointer to the sess_t as I also need it in check_passsword. Similar issues, I need to store the authentication token for a successful verification. Also, this way the APIs are uniform. This is good stuff. These changes help any organization providing centralized token based authentication where the resource specific and user session needs to be stashed away properly. I should be able to get a patch/pull reques back out by EOD today. Thanks, Shawn
Re: c2s per session user data & authreg auth API extension
Dnia 2014-08-14, czw o godzinie 04:27 +, Shawn Debnath pisze: > - Build a hash table of relevant data and store it in the authreg_t > private data member. Agreed, that needed internal bookkeeping makes it not feasible. > - Retrofit existing interfaces with the necessary data. > a. Introduce void *sess_private in sess_t. It's not really sess_private, but authreg_private, right? > 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. Agreed. I think we should extend all authreg calls with a pointer to session attached, authreg private data. In the simplest case it could be even set to point to sess_st, for the mechanizm to dig in session by itself. This is how it is done all around jabberd2. Also good point, that create_challenge misses realm parameter. If we go for it, we will just release 2.4.x line which hints API breakage. ;-) > /* 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); I don't like this approach for two reasons. - custom_auth does not really mean anything. as it is now it is clean - either we have password verification, or challenge/response. - custom_auth is used in "ar_mechs & AR_MECH_TRAD_CRAMMD5", so it is not really custom, but CRAM-MD5, right? Let's just implement CRAM-MD5 properly, with all needed features, even if it means API changes. We're open source - we are not afraid to change things. :-) -- smk
