Mailing List Archive

[PATCH 5/5] support password helper
Allows to integrate UI, similar to ssh-askpass, program prompt user
for password and echo result to stdout.

Settings:
---
Password Helper /home/alonbl/vpnc/vpnc-getpass
Xauth interactive
---

vpn-getpass script for KDE:
---
prompt="$1"
exec kdialog --title "vpnc" --password "$prompt";
---

vpn-getpass script for KDE with SecurID:
---
prompt="$1"
pass="$(kdialog --title "vpnc" --password "$prompt")" || exit 1
otp="$(RSA_SecurID_getpasswd)" || exit 1
echo "${pass}${otp}"
exit 0
---

Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
---
config.c | 17 +++++++-
config.h | 1 +
tunip.c | 2 +-
vpnc.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
vpnc.h | 2 +
5 files changed, 155 insertions(+), 4 deletions(-)

diff --git a/config.c b/config.c
index 7080630..36227b7 100644
--- a/config.c
+++ b/config.c
@@ -469,6 +469,13 @@ static const struct config_names_s {
"Target network in dotted decimal or CIDR notation\n",
config_def_target_network
}, {
+ CONFIG_PASSWORD_HELPER, 1, 1,
+ "--password-helper",
+ "Password helper ",
+ "<executable>",
+ "path to password program or helper name\n",
+ NULL
+ }, {
0, 0, 0, NULL, NULL, NULL, NULL, NULL
}
};
@@ -632,6 +639,7 @@ static void print_version(void)

void do_config(int argc, char **argv)
{
+ char _pass[1024];
char *s;
int i, c, known;
int got_conffile = 0, print_config = 0;
@@ -799,7 +807,14 @@ void do_config(int argc, char **argv)
switch (i) {
case CONFIG_IPSEC_SECRET:
case CONFIG_XAUTH_PASSWORD:
- s = strdup(getpass(""));
+ if (!vpnc_getpass(
+ config[CONFIG_PASSWORD_HELPER],
+ "",
+ _pass,
+ sizeof(_pass))) {
+ error(2, 0, "authentication unsuccessful");
+ }
+ s = _pass;
break;
case CONFIG_IPSEC_GATEWAY:
case CONFIG_IPSEC_ID:
diff --git a/config.h b/config.h
index 6fbd231..610feee 100644
--- a/config.h
+++ b/config.h
@@ -59,6 +59,7 @@ enum config_enum {
CONFIG_AUTH_MODE,
CONFIG_CA_FILE,
CONFIG_CA_DIR,
+ CONFIG_PASSWORD_HELPER,
LAST_CONFIG
};

diff --git a/tunip.c b/tunip.c
index 460459c..156f8f9 100644
--- a/tunip.c
+++ b/tunip.c
@@ -1049,7 +1049,7 @@ void vpnc_doit(struct sa_block *s)
setsid();
} else {
printf("VPNC started in background (pid: %d)...\n", (int)pid);
- exit(0);
+ _exit(0);
}
openlog("vpnc", LOG_PID | LOG_PERROR, LOG_DAEMON);
logmsg = syslog;
diff --git a/vpnc.c b/vpnc.c
index 66e3560..ab7804c 100644
--- a/vpnc.c
+++ b/vpnc.c
@@ -37,6 +37,7 @@
#include <poll.h>
#include <sys/ioctl.h>
#include <sys/utsname.h>
+#include <sys/wait.h>

#include <gcrypt.h>

@@ -161,6 +162,130 @@ const struct vid_element vid_list[] = {
static uint8_t r_packet[8192];
static ssize_t r_length;

+static int _vpnc_getpass_program(const char * const program, const char *const prompt,
+ char *const input, const size_t input_size)
+{
+ int status;
+ pid_t pid = -1;
+ int fds[2] = {-1, -1};
+ int r = 0;
+ int rc;
+
+ /*
+ * Make sure we don't reuse input
+ */
+ if (input)
+ memset(input, 0, input_size);
+
+ if (program == NULL) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (pipe(fds) == -1) {
+ rc = -errno;
+ goto out;
+ }
+
+ if ((pid = fork()) == -1) {
+ rc = -errno;
+ goto out;
+ }
+
+ if (pid == 0) {
+ close (fds[0]);
+ fds[0] = -1;
+
+ if (dup2(fds[1], 1) == -1) {
+ exit (1);
+ }
+
+ close(fds[1]);
+ fds[1] = -1;
+
+ execl(program, program, prompt, NULL);
+
+ exit(1);
+ }
+
+ close(fds[1]);
+ fds[1] = -1;
+
+ while (
+ (r=waitpid(pid, &status, 0)) == 0 ||
+ (r == -1 && errno == EINTR)
+ );
+
+ if (r == -1) {
+ rc = -errno;
+ goto out;
+ }
+
+ if (!WIFEXITED(status)) {
+ rc = -EFAULT;
+ goto out;
+ }
+
+ if (WEXITSTATUS(status) != 0) {
+ rc = -EIO;
+ goto out;
+ }
+
+ if (input != NULL) {
+ ssize_t bytes;
+
+ if ((bytes = read (fds[0], input, input_size)) == -1) {
+ rc = -errno;
+ goto out;
+ }
+
+ input[bytes] = '\0';
+
+ if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\n')
+ input[(int)strlen (input)-1] = '\0';
+ /* DOS cygwin */
+ if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\r')
+ input[(int)strlen (input)-1] = '\0';
+ }
+
+ rc = 0;
+
+out:
+ if (rc != 0) {
+ if (input)
+ memset(input, 0, input_size);
+ }
+
+ if (fds[0] != -1) {
+ close(fds[0]);
+ fds[0] = -1;
+ }
+
+ if (fds[1] != -1) {
+ close(fds[1]);
+ fds[1] = -1;
+ }
+
+ return rc;
+}
+
+int vpnc_getpass(const char * const helper, const char *const prompt,
+ char *const input, const size_t input_size)
+{
+ if (helper == NULL) {
+ char *pass = getpass(prompt);
+ if (pass == NULL) {
+ return 0;
+ }
+ strncpy(input, pass, input_size);
+ memset(pass, 0, strlen(pass));
+ return 1;
+ }
+ else {
+ return _vpnc_getpass_program(helper, prompt, input, input_size) == 0;
+ }
+}
+
void print_vid(const unsigned char *vid, uint16_t len) {

int vid_index = 0;
@@ -2322,7 +2447,8 @@ static int do_phase2_xauth(struct sa_block *s)
phase2_fatal(s, "noninteractive can't reuse password", reject);
error(2, 0, "authentication failed (requires interactive mode)");
} else if (seen_answer || passwd_used || config[CONFIG_XAUTH_INTERACTIVE]) {
- char *pass, *prompt = NULL;
+ char pass[1024];
+ char *prompt = NULL;

asprintf(&prompt, "%s for VPN %s@%s: ",
(ap->type == ISAKMP_XAUTH_06_ATTRIB_ANSWER) ?
@@ -2330,7 +2456,14 @@ static int do_phase2_xauth(struct sa_block *s)
(ap->type == ISAKMP_XAUTH_06_ATTRIB_USER_PASSWORD) ?
"Password" : "Passcode",
config[CONFIG_XAUTH_USERNAME], ntop_buf);
- pass = getpass(prompt);
+ if (!vpnc_getpass(
+ config[CONFIG_PASSWORD_HELPER],
+ prompt,
+ pass,
+ sizeof(pass))) {
+ free(prompt);
+ error(2, 0, "authentication unsuccessful");
+ }
free(prompt);

na = new_isakmp_attribute(ap->type, NULL);
diff --git a/vpnc.h b/vpnc.h
index 2bacc08..f817fbf 100644
--- a/vpnc.h
+++ b/vpnc.h
@@ -27,5 +27,7 @@ void process_late_ike(struct sa_block *s, uint8_t *r_packet, ssize_t r_length);
void keepalive_ike(struct sa_block *s);
void dpd_ike(struct sa_block *s);
void print_vid(const unsigned char *vid, uint16_t len);
+int vpnc_getpass(const char * const helper, const char *const prompt,
+ char *const input, const size_t input_size);

#endif
--
1.8.1.5

_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 5/5] support password helper [ In reply to ]
On Sun, Dec 15, 2013 at 8:52 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> Allows to integrate UI, similar to ssh-askpass, program prompt user
> for password and echo result to stdout.
>
> Settings:
> ---
> Password Helper /home/alonbl/vpnc/vpnc-getpass
> Xauth interactive
> ---
>
> vpn-getpass script for KDE:
> ---
> prompt="$1"
> exec kdialog --title "vpnc" --password "$prompt";
> ---
>
> vpn-getpass script for KDE with SecurID:
> ---
> prompt="$1"
> pass="$(kdialog --title "vpnc" --password "$prompt")" || exit 1
> otp="$(RSA_SecurID_getpasswd)" || exit 1
> echo "${pass}${otp}"
> exit 0
> ---
>
> Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
> ---
> config.c | 17 +++++++-
> config.h | 1 +
> tunip.c | 2 +-
> vpnc.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> vpnc.h | 2 +
> 5 files changed, 155 insertions(+), 4 deletions(-)
>
> diff --git a/config.c b/config.c
> index 7080630..36227b7 100644
> --- a/config.c
> +++ b/config.c
> @@ -469,6 +469,13 @@ static const struct config_names_s {
> "Target network in dotted decimal or CIDR notation\n",
> config_def_target_network
> }, {
> + CONFIG_PASSWORD_HELPER, 1, 1,
> + "--password-helper",
> + "Password helper ",

Hi Alon,

since r530 there is no need to add a space after the config file string.
I can remove it when commit.

I'm also trying to use the "checkpatch" included in Linux kernel,
ignoring for the moment only the line-length > 80.
This patch has some minor issue. I can fix them too.


> + "<executable>",
> + "path to password program or helper name\n",
> + NULL
> + }, {
> 0, 0, 0, NULL, NULL, NULL, NULL, NULL
> }
> };
> @@ -632,6 +639,7 @@ static void print_version(void)
>
> void do_config(int argc, char **argv)
> {
> + char _pass[1024];

Any special reason to use "_" as first char of the name?

> char *s;
> int i, c, known;
> int got_conffile = 0, print_config = 0;
> @@ -799,7 +807,14 @@ void do_config(int argc, char **argv)
> switch (i) {
> case CONFIG_IPSEC_SECRET:
> case CONFIG_XAUTH_PASSWORD:
> - s = strdup(getpass(""));
> + if (!vpnc_getpass(
> + config[CONFIG_PASSWORD_HELPER],
> + "",
> + _pass,
> + sizeof(_pass))) {
> + error(2, 0, "authentication unsuccessful");
> + }
> + s = _pass;
> break;
> case CONFIG_IPSEC_GATEWAY:
> case CONFIG_IPSEC_ID:
> diff --git a/config.h b/config.h
> index 6fbd231..610feee 100644
> --- a/config.h
> +++ b/config.h
> @@ -59,6 +59,7 @@ enum config_enum {
> CONFIG_AUTH_MODE,
> CONFIG_CA_FILE,
> CONFIG_CA_DIR,
> + CONFIG_PASSWORD_HELPER,
> LAST_CONFIG
> };
>
> diff --git a/tunip.c b/tunip.c
> index 460459c..156f8f9 100644
> --- a/tunip.c
> +++ b/tunip.c
> @@ -1049,7 +1049,7 @@ void vpnc_doit(struct sa_block *s)
> setsid();
> } else {
> printf("VPNC started in background (pid: %d)...\n", (int)pid);
> - exit(0);
> + _exit(0);

This change doesn't seams part of the patch.
Does it need to be in a separate patch?

> }
> openlog("vpnc", LOG_PID | LOG_PERROR, LOG_DAEMON);
> logmsg = syslog;
> diff --git a/vpnc.c b/vpnc.c
> index 66e3560..ab7804c 100644
> --- a/vpnc.c
> +++ b/vpnc.c
> @@ -37,6 +37,7 @@
> #include <poll.h>
> #include <sys/ioctl.h>
> #include <sys/utsname.h>
> +#include <sys/wait.h>
>
> #include <gcrypt.h>
>
> @@ -161,6 +162,130 @@ const struct vid_element vid_list[] = {
> static uint8_t r_packet[8192];
> static ssize_t r_length;
>
> +static int _vpnc_getpass_program(const char * const program, const char *const prompt,
> + char *const input, const size_t input_size)

Again "_" as first char

> +{
> + int status;
> + pid_t pid = -1;
> + int fds[2] = {-1, -1};
> + int r = 0;
> + int rc;
> +
> + /*
> + * Make sure we don't reuse input
> + */
> + if (input)
> + memset(input, 0, input_size);

If (input == NULL) this function will do nothing useful. For sure will
not read the password.
Can be simplified assuming that input is allocated or checking it only
once at function entrance?
Could "input" be allocated here or in vpnc_getpass() below?

Thanks,
Antonio

> +
> + if (program == NULL) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + if (pipe(fds) == -1) {
> + rc = -errno;
> + goto out;
> + }
> +
> + if ((pid = fork()) == -1) {
> + rc = -errno;
> + goto out;
> + }
> +
> + if (pid == 0) {
> + close (fds[0]);
> + fds[0] = -1;
> +
> + if (dup2(fds[1], 1) == -1) {
> + exit (1);
> + }
> +
> + close(fds[1]);
> + fds[1] = -1;
> +
> + execl(program, program, prompt, NULL);
> +
> + exit(1);
> + }
> +
> + close(fds[1]);
> + fds[1] = -1;
> +
> + while (
> + (r=waitpid(pid, &status, 0)) == 0 ||
> + (r == -1 && errno == EINTR)
> + );
> +
> + if (r == -1) {
> + rc = -errno;
> + goto out;
> + }
> +
> + if (!WIFEXITED(status)) {
> + rc = -EFAULT;
> + goto out;
> + }
> +
> + if (WEXITSTATUS(status) != 0) {
> + rc = -EIO;
> + goto out;
> + }
> +
> + if (input != NULL) {
> + ssize_t bytes;
> +
> + if ((bytes = read (fds[0], input, input_size)) == -1) {
> + rc = -errno;
> + goto out;
> + }
> +
> + input[bytes] = '\0';
> +
> + if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\n')
> + input[(int)strlen (input)-1] = '\0';
> + /* DOS cygwin */
> + if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\r')
> + input[(int)strlen (input)-1] = '\0';
> + }
> +
> + rc = 0;
> +
> +out:
> + if (rc != 0) {
> + if (input)
> + memset(input, 0, input_size);
> + }
> +
> + if (fds[0] != -1) {
> + close(fds[0]);
> + fds[0] = -1;
> + }
> +
> + if (fds[1] != -1) {
> + close(fds[1]);
> + fds[1] = -1;
> + }
> +
> + return rc;
> +}
> +
> +int vpnc_getpass(const char * const helper, const char *const prompt,
> + char *const input, const size_t input_size)
> +{
> + if (helper == NULL) {
> + char *pass = getpass(prompt);
> + if (pass == NULL) {
> + return 0;
> + }
> + strncpy(input, pass, input_size);
> + memset(pass, 0, strlen(pass));
> + return 1;
> + }
> + else {
> + return _vpnc_getpass_program(helper, prompt, input, input_size) == 0;
> + }
> +}
> +
> void print_vid(const unsigned char *vid, uint16_t len) {
>
> int vid_index = 0;
> @@ -2322,7 +2447,8 @@ static int do_phase2_xauth(struct sa_block *s)
> phase2_fatal(s, "noninteractive can't reuse password", reject);
> error(2, 0, "authentication failed (requires interactive mode)");
> } else if (seen_answer || passwd_used || config[CONFIG_XAUTH_INTERACTIVE]) {
> - char *pass, *prompt = NULL;
> + char pass[1024];
> + char *prompt = NULL;
>
> asprintf(&prompt, "%s for VPN %s@%s: ",
> (ap->type == ISAKMP_XAUTH_06_ATTRIB_ANSWER) ?
> @@ -2330,7 +2456,14 @@ static int do_phase2_xauth(struct sa_block *s)
> (ap->type == ISAKMP_XAUTH_06_ATTRIB_USER_PASSWORD) ?
> "Password" : "Passcode",
> config[CONFIG_XAUTH_USERNAME], ntop_buf);
> - pass = getpass(prompt);
> + if (!vpnc_getpass(
> + config[CONFIG_PASSWORD_HELPER],
> + prompt,
> + pass,
> + sizeof(pass))) {
> + free(prompt);
> + error(2, 0, "authentication unsuccessful");
> + }
> free(prompt);
>
> na = new_isakmp_attribute(ap->type, NULL);
> diff --git a/vpnc.h b/vpnc.h
> index 2bacc08..f817fbf 100644
> --- a/vpnc.h
> +++ b/vpnc.h
> @@ -27,5 +27,7 @@ void process_late_ike(struct sa_block *s, uint8_t *r_packet, ssize_t r_length);
> void keepalive_ike(struct sa_block *s);
> void dpd_ike(struct sa_block *s);
> void print_vid(const unsigned char *vid, uint16_t len);
> +int vpnc_getpass(const char * const helper, const char *const prompt,
> + char *const input, const size_t input_size);
>
> #endif
> --
> 1.8.1.5
>
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 5/5] support password helper [ In reply to ]
On Sun, Dec 15, 2013 at 5:20 PM, Antonio Borneo
<borneo.antonio@gmail.com> wrote:
> On Sun, Dec 15, 2013 at 8:52 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>> Allows to integrate UI, similar to ssh-askpass, program prompt user
>> for password and echo result to stdout.
>>
>> Settings:
>> ---
>> Password Helper /home/alonbl/vpnc/vpnc-getpass
>> Xauth interactive
>> ---
>>
>> vpn-getpass script for KDE:
>> ---
>> prompt="$1"
>> exec kdialog --title "vpnc" --password "$prompt";
>> ---
>>
>> vpn-getpass script for KDE with SecurID:
>> ---
>> prompt="$1"
>> pass="$(kdialog --title "vpnc" --password "$prompt")" || exit 1
>> otp="$(RSA_SecurID_getpasswd)" || exit 1
>> echo "${pass}${otp}"
>> exit 0
>> ---
>>
>> Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
>> ---
>> config.c | 17 +++++++-
>> config.h | 1 +
>> tunip.c | 2 +-
>> vpnc.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> vpnc.h | 2 +
>> 5 files changed, 155 insertions(+), 4 deletions(-)
>>
>> diff --git a/config.c b/config.c
>> index 7080630..36227b7 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -469,6 +469,13 @@ static const struct config_names_s {
>> "Target network in dotted decimal or CIDR notation\n",
>> config_def_target_network
>> }, {
>> + CONFIG_PASSWORD_HELPER, 1, 1,
>> + "--password-helper",
>> + "Password helper ",
>
> Hi Alon,
>
> since r530 there is no need to add a space after the config file string.
> I can remove it when commit.
>
> I'm also trying to use the "checkpatch" included in Linux kernel,
> ignoring for the moment only the line-length > 80.
> This patch has some minor issue. I can fix them too.

OK, I will fix this.

>
>
>> + "<executable>",
>> + "path to password program or helper name\n",
>> + NULL
>> + }, {
>> 0, 0, 0, NULL, NULL, NULL, NULL, NULL
>> }
>> };
>> @@ -632,6 +639,7 @@ static void print_version(void)
>>
>> void do_config(int argc, char **argv)
>> {
>> + char _pass[1024];
>
> Any special reason to use "_" as first char of the name?

It is helper function which is not exported out of the scope of the
module, I can remove the leading "_", no problem.

>
>> char *s;
>> int i, c, known;
>> int got_conffile = 0, print_config = 0;
>> @@ -799,7 +807,14 @@ void do_config(int argc, char **argv)
>> switch (i) {
>> case CONFIG_IPSEC_SECRET:
>> case CONFIG_XAUTH_PASSWORD:
>> - s = strdup(getpass(""));
>> + if (!vpnc_getpass(
>> + config[CONFIG_PASSWORD_HELPER],
>> + "",
>> + _pass,
>> + sizeof(_pass))) {
>> + error(2, 0, "authentication unsuccessful");
>> + }
>> + s = _pass;
>> break;
>> case CONFIG_IPSEC_GATEWAY:
>> case CONFIG_IPSEC_ID:
>> diff --git a/config.h b/config.h
>> index 6fbd231..610feee 100644
>> --- a/config.h
>> +++ b/config.h
>> @@ -59,6 +59,7 @@ enum config_enum {
>> CONFIG_AUTH_MODE,
>> CONFIG_CA_FILE,
>> CONFIG_CA_DIR,
>> + CONFIG_PASSWORD_HELPER,
>> LAST_CONFIG
>> };
>>
>> diff --git a/tunip.c b/tunip.c
>> index 460459c..156f8f9 100644
>> --- a/tunip.c
>> +++ b/tunip.c
>> @@ -1049,7 +1049,7 @@ void vpnc_doit(struct sa_block *s)
>> setsid();
>> } else {
>> printf("VPNC started in background (pid: %d)...\n", (int)pid);
>> - exit(0);
>> + _exit(0);
>
> This change doesn't seams part of the patch.
> Does it need to be in a separate patch?
>

This is required to avoid closing file descriptors at atexit. I can
submit a separate patch for this, however, as far as I remember this
problem is introduced and related to this patch.

>> }
>> openlog("vpnc", LOG_PID | LOG_PERROR, LOG_DAEMON);
>> logmsg = syslog;
>> diff --git a/vpnc.c b/vpnc.c
>> index 66e3560..ab7804c 100644
>> --- a/vpnc.c
>> +++ b/vpnc.c
>> @@ -37,6 +37,7 @@
>> #include <poll.h>
>> #include <sys/ioctl.h>
>> #include <sys/utsname.h>
>> +#include <sys/wait.h>
>>
>> #include <gcrypt.h>
>>
>> @@ -161,6 +162,130 @@ const struct vid_element vid_list[] = {
>> static uint8_t r_packet[8192];
>> static ssize_t r_length;
>>
>> +static int _vpnc_getpass_program(const char * const program, const char *const prompt,
>> + char *const input, const size_t input_size)
>
> Again "_" as first char

surprise :)

>> +{
>> + int status;
>> + pid_t pid = -1;
>> + int fds[2] = {-1, -1};
>> + int r = 0;
>> + int rc;
>> +
>> + /*
>> + * Make sure we don't reuse input
>> + */
>> + if (input)
>> + memset(input, 0, input_size);
>
> If (input == NULL) this function will do nothing useful. For sure will
> not read the password.
> Can be simplified assuming that input is allocated or checking it only
> once at function entrance?
> Could "input" be allocated here or in vpnc_getpass() below?

It is a safe guard for not accepting password even if something fails,
I can drop this if you do not think it is that important.
The input is statically allocated at do_phase2_xauth.

> Thanks,
> Antonio
>
>> +
>> + if (program == NULL) {
>> + rc = -EINVAL;
>> + goto out;
>> + }
>> +
>> + if (pipe(fds) == -1) {
>> + rc = -errno;
>> + goto out;
>> + }
>> +
>> + if ((pid = fork()) == -1) {
>> + rc = -errno;
>> + goto out;
>> + }
>> +
>> + if (pid == 0) {
>> + close (fds[0]);
>> + fds[0] = -1;
>> +
>> + if (dup2(fds[1], 1) == -1) {
>> + exit (1);
>> + }
>> +
>> + close(fds[1]);
>> + fds[1] = -1;
>> +
>> + execl(program, program, prompt, NULL);
>> +
>> + exit(1);
>> + }
>> +
>> + close(fds[1]);
>> + fds[1] = -1;
>> +
>> + while (
>> + (r=waitpid(pid, &status, 0)) == 0 ||
>> + (r == -1 && errno == EINTR)
>> + );
>> +
>> + if (r == -1) {
>> + rc = -errno;
>> + goto out;
>> + }
>> +
>> + if (!WIFEXITED(status)) {
>> + rc = -EFAULT;
>> + goto out;
>> + }
>> +
>> + if (WEXITSTATUS(status) != 0) {
>> + rc = -EIO;
>> + goto out;
>> + }
>> +
>> + if (input != NULL) {
>> + ssize_t bytes;
>> +
>> + if ((bytes = read (fds[0], input, input_size)) == -1) {
>> + rc = -errno;
>> + goto out;
>> + }
>> +
>> + input[bytes] = '\0';
>> +
>> + if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\n')
>> + input[(int)strlen (input)-1] = '\0';
>> + /* DOS cygwin */
>> + if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\r')
>> + input[(int)strlen (input)-1] = '\0';
>> + }
>> +
>> + rc = 0;
>> +
>> +out:
>> + if (rc != 0) {
>> + if (input)
>> + memset(input, 0, input_size);
>> + }
>> +
>> + if (fds[0] != -1) {
>> + close(fds[0]);
>> + fds[0] = -1;
>> + }
>> +
>> + if (fds[1] != -1) {
>> + close(fds[1]);
>> + fds[1] = -1;
>> + }
>> +
>> + return rc;
>> +}
>> +
>> +int vpnc_getpass(const char * const helper, const char *const prompt,
>> + char *const input, const size_t input_size)
>> +{
>> + if (helper == NULL) {
>> + char *pass = getpass(prompt);
>> + if (pass == NULL) {
>> + return 0;
>> + }
>> + strncpy(input, pass, input_size);
>> + memset(pass, 0, strlen(pass));
>> + return 1;
>> + }
>> + else {
>> + return _vpnc_getpass_program(helper, prompt, input, input_size) == 0;
>> + }
>> +}
>> +
>> void print_vid(const unsigned char *vid, uint16_t len) {
>>
>> int vid_index = 0;
>> @@ -2322,7 +2447,8 @@ static int do_phase2_xauth(struct sa_block *s)
>> phase2_fatal(s, "noninteractive can't reuse password", reject);
>> error(2, 0, "authentication failed (requires interactive mode)");
>> } else if (seen_answer || passwd_used || config[CONFIG_XAUTH_INTERACTIVE]) {
>> - char *pass, *prompt = NULL;
>> + char pass[1024];
>> + char *prompt = NULL;
>>
>> asprintf(&prompt, "%s for VPN %s@%s: ",
>> (ap->type == ISAKMP_XAUTH_06_ATTRIB_ANSWER) ?
>> @@ -2330,7 +2456,14 @@ static int do_phase2_xauth(struct sa_block *s)
>> (ap->type == ISAKMP_XAUTH_06_ATTRIB_USER_PASSWORD) ?
>> "Password" : "Passcode",
>> config[CONFIG_XAUTH_USERNAME], ntop_buf);
>> - pass = getpass(prompt);
>> + if (!vpnc_getpass(
>> + config[CONFIG_PASSWORD_HELPER],
>> + prompt,
>> + pass,
>> + sizeof(pass))) {
>> + free(prompt);
>> + error(2, 0, "authentication unsuccessful");
>> + }
>> free(prompt);
>>
>> na = new_isakmp_attribute(ap->type, NULL);
>> diff --git a/vpnc.h b/vpnc.h
>> index 2bacc08..f817fbf 100644
>> --- a/vpnc.h
>> +++ b/vpnc.h
>> @@ -27,5 +27,7 @@ void process_late_ike(struct sa_block *s, uint8_t *r_packet, ssize_t r_length);
>> void keepalive_ike(struct sa_block *s);
>> void dpd_ike(struct sa_block *s);
>> void print_vid(const unsigned char *vid, uint16_t len);
>> +int vpnc_getpass(const char * const helper, const char *const prompt,
>> + char *const input, const size_t input_size);
>>
>> #endif
>> --
>> 1.8.1.5
>>
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
[PATCH 5/5] support password helper [ In reply to ]
Allows to integrate UI, similar to ssh-askpass, program prompt user
for password and echo result to stdout.

Settings:
---
Password Helper /home/alonbl/vpnc/vpnc-getpass
Xauth interactive
---

vpn-getpass script for KDE:
---
prompt="$1"
exec kdialog --title "vpnc" --password "$prompt";
---

vpn-getpass script for KDE with SecurID:
---
prompt="$1"
pass="$(kdialog --title "vpnc" --password "$prompt")" || exit 1
otp="$(RSA_SecurID_getpasswd)" || exit 1
echo "${pass}${otp}"
exit 0
---

Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
---
config.c | 17 +++++++-
config.h | 1 +
tunip.c | 2 +-
vpnc.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
vpnc.h | 2 +
5 files changed, 155 insertions(+), 4 deletions(-)

diff --git a/config.c b/config.c
index 7080630..36227b7 100644
--- a/config.c
+++ b/config.c
@@ -469,6 +469,13 @@ static const struct config_names_s {
"Target network in dotted decimal or CIDR notation\n",
config_def_target_network
}, {
+ CONFIG_PASSWORD_HELPER, 1, 1,
+ "--password-helper",
+ "Password helper",
+ "<executable>",
+ "path to password program or helper name\n",
+ NULL
+ }, {
0, 0, 0, NULL, NULL, NULL, NULL, NULL
}
};
@@ -632,6 +639,7 @@ static void print_version(void)

void do_config(int argc, char **argv)
{
+ char _pass[1024];
char *s;
int i, c, known;
int got_conffile = 0, print_config = 0;
@@ -799,7 +807,14 @@ void do_config(int argc, char **argv)
switch (i) {
case CONFIG_IPSEC_SECRET:
case CONFIG_XAUTH_PASSWORD:
- s = strdup(getpass(""));
+ if (!vpnc_getpass(
+ config[CONFIG_PASSWORD_HELPER],
+ "",
+ _pass,
+ sizeof(_pass))) {
+ error(2, 0, "authentication unsuccessful");
+ }
+ s = _pass;
break;
case CONFIG_IPSEC_GATEWAY:
case CONFIG_IPSEC_ID:
diff --git a/config.h b/config.h
index 6fbd231..610feee 100644
--- a/config.h
+++ b/config.h
@@ -59,6 +59,7 @@ enum config_enum {
CONFIG_AUTH_MODE,
CONFIG_CA_FILE,
CONFIG_CA_DIR,
+ CONFIG_PASSWORD_HELPER,
LAST_CONFIG
};

diff --git a/tunip.c b/tunip.c
index 460459c..156f8f9 100644
--- a/tunip.c
+++ b/tunip.c
@@ -1049,7 +1049,7 @@ void vpnc_doit(struct sa_block *s)
setsid();
} else {
printf("VPNC started in background (pid: %d)...\n", (int)pid);
- exit(0);
+ _exit(0);
}
openlog("vpnc", LOG_PID | LOG_PERROR, LOG_DAEMON);
logmsg = syslog;
diff --git a/vpnc.c b/vpnc.c
index 66e3560..ab7804c 100644
--- a/vpnc.c
+++ b/vpnc.c
@@ -37,6 +37,7 @@
#include <poll.h>
#include <sys/ioctl.h>
#include <sys/utsname.h>
+#include <sys/wait.h>

#include <gcrypt.h>

@@ -161,6 +162,129 @@ const struct vid_element vid_list[] = {
static uint8_t r_packet[8192];
static ssize_t r_length;

+static int vpnc_getpass_program(const char * const program,
+ const char *const prompt, char *const input, const size_t input_size)
+{
+ int status;
+ pid_t pid = -1;
+ int fds[2] = {-1, -1};
+ int r = 0;
+ int rc;
+
+ /*
+ * Make sure we don't reuse input
+ */
+ if (input)
+ memset(input, 0, input_size);
+
+ if (program == NULL) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (pipe(fds) == -1) {
+ rc = -errno;
+ goto out;
+ }
+
+ pid = fork();
+ if (pid == -1) {
+ rc = -errno;
+ goto out;
+ }
+
+ if (pid == 0) {
+ close(fds[0]);
+ fds[0] = -1;
+
+ if (dup2(fds[1], 1) == -1)
+ exit(1);
+
+ close(fds[1]);
+ fds[1] = -1;
+
+ execl(program, program, prompt, NULL);
+
+ exit(1);
+ }
+
+ close(fds[1]);
+ fds[1] = -1;
+
+ while ((r = waitpid(pid, &status, 0)) == 0 ||
+ (r == -1 && errno == EINTR))
+ ;
+
+ if (r == -1) {
+ rc = -errno;
+ goto out;
+ }
+
+ if (!WIFEXITED(status)) {
+ rc = -EFAULT;
+ goto out;
+ }
+
+ if (WEXITSTATUS(status) != 0) {
+ rc = -EIO;
+ goto out;
+ }
+
+ if (input != NULL) {
+ ssize_t bytes;
+
+ bytes = read(fds[0], input, input_size);
+ if (bytes == -1) {
+ rc = -errno;
+ goto out;
+ }
+
+ input[bytes] = '\0';
+
+ if (strlen(input) > 0 && input[(int)strlen(input)-1] == '\n')
+ input[(int)strlen(input)-1] = '\0';
+ /* DOS cygwin */
+ if (strlen(input) > 0 && input[(int)strlen(input)-1] == '\r')
+ input[(int)strlen(input)-1] = '\0';
+ }
+
+ rc = 0;
+
+out:
+ if (rc != 0) {
+ if (input)
+ memset(input, 0, input_size);
+ }
+
+ if (fds[0] != -1) {
+ close(fds[0]);
+ fds[0] = -1;
+ }
+
+ if (fds[1] != -1) {
+ close(fds[1]);
+ fds[1] = -1;
+ }
+
+ return rc;
+}
+
+int vpnc_getpass(const char * const helper, const char *const prompt,
+ char *const input, const size_t input_size)
+{
+ if (helper == NULL) {
+ char *pass = getpass(prompt);
+ if (pass == NULL)
+ return 0;
+ strncpy(input, pass, input_size);
+ memset(pass, 0, strlen(pass));
+ return 1;
+ } else {
+ return vpnc_getpass_program(helper, prompt, input,
+ input_size) == 0;
+ }
+}
+
void print_vid(const unsigned char *vid, uint16_t len) {

int vid_index = 0;
@@ -2322,7 +2447,8 @@ static int do_phase2_xauth(struct sa_block *s)
phase2_fatal(s, "noninteractive can't reuse password", reject);
error(2, 0, "authentication failed (requires interactive mode)");
} else if (seen_answer || passwd_used || config[CONFIG_XAUTH_INTERACTIVE]) {
- char *pass, *prompt = NULL;
+ char pass[1024];
+ char *prompt = NULL;

asprintf(&prompt, "%s for VPN %s@%s: ",
(ap->type == ISAKMP_XAUTH_06_ATTRIB_ANSWER) ?
@@ -2330,7 +2456,14 @@ static int do_phase2_xauth(struct sa_block *s)
(ap->type == ISAKMP_XAUTH_06_ATTRIB_USER_PASSWORD) ?
"Password" : "Passcode",
config[CONFIG_XAUTH_USERNAME], ntop_buf);
- pass = getpass(prompt);
+ if (!vpnc_getpass(
+ config[CONFIG_PASSWORD_HELPER],
+ prompt,
+ pass,
+ sizeof(pass))) {
+ free(prompt);
+ error(2, 0, "authentication unsuccessful");
+ }
free(prompt);

na = new_isakmp_attribute(ap->type, NULL);
diff --git a/vpnc.h b/vpnc.h
index 2bacc08..f817fbf 100644
--- a/vpnc.h
+++ b/vpnc.h
@@ -27,5 +27,7 @@ void process_late_ike(struct sa_block *s, uint8_t *r_packet, ssize_t r_length);
void keepalive_ike(struct sa_block *s);
void dpd_ike(struct sa_block *s);
void print_vid(const unsigned char *vid, uint16_t len);
+int vpnc_getpass(const char * const helper, const char *const prompt,
+ char *const input, const size_t input_size);

#endif
--
1.8.1.5

_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 5/5] support password helper [ In reply to ]
Confirm to kernel's checkpatch.pl
Waiting for farther input regarding the _exit() if you indeed want it
to at separate patch, and the input buffer clean.

On Sun, Dec 15, 2013 at 6:08 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> Allows to integrate UI, similar to ssh-askpass, program prompt user
> for password and echo result to stdout.
>
> Settings:
> ---
> Password Helper /home/alonbl/vpnc/vpnc-getpass
> Xauth interactive
> ---
>
> vpn-getpass script for KDE:
> ---
> prompt="$1"
> exec kdialog --title "vpnc" --password "$prompt";
> ---
>
> vpn-getpass script for KDE with SecurID:
> ---
> prompt="$1"
> pass="$(kdialog --title "vpnc" --password "$prompt")" || exit 1
> otp="$(RSA_SecurID_getpasswd)" || exit 1
> echo "${pass}${otp}"
> exit 0
> ---
>
> Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
> ---
> config.c | 17 +++++++-
> config.h | 1 +
> tunip.c | 2 +-
> vpnc.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> vpnc.h | 2 +
> 5 files changed, 155 insertions(+), 4 deletions(-)
>
> diff --git a/config.c b/config.c
> index 7080630..36227b7 100644
> --- a/config.c
> +++ b/config.c
> @@ -469,6 +469,13 @@ static const struct config_names_s {
> "Target network in dotted decimal or CIDR notation\n",
> config_def_target_network
> }, {
> + CONFIG_PASSWORD_HELPER, 1, 1,
> + "--password-helper",
> + "Password helper",
> + "<executable>",
> + "path to password program or helper name\n",
> + NULL
> + }, {
> 0, 0, 0, NULL, NULL, NULL, NULL, NULL
> }
> };
> @@ -632,6 +639,7 @@ static void print_version(void)
>
> void do_config(int argc, char **argv)
> {
> + char _pass[1024];
> char *s;
> int i, c, known;
> int got_conffile = 0, print_config = 0;
> @@ -799,7 +807,14 @@ void do_config(int argc, char **argv)
> switch (i) {
> case CONFIG_IPSEC_SECRET:
> case CONFIG_XAUTH_PASSWORD:
> - s = strdup(getpass(""));
> + if (!vpnc_getpass(
> + config[CONFIG_PASSWORD_HELPER],
> + "",
> + _pass,
> + sizeof(_pass))) {
> + error(2, 0, "authentication unsuccessful");
> + }
> + s = _pass;
> break;
> case CONFIG_IPSEC_GATEWAY:
> case CONFIG_IPSEC_ID:
> diff --git a/config.h b/config.h
> index 6fbd231..610feee 100644
> --- a/config.h
> +++ b/config.h
> @@ -59,6 +59,7 @@ enum config_enum {
> CONFIG_AUTH_MODE,
> CONFIG_CA_FILE,
> CONFIG_CA_DIR,
> + CONFIG_PASSWORD_HELPER,
> LAST_CONFIG
> };
>
> diff --git a/tunip.c b/tunip.c
> index 460459c..156f8f9 100644
> --- a/tunip.c
> +++ b/tunip.c
> @@ -1049,7 +1049,7 @@ void vpnc_doit(struct sa_block *s)
> setsid();
> } else {
> printf("VPNC started in background (pid: %d)...\n", (int)pid);
> - exit(0);
> + _exit(0);
> }
> openlog("vpnc", LOG_PID | LOG_PERROR, LOG_DAEMON);
> logmsg = syslog;
> diff --git a/vpnc.c b/vpnc.c
> index 66e3560..ab7804c 100644
> --- a/vpnc.c
> +++ b/vpnc.c
> @@ -37,6 +37,7 @@
> #include <poll.h>
> #include <sys/ioctl.h>
> #include <sys/utsname.h>
> +#include <sys/wait.h>
>
> #include <gcrypt.h>
>
> @@ -161,6 +162,129 @@ const struct vid_element vid_list[] = {
> static uint8_t r_packet[8192];
> static ssize_t r_length;
>
> +static int vpnc_getpass_program(const char * const program,
> + const char *const prompt, char *const input, const size_t input_size)
> +{
> + int status;
> + pid_t pid = -1;
> + int fds[2] = {-1, -1};
> + int r = 0;
> + int rc;
> +
> + /*
> + * Make sure we don't reuse input
> + */
> + if (input)
> + memset(input, 0, input_size);
> +
> + if (program == NULL) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + if (pipe(fds) == -1) {
> + rc = -errno;
> + goto out;
> + }
> +
> + pid = fork();
> + if (pid == -1) {
> + rc = -errno;
> + goto out;
> + }
> +
> + if (pid == 0) {
> + close(fds[0]);
> + fds[0] = -1;
> +
> + if (dup2(fds[1], 1) == -1)
> + exit(1);
> +
> + close(fds[1]);
> + fds[1] = -1;
> +
> + execl(program, program, prompt, NULL);
> +
> + exit(1);
> + }
> +
> + close(fds[1]);
> + fds[1] = -1;
> +
> + while ((r = waitpid(pid, &status, 0)) == 0 ||
> + (r == -1 && errno == EINTR))
> + ;
> +
> + if (r == -1) {
> + rc = -errno;
> + goto out;
> + }
> +
> + if (!WIFEXITED(status)) {
> + rc = -EFAULT;
> + goto out;
> + }
> +
> + if (WEXITSTATUS(status) != 0) {
> + rc = -EIO;
> + goto out;
> + }
> +
> + if (input != NULL) {
> + ssize_t bytes;
> +
> + bytes = read(fds[0], input, input_size);
> + if (bytes == -1) {
> + rc = -errno;
> + goto out;
> + }
> +
> + input[bytes] = '\0';
> +
> + if (strlen(input) > 0 && input[(int)strlen(input)-1] == '\n')
> + input[(int)strlen(input)-1] = '\0';
> + /* DOS cygwin */
> + if (strlen(input) > 0 && input[(int)strlen(input)-1] == '\r')
> + input[(int)strlen(input)-1] = '\0';
> + }
> +
> + rc = 0;
> +
> +out:
> + if (rc != 0) {
> + if (input)
> + memset(input, 0, input_size);
> + }
> +
> + if (fds[0] != -1) {
> + close(fds[0]);
> + fds[0] = -1;
> + }
> +
> + if (fds[1] != -1) {
> + close(fds[1]);
> + fds[1] = -1;
> + }
> +
> + return rc;
> +}
> +
> +int vpnc_getpass(const char * const helper, const char *const prompt,
> + char *const input, const size_t input_size)
> +{
> + if (helper == NULL) {
> + char *pass = getpass(prompt);
> + if (pass == NULL)
> + return 0;
> + strncpy(input, pass, input_size);
> + memset(pass, 0, strlen(pass));
> + return 1;
> + } else {
> + return vpnc_getpass_program(helper, prompt, input,
> + input_size) == 0;
> + }
> +}
> +
> void print_vid(const unsigned char *vid, uint16_t len) {
>
> int vid_index = 0;
> @@ -2322,7 +2447,8 @@ static int do_phase2_xauth(struct sa_block *s)
> phase2_fatal(s, "noninteractive can't reuse password", reject);
> error(2, 0, "authentication failed (requires interactive mode)");
> } else if (seen_answer || passwd_used || config[CONFIG_XAUTH_INTERACTIVE]) {
> - char *pass, *prompt = NULL;
> + char pass[1024];
> + char *prompt = NULL;
>
> asprintf(&prompt, "%s for VPN %s@%s: ",
> (ap->type == ISAKMP_XAUTH_06_ATTRIB_ANSWER) ?
> @@ -2330,7 +2456,14 @@ static int do_phase2_xauth(struct sa_block *s)
> (ap->type == ISAKMP_XAUTH_06_ATTRIB_USER_PASSWORD) ?
> "Password" : "Passcode",
> config[CONFIG_XAUTH_USERNAME], ntop_buf);
> - pass = getpass(prompt);
> + if (!vpnc_getpass(
> + config[CONFIG_PASSWORD_HELPER],
> + prompt,
> + pass,
> + sizeof(pass))) {
> + free(prompt);
> + error(2, 0, "authentication unsuccessful");
> + }
> free(prompt);
>
> na = new_isakmp_attribute(ap->type, NULL);
> diff --git a/vpnc.h b/vpnc.h
> index 2bacc08..f817fbf 100644
> --- a/vpnc.h
> +++ b/vpnc.h
> @@ -27,5 +27,7 @@ void process_late_ike(struct sa_block *s, uint8_t *r_packet, ssize_t r_length);
> void keepalive_ike(struct sa_block *s);
> void dpd_ike(struct sa_block *s);
> void print_vid(const unsigned char *vid, uint16_t len);
> +int vpnc_getpass(const char * const helper, const char *const prompt,
> + char *const input, const size_t input_size);
>
> #endif
> --
> 1.8.1.5
>
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 5/5] support password helper [ In reply to ]
On Sun, Dec 15, 2013 at 11:43 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> On Sun, Dec 15, 2013 at 5:20 PM, Antonio Borneo
> <borneo.antonio@gmail.com> wrote:
>> On Sun, Dec 15, 2013 at 8:52 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>>> Allows to integrate UI, similar to ssh-askpass, program prompt user
>>> for password and echo result to stdout.
>>>
>>> Settings:
>>> ---
>>> Password Helper /home/alonbl/vpnc/vpnc-getpass
>>> Xauth interactive
>>> ---
>>>
>>> vpn-getpass script for KDE:
>>> ---
>>> prompt="$1"
>>> exec kdialog --title "vpnc" --password "$prompt";
>>> ---
>>>
>>> vpn-getpass script for KDE with SecurID:
>>> ---
>>> prompt="$1"
>>> pass="$(kdialog --title "vpnc" --password "$prompt")" || exit 1
>>> otp="$(RSA_SecurID_getpasswd)" || exit 1
>>> echo "${pass}${otp}"
>>> exit 0
>>> ---
>>>
>>> Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
>>> ---
>>> config.c | 17 +++++++-
>>> config.h | 1 +
>>> tunip.c | 2 +-
>>> vpnc.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>> vpnc.h | 2 +
>>> 5 files changed, 155 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/config.c b/config.c
>>> index 7080630..36227b7 100644
>>> --- a/config.c
>>> +++ b/config.c
>>> @@ -469,6 +469,13 @@ static const struct config_names_s {
>>> "Target network in dotted decimal or CIDR notation\n",
>>> config_def_target_network
>>> }, {
>>> + CONFIG_PASSWORD_HELPER, 1, 1,
>>> + "--password-helper",
>>> + "Password helper ",
>>
>> Hi Alon,
>>
>> since r530 there is no need to add a space after the config file string.
>> I can remove it when commit.
>>
>> I'm also trying to use the "checkpatch" included in Linux kernel,
>> ignoring for the moment only the line-length > 80.
>> This patch has some minor issue. I can fix them too.
>
> OK, I will fix this.

Thanks!

>
>>
>>
>>> + "<executable>",
>>> + "path to password program or helper name\n",
>>> + NULL
>>> + }, {
>>> 0, 0, 0, NULL, NULL, NULL, NULL, NULL
>>> }
>>> };
>>> @@ -632,6 +639,7 @@ static void print_version(void)
>>>
>>> void do_config(int argc, char **argv)
>>> {
>>> + char _pass[1024];
>>
>> Any special reason to use "_" as first char of the name?
>
> It is helper function which is not exported out of the scope of the
> module, I can remove the leading "_", no problem.
>
>>
>>> char *s;
>>> int i, c, known;
>>> int got_conffile = 0, print_config = 0;
>>> @@ -799,7 +807,14 @@ void do_config(int argc, char **argv)
>>> switch (i) {
>>> case CONFIG_IPSEC_SECRET:
>>> case CONFIG_XAUTH_PASSWORD:
>>> - s = strdup(getpass(""));
>>> + if (!vpnc_getpass(
>>> + config[CONFIG_PASSWORD_HELPER],
>>> + "",
>>> + _pass,
>>> + sizeof(_pass))) {
>>> + error(2, 0, "authentication unsuccessful");
>>> + }
>>> + s = _pass;
>>> break;
>>> case CONFIG_IPSEC_GATEWAY:
>>> case CONFIG_IPSEC_ID:
>>> diff --git a/config.h b/config.h
>>> index 6fbd231..610feee 100644
>>> --- a/config.h
>>> +++ b/config.h
>>> @@ -59,6 +59,7 @@ enum config_enum {
>>> CONFIG_AUTH_MODE,
>>> CONFIG_CA_FILE,
>>> CONFIG_CA_DIR,
>>> + CONFIG_PASSWORD_HELPER,
>>> LAST_CONFIG
>>> };
>>>
>>> diff --git a/tunip.c b/tunip.c
>>> index 460459c..156f8f9 100644
>>> --- a/tunip.c
>>> +++ b/tunip.c
>>> @@ -1049,7 +1049,7 @@ void vpnc_doit(struct sa_block *s)
>>> setsid();
>>> } else {
>>> printf("VPNC started in background (pid: %d)...\n", (int)pid);
>>> - exit(0);
>>> + _exit(0);
>>
>> This change doesn't seams part of the patch.
>> Does it need to be in a separate patch?
>>
>
> This is required to avoid closing file descriptors at atexit. I can
> submit a separate patch for this, however, as far as I remember this
> problem is introduced and related to this patch.

I'm reading the man-page looking for a reason of _exit() here, but
cannot find any.
The only call to atexit() has been introduced just recently. Was not
there when you submitted the patch.
This patch mainly adds fork&exec for the password helper, but helper
is already completed when execution arrives here.
Is there any issue in waiting the helper to finish? Zombie processes?

>
>>> }
>>> openlog("vpnc", LOG_PID | LOG_PERROR, LOG_DAEMON);
>>> logmsg = syslog;
>>> diff --git a/vpnc.c b/vpnc.c
>>> index 66e3560..ab7804c 100644
>>> --- a/vpnc.c
>>> +++ b/vpnc.c
>>> @@ -37,6 +37,7 @@
>>> #include <poll.h>
>>> #include <sys/ioctl.h>
>>> #include <sys/utsname.h>
>>> +#include <sys/wait.h>
>>>
>>> #include <gcrypt.h>
>>>
>>> @@ -161,6 +162,130 @@ const struct vid_element vid_list[] = {
>>> static uint8_t r_packet[8192];
>>> static ssize_t r_length;
>>>
>>> +static int _vpnc_getpass_program(const char * const program, const char *const prompt,
>>> + char *const input, const size_t input_size)
>>
>> Again "_" as first char
>
> surprise :)
>
>>> +{
>>> + int status;
>>> + pid_t pid = -1;
>>> + int fds[2] = {-1, -1};
>>> + int r = 0;
>>> + int rc;
>>> +
>>> + /*
>>> + * Make sure we don't reuse input
>>> + */
>>> + if (input)
>>> + memset(input, 0, input_size);
>>
>> If (input == NULL) this function will do nothing useful. For sure will
>> not read the password.
>> Can be simplified assuming that input is allocated or checking it only
>> once at function entrance?
>> Could "input" be allocated here or in vpnc_getpass() below?
>
> It is a safe guard for not accepting password even if something fails,
> I can drop this if you do not think it is that important.

I'm not saying that value of input should not be checked. Let's move
few lines below ....

> The input is statically allocated at do_phase2_xauth.
>
>> Thanks,
>> Antonio
>>
>>> +
>>> + if (program == NULL) {
>>> + rc = -EINVAL;
>>> + goto out;
>>> + }
>>> +
>>> + if (pipe(fds) == -1) {
>>> + rc = -errno;
>>> + goto out;
>>> + }
>>> +
>>> + if ((pid = fork()) == -1) {
>>> + rc = -errno;
>>> + goto out;
>>> + }
>>> +
>>> + if (pid == 0) {
>>> + close (fds[0]);
>>> + fds[0] = -1;
>>> +
>>> + if (dup2(fds[1], 1) == -1) {
>>> + exit (1);
>>> + }
>>> +
>>> + close(fds[1]);
>>> + fds[1] = -1;
>>> +
>>> + execl(program, program, prompt, NULL);
>>> +
>>> + exit(1);
>>> + }
>>> +
>>> + close(fds[1]);
>>> + fds[1] = -1;
>>> +
>>> + while (
>>> + (r=waitpid(pid, &status, 0)) == 0 ||
>>> + (r == -1 && errno == EINTR)
>>> + );
>>> +
>>> + if (r == -1) {
>>> + rc = -errno;
>>> + goto out;
>>> + }
>>> +
>>> + if (!WIFEXITED(status)) {
>>> + rc = -EFAULT;
>>> + goto out;
>>> + }
>>> +
>>> + if (WEXITSTATUS(status) != 0) {
>>> + rc = -EIO;
>>> + goto out;
>>> + }
>>> +
>>> + if (input != NULL) {

.... this part have no meaning if input == NULL.
I mean, you have just done fork and execl, but then you discard the
result because input == NULL.
Makes much more sense to return from this function at the beginning if
input == NULL. Skip everything and return before the fork.

Antonio

>>> + ssize_t bytes;
>>> +
>>> + if ((bytes = read (fds[0], input, input_size)) == -1) {
>>> + rc = -errno;
>>> + goto out;
>>> + }
>>> +
>>> + input[bytes] = '\0';
>>> +
>>> + if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\n')
>>> + input[(int)strlen (input)-1] = '\0';
>>> + /* DOS cygwin */
>>> + if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\r')
>>> + input[(int)strlen (input)-1] = '\0';
>>> + }
>>> +
>>> + rc = 0;
>>> +
>>> +out:
>>> + if (rc != 0) {
>>> + if (input)
>>> + memset(input, 0, input_size);
>>> + }
>>> +
>>> + if (fds[0] != -1) {
>>> + close(fds[0]);
>>> + fds[0] = -1;
>>> + }
>>> +
>>> + if (fds[1] != -1) {
>>> + close(fds[1]);
>>> + fds[1] = -1;
>>> + }
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +int vpnc_getpass(const char * const helper, const char *const prompt,
>>> + char *const input, const size_t input_size)
>>> +{
>>> + if (helper == NULL) {
>>> + char *pass = getpass(prompt);
>>> + if (pass == NULL) {
>>> + return 0;
>>> + }
>>> + strncpy(input, pass, input_size);
>>> + memset(pass, 0, strlen(pass));
>>> + return 1;
>>> + }
>>> + else {
>>> + return _vpnc_getpass_program(helper, prompt, input, input_size) == 0;
>>> + }
>>> +}
>>> +
>>> void print_vid(const unsigned char *vid, uint16_t len) {
>>>
>>> int vid_index = 0;
>>> @@ -2322,7 +2447,8 @@ static int do_phase2_xauth(struct sa_block *s)
>>> phase2_fatal(s, "noninteractive can't reuse password", reject);
>>> error(2, 0, "authentication failed (requires interactive mode)");
>>> } else if (seen_answer || passwd_used || config[CONFIG_XAUTH_INTERACTIVE]) {
>>> - char *pass, *prompt = NULL;
>>> + char pass[1024];
>>> + char *prompt = NULL;
>>>
>>> asprintf(&prompt, "%s for VPN %s@%s: ",
>>> (ap->type == ISAKMP_XAUTH_06_ATTRIB_ANSWER) ?
>>> @@ -2330,7 +2456,14 @@ static int do_phase2_xauth(struct sa_block *s)
>>> (ap->type == ISAKMP_XAUTH_06_ATTRIB_USER_PASSWORD) ?
>>> "Password" : "Passcode",
>>> config[CONFIG_XAUTH_USERNAME], ntop_buf);
>>> - pass = getpass(prompt);
>>> + if (!vpnc_getpass(
>>> + config[CONFIG_PASSWORD_HELPER],
>>> + prompt,
>>> + pass,
>>> + sizeof(pass))) {
>>> + free(prompt);
>>> + error(2, 0, "authentication unsuccessful");
>>> + }
>>> free(prompt);
>>>
>>> na = new_isakmp_attribute(ap->type, NULL);
>>> diff --git a/vpnc.h b/vpnc.h
>>> index 2bacc08..f817fbf 100644
>>> --- a/vpnc.h
>>> +++ b/vpnc.h
>>> @@ -27,5 +27,7 @@ void process_late_ike(struct sa_block *s, uint8_t *r_packet, ssize_t r_length);
>>> void keepalive_ike(struct sa_block *s);
>>> void dpd_ike(struct sa_block *s);
>>> void print_vid(const unsigned char *vid, uint16_t len);
>>> +int vpnc_getpass(const char * const helper, const char *const prompt,
>>> + char *const input, const size_t input_size);
>>>
>>> #endif
>>> --
>>> 1.8.1.5
>>>
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 5/5] support password helper [ In reply to ]
On Sun, Dec 15, 2013 at 6:57 PM, Antonio Borneo
<borneo.antonio@gmail.com> wrote:
> On Sun, Dec 15, 2013 at 11:43 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>> On Sun, Dec 15, 2013 at 5:20 PM, Antonio Borneo
>> <borneo.antonio@gmail.com> wrote:
>>> On Sun, Dec 15, 2013 at 8:52 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>>>> Allows to integrate UI, similar to ssh-askpass, program prompt user
>>>> for password and echo result to stdout.
>>>>
>>>> Settings:
>>>> ---
>>>> Password Helper /home/alonbl/vpnc/vpnc-getpass
>>>> Xauth interactive
>>>> ---
>>>>
>>>> vpn-getpass script for KDE:
>>>> ---
>>>> prompt="$1"
>>>> exec kdialog --title "vpnc" --password "$prompt";
>>>> ---
>>>>
>>>> vpn-getpass script for KDE with SecurID:
>>>> ---
>>>> prompt="$1"
>>>> pass="$(kdialog --title "vpnc" --password "$prompt")" || exit 1
>>>> otp="$(RSA_SecurID_getpasswd)" || exit 1
>>>> echo "${pass}${otp}"
>>>> exit 0
>>>> ---
>>>>
>>>> Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
>>>> ---
>>>> config.c | 17 +++++++-
>>>> config.h | 1 +
>>>> tunip.c | 2 +-
>>>> vpnc.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>> vpnc.h | 2 +
>>>> 5 files changed, 155 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/config.c b/config.c
>>>> index 7080630..36227b7 100644
>>>> --- a/config.c
>>>> +++ b/config.c
>>>> @@ -469,6 +469,13 @@ static const struct config_names_s {
>>>> "Target network in dotted decimal or CIDR notation\n",
>>>> config_def_target_network
>>>> }, {
>>>> + CONFIG_PASSWORD_HELPER, 1, 1,
>>>> + "--password-helper",
>>>> + "Password helper ",
>>>
>>> Hi Alon,
>>>
>>> since r530 there is no need to add a space after the config file string.
>>> I can remove it when commit.
>>>
>>> I'm also trying to use the "checkpatch" included in Linux kernel,
>>> ignoring for the moment only the line-length > 80.
>>> This patch has some minor issue. I can fix them too.
>>
>> OK, I will fix this.
>
> Thanks!
>
>>
>>>
>>>
>>>> + "<executable>",
>>>> + "path to password program or helper name\n",
>>>> + NULL
>>>> + }, {
>>>> 0, 0, 0, NULL, NULL, NULL, NULL, NULL
>>>> }
>>>> };
>>>> @@ -632,6 +639,7 @@ static void print_version(void)
>>>>
>>>> void do_config(int argc, char **argv)
>>>> {
>>>> + char _pass[1024];
>>>
>>> Any special reason to use "_" as first char of the name?
>>
>> It is helper function which is not exported out of the scope of the
>> module, I can remove the leading "_", no problem.
>>
>>>
>>>> char *s;
>>>> int i, c, known;
>>>> int got_conffile = 0, print_config = 0;
>>>> @@ -799,7 +807,14 @@ void do_config(int argc, char **argv)
>>>> switch (i) {
>>>> case CONFIG_IPSEC_SECRET:
>>>> case CONFIG_XAUTH_PASSWORD:
>>>> - s = strdup(getpass(""));
>>>> + if (!vpnc_getpass(
>>>> + config[CONFIG_PASSWORD_HELPER],
>>>> + "",
>>>> + _pass,
>>>> + sizeof(_pass))) {
>>>> + error(2, 0, "authentication unsuccessful");
>>>> + }
>>>> + s = _pass;
>>>> break;
>>>> case CONFIG_IPSEC_GATEWAY:
>>>> case CONFIG_IPSEC_ID:
>>>> diff --git a/config.h b/config.h
>>>> index 6fbd231..610feee 100644
>>>> --- a/config.h
>>>> +++ b/config.h
>>>> @@ -59,6 +59,7 @@ enum config_enum {
>>>> CONFIG_AUTH_MODE,
>>>> CONFIG_CA_FILE,
>>>> CONFIG_CA_DIR,
>>>> + CONFIG_PASSWORD_HELPER,
>>>> LAST_CONFIG
>>>> };
>>>>
>>>> diff --git a/tunip.c b/tunip.c
>>>> index 460459c..156f8f9 100644
>>>> --- a/tunip.c
>>>> +++ b/tunip.c
>>>> @@ -1049,7 +1049,7 @@ void vpnc_doit(struct sa_block *s)
>>>> setsid();
>>>> } else {
>>>> printf("VPNC started in background (pid: %d)...\n", (int)pid);
>>>> - exit(0);
>>>> + _exit(0);
>>>
>>> This change doesn't seams part of the patch.
>>> Does it need to be in a separate patch?
>>>
>>
>> This is required to avoid closing file descriptors at atexit. I can
>> submit a separate patch for this, however, as far as I remember this
>> problem is introduced and related to this patch.
>
> I'm reading the man-page looking for a reason of _exit() here, but
> cannot find any.
> The only call to atexit() has been introduced just recently. Was not
> there when you submitted the patch.
> This patch mainly adds fork&exec for the password helper, but helper
> is already completed when execution arrives here.
> Is there any issue in waiting the helper to finish? Zombie processes?

What was merge is a partial patch of mine, this patch was submitted at
order later than the tun cleanup patch, not sure why only partially
merged.
I now re-order patches and will send it at new order with all
functionality I added.

>
>>
>>>> }
>>>> openlog("vpnc", LOG_PID | LOG_PERROR, LOG_DAEMON);
>>>> logmsg = syslog;
>>>> diff --git a/vpnc.c b/vpnc.c
>>>> index 66e3560..ab7804c 100644
>>>> --- a/vpnc.c
>>>> +++ b/vpnc.c
>>>> @@ -37,6 +37,7 @@
>>>> #include <poll.h>
>>>> #include <sys/ioctl.h>
>>>> #include <sys/utsname.h>
>>>> +#include <sys/wait.h>
>>>>
>>>> #include <gcrypt.h>
>>>>
>>>> @@ -161,6 +162,130 @@ const struct vid_element vid_list[] = {
>>>> static uint8_t r_packet[8192];
>>>> static ssize_t r_length;
>>>>
>>>> +static int _vpnc_getpass_program(const char * const program, const char *const prompt,
>>>> + char *const input, const size_t input_size)
>>>
>>> Again "_" as first char
>>
>> surprise :)
>>
>>>> +{
>>>> + int status;
>>>> + pid_t pid = -1;
>>>> + int fds[2] = {-1, -1};
>>>> + int r = 0;
>>>> + int rc;
>>>> +
>>>> + /*
>>>> + * Make sure we don't reuse input
>>>> + */
>>>> + if (input)
>>>> + memset(input, 0, input_size);
>>>
>>> If (input == NULL) this function will do nothing useful. For sure will
>>> not read the password.
>>> Can be simplified assuming that input is allocated or checking it only
>>> once at function entrance?
>>> Could "input" be allocated here or in vpnc_getpass() below?
>>
>> It is a safe guard for not accepting password even if something fails,
>> I can drop this if you do not think it is that important.
>
> I'm not saying that value of input should not be checked. Let's move
> few lines below ....
>
>> The input is statically allocated at do_phase2_xauth.
>>
>>> Thanks,
>>> Antonio
>>>
>>>> +
>>>> + if (program == NULL) {
>>>> + rc = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + if (pipe(fds) == -1) {
>>>> + rc = -errno;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + if ((pid = fork()) == -1) {
>>>> + rc = -errno;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + if (pid == 0) {
>>>> + close (fds[0]);
>>>> + fds[0] = -1;
>>>> +
>>>> + if (dup2(fds[1], 1) == -1) {
>>>> + exit (1);
>>>> + }
>>>> +
>>>> + close(fds[1]);
>>>> + fds[1] = -1;
>>>> +
>>>> + execl(program, program, prompt, NULL);
>>>> +
>>>> + exit(1);
>>>> + }
>>>> +
>>>> + close(fds[1]);
>>>> + fds[1] = -1;
>>>> +
>>>> + while (
>>>> + (r=waitpid(pid, &status, 0)) == 0 ||
>>>> + (r == -1 && errno == EINTR)
>>>> + );
>>>> +
>>>> + if (r == -1) {
>>>> + rc = -errno;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + if (!WIFEXITED(status)) {
>>>> + rc = -EFAULT;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + if (WEXITSTATUS(status) != 0) {
>>>> + rc = -EIO;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + if (input != NULL) {
>
> .... this part have no meaning if input == NULL.
> I mean, you have just done fork and execl, but then you discard the
> result because input == NULL.
> Makes much more sense to return from this function at the beginning if
> input == NULL. Skip everything and return before the fork.

OK, will return an error.

>
> Antonio
>
>>>> + ssize_t bytes;
>>>> +
>>>> + if ((bytes = read (fds[0], input, input_size)) == -1) {
>>>> + rc = -errno;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + input[bytes] = '\0';
>>>> +
>>>> + if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\n')
>>>> + input[(int)strlen (input)-1] = '\0';
>>>> + /* DOS cygwin */
>>>> + if (strlen (input) > 0 && input[(int)strlen (input)-1] == '\r')
>>>> + input[(int)strlen (input)-1] = '\0';
>>>> + }
>>>> +
>>>> + rc = 0;
>>>> +
>>>> +out:
>>>> + if (rc != 0) {
>>>> + if (input)
>>>> + memset(input, 0, input_size);
>>>> + }
>>>> +
>>>> + if (fds[0] != -1) {
>>>> + close(fds[0]);
>>>> + fds[0] = -1;
>>>> + }
>>>> +
>>>> + if (fds[1] != -1) {
>>>> + close(fds[1]);
>>>> + fds[1] = -1;
>>>> + }
>>>> +
>>>> + return rc;
>>>> +}
>>>> +
>>>> +int vpnc_getpass(const char * const helper, const char *const prompt,
>>>> + char *const input, const size_t input_size)
>>>> +{
>>>> + if (helper == NULL) {
>>>> + char *pass = getpass(prompt);
>>>> + if (pass == NULL) {
>>>> + return 0;
>>>> + }
>>>> + strncpy(input, pass, input_size);
>>>> + memset(pass, 0, strlen(pass));
>>>> + return 1;
>>>> + }
>>>> + else {
>>>> + return _vpnc_getpass_program(helper, prompt, input, input_size) == 0;
>>>> + }
>>>> +}
>>>> +
>>>> void print_vid(const unsigned char *vid, uint16_t len) {
>>>>
>>>> int vid_index = 0;
>>>> @@ -2322,7 +2447,8 @@ static int do_phase2_xauth(struct sa_block *s)
>>>> phase2_fatal(s, "noninteractive can't reuse password", reject);
>>>> error(2, 0, "authentication failed (requires interactive mode)");
>>>> } else if (seen_answer || passwd_used || config[CONFIG_XAUTH_INTERACTIVE]) {
>>>> - char *pass, *prompt = NULL;
>>>> + char pass[1024];
>>>> + char *prompt = NULL;
>>>>
>>>> asprintf(&prompt, "%s for VPN %s@%s: ",
>>>> (ap->type == ISAKMP_XAUTH_06_ATTRIB_ANSWER) ?
>>>> @@ -2330,7 +2456,14 @@ static int do_phase2_xauth(struct sa_block *s)
>>>> (ap->type == ISAKMP_XAUTH_06_ATTRIB_USER_PASSWORD) ?
>>>> "Password" : "Passcode",
>>>> config[CONFIG_XAUTH_USERNAME], ntop_buf);
>>>> - pass = getpass(prompt);
>>>> + if (!vpnc_getpass(
>>>> + config[CONFIG_PASSWORD_HELPER],
>>>> + prompt,
>>>> + pass,
>>>> + sizeof(pass))) {
>>>> + free(prompt);
>>>> + error(2, 0, "authentication unsuccessful");
>>>> + }
>>>> free(prompt);
>>>>
>>>> na = new_isakmp_attribute(ap->type, NULL);
>>>> diff --git a/vpnc.h b/vpnc.h
>>>> index 2bacc08..f817fbf 100644
>>>> --- a/vpnc.h
>>>> +++ b/vpnc.h
>>>> @@ -27,5 +27,7 @@ void process_late_ike(struct sa_block *s, uint8_t *r_packet, ssize_t r_length);
>>>> void keepalive_ike(struct sa_block *s);
>>>> void dpd_ike(struct sa_block *s);
>>>> void print_vid(const unsigned char *vid, uint16_t len);
>>>> +int vpnc_getpass(const char * const helper, const char *const prompt,
>>>> + char *const input, const size_t input_size);
>>>>
>>>> #endif
>>>> --
>>>> 1.8.1.5
>>>>
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/