Allow to limit SVDRP port to given IP

Message ID 20100109170057.31040@gmx.net
State New
Headers

Commit Message

Manuel Reimer Jan. 9, 2010, 5 p.m. UTC
  Hello,

I've attached a second patch. This patch changes VDR's svdrp port handling in the following way: If only the localhost item is found in svdrphosts.conf, then the port is attached to "INADDR_LOOPBACK", which makes it impossible to reach the port from outside. As soon as even one additional item is added, the port is opened to "INADDR_ANY", again, so the port is accessible from network. This way, the default configuration of VDR is "rock solid" and there should be no need to disable svdrp at all. We also have no additional configuration mechanisms. The user only has to work with svdrphosts.conf, as he did with previous VDR versions.

CU

Manuel
  

Comments

Theunis Potgieter Jan. 9, 2010, 5:10 p.m. UTC | #1
Sorry to make things complicated, but would it not satisfy everybody
needs if you could bind to an ip address, which could be any one you
specify? For example, I would prefer mine to be bind to my eth0's ip
for internal lan clients to connect, but not accessible via ppp0 for
example.

So I guess what I'm asking is, if you could start vdr with a listen
parameter switch and could specify for each listen switch a source ip
address or even a interface name?

Theunis

2010/1/9 Manuel Reimer <Manuel.Reimer@gmx.de>:
> Hello,
>
> I've attached a second patch. This patch changes VDR's svdrp port handling in the following way: If only the localhost item is found in svdrphosts.conf, then the port is attached to "INADDR_LOOPBACK", which makes it impossible to reach the port from outside. As soon as even one additional item is added, the port is opened to "INADDR_ANY", again, so the port is accessible from network. This way, the default configuration of VDR is "rock solid" and there should be no need to disable svdrp at all. We also have no additional configuration mechanisms. The user only has to work with svdrphosts.conf, as he did with previous VDR versions.
>
> CU
>
> Manuel
> --
> ()  ascii ribbon campaign - against html mail
> /\                        - gegen HTML-Mail
> answers as html mail will be deleted automatically!
> Antworten als HTML-Mail werden automatisch gelöscht!
>
> GRATIS für alle GMX-Mitglieder: Die maxdome Movie-FLAT!
> Jetzt freischalten unter http://portal.gmx.net/de/go/maxdome01
>
> _______________________________________________
> vdr mailing list
> vdr@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
>
>
  
Diego Pierotto Jan. 9, 2010, 7:01 p.m. UTC | #2
Il 09/01/2010 18:10, Theunis Potgieter ha scritto:
> Sorry to make things complicated, but would it not satisfy everybody
> needs if you could bind to an ip address, which could be any one you
> specify? For example, I would prefer mine to be bind to my eth0's ip
> for internal lan clients to connect, but not accessible via ppp0 for
> example.
>
> So I guess what I'm asking is, if you could start vdr with a listen
> parameter switch and could specify for each listen switch a source ip
> address or even a interface name?
>
> Theunis
>
> 2010/1/9 Manuel Reimer <Manuel.Reimer@gmx.de>:
>   
>> Hello,
>>
>> I've attached a second patch. This patch changes VDR's svdrp port handling in the following way: If only the localhost item is found in svdrphosts.conf, then the port is attached to "INADDR_LOOPBACK", which makes it impossible to reach the port from outside. As soon as even one additional item is added, the port is opened to "INADDR_ANY", again, so the port is accessible from network. This way, the default configuration of VDR is "rock solid" and there should be no need to disable svdrp at all. We also have no additional configuration mechanisms. The user only has to work with svdrphosts.conf, as he did with previous VDR versions.
>>
>> CU
>>
>> Manuel
>> --
>> ()  ascii ribbon campaign - against html mail
>> /\                        - gegen HTML-Mail
>> answers as html mail will be deleted automatically!
>> Antworten als HTML-Mail werden automatisch gelöscht!
>>
>> GRATIS für alle GMX-Mitglieder: Die maxdome Movie-FLAT!
>> Jetzt freischalten unter http://portal.gmx.net/de/go/maxdome01
>>
>> _______________________________________________
>> vdr mailing list
>> vdr@linuxtv.org
>> http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
>>
>>
>>     
> _______________________________________________
> vdr mailing list
> vdr@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
>
>   
Right!!!

>From a simple user (not a technical one) i appreciate an item menu so i
can set the local IP to enable, without need to modify configuration
files on the file system.

That's a simple, functional and easy solution, obvious IMHO.

Diego
  
Olaf Titz Jan. 9, 2010, 7:32 p.m. UTC | #3
> I've attached a second patch. This patch changes VDR's svdrp port
> handling in the following way: If only the localhost item is found in
> svdrphosts.conf, then the port is attached to "INADDR_LOOPBACK", which
> makes it impossible to reach the port from outside. As soon as even one
> additional item is added, the port is opened to "INADDR_ANY", again, so
> the port is accessible from network. This way, the default configuration

I was just about to prepare a patch which does this, and apart from
the naming of functions it looks _identical_ to this one.
So it has to be correct :-) (I've also tested that this works with
empty or missing svdrphosts.conf too, although in this case we could
as well disable SVDRP entirely.)

Documentation and sample is correct in this case, too.

Olaf
  
Manuel Reimer Jan. 10, 2010, 10:48 a.m. UTC | #4
Theunis Potgieter wrote:
> Sorry to make things complicated, but would it not satisfy everybody
> needs if you could bind to an ip address, which could be any one you
> specify? For example, I would prefer mine to be bind to my eth0's ip
> for internal lan clients to connect, but not accessible via ppp0 for
> example.
> 
> So I guess what I'm asking is, if you could start vdr with a listen
> parameter switch and could specify for each listen switch a source ip
> address or even a interface name?

... where we would be back at my first patch, which added the possiblity to specify the svdrp port in the way "$IP:$PORT".

Maybe it would be an idea to combine the two. If the IP is given at command line with the -p switch, then this setting is preferred. If the IP isn't given at command line, then it is 127.0.0.1 if svdrphosts.conf only contains "127.0.0.1" and 0.0.0.0 otherwise.

IMHO we shouldn't get too complicated at this point, as it isn't VDRs main usage to be a network daemon, unlike apache, where there are much more and much more complicated ways to listen to one or multiple ports and to filter based on IP.

CU

Manuel
  

Patch

diff -ruN -x Make.config -x Makefile -x dvbdevice.h vdr-1.6.0-2.org/config.c vdr-1.6.0-2/config.c
--- vdr-1.6.0-2.org/config.c	2008-02-17 14:39:00.000000000 +0100
+++ vdr-1.6.0-2/config.c	2010-01-09 18:32:22.626879490 +0100
@@ -121,6 +121,11 @@ 
   return (Address & mask) == (addr.s_addr & mask);
 }
 
+bool cSVDRPhost::IsLocalhost(void)
+{
+  return (addr.s_addr == htonl(INADDR_LOOPBACK));
+}
+
 // --- cCommands -------------------------------------------------------------
 
 cCommands Commands;
@@ -141,6 +146,17 @@ 
   return false;
 }
 
+bool cSVDRPhosts::LocalhostOnly(void)
+{
+  cSVDRPhost *h = First();
+  while (h) {
+        if (!h->IsLocalhost())
+           return false;
+        h = (cSVDRPhost *)h->Next();
+        }
+  return true;
+}
+
 // --- cSetupLine ------------------------------------------------------------
 
 cSetupLine::cSetupLine(void)
diff -ruN -x Make.config -x Makefile -x dvbdevice.h vdr-1.6.0-2.org/config.h vdr-1.6.0-2/config.h
--- vdr-1.6.0-2.org/config.h	2010-01-09 18:45:11.477536875 +0100
+++ vdr-1.6.0-2/config.h	2010-01-09 18:29:36.565516740 +0100
@@ -73,6 +73,7 @@ 
   cSVDRPhost(void);
   bool Parse(const char *s);
   bool Accepts(in_addr_t Address);
+  bool IsLocalhost(void);
   };
 
 template<class T> class cConfig : public cList<T> {
@@ -164,6 +165,7 @@ 
 class cSVDRPhosts : public cConfig<cSVDRPhost> {
 public:
   bool Acceptable(in_addr_t Address);
+  bool LocalhostOnly(void);
   };
 
 extern cCommands Commands;
diff -ruN -x Make.config -x Makefile -x dvbdevice.h vdr-1.6.0-2.org/svdrp.c vdr-1.6.0-2/svdrp.c
--- vdr-1.6.0-2.org/svdrp.c	2010-01-09 18:45:11.535528281 +0100
+++ vdr-1.6.0-2/svdrp.c	2010-01-09 18:34:11.211458832 +0100
@@ -79,7 +79,7 @@ 
      struct sockaddr_in name;
      name.sin_family = AF_INET;
      name.sin_port = htons(port);
-     name.sin_addr.s_addr = htonl(INADDR_ANY);
+     name.sin_addr.s_addr = SVDRPhosts.LocalhostOnly() ? htonl(INADDR_LOOPBACK) : htonl(INADDR_ANY);
      if (bind(sock, (struct sockaddr *)&name, sizeof(name)) < 0) {
         LOG_ERROR;
         Close();