From 4c4ca6a70525d81b502bd2298928b447a9966d7f Mon Sep 17 00:00:00 2001 From: Anthony DeRobertis Date: Wed, 10 Jun 2020 18:58:51 -0400 Subject: [PATCH] Down VPN if pam_open_session errors out. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AFAICT, the core pluto daemon is single-threaded, and I don't see any locking around states, so it's very unlikely it's safe to call delete_state directly from the xauth thread. So there are two broad approaches I considered to doing this: a timer on the main thread to check status or using the existing whack interface. A timer would have the downside that we'd have to decide how long is too long to wait for PAM, and we could have an issue where we give up waiting, down the VPN, then pam comes back and says OK. And even if PAM comes back and says no, we'd have to then keep polling (to notice quickly) or just deal with it taking 30s–1min to down the VPN after we get the failure back, which seems unacceptable. Just using the existing whack interface has an advantage: it's easy. It's also relatively immediate. So I picked this. Instead of copy & pasting the code to generate/send the message, just exec'd the program. This should almost never happen anyway, so cost of that is minimal. TODO: Internal tickets TABLET-1402 and TABLET-1403. --- programs/pluto/pam_conv.c | 66 ++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 21 deletions(-) diff --git a/programs/pluto/pam_conv.c b/programs/pluto/pam_conv.c index 5900c3caa39..249660bbfd0 100644 --- a/programs/pluto/pam_conv.c +++ b/programs/pluto/pam_conv.c @@ -30,6 +30,7 @@ #include #include #include /* needed for pam_handle_t */ +#include #include "defs.h" #include "lswlog.h" @@ -41,6 +42,7 @@ #include "log.h" #include "state.h" +extern char **environ; /* because apparently not _GNU_SOURCE */ static char *pam_state_enum[] = { "PAM_AUTH", "PAM_CALLBACK", "PAM_SESSION_START", "PAM_SESSION_END", "PAM_TERM", "PAM_STATE_UNKNOWN", "PAM_DO_NOTHING" @@ -273,31 +275,53 @@ void *pam_thread(void *parg) if (atomic_flag_test_and_set(&ptr_xauth->vpn_still_starting)) { log_pam_step(&ptr_xauth->ptarg, "waiting for VPN up"); } else { - for (int i = 0; i < 5; i++) { - what = "pam_open_session"; - retval = pam_open_session(pamh, PAM_SILENT); - log_pam_step((struct pam_thread_arg *)&ptr_xauth->ptarg, what); - if (retval == PAM_SUCCESS) { - - bool success = TRUE; - struct state *st = state_with_serialno(ptr_xauth->serialno); - //passert(st != NULL); - if(st != NULL) { - libreswan_log("XAUTH: #%lu: completed for user '%s' with status %s ::: pam_open_session", - ptr_xauth->ptarg.st_serialno, ptr_xauth->ptarg.name, - success ? "SUCCESSS" : "FAILURE"); + what = "pam_open_session"; + retval = pam_open_session(pamh, PAM_SILENT); + log_pam_step((struct pam_thread_arg *)&ptr_xauth->ptarg, what); - } + struct state *st = state_with_serialno(ptr_xauth->serialno); + if (!st) { + /* should never happen, just log if it does */ + libreswan_log("XAUTH: #%lu: INTERNAL ERROR Could not find SA state object for user '%s' ::: pam_open_session", + ptr_xauth->ptarg.st_serialno, ptr_xauth->ptarg.name); + } + libreswan_log("XAUTH: #%lu: completed for user '%s' with status %s ::: pam_open_session", + ptr_xauth->ptarg.st_serialno, ptr_xauth->ptarg.name, + (retval == PAM_SUCCESS ? "SUCCESS" : "FAILURE")); - ptr_xauth->ptarg.pam_state = PAM_SESSION_START_SUCCESS; - ptr_xauth->ptarg.pam_do_state = PAM_DO_NOTHING; + if (retval == PAM_SUCCESS) { + ptr_xauth->ptarg.pam_state = PAM_SESSION_START_SUCCESS; + ptr_xauth->ptarg.pam_do_state = PAM_DO_NOTHING; + } else { + ptr_xauth->ptarg.pam_state = PAM_SESSION_START_FAIL; + ptr_xauth->ptarg.pam_do_state = PAM_TERM; - break; - } else { - ptr_xauth->ptarg.pam_state = PAM_SESSION_START_FAIL; - ptr_xauth->ptarg.pam_do_state = PAM_TERM; - /* FIXME: Need to kill the VPN here */ + /* + * Ideally, sending the message would be a library, but it's + * not... so only way to re-use that code is to just call ipsec + * whack. Hopefully posix_spawn avoids the pthread/fork issues. + */ + pid_t child; + int res; + char buf[32]; /* 2^64-1 needs 21 */ + res = snprintf(buf, sizeof(buf), "%lu", ptr_xauth->serialno); + if (res < 0 || (unsigned int)res >= sizeof(buf)) { + libreswan_log("XAUTH: #%lu: snprintf failure while downing VPN for user '%s'", + ptr_xauth->ptarg.st_serialno, ptr_xauth->ptarg.name); + continue; + } + char *args[] = { + "ipsec", "whack", "--deletestate", buf, NULL + }; + int err = posix_spawnp(&child, "ipsec", NULL, NULL, args, environ); + if (err) { + libreswan_log("XAUTH: #%lu: posix_spawnp failure while downing VPN for user '%s'", + ptr_xauth->ptarg.st_serialno, ptr_xauth->ptarg.name); + continue; } + + libreswan_log("XAUTH: #%lu: sent VPN down command for user '%s'", + ptr_xauth->ptarg.st_serialno, ptr_xauth->ptarg.name); } /* Failed pam_open_session */ }