Looks like the init and destroy methods went away 5 days ago. My module (rlm_otp) needs them. - How was this tested? struct module_t rlm_otp still contains these methods. How can this even compile? - What are the chances of getting these back? I can do something like pthread_once(otp_init()) and add a refcount to cleanup in detach, but really, init and destroy are superior. -frank
Frank Cusack <fcusack@fcusack.com> wrote:
Looks like the init and destroy methods went away 5 days ago. My module (rlm_otp) needs them.
Yes. It was discussed here, but I meant to email you privately.
- How was this tested? struct module_t rlm_otp still contains these methods. How can this even compile?
I delete rlm_otp from the makefile.
- What are the chances of getting these back? I can do something like pthread_once(otp_init()) and add a refcount to cleanup in detach, but really, init and destroy are superior.
I'm not convinced any module needs them. The other modules that used them could be modified easily to not use them. I took a quick look through rlm_otp, and didn't see anything that struck me as a show-stopper. If necessary, they can be added back, but I'd like to know what rlm_otp is doing with them, and if another approach is equivalent. Alan DeKok.
On August 23, 2005 11:28:20 AM -0400 Alan DeKok <aland@ox.org> wrote:
Frank Cusack <fcusack@fcusack.com> wrote:
Looks like the init and destroy methods went away 5 days ago. My module (rlm_otp) needs them.
Yes. It was discussed here, but I meant to email you privately.
- How was this tested? struct module_t rlm_otp still contains these methods. How can this even compile?
I delete rlm_otp from the makefile.
- What are the chances of getting these back? I can do something like pthread_once(otp_init()) and add a refcount to cleanup in detach, but really, init and destroy are superior.
I'm not convinced any module needs them. The other modules that used them could be modified easily to not use them. I took a quick look through rlm_otp, and didn't see anything that struck me as a show-stopper.
If necessary, they can be added back, but I'd like to know what rlm_otp is doing with them, and if another approach is equivalent.
It's fairly obvious from the code. I open a single file handle to /dev/random, shared by all instances, and I setup an hmac key which I need to sign access-challenge STATE attributes. I close the fd and clear the hmac in destroy. You're right, no module *needs* init and destroy. These are easily simulated with pthread_once() and a refcount, but you can consider the modules to be classes, and a static constructor and destructor is a natural for them. init and destroy methods make sense. There's no reason a module *has* to use them, but they should be there if you want them. Please advise. -frank
Frank Cusack <fcusack@fcusack.com> wrote:
It's fairly obvious from the code.
OK. A work-around can be to do: static int initialized_flag = FALSE; instantiate() { if (!initialized_flag) { do stuff... } } I wouldn't worry about the "destroy" function.
I open a single file handle to /dev/random, shared by all instances, and I setup an hmac key which I need to sign access-challenge STATE attributes. I close the fd and clear the hmac in destroy.
A *larger* issue is that you shouldn't be using /dev/random, as it's blocking. It's also non-portable (for what that's worth). I suggest using lrad_rand(), which returns a cryptographically strong random 32-bit integer. As for signing the State attribute, the HMAC key can also be generated by using lrad_rand().
You're right, no module *needs* init and destroy. These are easily simulated with pthread_once() and a refcount, but you can consider the modules to be classes, and a static constructor and destructor is a natural for them. init and destroy methods make sense.
And LD_PRELOAD as class overloading... (yes, I've done it)
There's no reason a module *has* to use them, but they should be there if you want them.
We can add them back in, but I don't think they're *required* for what you're doing. The hmac key for signing State SHOULD be per-instance, too. Otherwise you run into the issue of one OTP module getting a State from another one, and not knowing what to do with it. Alan DeKok.
On August 23, 2005 6:55:09 PM -0400 Alan DeKok <aland@ox.org> wrote:
Frank Cusack <fcusack@fcusack.com> wrote:
It's fairly obvious from the code.
OK. A work-around can be to do:
static int initialized_flag = FALSE;
instantiate() { if (!initialized_flag) { do stuff... } }
Sure, but that's awkward compared to having an init method. As an aside, I thought instantiate ran on each thread creation, but I see now it's a config-based thing and runs in a single thread (so I don't need pthread_once()).
I wouldn't worry about the "destroy" function.
I open a single file handle to /dev/random, shared by all instances, and I setup an hmac key which I need to sign access-challenge STATE attributes. I close the fd and clear the hmac in destroy.
A *larger* issue is that you shouldn't be using /dev/random, as it's blocking.
I misspoke; I actually use /dev/urandom.
It's also non-portable (for what that's worth).
It's as portable as I require. Systems that don't have it really aren't worth running on. :-)
I suggest using lrad_rand(), which returns a cryptographically strong random 32-bit integer.
I also need it to generate challenges, which is also found in my other otp code (non-freeradius), and much code is shared with rlm_otp. It doesn't really help me to use lrad_rand().
We can add them back in, but I don't think they're *required* for what you're doing.
They're not; I don't think I ever disputed that. I just don't see the point of removing them, and making instantiate() awkward for the few modules (now and perhaps later) that need them.
The hmac key for signing State SHOULD be per-instance, too. Otherwise you run into the issue of one OTP module getting a State from another one, and not knowing what to do with it.
I don't see how that can happen. If one instance of the OTP module generates State, that same instance will be the one to process it. It works at my site, at least. -frank
On August 23, 2005 5:15:54 PM -0700 Frank Cusack <fcusack@fcusack.com> wrote:
On August 23, 2005 6:55:09 PM -0400 Alan DeKok <aland@ox.org> wrote:
We can add them back in, but I don't think they're *required* for what you're doing.
They're not; I don't think I ever disputed that. I just don't see the point of removing them, and making instantiate() awkward for the few modules (now and perhaps later) that need them.
Please let me know your final decision, so that I can get rlm_otp working again. thanks -frank
Frank Cusack <fcusack@fcusack.com> wrote:
Please let me know your final decision, so that I can get rlm_otp working again.
Sorry, I've been thinking about the best way to fix it. For FreeRADIUS, you can do: #ifdef _LIBRADIUS_H /* freeradius code, including calls to lrad_rand() */ #else /* other code */ #endif If you're OK with that, we can move ahead. Otherwise, we'll have to look at re-adding init & destroy. Alan DeKok.
participants (2)
-
Alan DeKok -
Frank Cusack