[Home]

Summary:ASTERISK-01643: Allow interface restriction for RTP
Reporter:khb (khb)Labels:
Date Opened:2004-05-17 20:18:52Date Closed:2011-06-07 14:05:26
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) manager_patch_listenaddr.txt
( 1) new_rtp_c_patch.txt
( 2) new_rtp_h_patch.txt
( 3) new_rtp_patch_readme.txt
( 4) new_sip_c_patch.txt
( 5) new_sip_conf_patch.txt
( 6) rtp_patch_bindaddr.txt
( 7) rtp.conf.sample
Description:RTP always binds to all interfaces (0.0.0.0) on a machine.
The patch allows restriction to one interface similar to what is provided for other modules.

****** ADDITIONAL INFORMATION ******

This introduces a
bindaddr=<ipaddress or hostname>
option in the general section of rtp.conf
documentation change also included.

Comments:By: khb (khb) 2004-05-17 20:28:41

I really would like to weed out the ugly 'bindaddr' nomenclature and rename all such options in asterisk to 'interface=' but left it here for uniformity, for now.
Also, the rtpstart and rtpend options would be more meaningfull if they were named
portstart and portend.  Shall I patch that with deprecation notices for a while?

I am also attaching a tweak to the manager.c code to print into the log both the IP address and port, not just the port.  That patch also removes the recognition of the long deprecated portno= option.

By: Mark Spencer (markster) 2004-05-18 01:02:55

As I said on the other bug/feature request about this, it is chan_sip, chan_mgcp, et al that should say what the bind address is based on their own values.

For example, I might want chan_sip to bind to my public IP while i want chan_mgcp to bind to a local private interface.  It doesn't belong in rtp.conf.  If you fix it so that chan_sip passes a bind address (or NULL to use 0.0.0.0) then i'll be happy to merge it.

By: khb (khb) 2004-05-18 01:29:04

Yes, you are absolutely right about that.
We'll redo.

By: Mark Spencer (markster) 2004-05-18 01:41:44

Thanks, there will be a lot of people happy to see this.  Also with respect to the manager patch, I went ahead and updated the coding guidelines to discourage nested blocks with no braces (eg. if (foo) if(bar) baz(); ).

By: flavour (flavour) 2004-05-18 10:14:45

2 other implementations of this:
ASTERISK-1013
ASTERISK-1316

By: khb (khb) 2004-05-18 20:26:14

Sorry, I totally overlooked the previous tracks on this. Looking pretty stupid.
But here is a new solution.

By: khb (khb) 2004-05-18 20:35:34

RTP port management
===================

This patch provides additional API capabilities for RTP
for the allocation of ports that RTP uses when creating streams.

Up to now RTP obtains a range of ports to use from its configuration
file 'rtp.conf'.  All ports were opened an all interfaces of the host.

This package allows each client channel (SIP, MGCP, H323, etc)
to select its own range of ports and allows the restriction
of binding only on the channel's own interface.

The rtp function 'ast_rtp_new' was used to open an RTP
channel pair. This has been deprecated, but is still available
for code that has not been converted to take advantage of
the new features.

The new function is 'ast_rtp_create' (as an opposite to
ast_rtp_destroy).  ast_rtp_create requires an additional
argument of *sockaddr_in which contains the IP address
information to use in binding. if the pointer to the
structure is NULL, RTP will revert to its old behavior.

It's the responsibility of the caller to setup the
structure correctly. RTP will not check the validity
of the address until it fails on bind().

If the address structure contains a non-zero port number,
RTP will try to bind to this port and if rtcp is needed,
also to port+1.  If the port was zero or cannot be
successfully bound, RTP will search in its default port
range for a successful pair.

The default port range can be overruled by the calling
channel driver with a call to 'ast_rtp_set_portrange(start,end)'
This allows each driver to use custom port ranges.

The currently installed RTP port range can be queried
with a call to 'ast_rtp_get_portrange(&start, &end)'

The creation of RTP ports can be monitored on the console
by using  'set verbose 5'

First implementation is for chan_sip.
It does not allow SIP to use a different ip address to bind RTP to
than it's own address.  I don't know if anyone would want this.
But it introduces a new SIP option to set the port range.
This really makes the current contents of rtp.conf obsolete.

rtpports = <start>, <end>

Any better naming suggestions?

The files all start with new_  and contents is obvious from name.

This code should be tested a bit before release.  I like to get some feedback
for problems and suggestions that others may have.
The port search code tries a little harder than before to find an adjacent free port pair on an even start, but in the process it closes and re-opens potentially a lot of sockets, which might be bad.

khb@brose.com

edited on: 05-18-04 20:51

By: Mark Spencer (markster) 2004-05-18 22:37:52

Port ranges have to be passed as parameters (and passing 0, 0 should use the defaults in rtp.conf) otherwise there exists a race if SIP and MGCP for example are making channels at the same time.  I don't really care about API backwards compatibility (although obviously it's trivial to maintain it so we mind as well) but you'll also need to update chan_skinny and chan_mgcp, even if you don't add the ports option to them.  Don't take the port functionality out of rtp.conf -- use it when the range isn't specified.  Be sure that on a reload, the begin/end for the ports in sip.conf defaults to 0/0 so tha it will follow rtp.conf for *functional* backwards compatibility.

Definitely getting closer now.

Also, make sure that rtp.conf can use the same syntax for ports as sip.conf, and deprecate the rtpstart and rtpend parameters (unless you'd rather have rtpstart and rtpend in both places, but rtpports or "rtprange" would sound better).  

Finally, I would imagine that rtpports => 10000-20000 looks better than 10000, 20000.

By: khb (khb) 2004-05-19 00:28:54

Ok, good suggestions.
Have actually already gone through the parameter only version,
which indeed now seems the better solutions. Since chan_sip is calling the
rtp_set_portrange before rtp_create everytime, it might as well be
one call.
Ok, another round.  There seems to be a fd leak in there also.

What about the IP address question, does it make any sense ever to
send RTP on a different interface? Need to worry about proper SDP settings then. Probably gets too complex for the cause, which was to remove RTP from other 0.0.0.0 interfaces.

By: Mark Spencer (markster) 2004-05-19 09:17:34

I don't believe it ever makes sense to have signalling and RTP on different bind addr's.  If someone wants it, we'll let them ask for it.

By: Mark Spencer (markster) 2004-05-21 23:16:50

Any update here?

By: Mark Spencer (markster) 2004-05-23 22:07:28

Still intererested in finishing this patch?

By: khb (khb) 2004-05-23 23:31:50

I'll get back on it after Monday afternoon. Had a project priority.

By: Mark Spencer (markster) 2004-06-03 14:40:21

Just wanting to touch base here...

By: Mark Spencer (markster) 2004-06-21 15:06:36

Looks like interest has been lost in this project.