Add support for Linux VRF (SO_BINDTODEVICE), use per-server sockets if src_ip or vrf are requested

[bluca]

Linux VRF doc: https://www.kernel.org/doc/Documentation/networking/vrf.txt

I've tested this on Debian 9 with the following config file:

127.0.0.1 secret 1
192.168.122.122 secret 1 0 vrf-blue
192.168.1.1 secret 1 192.168.1.10
192.168.122.122 secret 1 192.168.122.2 vrf-blue

The 192.168.122.x is on a virtual interface enslaved to a VRF. 192.168.1.x is on an interface with 2 IP addresses, and the config asks to use the second one (to test src_ip).

With Wireshark I could observe all 4 packets going out from the correct interfaces.

I've done a small refactor w.r.t. socket initialisation, to avoid code duplication, and in error handling (to correctly clean up all open sockets).

I've also completed one item that was listed as TODO in the code - creating per-server sockets, as it was necessary to be able to use multiple servers in different VRFs. It will also allow to use different src_ip for different servers.

[alandekok]

Overall it looks pretty good. Tho the duplication of v4/v6 sockets could be fixed. i.e. if it's talking to a server on a v4 socket, it doesn't need a v6 socket for that server.

[bluca]

Thanks for the merge!

Overall it looks pretty good. Tho the duplication of v4/v6 sockets could be fixed. i.e. if it's talking to a server on a v4 socket, it doesn't need a v6 socket for that server.

True. Although it would need a bit of refactoring since the hostname is only parsed later at runtime, rather than at initialisation time, so when creating the socket and choosing AF_INET or AF_INET6 it is not yet known what is needed to talk to that specific server.

[bluca]

@dsahern FYI - just in case, not sure if Cumulus Linux uses libpam-radius-auth

[dsahern]

Adding @daveolson53. He handles the pam stuff for CL.

[daveolson53]

David Ahern notifications@github.com wrote:

Adding @daveolson53. He handles the pam stuff for CL.

I had implemented this for Cumulus Linux, but haven't pushed the changes anywhere to github yet. My apologies for that.

I'm not seeing the issue or pull request. Am I looking at the right repo? I'm looking at: https://github.com/FreeRADIUS/pam_radius

I'd like to look at them and compare them to what I did; there may be a better approach than mine.

My changes were based on commit a11bbaeb6b0635dd852fb511a28f67da80327fef from that repo.

Ignoring documentation and other changes, the vrf changes I did were not too large. I see some minor whitespace issues in the attached patch.

You can also ignore the SUDO_PROMPT changes, I did those for some other work I did to allow radius account login without adding the user to the local password file or database. That requires another library and config file to map radius users to a local fixed account, similar to what I did with the tacacs client code under daveolson53 on github.

I'll get all these changes on github "soon". I meant to do that earlier but got sidetracked. The tacacs client repos are under https://github.com/daveolson53/ if you want to see what I did there. I'll put the radius client repos in the same place.

Dave Olson olson@cumulusnetworks.com

diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index a9fd518..3631db7 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -227,7 +227,6 @@ static int host2server(int debug, radius_server_t *server)

server->ip = (struct sockaddr *)&server->ip_storage;
r = get_ipaddr(hostname, server->ip, port);

• DPRINT(LOG_DEBUG, "DEBUG: get_ipaddr(%s) returned %d.\n", hostname, r); return r; }

@@ -525,13 +524,14 @@ static int initialize(radius_conf_t *conf, int accounting) struct sockaddr_storage salocal6; char hostname[BUFFER_SIZE]; char secret[BUFFER_SIZE];

• char vrfname[64];

  char buffer[BUFFER_SIZE]; char p; FILE fserver;

• radius_server_t *server = NULL;
• radius_server_t server = NULL, tmp; int timeout;
• int line = 0;
• int line = 0, scancnt; char src_ip[MAX_IP_LEN]; int seen_v6 = 0;

@@ -549,6 +549,7 @@ static int initialize(radius_conf_t *conf, int accounting) return PAM_ABORT; }

• vrfname[0] = '\0'; while (!feof(fserver) && (fgets (buffer, sizeof(buffer), fserver) != (char) NULL) && (!ferror(fserver))) { line++; p = buffer; @@ -572,48 +573,73 @@ static int initialize(radius_conf_t conf, int accounting) break; }

• timeout = 3;

• src_ip[0] = 0;

• if (sscanf(p, "%s %s %d %s", hostname, secret, &timeout, src_ip) < 2) {

• _pam_log(LOG_ERR, "ERROR reading %s, line %d: Could not read hostname or secret\n",

• conf->conf_file, line);

• continue; / invalid line /

• } else { / read it in and save the data /

• radius_server_t *tmp;

• tmp = malloc(sizeof(radius_server_t));

• if (server) {

• server->next = tmp;

• server = server->next;

• } else {

• conf->server = tmp;

• server= tmp; / first time /

• }

• scancnt = sscanf(p, "%s %s %d %s", hostname, secret, &timeout, src_ip);

• / sometime later do memory checks here /

• server->hostname = strdup(hostname);

• server->secret = strdup(secret);

• server->accounting = accounting;

• / is it the name of a vrf we should bind to? /

• if (!strcmp(hostname, "vrf-name")) {

• if (scancnt < 2)

• _pam_log(LOG_ERR, "ERROR reading %s, line %d: only %d fields\n",

• conf->conf_file, line, scancnt);

• else

• snprintf(vrfname, sizeof vrfname, "%s", secret);

• continue;

• }

• if ((timeout < 1) || (timeout > 60)) {

• server->timeout = 3;

• } else {

• server->timeout = timeout;

• }

• server->next = NULL;

• / allow setting debug in config file as well /

• if (!strcmp(hostname, "debug")) {

• if (scancnt < 1)

• _pam_log(LOG_ERR, "ERROR reading %s, line %d: only %d fields\n",

• conf->conf_file, line, scancnt);

• else

• conf->debug = 1;

• continue;

• }

• if (src_ip[0]) {

• memset(&salocal, 0, sizeof(salocal));

• get_ipaddr(src_ip, (struct sockaddr *)&salocal, NULL);

• switch (salocal.ss_family) {

• case AF_INET:

• memcpy(&salocal4, &salocal, sizeof(salocal));

• break;

• case AF_INET6:

• seen_v6 = 1;

• memcpy(&salocal6, &salocal, sizeof(salocal));

• break;

• }

• if (scancnt < 2) {

• _pam_log(LOG_ERR, "ERROR reading %s, line %d: only %d fields\n",

• conf->conf_file, line, scancnt);

• continue; / invalid line /

• }

• if (scancnt < 4) {

• src_ip[0] = 0;

• if (scancnt < 3)

• timeout = 3; / default timeout /

• }

• / read it in and save the data /

• tmp = malloc(sizeof(radius_server_t));

• if (server) {

• server->next = tmp;

• server = server->next;

• } else {

• conf->server = tmp;

• server= tmp; / first time /

• }

• / sometime later do memory checks here /

• server->hostname = strdup(hostname);

• server->secret = strdup(secret);

• server->accounting = accounting;

• memset(&server->ip, 0, sizeof server->ip);

• if ((timeout < 1) || (timeout > 60)) {

• server->timeout = 3;

• } else {

• server->timeout = timeout;

• }

• server->next = NULL;

• if (src_ip[0]) {

• memset(&salocal, 0, sizeof(salocal));

• get_ipaddr(src_ip, (struct sockaddr *)&salocal, NULL);

• switch (salocal.ss_family) {

• case AF_INET:

• memcpy(&salocal4, &salocal, sizeof(salocal));

• break;

• case AF_INET6:

• seen_v6 = 1;

• memcpy(&salocal6, &salocal, sizeof(salocal));

• break; } } } @@ -647,6 +673,16 @@ static int initialize(radius_conf_t *conf, int accounting) }

  endif

• if (vrfname[0]) {

• / do not fail if the bind fails, connection may succeed /

• if (setsockopt(conf->sockfd, SOL_SOCKET, SO_BINDTODEVICE,

• vrfname, strlen(vrfname)+1) < 0)

• _pam_log(LOG_WARNING, "Binding IPv4 socket to VRF %s failed: %m",

• vrfname);

• else if(conf->debug)

• _pam_log(LOG_DEBUG, "Configured IPv4 vrf as: %s", vrfname);

• }

• / set up the local end of the socket communications / if (bind(conf->sockfd, (struct sockaddr )&salocal4, sizeof (struct sockaddr_in)) < 0) { char error_string[BUFFER_SIZE]; @@ -675,6 +711,15 @@ static int initialize(radius_conf_t conf, int accounting) return PAM_AUTHINFO_UNAVAIL; }

  endif

• if (vrfname[0]) {

• / do not fail if the bind fails, connection may succeed /

• if (setsockopt(conf->sockfd6, SOL_SOCKET, SO_BINDTODEVICE,

• vrfname, strlen(vrfname)+1) < 0)

• _pam_log(LOG_WARNING, "Binding IPv6 socket to VRF %s failed: %m",

• vrfname);

• else if(conf->debug)

• _pam_log(LOG_DEBUG, "Configured IPv6 vrf as: %s", vrfname);

• }

  / set up the local end of the socket communications / if (bind(conf->sockfd6, (struct sockaddr )&salocal6, sizeof (struct sockaddr_in6)) < 0) { @@ -1642,8 +1687,39 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t pamh, int flags, int argc, CONST c / PAM_EXTERN int pam_sm_acct_mgmt(pam_handle_t pamh,int flags,int argc,CONST char **argv) {

• int retval;

• retval = PAM_SUCCESS;

• int retval = PAM_SUCCESS;

• CONST char *user;

• radius_conf_t config;

• (void) _pam_parse(argc, argv, &config);

• / grab the user name /

• retval = pam_get_user(pamh, &user, NULL);

• if (retval != PAM_SUCCESS || user == NULL || strlen(user) > MAXPWNAM) {

• return PAM_USER_UNKNOWN;

• }

• /*

• • parse the config file. We don't make any connections here, so ignore
• • any failures. For consistency only.

• */

• retval = initialize(&config, FALSE);

• /*

• • set SUDO_PROMPT in env so that it prompts as the login user, not the mapped
• • user, unless (unlikely) the prompt has already been set.
• • It won't hurt to do this if the user wasn't mapped.

• */

• if (!pam_getenv(pamh, "SUDO_PROMPT")) {

• char nprompt[strlen("SUDO_PROMPT=[sudo] password for ") +

• strlen(user) + 3]; / + 3 for ": " and the \0 /

• snprintf(nprompt, sizeof nprompt,

• "SUDO_PROMPT=[sudo] password for %s: ", user);

• if (pam_putenv(pamh, nprompt) != PAM_SUCCESS)

• _pam_log(LOG_NOTICE, "failed to set PAM sudo prompt "

• "(%s)", nprompt);

• }

• return retval; }

[daveolson53]

Dave Olson olson@cumulusnetworks.com wrote:

David Ahern notifications@github.com wrote:

Adding @daveolson53. He handles the pam stuff for CL.

I had implemented this for Cumulus Linux, but haven't pushed the changes anywhere to github yet. My apologies for that.

I'm not seeing the issue or pull request. Am I looking at the right repo? I'm looking at: https://github.com/FreeRADIUS/pam_radius I'd like to look at them and compare them to what I did; there may be a better approach than mine.

Never mind. I didn't find it in the issues because it was already merged. The changes look more thorough than mine, but I won't have time to look at them thoroughly until next week, most likely.

Thanks for doing this work!

Dave Olson olson@cumulusnetworks.com

[bluca]

No problem, thanks @daveolson53 !

The changes that were merged will allow to use different servers on different VRFs, which unless I'm mistaken was not the case with the patch you posted.

The main difference in behaviour is on failure to set SO_BINDTODEVICE - I choose to make it a hard failure, because the same address might be reachable outside of the requested VRF but might be the wrong server, which is a security concern given it's dealing with authentication. At the very least it would leak information. What do you think?

[daveolson53]

Luca Boccassi notifications@github.com wrote:

No problem!

The changes that were merged will allow to use different servers on different VRFs, which unless I'm mistaken was not the case with the patch you posted.

That's correct, mine was more limited.

The main difference in behaviour is on failure to set SO_BINDTODEVICE - I choose to make it a hard failure, because the same address might be reachable outside of the requested VRF but might be the wrong server, which is a security concern given it's dealing with authentication. At the very least it would leak information. What do you think?

I hadn't thought about that potential security issue, and should have.

The other reason I didn't make it a hard failure was that sometimes people forget to change config files when they change vrf configuration (including removing it completely). But with reasonable error messages, and given the security issue, I agree that you made the right choice.

I'm in the middle of some short term deliverables, but after that I'll merge what you did with my code, possibly with some changes so it's compatible with what we already shipped to customers (should be minor, if any).

Thanks,

Dave Olson olson@cumulusnetworks.com