Mailing List Archive

Allows to integrate UI, similar to ssh-askpass, program prompt user
for password and echo result to stdout.

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

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

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

diff --git a/config.c b/config.c
index fae2799..4485063 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_PROGRAM, 1, 1,
+ "--password-program",
+ "Password Program ",
+ "<executable>",
+ "path to password program\n",
+ NULL
+ }, {
0, 0, 0, NULL, NULL, NULL, NULL, NULL
}
};
diff --git a/config.h b/config.h
index a065a58..2016e0b 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_PROGRAM,
LAST_CONFIG
};

diff --git a/tunip.c b/tunip.c
index d0bc971..ae68b34 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 f8f0828..f72d8ec 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,114 @@ const struct vid_element vid_list[] = {
static uint8_t r_packet[8192];
static ssize_t r_length;

+static int
+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;
+}
+
void print_vid(const unsigned char *vid, uint16_t len) {

int vid_index = 0;
@@ -2310,6 +2419,7 @@ 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[1024];
char *pass, *prompt = NULL;

asprintf(&prompt, "%s for VPN %s@%s: ",
@@ -2318,7 +2428,20 @@ 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 (config[CONFIG_PASSWORD_PROGRAM] == NULL) {
+ pass = getpass(prompt);
+ } else {
+ if (getpass_program(
+ config[CONFIG_PASSWORD_PROGRAM],
+ prompt,
+ _pass,
+ sizeof(_pass)) != 0
+ ) {
+ free(prompt);
+ error(2, 0, "authentication unsuccessful");
+ }
+ pass = _pass;
+ }
free(prompt);

na = new_isakmp_attribute(ap->type, NULL);
--
1.7.8.6

_______________________________________________
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/

On Wed, Jul 4, 2012 at 3:13 AM, 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.

Few comments on this patch.

What about entering username? Do you need UI for this too?

Passwords can be passed through config file. Would not be more general
to have a UI that creates the configuration as temporary file and then
runs vpnc? I expect this is the way NetworkManager works.

There are two calls to getpass() in vpnc code, while you are patching
only one call.
Do you need to extend your code to support the other case or is there
any reason to replace getpass() in just one case?

Instead of inlineing
+ if (config[CONFIG_PASSWORD_PROGRAM] == NULL) {
+ pass = getpass(prompt);
+ } else {
+ if (getpass_program(
and mainly if you have to replace both call to getpass(), I would
prefer using a wrapper around this code.
The overall code readability would be improved.
The wrap function will decide how to get the password, through
getpass() or getpass_program or some other method.

Also, the manpage of getpass() reports "This function is obsolete. Do
not use it.". A wrapper could be the right starting point to replace
it (don't know what could be a suitable replacement).

At last, there are cases where we don't care about obfuscating the
text when entering the password, for example when we copy a
one-time-token from a HW device like RSA key or ActivIdentity. Having
the password typed as clear text can let us checking if it's typed
correctly. A wrapper around getpass() could simplify adding the
additional feature to enter password as cleartext. (I'm not asking you
to implement this feature, just highlighting further development
directions).

Best Regards,
Antonio Borneo
_______________________________________________
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/

Hello Antonio,

Thank you for the review!

On Wed, Jul 4, 2012 at 10:52 AM, Antonio Borneo
<borneo.antonio@gmail.com> wrote:
> On Wed, Jul 4, 2012 at 3:13 AM, 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.
>
> Few comments on this patch.
>
> What about entering username? Do you need UI for this too?

Hmmm... we can do this too... I am new to vpnc, and did the minimal to
set my up :)

I don't see much advantage in user as most probably each user will
have different configuration file... or in my case as I run this
within my unprivileged account, it is obvious I have static user.

While executing a program at the context of root, it is usually
impossible to present anything to the user without great risk... so
this solution is for unprivileged mode only.

If we want to do this properly we would need something similar to the
openvpn management interface, communicate via socket/usock to provide
details to the daemon.

> Passwords can be passed through config file. Would not be more general
> to have a UI that creates the configuration as temporary file and then
> runs vpnc? I expect this is the way NetworkManager works.

I don't like storing passwords within files, as files are written...
of course we can put them on sysfs or similar, but then we need
special privileges to create the file system... and there can be
always be race if we require to delete these.

> There are two calls to getpass() in vpnc code, while you are patching
> only one call.
> Do you need to extend your code to support the other case or is there
> any reason to replace getpass() in just one case?

I patched the code only for the "Interactive" prompt... As far as I
examined the code, the none interactive prompt is not useful with this
feature.

>
> Instead of inlineing
> + if (config[CONFIG_PASSWORD_PROGRAM] == NULL) {
> + pass = getpass(prompt);
> + } else {
> + if (getpass_program(
> and mainly if you have to replace both call to getpass(), I would
> prefer using a wrapper around this code.
> The overall code readability would be improved.
> The wrap function will decide how to get the password, through
> getpass() or getpass_program or some other method.

OK, I will do this.

> Also, the manpage of getpass() reports "This function is obsolete. Do
> not use it.". A wrapper could be the right starting point to replace
> it (don't know what could be a suitable replacement).

There is none [simple cross platform] as far as I know without using ncurses.

> At last, there are cases where we don't care about obfuscating the
> text when entering the password, for example when we copy a
> one-time-token from a HW device like RSA key or ActivIdentity. Having
> the password typed as clear text can let us checking if it's typed
> correctly. A wrapper around getpass() could simplify adding the
> additional feature to enter password as cleartext. (I'm not asking you
> to implement this feature, just highlighting further development
> directions).

I don't follow you... what do you mean "wrapper" and "simply adding"?
do you suggest to always run a script to acquire password? similar to
vpnc-script? I can do this.

Alon.
_______________________________________________
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/

On Wed, 2012-07-04 at 15:52 +0800, Antonio Borneo wrote:
> Also, the manpage of getpass() reports "This function is obsolete. Do
> not use it.". A wrapper could be the right starting point to replace
> it (don't know what could be a suitable replacement).

Perhaps something like
http://git.infradead.org/users/dwmw2/openconnect.git/blob/v4.03:/main.c#l1140

--
dwmw2

On Wed, Jul 4, 2012 at 11:22 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2012-07-04 at 15:52 +0800, Antonio Borneo wrote:
>> Also, the manpage of getpass() reports "This function is obsolete. Do
>> not use it.". A wrapper could be the right starting point to replace
>> it (don't know what could be a suitable replacement).
>
> Perhaps something like
> http://git.infradead.org/users/dwmw2/openconnect.git/blob/v4.03:/main.c#l1140

Yes... it will work in most cases... I fear from the cases not.
I would have kept getpass() for now... :)

Alon.
_______________________________________________
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/

On Wednesday July 4 2012 3:52:31 PM Antonio Borneo wrote:
> On Wed, Jul 4, 2012 at 3:13 AM, 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.
>
> Few comments on this patch.
>
> What about entering username? Do you need UI for this too?

Generally, no. The username is generally far less hidden. For example, at
$work, we use our internal email addresses as our user names. Any coworker
can easily find my user name anyway.

Further, the user name is unlikely to change frequently.

On the other hand, a password is the secret we keep, and it'd be nice-to-have
if there was some simple way for a user to store that password in a more-
secure manner than the config file, but yet still make it accessible enough to
modify regularly.

To the latter end (modify regularly), I have a perl script that, whenever I
have to change my password, I run that script and it updates something in
KWallet as well as my vpnc configuration. (I still have to figure out how to
patch Firefox's password database automatically, but that's not on topic
here.)

However, if we (the community, not necessarily vpnc devs) could produce a
"vpnc-askpass" type application that could read an arbitrary location in
KWallet (and/or other equivalents), then that's just one less place to have to
modify. And I could set the group ownership of the vpnc config file back to
root. And those passwords are encrypted, so it's also one less plain-text
location for the password.

Of course, this brings in a bunch of other issues (such as, what happens if
DBus isn't working, e.g., X isn't running, I'm not logged in?), but as long as
there are well-defined return codes here, most of that can be pushed on to the
authors and users of these extra features. ("VPNC with this plugin only works
when you're logged in to your desktop environment. If you're not, you won't
get a connection to your VPN." Sometimes this could be construed as a
feature.)

return codes: e.g., 0 == "got password, try it", 1 == "user hit cancel", etc.

Also, like ssh-agent, this could have an in-memory persistance. That is, you
enter the password once, and, as long as its agent stays running, it doesn't
have to ask again. But, again, this is pushed to the authors of those extra
features, and not part of vpnc's core code.

Challenge for these apps will be figuring out a display - I run vpnc from a
cron job :-)

So, perhaps one example of this plugin and setup would be running a vpnc-agent
as part of the boot, and a vpnc-setpass that tells the agent what password to
use which gets to be run manually after each reboot. Then the cron job that
starts vpnc (since it has to run again after the vpnc-setpass that is run
manually) would eventually run again and pick up the password to use. Prior
to vpnc-setpass, the vpnc-askpass that is part of this suite would just return
whatever return code that says "password not available" (maybe that's the same
as "cancel") which then would cause vpnc to exit. (Communication between -
askpass and -agent is another story, perhaps a socket that is set to root
only?)
In this example, vpnc would be able to run as many times as desired, but
only setting the password once, and it would be in cleartext nowhere.

Another example is running vpnc manually, and -askpass could check if there's
a DISPLAY - if there is, pop up a graphical query, if not, a CLI. This would
imply that the user gets asked every time. This can be a valid use case as
well.

All VPNC would have to do is allow this, and define the protocol (query string,
how to return password, how to inform vpnc of various situations that vpnc
might care about). The rest would be up to someone else.

> Passwords can be passed through config file. Would not be more general
> to have a UI that creates the configuration as temporary file and then
> runs vpnc? I expect this is the way NetworkManager works.

I wouldn't want to overwrite the entire config file. If this were the case, it
should be a separate config file that can be fed to vpnc, making it easier to
write without accidentally clobbering anything else. However, again, you're
putting an unencrypted password on disk, which is something to generally
avoid. Personally, I'm not all up-in-arms about it if we can use filesystem
permissions to restrict it to root, but that requires the entire stack to be
running as root, which is generally bad. And, worse, some of our customers at
$work like to throw a fit any time there's a plain text password anywhere, even
if it's only readable by root (basically, if you can see this password, you
already have broken in to the system as someone to whom this password won't
gain any extra privileges).


_______________________________________________
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/

On Wed, Jul 4, 2012 at 11:27 AM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> On Wed, Jul 4, 2012 at 11:22 AM, David Woodhouse <dwmw2@infradead.org> wrote:
>> On Wed, 2012-07-04 at 15:52 +0800, Antonio Borneo wrote:
>>> Also, the manpage of getpass() reports "This function is obsolete. Do
>>> not use it.". A wrapper could be the right starting point to replace
>>> it (don't know what could be a suitable replacement).
>>
>> Perhaps something like
>> http://git.infradead.org/users/dwmw2/openconnect.git/blob/v4.03:/main.c#l1140
>
> Yes... it will work in most cases... I fear from the cases not.
> I would have kept getpass() for now... :)
>
> Alon.

I modified the patch.
Hope you find it better, still using getpass().

Patch series is at[1].
Patch is at[2].

[1] https://github.com/alonbl/vpnc/compare/master...unprivileged
[2] https://github.com/alonbl/vpnc/commit/84b94feb48178df90bcb57170bc9d90d914863d5
_______________________________________________
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/

ping...

On Sat, Jul 7, 2012 at 6:35 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> On Wed, Jul 4, 2012 at 11:27 AM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>> On Wed, Jul 4, 2012 at 11:22 AM, David Woodhouse <dwmw2@infradead.org> wrote:
>>> On Wed, 2012-07-04 at 15:52 +0800, Antonio Borneo wrote:
>>>> Also, the manpage of getpass() reports "This function is obsolete. Do
>>>> not use it.". A wrapper could be the right starting point to replace
>>>> it (don't know what could be a suitable replacement).
>>>
>>> Perhaps something like
>>> http://git.infradead.org/users/dwmw2/openconnect.git/blob/v4.03:/main.c#l1140
>>
>> Yes... it will work in most cases... I fear from the cases not.
>> I would have kept getpass() for now... :)
>>
>> Alon.
>
> I modified the patch.
> Hope you find it better, still using getpass().
>
> Patch series is at[1].
> Patch is at[2].
>
> [1] https://github.com/alonbl/vpnc/compare/master...unprivileged
> [2] https://github.com/alonbl/vpnc/commit/84b94feb48178df90bcb57170bc9d90d914863d5
_______________________________________________
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/