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);