Closed formula1 closed 9 years ago
Interesting stuff. I will reply back later today. On Dec 16, 2014 2:48 AM, "Sam Tobia" notifications@github.com wrote:
Able to start new servers with the new operator. I could probably at this point make it so that there is a TCP and UDP server. However, looking forward I found 2 major obstacles that I am not confident I can script kitty my way through.
- libuv https://github.com/libuv/libuv - This is the main loop that runs in node. It is not necessary to participate in it but I am left to wonder if what I'm doing is creating a c++ wrapper to interface with nodejs. Granted, yes however I'm not sure if that is the best method. That being said, I'll see about just incorperating a uv_run to ensure the stun server stays alive.
- Asyncronous Auth Check https://github.com/formula1/stunserver/blob/master/stuncore/messagehandler.cpp#L131
- Nodejs prodominately works off of an asyncronous flow. Usually resulting in callback in callback in callback or heavilly relient on event emitters, however it also has some useful utilities like async https://github.com/caolan/async and promises https://github.com/jakearchibald/es6-promise which makes it bearable. That being said, because the authorization checks are syncronous, it is not as simple to integrate with it.
Here are some common databases available in node
- CouchDB - https://www.npmjs.com/package/node-couchdb
- MongoDB - https://github.com/learnboost/mongoose
- MySql - https://github.com/brianc/node-sql (though it is available, kind of taboo)
Most filesystem calls are also expected to be async - http://nodejs.org/api/fs.html
If I were to work with the server in php, this wouldn't be an issue as in php I wouldn't care how long it took. However, I node
As a result there becomes a few options
- Make the auth function poll for a response from node - There would be a mapping between the request and the hopeful response. This however will cause a block from within the server
- Make the auth function ayncronous - This is what I was going to try and do and it doesn't seem like its completely undoable as the main failure to worry about https://github.com/formula1/stunserver/blob/master/server/stunsocketthread.cpp#L369 seems to only have a small amount of methods afterwards. In addition, the return value isn't used so it doesn't technically matter
- Move completely towards libuv however continue to parse the data in the same manner - the message handler https://github.com/formula1/stunserver/blob/master/stuncore/messagehandler.cpp would have to start becoming more independent, the config file using uv addresses, getting rid of the threading. There would be many many changes for which I had no more desire to count.
However, long story short, I would have to start editing the base. Whether
its small or large
You can merge this Pull Request by running
git pull https://github.com/formula1/stunserver new_operator
Or view, comment on, or merge it at:
https://github.com/jselbie/stunserver/pull/13 Commit Summary
- handling more than one argument as options object or as native arguments
- proposal 2 and other changes
- small error and breif test
- testing every permutation
- improper modulous
- one arg and configuration file building
- fixes and making some tests expect errors
- Can create stun servers with new operator
File Changes
- M .gitignore https://github.com/jselbie/stunserver/pull/13/files#diff-0 (2)
- M nodejs/binding.cpp https://github.com/jselbie/stunserver/pull/13/files#diff-1 (88)
- M nodejs/binding.gyp https://github.com/jselbie/stunserver/pull/13/files#diff-2 (3)
- A nodejs/nodestun_args.cpp https://github.com/jselbie/stunserver/pull/13/files#diff-3 (558)
- A nodejs/nodestun_args.h https://github.com/jselbie/stunserver/pull/13/files#diff-4 (21)
- A nodejs/nodestun_object.cc https://github.com/jselbie/stunserver/pull/13/files#diff-5 (113)
- A nodejs/nodestun_object.h https://github.com/jselbie/stunserver/pull/13/files#diff-6 (23)
- M nodejs/test.js https://github.com/jselbie/stunserver/pull/13/files#diff-7 (120)
- A nodejs/tests/multi_instance.js https://github.com/jselbie/stunserver/pull/13/files#diff-8 (60)
- A nodejs/threeargstest.js https://github.com/jselbie/stunserver/pull/13/files#diff-9 (49)
Patch Links:
- https://github.com/jselbie/stunserver/pull/13.patch
- https://github.com/jselbie/stunserver/pull/13.diff
— Reply to this email directly or view it on GitHub https://github.com/jselbie/stunserver/pull/13.
Sam,
This discussion is getting fragmented between the original thread in the "issues" section and this thread in the pull request. Can we continue this conversation over email (jselbie at g mail dot com) ? It would be a lot easier for me.
Back to the discussion.
First, I want to ask. Are you absolutely certain that you need to have your STUN server support authentication? Do you know if it supports the "long term credential" form of auth, "short term (password) credential" form, or both? I ask because I do not know of any client library, including WebRTC, that will do the authentication flow with STUN as they might with TURN. I can't remember why I thought WebRTC doesn't auth with STUN. Maybe that has changed since last year. I suppose we can hack up an instance of STUNTMAN using the sample auth provider code and see if it will do the auth flow with a webclient. But I think we should validate that assumption before doing a lot of hacking to an add auth layer through NodeJS.
Weird - I just noticed that STUNTMAN defines the max length for USERNAME, NONCE, and REALM as 64 characters. But RFC clearly defines these fields to have larger values. I should probably re-validate that it was done correctly.
As for the asynchronous vs synchronous problem.
STUNTMAN was written to assume that authentication could be done synchronously. While NodeJS has asynchronous i/o all the way. The way I see this working is this:
The implementation of IStunAuth::DoAuthCheck should invoke an asynchronous method on NodeJS. And then just wait on an event handle (conditional variable) that gets signaled by another object or method exposed by the C++ bindings that Node knows how to talk to. The Node code, in response to a DoAuthCheck callback, should fire an event to a different thread telling it to do the auth lookup for the given {username, password, realm, nonce} fields that are passed in. When Node completes the auth lookup, it invokes something (on a different thread from the one that DoAuthCheck originated on) to signal the event. Something like the following:
class AuthFromNodeJS : public IStunAuth
{
HRESULT DoAuthCheck(AuthAttributes* pAuthAttributes, AuthResponse* pResponse)
{
Authenticator auth;
return auth.DoAuthCheck(pAuthAttributes, pResponse);
}
}
class Authenticator
{
bool _isCompleted;
pthread_cond _cond;
pthread_mutex _mutex;
AuthResponse _response;
HRESULT DoAuthCheck(AuthAttributes* pAuthAttributes, AuthResponse* pResponse)
{
_isCompleted = false;
// NOT SHOWN - invoke some Node object that has already been passed in during server intialization does the work of asynchronously starting an auth request.
pthread_mutex_lock(&_mutex);
while (_isCompleted == false)
{
pthread_cond_wait(&_cond, &_mutex);
}
pthread_mutex_unlock(&_mutex);
memcpy(pResponse, &_authresponse, sizeof(AuthResponse));
}
// NOT SHOWN - that other NodeJS object mentioned above, upon getting the callback that the async auth request has completed, call this methods
void onNodeAuthCallback(AuthResponse* pResponse)
{
_isCompleted = true;
pthread_cond_signal(&_cond);
}
};
Absolutely. Thank you for being patient with me. I'll send you my response email.
Able to start new servers with the new operator. I could probably at this point make it so that there is a TCP and UDP server. However, looking forward I found 2 major obstacles that I am not confident I can script kitty my way through.
uv_run
to ensure the stun server stays alive.Here are some common databases available in node
Most filesystem calls are also expected to be async - http://nodejs.org/api/fs.html
If I were to work with the server in php, this wouldn't be an issue as in php I wouldn't care how long it took. However, I node
As a result there becomes a few options
However, long story short, I would have to start editing the base. Whether its small or large