Package: openvpn-auth-radius
Version: 2.1-6+b1
Severity: normal

Dear maintainers,

When using several openvpn instances on the same subnet, tcp and udp for
example, we noticed routes specified by Framed-Route radius attribute
were not correctly inserted. As create route string doesn't specify the
output device, the routes are always inserted with the first device
matching the Framed-IP-Address (which is always the first launched
openvpn instance in our case).

Here is a patch proposal. It adds the openvpn instance device (given in
the environment) to the create/delete route string.

Kind regards,
-- 
Olivier Le Brouster
Subject: add output device to create/delete route string

When several openvpn instances is running on the same subnet, for example tcp
and udp, routes need to be inserted with a specific output device depending on
which instances the user is connecting to (given by openvpn in "dev"
environment variable).

--- a/AccountingProcess.cpp
+++ b/AccountingProcess.cpp
@@ -92,6 +92,7 @@
                                        
                                        //get the information from the 
foreground process
                                        
user->setUsername(context->acctsocketforegr.recvStr());
+                                       
user->setDevice(context->acctsocketforegr.recvStr());
                                        
user->setSessionId(context->acctsocketforegr.recvStr()) ;
                                        
user->setPortnumber(context->acctsocketforegr.recvInt()); 
                                        
user->setCallingStationId(context->acctsocketforegr.recvStr()); 
@@ -104,7 +105,7 @@
                                        
user->setUntrustedPort(context->acctsocketforegr.recvStr());
                                        context->acctsocketforegr.recvBuf(user);
                                        if (DEBUG (context->getVerbosity()))
-                                       cerr << getTime() << "RADIUS-PLUGIN: 
BACKGROUND ACCT: New user acct: username: " << user->getUsername() << ", 
interval: " << user->getAcctInterimInterval() << ", calling station: " << 
user->getCallingStationId() << ", commonname: " << user->getCommonname() << ", 
framed ip: " << user->getFramedIp() <<".\n";
+                                               cerr << getTime() << 
"RADIUS-PLUGIN: BACKGROUND ACCT: New user acct: username: " << 
user->getUsername() << ", dev: " << user->getDevice() << ", interval: " << 
user->getAcctInterimInterval() << ", calling station: " << 
user->getCallingStationId() << ", commonname: " << user->getCommonname() << ", 
framed ip: " << user->getFramedIp() <<".\n";
                                        
                                        
                                        //set the starttime
--- a/radiusplugin.cpp
+++ b/radiusplugin.cpp
@@ -558,6 +558,15 @@
                                }
                                if ( DEBUG ( context->getVerbosity() ) )
                                        cerr << getTime() << "RADIUS-PLUGIN: 
FOREGROUND: Set FramedIP to the IP (" << newuser->getFramedIp() << ") OpenVPN 
assigned to the user " << newuser->getUsername() << "\n";
+                               //set the device of the user
+                               if(get_env ( "dev", envp ) !=NULL)
+                               {
+                                       newuser->setDevice( string ( get_env ( 
"dev", envp ) ) );
+                               }
+
+                               if ( DEBUG ( context->getVerbosity() ) )
+                                       cerr << getTime() << "RADIUS-PLUGIN: 
FOREGROUND: Set device (" << newuser->getDevice() << ") to the user " << 
newuser->getUsername() << "\n";
+                               
                                //the user must be there and must be 
authenticated but not accounted
                                // isAccounted and isAuthenticated is true it 
is client connect for renegotiation, the user is already in the accounting 
process
                                if ( newuser!=NULL && newuser->isAccounted() 
==false && newuser->isAuthenticated() )
@@ -570,6 +579,7 @@
                                        //send information to the background 
process
                                        context->acctsocketbackgr.send ( 
ADD_USER );
                                        context->acctsocketbackgr.send ( 
newuser->getUsername() );
+                                       context->acctsocketbackgr.send ( 
newuser->getDevice() );
                                        context->acctsocketbackgr.send ( 
newuser->getSessionId() );
                                        context->acctsocketbackgr.send ( 
newuser->getPortnumber() );
                                        context->acctsocketbackgr.send ( 
newuser->getCallingStationId() );
--- a/UserAcct.cpp
+++ b/UserAcct.cpp
@@ -705,6 +705,11 @@
                                        strncat(routestring, " metric ", 8);
                                        strncat(routestring, framedmetric , 5);
                                }
+                               if (this->getDevice().size() > 0)
+                               {
+                                       strncat(routestring, " dev ", 5);
+                                       strncat(routestring, 
this->getDevice().c_str(), this->getDevice().size());
+                               }
                                //redirect the output stderr to /dev/null
                                strncat(routestring," 2> /dev/null",13);
                                
@@ -859,6 +864,12 @@
                                        strncat(routestring, " metric ", 8);
                                        strncat(routestring, framedmetric , 5);
                                }
+                               if (this->getDevice().size() > 0)
+                               {
+                                       strncat(routestring, " dev ", 5);
+                                       strncat(routestring, 
this->getDevice().c_str(), this->getDevice().size());
+                               }
+
                                //redirect the output stderr to /dev/null
                                strncat(routestring," 2> /dev/null",13);
                                
--- a/User.cpp
+++ b/User.cpp
@@ -35,6 +35,7 @@
        this->portnumber=0;
        this->vsabuf=NULL;
        this->vsabuflen=0;
+       this->device="";
 }
 
 /** The constructor sets the acctinteriminterval to 0 and the portnumber to 
num.
@@ -100,6 +101,7 @@
 {
        this->username=u.username;
        this->commonname=u.commonname;
+       this->device=u.device;
        this->framedroutes=u.framedroutes;
        this->framedip=u.framedip;
        this->key=u.key;
@@ -149,6 +151,19 @@
        this->commonname=cn;
 }
 
+/** The getter method for the device.
+ *  @return The device as a string.*/
+string User::getDevice(void)
+{
+       return this->device;
+}
+/** The setter method for the commonname.
+ * @param cn The commonname.*/
+void User::setDevice(string device)
+{
+       this->device=device;
+}
+
 /** The getter method for the framed routes.
  *  @return The framed routes as a string.*/   
 string User::getFramedRoutes(void)
--- a/User.h
+++ b/User.h
@@ -44,6 +44,7 @@
 protected:
        string username;                        /**<The username.*/
        string commonname;                      /**<The commonname.*/
+       string device;
        string framedroutes;            /**<The framedroutes, they are stored 
as a string. if there are more routes, they must be delimted by an ';'*/
        string framedip;                /**<The framed ip.*/
        string callingstationid;        /**<The calling station id, in this 
case the real ip addres of the client.*/
@@ -72,7 +73,9 @@
        string getCommonname(void);
        void setCommonname(string);
                
-       
+       void setDevice(string);
+       string getDevice(void);
+
        string getFramedRoutes(void);
        void setFramedRoutes(string);
        

Reply via email to