[Live-devel] RTSP server extending

[Gustaf Räntilä]

Ross Finlayson wrote:
>> Actually there is problem here with sockaddr_in. It explicitly
>> excludes sockaddr_in6. It would better just to use sockaddr to
>> simplify IPV6 porting later.
>>     
>
> The actual parameter to the call is a "sockaddr_in", so that's what I 
> used for the formal parameter.
>
> There are many many places in the code that are IPv4-specific, 
> unfortunately.  (The networking (groupsock) code will need to undergo 
> a major restructuring before IPv6 could be supported...)
>   
The second patch merely fixes a part of what was missing, and it really 
makes no sense to try to solve it all with one function.

Therefore I send in my solution as a patch (to the latest release). It 
extends the other patches from this thread in the following manner:

Connections are controlled by a virtual function (accept/deny). It 
brings a file descriptor of the socket of the session that will be 
created as well as the address of the socket.
When the connection for the session is lost (client disconnected), the 
same function is run again with the address parameter set to NULL. The 
user implementation can then (if needed off course), track sessions 
(with certain IP/port) and what resources they're accessing (see below). 
It has also shown to be good for debugging (what clients from what 
location is connecting/disconnecting).
Definition:
virtual Boolean sessionAccept(int clientSocket, const struct sockaddr* 
clientAddr);

Note: The address is not necessary since you can get that information 
from the file descriptor through getsockname, but why not give it to the 
users who want it? And it is used to show that a session is disconnected 
(no use for yet another virtual function).

Alternative implementation: The address is a "struct sockaddr" which is 
already exposed in the headers from the NetCommon.hh. If Ross (or anyone 
else) finds it more beautiful to use the NetAddress and Port classes, 
feel free to change this. But note that neither of these classes are 
very useful at the moment. They do not expose any comparison operators 
whatsoever, and I didn't want to go into that part (it's not useful for me).
As Igor noted, the sockaddr_in is only for IPv4, and type-casting it to 
anything else (given the size was enough) is bulky with references, so a 
pointer makes more sense. In my patch, I use sockaddr_in6 internally 
which should be large enough for most protocols (do we really need more 
than IPv4 and IPv6 support?). This is done _internally_ so it doesn't 
require anything from anyone, just regular sockaddr* management which 
everyone should be comfortable with. If Ross still wants this as a 
strict IPv4, feel free to change this.

Since this new virtual function provides a way for tracking sessions 
(and thereby gives you the opportunity to store their IP/port in 
whatever fashion you prefer, if at all used), the previous patch's 
clientSocket parameter acts as a reference to the accepted session. 
However, this function is currently only called for the DESCRIBE 
command. A program using liveMedia can never know/track what commands a 
client are sending. This is (as far as I can understand) what the 
original author of this thread was asking for (and what I have been 
using for a while now). I can't however see the reason for overloading 
incomingRequestHandler1 since it is doing all the socket reading for 
you. Therefore, the second part of my patch:

To know whatever successful commands have been received by liveMedia, 
another virtual function is available. This enables an end application 
to track whatever a client is doing. This does not return Boolean, since 
it cannot break (deny access to) the liveMedia functionality. The 
command has already been accepted by the specialClientAccessCheck.

Definition:
virtual void commandSuccess(int clientSocket, char const* cmd, char 
const* urlSuffix, const char* fullRequestStr);

Obviously, this also uses the clientSocket as a reference to what 
session/client it refers to. The cmd is the command being sent from the 
client (OPTIONS, DESCRIBE, TEARDOWN, PLAY, PAUSE, SETUP, etc). The 
urlSuffix is the server resource being requested (whatever is after the 
first slash after the ip/domainname). fullRequestStr provides the entire 
header, which can be especially useful in debugging.

Moreover, since the authorization (until now) only occurs on DESCRIBE 
(authenticationOK is only called from handleCmd_DESCRIBE), clients who 
ignores this (Amino) can still play streams, since the PLAY command 
isn't checked for authorization (trust me, I just tried the patch 
provided in this thread). This is a serious flaw. I have modified the 
specialClientAccessCheck so that it brings the command as well, and 
added it to the PLAY, PAUSE and TEARDOWN (add more if you want) 
functions, so that you can control exactly whatever commands are allowed 
for whatever resource from whatever client.

Comment 1:
I would suggest to rename specialClientAccessCheck into something like 
authorizeResource or authorizeSessionCommand. There is nothing "special" 
about this function, and Check? All virtual functions which are more 
than pure notifiers are checks somehow, should they also be called 
*Check? This is my personal view.
Other than that, I have tried to follow Ross' coding style to the extent 
I could see one.

Comment 2:
It is very difficult to follow the development of liveMedia and 
cooperate by sending patches when there is no source code repository to 
regularly update. Manually diffing the tar.gz tree is cumbersome. I 
wrote this patch mainly for the first release in this thread, but had to 
re-diff everything in the new (the source is small, ok, but it's still 
maybe easier not to send a patch at all, and who gains from that?). I 
just hope there's not yet another tarball to unpack and diff before I 
finally send this mail...

Cheers,
Gustaf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: authorize.patch
Type: text/x-patch
Size: 6823 bytes
Desc: not available
Url : http://lists.live555.com/pipermail/live-devel/attachments/20070424/a0683117/attachment.bin