Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. vpnc>
  3. devel
[PATCH 2/2] use custom getpass() implementation if caller wants to use stdin
dcbw at redhat
Jul 11, 2013, 3:22 PM
Post #1 of 20 (7722 views)
Permalink
getpass() tries to use /dev/tty by default, which requests input directly
from the user, which prevents a controlling process from writing the input
to stdin. Add a config option to always listen for input on stdin and
implement a replacement for getpass() that always reads from stdin. The
config option performs no TTY operations on stdin (eg, unlike getpass()
it does not disable echo) so the config option should only if vpnc is
spawned from another program and pipes are used for communication.
---
trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
trunk/config.h | 2 ++
trunk/vpnc.c | 6 +++++-
3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/trunk/config.c b/trunk/config.c
index e02a66f..fa7798b 100644
--- a/trunk/config.c
+++ b/trunk/config.c
@@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
return buf;
}

+char *vpnc_getpass (const char *prompt, int always_stdin)
+{
+ char *buf = NULL;
+ size_t llen = 0;
+
+ /* Standard getpass() tries to open the TTY first so if the caller
+ * always wants to use stdin, roll our own naive implementation.
+ */
+
+ if (!always_stdin) {
+ buf = getpass(prompt);
+ return buf ? strdup (buf) : NULL;
+ }
+
+ fprintf(stderr, "%s", prompt);
+ setbuf(stdin, NULL);
+ buf = vpnc_getline(&llen, stdin);
+ if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
+ /* Remove the newline. */
+ buf[llen - 1] = '\0';
+ }
+
+ return buf;
+}
+
static void config_deobfuscate(int obfuscated, int clear)
{
int ret, len = 0;
@@ -470,6 +495,13 @@ static const struct config_names_s {
"Don't ask anything, exit on missing options",
NULL
}, {
+ CONFIG_INTERACTIVE_STDIN, 0, 1,
+ "--interactive-stdin",
+ "InteractiveStdin",
+ NULL,
+ "Always look for interactive input on stdin instead of the TTY",
+ NULL
+ }, {
CONFIG_AUTH_MODE, 1, 1,
"--auth-mode",
"IKE Authmode ",
@@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
switch (i) {
case CONFIG_IPSEC_SECRET:
case CONFIG_XAUTH_PASSWORD:
- s = strdup(getpass(""));
+ s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
break;
case CONFIG_IPSEC_GATEWAY:
case CONFIG_IPSEC_ID:
diff --git a/trunk/config.h b/trunk/config.h
index 6fbd231..0460d83 100644
--- a/trunk/config.h
+++ b/trunk/config.h
@@ -59,6 +59,7 @@ enum config_enum {
CONFIG_AUTH_MODE,
CONFIG_CA_FILE,
CONFIG_CA_DIR,
+ CONFIG_INTERACTIVE_STDIN,
LAST_CONFIG
};

@@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;

extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
extern void do_config(int argc, char **argv);
+extern char *vpnc_getpass(const char *prompt, int always_stdin);

extern void (*logmsg)(int priority, const char *format, ...)
__attribute__ ((__format__ (__printf__, 2, 3)));
diff --git a/trunk/vpnc.c b/trunk/vpnc.c
index eaa29fa..9e548c4 100644
--- a/trunk/vpnc.c
+++ b/trunk/vpnc.c
@@ -2308,14 +2308,18 @@ 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);
+ pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
free(prompt);

+ if (pass == NULL)
+ phase2_fatal(s, "failed to read password", reject);
+
na = new_isakmp_attribute(ap->type, NULL);
na->u.lots.length = strlen(pass);
na->u.lots.data = xallocc(na->u.lots.length);
memcpy(na->u.lots.data, pass, na->u.lots.length);
memset(pass, 0, na->u.lots.length);
+ free(pass);
} else {
na = new_isakmp_attribute(ap->type, NULL);
na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
--
1.8.1.4


_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Jul 11, 2013, 9:04 PM
Post #2 of 20 (7636 views)
Permalink
Hi,

I think that this[1] patch that was not merged for some reason is superset
of this.

Regards,
Alon

[1]
http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html


On Fri, Jul 12, 2013 at 1:22 AM, Dan Williams <dcbw@redhat.com> wrote:

> getpass() tries to use /dev/tty by default, which requests input directly
> from the user, which prevents a controlling process from writing the input
> to stdin. Add a config option to always listen for input on stdin and
> implement a replacement for getpass() that always reads from stdin. The
> config option performs no TTY operations on stdin (eg, unlike getpass()
> it does not disable echo) so the config option should only if vpnc is
> spawned from another program and pipes are used for communication.
> ---
> trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
> trunk/config.h | 2 ++
> trunk/vpnc.c | 6 +++++-
> 3 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/trunk/config.c b/trunk/config.c
> index e02a66f..fa7798b 100644
> --- a/trunk/config.c
> +++ b/trunk/config.c
> @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE
> *stream)
> return buf;
> }
>
> +char *vpnc_getpass (const char *prompt, int always_stdin)
> +{
> + char *buf = NULL;
> + size_t llen = 0;
> +
> + /* Standard getpass() tries to open the TTY first so if the caller
> + * always wants to use stdin, roll our own naive implementation.
> + */
> +
> + if (!always_stdin) {
> + buf = getpass(prompt);
> + return buf ? strdup (buf) : NULL;
> + }
> +
> + fprintf(stderr, "%s", prompt);
> + setbuf(stdin, NULL);
> + buf = vpnc_getline(&llen, stdin);
> + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
> + /* Remove the newline. */
> + buf[llen - 1] = '\0';
> + }
> +
> + return buf;
> +}
> +
> static void config_deobfuscate(int obfuscated, int clear)
> {
> int ret, len = 0;
> @@ -470,6 +495,13 @@ static const struct config_names_s {
> "Don't ask anything, exit on missing options",
> NULL
> }, {
> + CONFIG_INTERACTIVE_STDIN, 0, 1,
> + "--interactive-stdin",
> + "InteractiveStdin",
> + NULL,
> + "Always look for interactive input on stdin instead of the
> TTY",
> + NULL
> + }, {
> CONFIG_AUTH_MODE, 1, 1,
> "--auth-mode",
> "IKE Authmode ",
> @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
> switch (i) {
> case CONFIG_IPSEC_SECRET:
> case CONFIG_XAUTH_PASSWORD:
> - s = strdup(getpass(""));
> + s = vpnc_getpass("",
> !!config[CONFIG_INTERACTIVE_STDIN]);
> break;
> case CONFIG_IPSEC_GATEWAY:
> case CONFIG_IPSEC_ID:
> diff --git a/trunk/config.h b/trunk/config.h
> index 6fbd231..0460d83 100644
> --- a/trunk/config.h
> +++ b/trunk/config.h
> @@ -59,6 +59,7 @@ enum config_enum {
> CONFIG_AUTH_MODE,
> CONFIG_CA_FILE,
> CONFIG_CA_DIR,
> + CONFIG_INTERACTIVE_STDIN,
> LAST_CONFIG
> };
>
> @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
>
> extern void hex_dump(const char *str, const void *data, ssize_t len,
> const struct debug_strings *decode);
> extern void do_config(int argc, char **argv);
> +extern char *vpnc_getpass(const char *prompt, int always_stdin);
>
> extern void (*logmsg)(int priority, const char *format, ...)
> __attribute__ ((__format__ (__printf__, 2, 3)));
> diff --git a/trunk/vpnc.c b/trunk/vpnc.c
> index eaa29fa..9e548c4 100644
> --- a/trunk/vpnc.c
> +++ b/trunk/vpnc.c
> @@ -2308,14 +2308,18 @@ 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);
> + pass = vpnc_getpass(prompt,
> !!config[CONFIG_INTERACTIVE_STDIN]);
> free(prompt);
>
> + if (pass == NULL)
> + phase2_fatal(s, "failed to
> read password", reject);
> +
> na =
> new_isakmp_attribute(ap->type, NULL);
> na->u.lots.length = strlen(pass);
> na->u.lots.data =
> xallocc(na->u.lots.length);
> memcpy(na->u.lots.data, pass,
> na->u.lots.length);
> memset(pass, 0, na->u.lots.length);
> + free(pass);
> } else {
> na =
> new_isakmp_attribute(ap->type, NULL);
> na->u.lots.length =
> strlen(config[CONFIG_XAUTH_PASSWORD]);
> --
> 1.8.1.4
>
>
> _______________________________________________
> 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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
borneo.antonio at gmail
Nov 30, 2013, 3:29 AM
Post #3 of 20 (7498 views)
Permalink
On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
> getpass() tries to use /dev/tty by default, which requests input directly
> from the user, which prevents a controlling process from writing the input
> to stdin. Add a config option to always listen for input on stdin and
> implement a replacement for getpass() that always reads from stdin. The
> config option performs no TTY operations on stdin (eg, unlike getpass()
> it does not disable echo) so the config option should only if vpnc is
> spawned from another program and pipes are used for communication.
> ---
> trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
> trunk/config.h | 2 ++
> trunk/vpnc.c | 6 +++++-
> 3 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/trunk/config.c b/trunk/config.c
> index e02a66f..fa7798b 100644
> --- a/trunk/config.c
> +++ b/trunk/config.c
> @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
> return buf;
> }
>
> +char *vpnc_getpass (const char *prompt, int always_stdin)
> +{
> + char *buf = NULL;
> + size_t llen = 0;
> +
> + /* Standard getpass() tries to open the TTY first so if the caller
> + * always wants to use stdin, roll our own naive implementation.
> + */
> +
> + if (!always_stdin) {
> + buf = getpass(prompt);
> + return buf ? strdup (buf) : NULL;
> + }
> +
> + fprintf(stderr, "%s", prompt);
> + setbuf(stdin, NULL);
> + buf = vpnc_getline(&llen, stdin);
> + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
> + /* Remove the newline. */
> + buf[llen - 1] = '\0';
> + }
> +
> + return buf;
> +}

Hi Dan,
we started talking about it, but then I did not went ahead.
As far as I understand, the target is to:
- use getpass() when a user types password on tty;
- use vpnc_getline() when password is arriving from stdin.
Or I'm missing some other useful case?

My question is:
do we really need to introduce a new command line flag?
Can we insted autodetect inside vpnc_getpass():
always_stdin = ! isatty(fileno(stdin));

Best Regards,
Antonio

> +
> static void config_deobfuscate(int obfuscated, int clear)
> {
> int ret, len = 0;
> @@ -470,6 +495,13 @@ static const struct config_names_s {
> "Don't ask anything, exit on missing options",
> NULL
> }, {
> + CONFIG_INTERACTIVE_STDIN, 0, 1,
> + "--interactive-stdin",
> + "InteractiveStdin",
> + NULL,
> + "Always look for interactive input on stdin instead of the TTY",
> + NULL
> + }, {
> CONFIG_AUTH_MODE, 1, 1,
> "--auth-mode",
> "IKE Authmode ",
> @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
> switch (i) {
> case CONFIG_IPSEC_SECRET:
> case CONFIG_XAUTH_PASSWORD:
> - s = strdup(getpass(""));
> + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
> break;
> case CONFIG_IPSEC_GATEWAY:
> case CONFIG_IPSEC_ID:
> diff --git a/trunk/config.h b/trunk/config.h
> index 6fbd231..0460d83 100644
> --- a/trunk/config.h
> +++ b/trunk/config.h
> @@ -59,6 +59,7 @@ enum config_enum {
> CONFIG_AUTH_MODE,
> CONFIG_CA_FILE,
> CONFIG_CA_DIR,
> + CONFIG_INTERACTIVE_STDIN,
> LAST_CONFIG
> };
>
> @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
>
> extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
> extern void do_config(int argc, char **argv);
> +extern char *vpnc_getpass(const char *prompt, int always_stdin);
>
> extern void (*logmsg)(int priority, const char *format, ...)
> __attribute__ ((__format__ (__printf__, 2, 3)));
> diff --git a/trunk/vpnc.c b/trunk/vpnc.c
> index eaa29fa..9e548c4 100644
> --- a/trunk/vpnc.c
> +++ b/trunk/vpnc.c
> @@ -2308,14 +2308,18 @@ 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);
> + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
> free(prompt);
>
> + if (pass == NULL)
> + phase2_fatal(s, "failed to read password", reject);
> +
> na = new_isakmp_attribute(ap->type, NULL);
> na->u.lots.length = strlen(pass);
> na->u.lots.data = xallocc(na->u.lots.length);
> memcpy(na->u.lots.data, pass, na->u.lots.length);
> memset(pass, 0, na->u.lots.length);
> + free(pass);
> } else {
> na = new_isakmp_attribute(ap->type, NULL);
> na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
> --
> 1.8.1.4
>
>
> _______________________________________________
> 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/
_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
dcbw at redhat
Dec 13, 2013, 8:46 AM
Post #4 of 20 (7481 views)
Permalink
On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
> On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
> > getpass() tries to use /dev/tty by default, which requests input directly
> > from the user, which prevents a controlling process from writing the input
> > to stdin. Add a config option to always listen for input on stdin and
> > implement a replacement for getpass() that always reads from stdin. The
> > config option performs no TTY operations on stdin (eg, unlike getpass()
> > it does not disable echo) so the config option should only if vpnc is
> > spawned from another program and pipes are used for communication.
> > ---
> > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
> > trunk/config.h | 2 ++
> > trunk/vpnc.c | 6 +++++-
> > 3 files changed, 40 insertions(+), 2 deletions(-)
> >
> > diff --git a/trunk/config.c b/trunk/config.c
> > index e02a66f..fa7798b 100644
> > --- a/trunk/config.c
> > +++ b/trunk/config.c
> > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
> > return buf;
> > }
> >
> > +char *vpnc_getpass (const char *prompt, int always_stdin)
> > +{
> > + char *buf = NULL;
> > + size_t llen = 0;
> > +
> > + /* Standard getpass() tries to open the TTY first so if the caller
> > + * always wants to use stdin, roll our own naive implementation.
> > + */
> > +
> > + if (!always_stdin) {
> > + buf = getpass(prompt);
> > + return buf ? strdup (buf) : NULL;
> > + }
> > +
> > + fprintf(stderr, "%s", prompt);
> > + setbuf(stdin, NULL);
> > + buf = vpnc_getline(&llen, stdin);
> > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
> > + /* Remove the newline. */
> > + buf[llen - 1] = '\0';
> > + }
> > +
> > + return buf;
> > +}
>
> Hi Dan,
> we started talking about it, but then I did not went ahead.
> As far as I understand, the target is to:
> - use getpass() when a user types password on tty;
> - use vpnc_getline() when password is arriving from stdin.
> Or I'm missing some other useful case?

IIRC the other use-case I was thinking of, which caused me to create the
flag, was for a command-line program that wanted to wrap vpnc and
deliver config via stdin, but still allow the user to type the password
into the TTY. GUI programs obviously wouldn't do this, and my use-cases
(NetworkManager-vpnc) would do that, but I thought it might be useful.

I also didn't want to change any existing behavior that other users
might depend on.

However, I agree that just using isatty() would be simpler, so if you
prefer that I'm happy to do that instead, and a config option could
always be added later. What do you think?

Dan

> My question is:
> do we really need to introduce a new command line flag?
> Can we insted autodetect inside vpnc_getpass():
> always_stdin = ! isatty(fileno(stdin));
>
> Best Regards,
> Antonio
>
> > +
> > static void config_deobfuscate(int obfuscated, int clear)
> > {
> > int ret, len = 0;
> > @@ -470,6 +495,13 @@ static const struct config_names_s {
> > "Don't ask anything, exit on missing options",
> > NULL
> > }, {
> > + CONFIG_INTERACTIVE_STDIN, 0, 1,
> > + "--interactive-stdin",
> > + "InteractiveStdin",
> > + NULL,
> > + "Always look for interactive input on stdin instead of the TTY",
> > + NULL
> > + }, {
> > CONFIG_AUTH_MODE, 1, 1,
> > "--auth-mode",
> > "IKE Authmode ",
> > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
> > switch (i) {
> > case CONFIG_IPSEC_SECRET:
> > case CONFIG_XAUTH_PASSWORD:
> > - s = strdup(getpass(""));
> > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
> > break;
> > case CONFIG_IPSEC_GATEWAY:
> > case CONFIG_IPSEC_ID:
> > diff --git a/trunk/config.h b/trunk/config.h
> > index 6fbd231..0460d83 100644
> > --- a/trunk/config.h
> > +++ b/trunk/config.h
> > @@ -59,6 +59,7 @@ enum config_enum {
> > CONFIG_AUTH_MODE,
> > CONFIG_CA_FILE,
> > CONFIG_CA_DIR,
> > + CONFIG_INTERACTIVE_STDIN,
> > LAST_CONFIG
> > };
> >
> > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
> >
> > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
> > extern void do_config(int argc, char **argv);
> > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
> >
> > extern void (*logmsg)(int priority, const char *format, ...)
> > __attribute__ ((__format__ (__printf__, 2, 3)));
> > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
> > index eaa29fa..9e548c4 100644
> > --- a/trunk/vpnc.c
> > +++ b/trunk/vpnc.c
> > @@ -2308,14 +2308,18 @@ 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);
> > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
> > free(prompt);
> >
> > + if (pass == NULL)
> > + phase2_fatal(s, "failed to read password", reject);
> > +
> > na = new_isakmp_attribute(ap->type, NULL);
> > na->u.lots.length = strlen(pass);
> > na->u.lots.data = xallocc(na->u.lots.length);
> > memcpy(na->u.lots.data, pass, na->u.lots.length);
> > memset(pass, 0, na->u.lots.length);
> > + free(pass);
> > } else {
> > na = new_isakmp_attribute(ap->type, NULL);
> > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
> > --
> > 1.8.1.4
> >
> >
> > _______________________________________________
> > 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/


_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Dec 13, 2013, 8:59 AM
Post #5 of 20 (7480 views)
Permalink
On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
>
> On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
> > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
> > > getpass() tries to use /dev/tty by default, which requests input directly
> > > from the user, which prevents a controlling process from writing the input
> > > to stdin. Add a config option to always listen for input on stdin and
> > > implement a replacement for getpass() that always reads from stdin. The
> > > config option performs no TTY operations on stdin (eg, unlike getpass()
> > > it does not disable echo) so the config option should only if vpnc is
> > > spawned from another program and pipes are used for communication.
> > > ---
> > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
> > > trunk/config.h | 2 ++
> > > trunk/vpnc.c | 6 +++++-
> > > 3 files changed, 40 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/trunk/config.c b/trunk/config.c
> > > index e02a66f..fa7798b 100644
> > > --- a/trunk/config.c
> > > +++ b/trunk/config.c
> > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
> > > return buf;
> > > }
> > >
> > > +char *vpnc_getpass (const char *prompt, int always_stdin)
> > > +{
> > > + char *buf = NULL;
> > > + size_t llen = 0;
> > > +
> > > + /* Standard getpass() tries to open the TTY first so if the caller
> > > + * always wants to use stdin, roll our own naive implementation.
> > > + */
> > > +
> > > + if (!always_stdin) {
> > > + buf = getpass(prompt);
> > > + return buf ? strdup (buf) : NULL;
> > > + }
> > > +
> > > + fprintf(stderr, "%s", prompt);
> > > + setbuf(stdin, NULL);
> > > + buf = vpnc_getline(&llen, stdin);
> > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
> > > + /* Remove the newline. */
> > > + buf[llen - 1] = '\0';
> > > + }
> > > +
> > > + return buf;
> > > +}
> >
> > Hi Dan,
> > we started talking about it, but then I did not went ahead.
> > As far as I understand, the target is to:
> > - use getpass() when a user types password on tty;
> > - use vpnc_getline() when password is arriving from stdin.
> > Or I'm missing some other useful case?
>
> IIRC the other use-case I was thinking of, which caused me to create the
> flag, was for a command-line program that wanted to wrap vpnc and
> deliver config via stdin, but still allow the user to type the password
> into the TTY. GUI programs obviously wouldn't do this, and my use-cases
> (NetworkManager-vpnc) would do that, but I thought it might be useful.
>
> I also didn't want to change any existing behavior that other users
> might depend on.
>
> However, I agree that just using isatty() would be simpler, so if you
> prefer that I'm happy to do that instead, and a config option could
> always be added later. What do you think?


I still think that [1] provides a superset solution for this and I
already use it to feed vpnc from graphical UI in my case it is
kdialog.

[1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html

>
>
> Dan
>
> > My question is:
> > do we really need to introduce a new command line flag?
> > Can we insted autodetect inside vpnc_getpass():
> > always_stdin = ! isatty(fileno(stdin));
> >
> > Best Regards,
> > Antonio
> >
> > > +
> > > static void config_deobfuscate(int obfuscated, int clear)
> > > {
> > > int ret, len = 0;
> > > @@ -470,6 +495,13 @@ static const struct config_names_s {
> > > "Don't ask anything, exit on missing options",
> > > NULL
> > > }, {
> > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
> > > + "--interactive-stdin",
> > > + "InteractiveStdin",
> > > + NULL,
> > > + "Always look for interactive input on stdin instead of the TTY",
> > > + NULL
> > > + }, {
> > > CONFIG_AUTH_MODE, 1, 1,
> > > "--auth-mode",
> > > "IKE Authmode ",
> > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
> > > switch (i) {
> > > case CONFIG_IPSEC_SECRET:
> > > case CONFIG_XAUTH_PASSWORD:
> > > - s = strdup(getpass(""));
> > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
> > > break;
> > > case CONFIG_IPSEC_GATEWAY:
> > > case CONFIG_IPSEC_ID:
> > > diff --git a/trunk/config.h b/trunk/config.h
> > > index 6fbd231..0460d83 100644
> > > --- a/trunk/config.h
> > > +++ b/trunk/config.h
> > > @@ -59,6 +59,7 @@ enum config_enum {
> > > CONFIG_AUTH_MODE,
> > > CONFIG_CA_FILE,
> > > CONFIG_CA_DIR,
> > > + CONFIG_INTERACTIVE_STDIN,
> > > LAST_CONFIG
> > > };
> > >
> > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
> > >
> > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
> > > extern void do_config(int argc, char **argv);
> > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
> > >
> > > extern void (*logmsg)(int priority, const char *format, ...)
> > > __attribute__ ((__format__ (__printf__, 2, 3)));
> > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
> > > index eaa29fa..9e548c4 100644
> > > --- a/trunk/vpnc.c
> > > +++ b/trunk/vpnc.c
> > > @@ -2308,14 +2308,18 @@ 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);
> > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
> > > free(prompt);
> > >
> > > + if (pass == NULL)
> > > + phase2_fatal(s, "failed to read password", reject);
> > > +
> > > na = new_isakmp_attribute(ap->type, NULL);
> > > na->u.lots.length = strlen(pass);
> > > na->u.lots.data = xallocc(na->u.lots.length);
> > > memcpy(na->u.lots.data, pass, na->u.lots.length);
> > > memset(pass, 0, na->u.lots.length);
> > > + free(pass);
> > > } else {
> > > na = new_isakmp_attribute(ap->type, NULL);
> > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
> > > --
> > > 1.8.1.4
> > >
> > >
> > > _______________________________________________
> > > 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/
>
>
> _______________________________________________
> 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/
_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
dcbw at redhat
Dec 13, 2013, 10:36 AM
Post #6 of 20 (7478 views)
Permalink
On Fri, 2013-12-13 at 18:59 +0200, Alon Bar-Lev wrote:
> On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
> >
> > On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
> > > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
> > > > getpass() tries to use /dev/tty by default, which requests input directly
> > > > from the user, which prevents a controlling process from writing the input
> > > > to stdin. Add a config option to always listen for input on stdin and
> > > > implement a replacement for getpass() that always reads from stdin. The
> > > > config option performs no TTY operations on stdin (eg, unlike getpass()
> > > > it does not disable echo) so the config option should only if vpnc is
> > > > spawned from another program and pipes are used for communication.
> > > > ---
> > > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
> > > > trunk/config.h | 2 ++
> > > > trunk/vpnc.c | 6 +++++-
> > > > 3 files changed, 40 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/trunk/config.c b/trunk/config.c
> > > > index e02a66f..fa7798b 100644
> > > > --- a/trunk/config.c
> > > > +++ b/trunk/config.c
> > > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
> > > > return buf;
> > > > }
> > > >
> > > > +char *vpnc_getpass (const char *prompt, int always_stdin)
> > > > +{
> > > > + char *buf = NULL;
> > > > + size_t llen = 0;
> > > > +
> > > > + /* Standard getpass() tries to open the TTY first so if the caller
> > > > + * always wants to use stdin, roll our own naive implementation.
> > > > + */
> > > > +
> > > > + if (!always_stdin) {
> > > > + buf = getpass(prompt);
> > > > + return buf ? strdup (buf) : NULL;
> > > > + }
> > > > +
> > > > + fprintf(stderr, "%s", prompt);
> > > > + setbuf(stdin, NULL);
> > > > + buf = vpnc_getline(&llen, stdin);
> > > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
> > > > + /* Remove the newline. */
> > > > + buf[llen - 1] = '\0';
> > > > + }
> > > > +
> > > > + return buf;
> > > > +}
> > >
> > > Hi Dan,
> > > we started talking about it, but then I did not went ahead.
> > > As far as I understand, the target is to:
> > > - use getpass() when a user types password on tty;
> > > - use vpnc_getline() when password is arriving from stdin.
> > > Or I'm missing some other useful case?
> >
> > IIRC the other use-case I was thinking of, which caused me to create the
> > flag, was for a command-line program that wanted to wrap vpnc and
> > deliver config via stdin, but still allow the user to type the password
> > into the TTY. GUI programs obviously wouldn't do this, and my use-cases
> > (NetworkManager-vpnc) would do that, but I thought it might be useful.
> >
> > I also didn't want to change any existing behavior that other users
> > might depend on.
> >
> > However, I agree that just using isatty() would be simpler, so if you
> > prefer that I'm happy to do that instead, and a config option could
> > always be added later. What do you think?
>
>
> I still think that [1] provides a superset solution for this and I
> already use it to feed vpnc from graphical UI in my case it is
> kdialog.

I didn't take this type of approach because it requires another process.
In my case there's already a process controlling vpnc's lifetime and
writing configuration to stdin, and that process knows more details
about everything required. With your patch, I would require a third
process that would have to communicate back to the first process to get
additional details, which seems sub-optimal.

While not really suitable for my uses, I certainly see how your approach
is valid for other use-cases and I have no problem with both approaches
co-existing. For example, if "Password program" were not specified,
then the isatty() behavior could kick in, otherwise the password program
would be spawned.

Does that sound OK?

Dan

> [1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html
>
> >
> >
> > Dan
> >
> > > My question is:
> > > do we really need to introduce a new command line flag?
> > > Can we insted autodetect inside vpnc_getpass():
> > > always_stdin = ! isatty(fileno(stdin));
> > >
> > > Best Regards,
> > > Antonio
> > >
> > > > +
> > > > static void config_deobfuscate(int obfuscated, int clear)
> > > > {
> > > > int ret, len = 0;
> > > > @@ -470,6 +495,13 @@ static const struct config_names_s {
> > > > "Don't ask anything, exit on missing options",
> > > > NULL
> > > > }, {
> > > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
> > > > + "--interactive-stdin",
> > > > + "InteractiveStdin",
> > > > + NULL,
> > > > + "Always look for interactive input on stdin instead of the TTY",
> > > > + NULL
> > > > + }, {
> > > > CONFIG_AUTH_MODE, 1, 1,
> > > > "--auth-mode",
> > > > "IKE Authmode ",
> > > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
> > > > switch (i) {
> > > > case CONFIG_IPSEC_SECRET:
> > > > case CONFIG_XAUTH_PASSWORD:
> > > > - s = strdup(getpass(""));
> > > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
> > > > break;
> > > > case CONFIG_IPSEC_GATEWAY:
> > > > case CONFIG_IPSEC_ID:
> > > > diff --git a/trunk/config.h b/trunk/config.h
> > > > index 6fbd231..0460d83 100644
> > > > --- a/trunk/config.h
> > > > +++ b/trunk/config.h
> > > > @@ -59,6 +59,7 @@ enum config_enum {
> > > > CONFIG_AUTH_MODE,
> > > > CONFIG_CA_FILE,
> > > > CONFIG_CA_DIR,
> > > > + CONFIG_INTERACTIVE_STDIN,
> > > > LAST_CONFIG
> > > > };
> > > >
> > > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
> > > >
> > > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
> > > > extern void do_config(int argc, char **argv);
> > > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
> > > >
> > > > extern void (*logmsg)(int priority, const char *format, ...)
> > > > __attribute__ ((__format__ (__printf__, 2, 3)));
> > > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
> > > > index eaa29fa..9e548c4 100644
> > > > --- a/trunk/vpnc.c
> > > > +++ b/trunk/vpnc.c
> > > > @@ -2308,14 +2308,18 @@ 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);
> > > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
> > > > free(prompt);
> > > >
> > > > + if (pass == NULL)
> > > > + phase2_fatal(s, "failed to read password", reject);
> > > > +
> > > > na = new_isakmp_attribute(ap->type, NULL);
> > > > na->u.lots.length = strlen(pass);
> > > > na->u.lots.data = xallocc(na->u.lots.length);
> > > > memcpy(na->u.lots.data, pass, na->u.lots.length);
> > > > memset(pass, 0, na->u.lots.length);
> > > > + free(pass);
> > > > } else {
> > > > na = new_isakmp_attribute(ap->type, NULL);
> > > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
> > > > --
> > > > 1.8.1.4
> > > >
> > > >
> > > > _______________________________________________
> > > > 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/
> >
> >
> > _______________________________________________
> > 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/
> _______________________________________________
> 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/


_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Dec 13, 2013, 10:41 AM
Post #7 of 20 (7484 views)
Permalink
On Fri, Dec 13, 2013 at 8:36 PM, Dan Williams <dcbw@redhat.com> wrote:
> On Fri, 2013-12-13 at 18:59 +0200, Alon Bar-Lev wrote:
>> On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >
>> > On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
>> > > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
>> > > > getpass() tries to use /dev/tty by default, which requests input directly
>> > > > from the user, which prevents a controlling process from writing the input
>> > > > to stdin. Add a config option to always listen for input on stdin and
>> > > > implement a replacement for getpass() that always reads from stdin. The
>> > > > config option performs no TTY operations on stdin (eg, unlike getpass()
>> > > > it does not disable echo) so the config option should only if vpnc is
>> > > > spawned from another program and pipes are used for communication.
>> > > > ---
>> > > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
>> > > > trunk/config.h | 2 ++
>> > > > trunk/vpnc.c | 6 +++++-
>> > > > 3 files changed, 40 insertions(+), 2 deletions(-)
>> > > >
>> > > > diff --git a/trunk/config.c b/trunk/config.c
>> > > > index e02a66f..fa7798b 100644
>> > > > --- a/trunk/config.c
>> > > > +++ b/trunk/config.c
>> > > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
>> > > > return buf;
>> > > > }
>> > > >
>> > > > +char *vpnc_getpass (const char *prompt, int always_stdin)
>> > > > +{
>> > > > + char *buf = NULL;
>> > > > + size_t llen = 0;
>> > > > +
>> > > > + /* Standard getpass() tries to open the TTY first so if the caller
>> > > > + * always wants to use stdin, roll our own naive implementation.
>> > > > + */
>> > > > +
>> > > > + if (!always_stdin) {
>> > > > + buf = getpass(prompt);
>> > > > + return buf ? strdup (buf) : NULL;
>> > > > + }
>> > > > +
>> > > > + fprintf(stderr, "%s", prompt);
>> > > > + setbuf(stdin, NULL);
>> > > > + buf = vpnc_getline(&llen, stdin);
>> > > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
>> > > > + /* Remove the newline. */
>> > > > + buf[llen - 1] = '\0';
>> > > > + }
>> > > > +
>> > > > + return buf;
>> > > > +}
>> > >
>> > > Hi Dan,
>> > > we started talking about it, but then I did not went ahead.
>> > > As far as I understand, the target is to:
>> > > - use getpass() when a user types password on tty;
>> > > - use vpnc_getline() when password is arriving from stdin.
>> > > Or I'm missing some other useful case?
>> >
>> > IIRC the other use-case I was thinking of, which caused me to create the
>> > flag, was for a command-line program that wanted to wrap vpnc and
>> > deliver config via stdin, but still allow the user to type the password
>> > into the TTY. GUI programs obviously wouldn't do this, and my use-cases
>> > (NetworkManager-vpnc) would do that, but I thought it might be useful.
>> >
>> > I also didn't want to change any existing behavior that other users
>> > might depend on.
>> >
>> > However, I agree that just using isatty() would be simpler, so if you
>> > prefer that I'm happy to do that instead, and a config option could
>> > always be added later. What do you think?
>>
>>
>> I still think that [1] provides a superset solution for this and I
>> already use it to feed vpnc from graphical UI in my case it is
>> kdialog.
>
> I didn't take this type of approach because it requires another process.
> In my case there's already a process controlling vpnc's lifetime and
> writing configuration to stdin, and that process knows more details
> about everything required. With your patch, I would require a third
> process that would have to communicate back to the first process to get
> additional details, which seems sub-optimal.
>
> While not really suitable for my uses, I certainly see how your approach
> is valid for other use-cases and I have no problem with both approaches
> co-existing. For example, if "Password program" were not specified,
> then the isatty() behavior could kick in, otherwise the password program
> would be spawned.
>
> Does that sound OK?

Why won't 'cat' as program will do the trick?

>
> Dan
>
>> [1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html
>>
>> >
>> >
>> > Dan
>> >
>> > > My question is:
>> > > do we really need to introduce a new command line flag?
>> > > Can we insted autodetect inside vpnc_getpass():
>> > > always_stdin = ! isatty(fileno(stdin));
>> > >
>> > > Best Regards,
>> > > Antonio
>> > >
>> > > > +
>> > > > static void config_deobfuscate(int obfuscated, int clear)
>> > > > {
>> > > > int ret, len = 0;
>> > > > @@ -470,6 +495,13 @@ static const struct config_names_s {
>> > > > "Don't ask anything, exit on missing options",
>> > > > NULL
>> > > > }, {
>> > > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
>> > > > + "--interactive-stdin",
>> > > > + "InteractiveStdin",
>> > > > + NULL,
>> > > > + "Always look for interactive input on stdin instead of the TTY",
>> > > > + NULL
>> > > > + }, {
>> > > > CONFIG_AUTH_MODE, 1, 1,
>> > > > "--auth-mode",
>> > > > "IKE Authmode ",
>> > > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
>> > > > switch (i) {
>> > > > case CONFIG_IPSEC_SECRET:
>> > > > case CONFIG_XAUTH_PASSWORD:
>> > > > - s = strdup(getpass(""));
>> > > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
>> > > > break;
>> > > > case CONFIG_IPSEC_GATEWAY:
>> > > > case CONFIG_IPSEC_ID:
>> > > > diff --git a/trunk/config.h b/trunk/config.h
>> > > > index 6fbd231..0460d83 100644
>> > > > --- a/trunk/config.h
>> > > > +++ b/trunk/config.h
>> > > > @@ -59,6 +59,7 @@ enum config_enum {
>> > > > CONFIG_AUTH_MODE,
>> > > > CONFIG_CA_FILE,
>> > > > CONFIG_CA_DIR,
>> > > > + CONFIG_INTERACTIVE_STDIN,
>> > > > LAST_CONFIG
>> > > > };
>> > > >
>> > > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
>> > > >
>> > > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
>> > > > extern void do_config(int argc, char **argv);
>> > > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
>> > > >
>> > > > extern void (*logmsg)(int priority, const char *format, ...)
>> > > > __attribute__ ((__format__ (__printf__, 2, 3)));
>> > > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
>> > > > index eaa29fa..9e548c4 100644
>> > > > --- a/trunk/vpnc.c
>> > > > +++ b/trunk/vpnc.c
>> > > > @@ -2308,14 +2308,18 @@ 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);
>> > > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
>> > > > free(prompt);
>> > > >
>> > > > + if (pass == NULL)
>> > > > + phase2_fatal(s, "failed to read password", reject);
>> > > > +
>> > > > na = new_isakmp_attribute(ap->type, NULL);
>> > > > na->u.lots.length = strlen(pass);
>> > > > na->u.lots.data = xallocc(na->u.lots.length);
>> > > > memcpy(na->u.lots.data, pass, na->u.lots.length);
>> > > > memset(pass, 0, na->u.lots.length);
>> > > > + free(pass);
>> > > > } else {
>> > > > na = new_isakmp_attribute(ap->type, NULL);
>> > > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
>> > > > --
>> > > > 1.8.1.4
>> > > >
>> > > >
>> > > > _______________________________________________
>> > > > 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/
>> >
>> >
>> > _______________________________________________
>> > 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/
>> _______________________________________________
>> 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/
>
>
> _______________________________________________
> 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/
_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
dcbw at redhat
Dec 13, 2013, 11:03 AM
Post #8 of 20 (7485 views)
Permalink
On Fri, 2013-12-13 at 20:41 +0200, Alon Bar-Lev wrote:
> On Fri, Dec 13, 2013 at 8:36 PM, Dan Williams <dcbw@redhat.com> wrote:
> > On Fri, 2013-12-13 at 18:59 +0200, Alon Bar-Lev wrote:
> >> On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> >
> >> > On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
> >> > > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
> >> > > > getpass() tries to use /dev/tty by default, which requests input directly
> >> > > > from the user, which prevents a controlling process from writing the input
> >> > > > to stdin. Add a config option to always listen for input on stdin and
> >> > > > implement a replacement for getpass() that always reads from stdin. The
> >> > > > config option performs no TTY operations on stdin (eg, unlike getpass()
> >> > > > it does not disable echo) so the config option should only if vpnc is
> >> > > > spawned from another program and pipes are used for communication.
> >> > > > ---
> >> > > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
> >> > > > trunk/config.h | 2 ++
> >> > > > trunk/vpnc.c | 6 +++++-
> >> > > > 3 files changed, 40 insertions(+), 2 deletions(-)
> >> > > >
> >> > > > diff --git a/trunk/config.c b/trunk/config.c
> >> > > > index e02a66f..fa7798b 100644
> >> > > > --- a/trunk/config.c
> >> > > > +++ b/trunk/config.c
> >> > > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
> >> > > > return buf;
> >> > > > }
> >> > > >
> >> > > > +char *vpnc_getpass (const char *prompt, int always_stdin)
> >> > > > +{
> >> > > > + char *buf = NULL;
> >> > > > + size_t llen = 0;
> >> > > > +
> >> > > > + /* Standard getpass() tries to open the TTY first so if the caller
> >> > > > + * always wants to use stdin, roll our own naive implementation.
> >> > > > + */
> >> > > > +
> >> > > > + if (!always_stdin) {
> >> > > > + buf = getpass(prompt);
> >> > > > + return buf ? strdup (buf) : NULL;
> >> > > > + }
> >> > > > +
> >> > > > + fprintf(stderr, "%s", prompt);
> >> > > > + setbuf(stdin, NULL);
> >> > > > + buf = vpnc_getline(&llen, stdin);
> >> > > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
> >> > > > + /* Remove the newline. */
> >> > > > + buf[llen - 1] = '\0';
> >> > > > + }
> >> > > > +
> >> > > > + return buf;
> >> > > > +}
> >> > >
> >> > > Hi Dan,
> >> > > we started talking about it, but then I did not went ahead.
> >> > > As far as I understand, the target is to:
> >> > > - use getpass() when a user types password on tty;
> >> > > - use vpnc_getline() when password is arriving from stdin.
> >> > > Or I'm missing some other useful case?
> >> >
> >> > IIRC the other use-case I was thinking of, which caused me to create the
> >> > flag, was for a command-line program that wanted to wrap vpnc and
> >> > deliver config via stdin, but still allow the user to type the password
> >> > into the TTY. GUI programs obviously wouldn't do this, and my use-cases
> >> > (NetworkManager-vpnc) would do that, but I thought it might be useful.
> >> >
> >> > I also didn't want to change any existing behavior that other users
> >> > might depend on.
> >> >
> >> > However, I agree that just using isatty() would be simpler, so if you
> >> > prefer that I'm happy to do that instead, and a config option could
> >> > always be added later. What do you think?
> >>
> >>
> >> I still think that [1] provides a superset solution for this and I
> >> already use it to feed vpnc from graphical UI in my case it is
> >> kdialog.
> >
> > I didn't take this type of approach because it requires another process.
> > In my case there's already a process controlling vpnc's lifetime and
> > writing configuration to stdin, and that process knows more details
> > about everything required. With your patch, I would require a third
> > process that would have to communicate back to the first process to get
> > additional details, which seems sub-optimal.
> >
> > While not really suitable for my uses, I certainly see how your approach
> > is valid for other use-cases and I have no problem with both approaches
> > co-existing. For example, if "Password program" were not specified,
> > then the isatty() behavior could kick in, otherwise the password program
> > would be spawned.
> >
> > Does that sound OK?
>
> Why won't 'cat' as program will do the trick?

Theoretically it might (though you'd lose the 'prompt' infromation), but
I would rather not spawn yet another process when one already running
and perfectly capable of doing what's required. It's a couple LoC to
replace getpass(), and in my opinion the patch is pretty simple, the
benefit outweights the additional LoC, especially if we use isatty()
instead of the config option as Antonio suggests.

But again, these two mechanisms are perfectly capable of co-existing.

Dan

> >
> > Dan
> >
> >> [1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html
> >>
> >> >
> >> >
> >> > Dan
> >> >
> >> > > My question is:
> >> > > do we really need to introduce a new command line flag?
> >> > > Can we insted autodetect inside vpnc_getpass():
> >> > > always_stdin = ! isatty(fileno(stdin));
> >> > >
> >> > > Best Regards,
> >> > > Antonio
> >> > >
> >> > > > +
> >> > > > static void config_deobfuscate(int obfuscated, int clear)
> >> > > > {
> >> > > > int ret, len = 0;
> >> > > > @@ -470,6 +495,13 @@ static const struct config_names_s {
> >> > > > "Don't ask anything, exit on missing options",
> >> > > > NULL
> >> > > > }, {
> >> > > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
> >> > > > + "--interactive-stdin",
> >> > > > + "InteractiveStdin",
> >> > > > + NULL,
> >> > > > + "Always look for interactive input on stdin instead of the TTY",
> >> > > > + NULL
> >> > > > + }, {
> >> > > > CONFIG_AUTH_MODE, 1, 1,
> >> > > > "--auth-mode",
> >> > > > "IKE Authmode ",
> >> > > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
> >> > > > switch (i) {
> >> > > > case CONFIG_IPSEC_SECRET:
> >> > > > case CONFIG_XAUTH_PASSWORD:
> >> > > > - s = strdup(getpass(""));
> >> > > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
> >> > > > break;
> >> > > > case CONFIG_IPSEC_GATEWAY:
> >> > > > case CONFIG_IPSEC_ID:
> >> > > > diff --git a/trunk/config.h b/trunk/config.h
> >> > > > index 6fbd231..0460d83 100644
> >> > > > --- a/trunk/config.h
> >> > > > +++ b/trunk/config.h
> >> > > > @@ -59,6 +59,7 @@ enum config_enum {
> >> > > > CONFIG_AUTH_MODE,
> >> > > > CONFIG_CA_FILE,
> >> > > > CONFIG_CA_DIR,
> >> > > > + CONFIG_INTERACTIVE_STDIN,
> >> > > > LAST_CONFIG
> >> > > > };
> >> > > >
> >> > > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
> >> > > >
> >> > > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
> >> > > > extern void do_config(int argc, char **argv);
> >> > > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
> >> > > >
> >> > > > extern void (*logmsg)(int priority, const char *format, ...)
> >> > > > __attribute__ ((__format__ (__printf__, 2, 3)));
> >> > > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
> >> > > > index eaa29fa..9e548c4 100644
> >> > > > --- a/trunk/vpnc.c
> >> > > > +++ b/trunk/vpnc.c
> >> > > > @@ -2308,14 +2308,18 @@ 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);
> >> > > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
> >> > > > free(prompt);
> >> > > >
> >> > > > + if (pass == NULL)
> >> > > > + phase2_fatal(s, "failed to read password", reject);
> >> > > > +
> >> > > > na = new_isakmp_attribute(ap->type, NULL);
> >> > > > na->u.lots.length = strlen(pass);
> >> > > > na->u.lots.data = xallocc(na->u.lots.length);
> >> > > > memcpy(na->u.lots.data, pass, na->u.lots.length);
> >> > > > memset(pass, 0, na->u.lots.length);
> >> > > > + free(pass);
> >> > > > } else {
> >> > > > na = new_isakmp_attribute(ap->type, NULL);
> >> > > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
> >> > > > --
> >> > > > 1.8.1.4
> >> > > >
> >> > > >
> >> > > > _______________________________________________
> >> > > > 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/
> >> >
> >> >
> >> > _______________________________________________
> >> > 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/
> >> _______________________________________________
> >> 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/
> >
> >
> > _______________________________________________
> > 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/
> _______________________________________________
> 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/


_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Dec 13, 2013, 11:06 AM
Post #9 of 20 (7471 views)
Permalink
On Fri, Dec 13, 2013 at 9:03 PM, Dan Williams <dcbw@redhat.com> wrote:
> On Fri, 2013-12-13 at 20:41 +0200, Alon Bar-Lev wrote:
>> On Fri, Dec 13, 2013 at 8:36 PM, Dan Williams <dcbw@redhat.com> wrote:
>> > On Fri, 2013-12-13 at 18:59 +0200, Alon Bar-Lev wrote:
>> >> On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> >
>> >> > On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
>> >> > > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
>> >> > > > getpass() tries to use /dev/tty by default, which requests input directly
>> >> > > > from the user, which prevents a controlling process from writing the input
>> >> > > > to stdin. Add a config option to always listen for input on stdin and
>> >> > > > implement a replacement for getpass() that always reads from stdin. The
>> >> > > > config option performs no TTY operations on stdin (eg, unlike getpass()
>> >> > > > it does not disable echo) so the config option should only if vpnc is
>> >> > > > spawned from another program and pipes are used for communication.
>> >> > > > ---
>> >> > > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
>> >> > > > trunk/config.h | 2 ++
>> >> > > > trunk/vpnc.c | 6 +++++-
>> >> > > > 3 files changed, 40 insertions(+), 2 deletions(-)
>> >> > > >
>> >> > > > diff --git a/trunk/config.c b/trunk/config.c
>> >> > > > index e02a66f..fa7798b 100644
>> >> > > > --- a/trunk/config.c
>> >> > > > +++ b/trunk/config.c
>> >> > > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
>> >> > > > return buf;
>> >> > > > }
>> >> > > >
>> >> > > > +char *vpnc_getpass (const char *prompt, int always_stdin)
>> >> > > > +{
>> >> > > > + char *buf = NULL;
>> >> > > > + size_t llen = 0;
>> >> > > > +
>> >> > > > + /* Standard getpass() tries to open the TTY first so if the caller
>> >> > > > + * always wants to use stdin, roll our own naive implementation.
>> >> > > > + */
>> >> > > > +
>> >> > > > + if (!always_stdin) {
>> >> > > > + buf = getpass(prompt);
>> >> > > > + return buf ? strdup (buf) : NULL;
>> >> > > > + }
>> >> > > > +
>> >> > > > + fprintf(stderr, "%s", prompt);
>> >> > > > + setbuf(stdin, NULL);
>> >> > > > + buf = vpnc_getline(&llen, stdin);
>> >> > > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
>> >> > > > + /* Remove the newline. */
>> >> > > > + buf[llen - 1] = '\0';
>> >> > > > + }
>> >> > > > +
>> >> > > > + return buf;
>> >> > > > +}
>> >> > >
>> >> > > Hi Dan,
>> >> > > we started talking about it, but then I did not went ahead.
>> >> > > As far as I understand, the target is to:
>> >> > > - use getpass() when a user types password on tty;
>> >> > > - use vpnc_getline() when password is arriving from stdin.
>> >> > > Or I'm missing some other useful case?
>> >> >
>> >> > IIRC the other use-case I was thinking of, which caused me to create the
>> >> > flag, was for a command-line program that wanted to wrap vpnc and
>> >> > deliver config via stdin, but still allow the user to type the password
>> >> > into the TTY. GUI programs obviously wouldn't do this, and my use-cases
>> >> > (NetworkManager-vpnc) would do that, but I thought it might be useful.
>> >> >
>> >> > I also didn't want to change any existing behavior that other users
>> >> > might depend on.
>> >> >
>> >> > However, I agree that just using isatty() would be simpler, so if you
>> >> > prefer that I'm happy to do that instead, and a config option could
>> >> > always be added later. What do you think?
>> >>
>> >>
>> >> I still think that [1] provides a superset solution for this and I
>> >> already use it to feed vpnc from graphical UI in my case it is
>> >> kdialog.
>> >
>> > I didn't take this type of approach because it requires another process.
>> > In my case there's already a process controlling vpnc's lifetime and
>> > writing configuration to stdin, and that process knows more details
>> > about everything required. With your patch, I would require a third
>> > process that would have to communicate back to the first process to get
>> > additional details, which seems sub-optimal.
>> >
>> > While not really suitable for my uses, I certainly see how your approach
>> > is valid for other use-cases and I have no problem with both approaches
>> > co-existing. For example, if "Password program" were not specified,
>> > then the isatty() behavior could kick in, otherwise the password program
>> > would be spawned.
>> >
>> > Does that sound OK?
>>
>> Why won't 'cat' as program will do the trick?
>
> Theoretically it might (though you'd lose the 'prompt' infromation), but
> I would rather not spawn yet another process when one already running
> and perfectly capable of doing what's required. It's a couple LoC to
> replace getpass(), and in my opinion the patch is pretty simple, the
> benefit outweights the additional LoC, especially if we use isatty()
> instead of the config option as Antonio suggests.
>
> But again, these two mechanisms are perfectly capable of co-existing.
>
> Dan

No need for two mechanism to be applied if one is superset of the other.
There is no theory in software only practical solutions, if executing
cat one time solves the issue for you there is no reason to have a
specific solution for your problem.

But up to maintainer to decide.

>
>> >
>> > Dan
>> >
>> >> [1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html
>> >>
>> >> >
>> >> >
>> >> > Dan
>> >> >
>> >> > > My question is:
>> >> > > do we really need to introduce a new command line flag?
>> >> > > Can we insted autodetect inside vpnc_getpass():
>> >> > > always_stdin = ! isatty(fileno(stdin));
>> >> > >
>> >> > > Best Regards,
>> >> > > Antonio
>> >> > >
>> >> > > > +
>> >> > > > static void config_deobfuscate(int obfuscated, int clear)
>> >> > > > {
>> >> > > > int ret, len = 0;
>> >> > > > @@ -470,6 +495,13 @@ static const struct config_names_s {
>> >> > > > "Don't ask anything, exit on missing options",
>> >> > > > NULL
>> >> > > > }, {
>> >> > > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
>> >> > > > + "--interactive-stdin",
>> >> > > > + "InteractiveStdin",
>> >> > > > + NULL,
>> >> > > > + "Always look for interactive input on stdin instead of the TTY",
>> >> > > > + NULL
>> >> > > > + }, {
>> >> > > > CONFIG_AUTH_MODE, 1, 1,
>> >> > > > "--auth-mode",
>> >> > > > "IKE Authmode ",
>> >> > > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
>> >> > > > switch (i) {
>> >> > > > case CONFIG_IPSEC_SECRET:
>> >> > > > case CONFIG_XAUTH_PASSWORD:
>> >> > > > - s = strdup(getpass(""));
>> >> > > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
>> >> > > > break;
>> >> > > > case CONFIG_IPSEC_GATEWAY:
>> >> > > > case CONFIG_IPSEC_ID:
>> >> > > > diff --git a/trunk/config.h b/trunk/config.h
>> >> > > > index 6fbd231..0460d83 100644
>> >> > > > --- a/trunk/config.h
>> >> > > > +++ b/trunk/config.h
>> >> > > > @@ -59,6 +59,7 @@ enum config_enum {
>> >> > > > CONFIG_AUTH_MODE,
>> >> > > > CONFIG_CA_FILE,
>> >> > > > CONFIG_CA_DIR,
>> >> > > > + CONFIG_INTERACTIVE_STDIN,
>> >> > > > LAST_CONFIG
>> >> > > > };
>> >> > > >
>> >> > > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
>> >> > > >
>> >> > > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
>> >> > > > extern void do_config(int argc, char **argv);
>> >> > > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
>> >> > > >
>> >> > > > extern void (*logmsg)(int priority, const char *format, ...)
>> >> > > > __attribute__ ((__format__ (__printf__, 2, 3)));
>> >> > > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
>> >> > > > index eaa29fa..9e548c4 100644
>> >> > > > --- a/trunk/vpnc.c
>> >> > > > +++ b/trunk/vpnc.c
>> >> > > > @@ -2308,14 +2308,18 @@ 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);
>> >> > > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
>> >> > > > free(prompt);
>> >> > > >
>> >> > > > + if (pass == NULL)
>> >> > > > + phase2_fatal(s, "failed to read password", reject);
>> >> > > > +
>> >> > > > na = new_isakmp_attribute(ap->type, NULL);
>> >> > > > na->u.lots.length = strlen(pass);
>> >> > > > na->u.lots.data = xallocc(na->u.lots.length);
>> >> > > > memcpy(na->u.lots.data, pass, na->u.lots.length);
>> >> > > > memset(pass, 0, na->u.lots.length);
>> >> > > > + free(pass);
>> >> > > > } else {
>> >> > > > na = new_isakmp_attribute(ap->type, NULL);
>> >> > > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
>> >> > > > --
>> >> > > > 1.8.1.4
>> >> > > >
>> >> > > >
>> >> > > > _______________________________________________
>> >> > > > 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/
>> >> >
>> >> >
>> >> > _______________________________________________
>> >> > 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/
>> >> _______________________________________________
>> >> 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/
>> >
>> >
>> > _______________________________________________
>> > 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/
>> _______________________________________________
>> 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/
>
>
> _______________________________________________
> 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/
_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
dcbw at redhat
Dec 13, 2013, 12:05 PM
Post #10 of 20 (7489 views)
Permalink
On Fri, 2013-12-13 at 21:06 +0200, Alon Bar-Lev wrote:
> On Fri, Dec 13, 2013 at 9:03 PM, Dan Williams <dcbw@redhat.com> wrote:
> > On Fri, 2013-12-13 at 20:41 +0200, Alon Bar-Lev wrote:
> >> On Fri, Dec 13, 2013 at 8:36 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> > On Fri, 2013-12-13 at 18:59 +0200, Alon Bar-Lev wrote:
> >> >> On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> >> >
> >> >> > On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
> >> >> > > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
> >> >> > > > getpass() tries to use /dev/tty by default, which requests input directly
> >> >> > > > from the user, which prevents a controlling process from writing the input
> >> >> > > > to stdin. Add a config option to always listen for input on stdin and
> >> >> > > > implement a replacement for getpass() that always reads from stdin. The
> >> >> > > > config option performs no TTY operations on stdin (eg, unlike getpass()
> >> >> > > > it does not disable echo) so the config option should only if vpnc is
> >> >> > > > spawned from another program and pipes are used for communication.
> >> >> > > > ---
> >> >> > > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
> >> >> > > > trunk/config.h | 2 ++
> >> >> > > > trunk/vpnc.c | 6 +++++-
> >> >> > > > 3 files changed, 40 insertions(+), 2 deletions(-)
> >> >> > > >
> >> >> > > > diff --git a/trunk/config.c b/trunk/config.c
> >> >> > > > index e02a66f..fa7798b 100644
> >> >> > > > --- a/trunk/config.c
> >> >> > > > +++ b/trunk/config.c
> >> >> > > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
> >> >> > > > return buf;
> >> >> > > > }
> >> >> > > >
> >> >> > > > +char *vpnc_getpass (const char *prompt, int always_stdin)
> >> >> > > > +{
> >> >> > > > + char *buf = NULL;
> >> >> > > > + size_t llen = 0;
> >> >> > > > +
> >> >> > > > + /* Standard getpass() tries to open the TTY first so if the caller
> >> >> > > > + * always wants to use stdin, roll our own naive implementation.
> >> >> > > > + */
> >> >> > > > +
> >> >> > > > + if (!always_stdin) {
> >> >> > > > + buf = getpass(prompt);
> >> >> > > > + return buf ? strdup (buf) : NULL;
> >> >> > > > + }
> >> >> > > > +
> >> >> > > > + fprintf(stderr, "%s", prompt);
> >> >> > > > + setbuf(stdin, NULL);
> >> >> > > > + buf = vpnc_getline(&llen, stdin);
> >> >> > > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
> >> >> > > > + /* Remove the newline. */
> >> >> > > > + buf[llen - 1] = '\0';
> >> >> > > > + }
> >> >> > > > +
> >> >> > > > + return buf;
> >> >> > > > +}
> >> >> > >
> >> >> > > Hi Dan,
> >> >> > > we started talking about it, but then I did not went ahead.
> >> >> > > As far as I understand, the target is to:
> >> >> > > - use getpass() when a user types password on tty;
> >> >> > > - use vpnc_getline() when password is arriving from stdin.
> >> >> > > Or I'm missing some other useful case?
> >> >> >
> >> >> > IIRC the other use-case I was thinking of, which caused me to create the
> >> >> > flag, was for a command-line program that wanted to wrap vpnc and
> >> >> > deliver config via stdin, but still allow the user to type the password
> >> >> > into the TTY. GUI programs obviously wouldn't do this, and my use-cases
> >> >> > (NetworkManager-vpnc) would do that, but I thought it might be useful.
> >> >> >
> >> >> > I also didn't want to change any existing behavior that other users
> >> >> > might depend on.
> >> >> >
> >> >> > However, I agree that just using isatty() would be simpler, so if you
> >> >> > prefer that I'm happy to do that instead, and a config option could
> >> >> > always be added later. What do you think?
> >> >>
> >> >>
> >> >> I still think that [1] provides a superset solution for this and I
> >> >> already use it to feed vpnc from graphical UI in my case it is
> >> >> kdialog.
> >> >
> >> > I didn't take this type of approach because it requires another process.
> >> > In my case there's already a process controlling vpnc's lifetime and
> >> > writing configuration to stdin, and that process knows more details
> >> > about everything required. With your patch, I would require a third
> >> > process that would have to communicate back to the first process to get
> >> > additional details, which seems sub-optimal.
> >> >
> >> > While not really suitable for my uses, I certainly see how your approach
> >> > is valid for other use-cases and I have no problem with both approaches
> >> > co-existing. For example, if "Password program" were not specified,
> >> > then the isatty() behavior could kick in, otherwise the password program
> >> > would be spawned.
> >> >
> >> > Does that sound OK?
> >>
> >> Why won't 'cat' as program will do the trick?
> >
> > Theoretically it might (though you'd lose the 'prompt' infromation), but
> > I would rather not spawn yet another process when one already running
> > and perfectly capable of doing what's required. It's a couple LoC to
> > replace getpass(), and in my opinion the patch is pretty simple, the
> > benefit outweights the additional LoC, especially if we use isatty()
> > instead of the config option as Antonio suggests.
> >
> > But again, these two mechanisms are perfectly capable of co-existing.
> >
> > Dan
>
> No need for two mechanism to be applied if one is superset of the other.
> There is no theory in software only practical solutions, if executing
> cat one time solves the issue for you there is no reason to have a
> specific solution for your problem.

I tried out the patch and modified NetworkManager-vpnc to
use /usr/bin/cat. There are three problems:

1) I'd have to include code to find /usr/bin/cat, /usr/bin/local/cat,
etc. This is not a huge problem.

2) Obviously, 'cat' does not accept arguments, and passing anything like
"Password required for dcbw@1.2.3.4:" causes cat to exit because that is
not a file cat can echo

3) When does 'cat' know to quit, so that vpnc can continue with the
connection?

Yes, I could write a small, custom program like 'cat' that does handle
the prompts, and accepts a special "terminate" character from
NetworkManager-vpnc that would cause it to quit so vpnc could continue.
But that's a lot more work than few more lines in vpnc.

Otherwise, you could modify your patch to do some parsing and terminate
the child process when a \n or \r was received (since responses are not
going to be multi-line), but that's more involved in vpnc.

But writing my own tool, or using cat is still a third unecessary
process that causes (IMHO) more complications than they are worth, for
my use-case at least. I do understand that it works well for your
use-case, and that's great.

I'm happy to rebase my patch on top of your patch if Antonio wants that.

Dan

> But up to maintainer to decide.
>
> >
> >> >
> >> > Dan
> >> >
> >> >> [1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html
> >> >>
> >> >> >
> >> >> >
> >> >> > Dan
> >> >> >
> >> >> > > My question is:
> >> >> > > do we really need to introduce a new command line flag?
> >> >> > > Can we insted autodetect inside vpnc_getpass():
> >> >> > > always_stdin = ! isatty(fileno(stdin));
> >> >> > >
> >> >> > > Best Regards,
> >> >> > > Antonio
> >> >> > >
> >> >> > > > +
> >> >> > > > static void config_deobfuscate(int obfuscated, int clear)
> >> >> > > > {
> >> >> > > > int ret, len = 0;
> >> >> > > > @@ -470,6 +495,13 @@ static const struct config_names_s {
> >> >> > > > "Don't ask anything, exit on missing options",
> >> >> > > > NULL
> >> >> > > > }, {
> >> >> > > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
> >> >> > > > + "--interactive-stdin",
> >> >> > > > + "InteractiveStdin",
> >> >> > > > + NULL,
> >> >> > > > + "Always look for interactive input on stdin instead of the TTY",
> >> >> > > > + NULL
> >> >> > > > + }, {
> >> >> > > > CONFIG_AUTH_MODE, 1, 1,
> >> >> > > > "--auth-mode",
> >> >> > > > "IKE Authmode ",
> >> >> > > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
> >> >> > > > switch (i) {
> >> >> > > > case CONFIG_IPSEC_SECRET:
> >> >> > > > case CONFIG_XAUTH_PASSWORD:
> >> >> > > > - s = strdup(getpass(""));
> >> >> > > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
> >> >> > > > break;
> >> >> > > > case CONFIG_IPSEC_GATEWAY:
> >> >> > > > case CONFIG_IPSEC_ID:
> >> >> > > > diff --git a/trunk/config.h b/trunk/config.h
> >> >> > > > index 6fbd231..0460d83 100644
> >> >> > > > --- a/trunk/config.h
> >> >> > > > +++ b/trunk/config.h
> >> >> > > > @@ -59,6 +59,7 @@ enum config_enum {
> >> >> > > > CONFIG_AUTH_MODE,
> >> >> > > > CONFIG_CA_FILE,
> >> >> > > > CONFIG_CA_DIR,
> >> >> > > > + CONFIG_INTERACTIVE_STDIN,
> >> >> > > > LAST_CONFIG
> >> >> > > > };
> >> >> > > >
> >> >> > > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
> >> >> > > >
> >> >> > > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
> >> >> > > > extern void do_config(int argc, char **argv);
> >> >> > > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
> >> >> > > >
> >> >> > > > extern void (*logmsg)(int priority, const char *format, ...)
> >> >> > > > __attribute__ ((__format__ (__printf__, 2, 3)));
> >> >> > > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
> >> >> > > > index eaa29fa..9e548c4 100644
> >> >> > > > --- a/trunk/vpnc.c
> >> >> > > > +++ b/trunk/vpnc.c
> >> >> > > > @@ -2308,14 +2308,18 @@ 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);
> >> >> > > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
> >> >> > > > free(prompt);
> >> >> > > >
> >> >> > > > + if (pass == NULL)
> >> >> > > > + phase2_fatal(s, "failed to read password", reject);
> >> >> > > > +
> >> >> > > > na = new_isakmp_attribute(ap->type, NULL);
> >> >> > > > na->u.lots.length = strlen(pass);
> >> >> > > > na->u.lots.data = xallocc(na->u.lots.length);
> >> >> > > > memcpy(na->u.lots.data, pass, na->u.lots.length);
> >> >> > > > memset(pass, 0, na->u.lots.length);
> >> >> > > > + free(pass);
> >> >> > > > } else {
> >> >> > > > na = new_isakmp_attribute(ap->type, NULL);
> >> >> > > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
> >> >> > > > --
> >> >> > > > 1.8.1.4
> >> >> > > >
> >> >> > > >
> >> >> > > > _______________________________________________
> >> >> > > > 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/
> >> >> >
> >> >> >
> >> >> > _______________________________________________
> >> >> > 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/
> >> >> _______________________________________________
> >> >> 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/
> >> >
> >> >
> >> > _______________________________________________
> >> > 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/
> >> _______________________________________________
> >> 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/
> >
> >
> > _______________________________________________
> > 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/
> _______________________________________________
> 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/


_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Dec 13, 2013, 12:11 PM
Post #11 of 20 (7491 views)
Permalink
On Fri, Dec 13, 2013 at 10:05 PM, Dan Williams <dcbw@redhat.com> wrote:
> On Fri, 2013-12-13 at 21:06 +0200, Alon Bar-Lev wrote:
>> On Fri, Dec 13, 2013 at 9:03 PM, Dan Williams <dcbw@redhat.com> wrote:
>> > On Fri, 2013-12-13 at 20:41 +0200, Alon Bar-Lev wrote:
>> >> On Fri, Dec 13, 2013 at 8:36 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> > On Fri, 2013-12-13 at 18:59 +0200, Alon Bar-Lev wrote:
>> >> >> On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> >> >
>> >> >> > On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
>> >> >> > > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
>> >> >> > > > getpass() tries to use /dev/tty by default, which requests input directly
>> >> >> > > > from the user, which prevents a controlling process from writing the input
>> >> >> > > > to stdin. Add a config option to always listen for input on stdin and
>> >> >> > > > implement a replacement for getpass() that always reads from stdin. The
>> >> >> > > > config option performs no TTY operations on stdin (eg, unlike getpass()
>> >> >> > > > it does not disable echo) so the config option should only if vpnc is
>> >> >> > > > spawned from another program and pipes are used for communication.
>> >> >> > > > ---
>> >> >> > > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
>> >> >> > > > trunk/config.h | 2 ++
>> >> >> > > > trunk/vpnc.c | 6 +++++-
>> >> >> > > > 3 files changed, 40 insertions(+), 2 deletions(-)
>> >> >> > > >
>> >> >> > > > diff --git a/trunk/config.c b/trunk/config.c
>> >> >> > > > index e02a66f..fa7798b 100644
>> >> >> > > > --- a/trunk/config.c
>> >> >> > > > +++ b/trunk/config.c
>> >> >> > > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
>> >> >> > > > return buf;
>> >> >> > > > }
>> >> >> > > >
>> >> >> > > > +char *vpnc_getpass (const char *prompt, int always_stdin)
>> >> >> > > > +{
>> >> >> > > > + char *buf = NULL;
>> >> >> > > > + size_t llen = 0;
>> >> >> > > > +
>> >> >> > > > + /* Standard getpass() tries to open the TTY first so if the caller
>> >> >> > > > + * always wants to use stdin, roll our own naive implementation.
>> >> >> > > > + */
>> >> >> > > > +
>> >> >> > > > + if (!always_stdin) {
>> >> >> > > > + buf = getpass(prompt);
>> >> >> > > > + return buf ? strdup (buf) : NULL;
>> >> >> > > > + }
>> >> >> > > > +
>> >> >> > > > + fprintf(stderr, "%s", prompt);
>> >> >> > > > + setbuf(stdin, NULL);
>> >> >> > > > + buf = vpnc_getline(&llen, stdin);
>> >> >> > > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
>> >> >> > > > + /* Remove the newline. */
>> >> >> > > > + buf[llen - 1] = '\0';
>> >> >> > > > + }
>> >> >> > > > +
>> >> >> > > > + return buf;
>> >> >> > > > +}
>> >> >> > >
>> >> >> > > Hi Dan,
>> >> >> > > we started talking about it, but then I did not went ahead.
>> >> >> > > As far as I understand, the target is to:
>> >> >> > > - use getpass() when a user types password on tty;
>> >> >> > > - use vpnc_getline() when password is arriving from stdin.
>> >> >> > > Or I'm missing some other useful case?
>> >> >> >
>> >> >> > IIRC the other use-case I was thinking of, which caused me to create the
>> >> >> > flag, was for a command-line program that wanted to wrap vpnc and
>> >> >> > deliver config via stdin, but still allow the user to type the password
>> >> >> > into the TTY. GUI programs obviously wouldn't do this, and my use-cases
>> >> >> > (NetworkManager-vpnc) would do that, but I thought it might be useful.
>> >> >> >
>> >> >> > I also didn't want to change any existing behavior that other users
>> >> >> > might depend on.
>> >> >> >
>> >> >> > However, I agree that just using isatty() would be simpler, so if you
>> >> >> > prefer that I'm happy to do that instead, and a config option could
>> >> >> > always be added later. What do you think?
>> >> >>
>> >> >>
>> >> >> I still think that [1] provides a superset solution for this and I
>> >> >> already use it to feed vpnc from graphical UI in my case it is
>> >> >> kdialog.
>> >> >
>> >> > I didn't take this type of approach because it requires another process.
>> >> > In my case there's already a process controlling vpnc's lifetime and
>> >> > writing configuration to stdin, and that process knows more details
>> >> > about everything required. With your patch, I would require a third
>> >> > process that would have to communicate back to the first process to get
>> >> > additional details, which seems sub-optimal.
>> >> >
>> >> > While not really suitable for my uses, I certainly see how your approach
>> >> > is valid for other use-cases and I have no problem with both approaches
>> >> > co-existing. For example, if "Password program" were not specified,
>> >> > then the isatty() behavior could kick in, otherwise the password program
>> >> > would be spawned.
>> >> >
>> >> > Does that sound OK?
>> >>
>> >> Why won't 'cat' as program will do the trick?
>> >
>> > Theoretically it might (though you'd lose the 'prompt' infromation), but
>> > I would rather not spawn yet another process when one already running
>> > and perfectly capable of doing what's required. It's a couple LoC to
>> > replace getpass(), and in my opinion the patch is pretty simple, the
>> > benefit outweights the additional LoC, especially if we use isatty()
>> > instead of the config option as Antonio suggests.
>> >
>> > But again, these two mechanisms are perfectly capable of co-existing.
>> >
>> > Dan
>>
>> No need for two mechanism to be applied if one is superset of the other.
>> There is no theory in software only practical solutions, if executing
>> cat one time solves the issue for you there is no reason to have a
>> specific solution for your problem.
>
> I tried out the patch and modified NetworkManager-vpnc to
> use /usr/bin/cat. There are three problems:
>
> 1) I'd have to include code to find /usr/bin/cat, /usr/bin/local/cat,
> etc. This is not a huge problem.
>
> 2) Obviously, 'cat' does not accept arguments, and passing anything like
> "Password required for dcbw@1.2.3.4:" causes cat to exit because that is
> not a file cat can echo
>
> 3) When does 'cat' know to quit, so that vpnc can continue with the
> connection?
>
> Yes, I could write a small, custom program like 'cat' that does handle
> the prompts, and accepts a special "terminate" character from
> NetworkManager-vpnc that would cause it to quit so vpnc could continue.
> But that's a lot more work than few more lines in vpnc.
>
> Otherwise, you could modify your patch to do some parsing and terminate
> the child process when a \n or \r was received (since responses are not
> going to be multi-line), but that's more involved in vpnc.
>
> But writing my own tool, or using cat is still a third unecessary
> process that causes (IMHO) more complications than they are worth, for
> my use-case at least. I do understand that it works well for your
> use-case, and that's great.

Do you mean write a complete program like:
---
#!/bin/sh
read line
echo "${line}"
---

OK, too complex.

Thanks!

>
> I'm happy to rebase my patch on top of your patch if Antonio wants that.
>
> Dan
>
>> But up to maintainer to decide.
>>
>> >
>> >> >
>> >> > Dan
>> >> >
>> >> >> [1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html
>> >> >>
>> >> >> >
>> >> >> >
>> >> >> > Dan
>> >> >> >
>> >> >> > > My question is:
>> >> >> > > do we really need to introduce a new command line flag?
>> >> >> > > Can we insted autodetect inside vpnc_getpass():
>> >> >> > > always_stdin = ! isatty(fileno(stdin));
>> >> >> > >
>> >> >> > > Best Regards,
>> >> >> > > Antonio
>> >> >> > >
>> >> >> > > > +
>> >> >> > > > static void config_deobfuscate(int obfuscated, int clear)
>> >> >> > > > {
>> >> >> > > > int ret, len = 0;
>> >> >> > > > @@ -470,6 +495,13 @@ static const struct config_names_s {
>> >> >> > > > "Don't ask anything, exit on missing options",
>> >> >> > > > NULL
>> >> >> > > > }, {
>> >> >> > > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
>> >> >> > > > + "--interactive-stdin",
>> >> >> > > > + "InteractiveStdin",
>> >> >> > > > + NULL,
>> >> >> > > > + "Always look for interactive input on stdin instead of the TTY",
>> >> >> > > > + NULL
>> >> >> > > > + }, {
>> >> >> > > > CONFIG_AUTH_MODE, 1, 1,
>> >> >> > > > "--auth-mode",
>> >> >> > > > "IKE Authmode ",
>> >> >> > > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
>> >> >> > > > switch (i) {
>> >> >> > > > case CONFIG_IPSEC_SECRET:
>> >> >> > > > case CONFIG_XAUTH_PASSWORD:
>> >> >> > > > - s = strdup(getpass(""));
>> >> >> > > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
>> >> >> > > > break;
>> >> >> > > > case CONFIG_IPSEC_GATEWAY:
>> >> >> > > > case CONFIG_IPSEC_ID:
>> >> >> > > > diff --git a/trunk/config.h b/trunk/config.h
>> >> >> > > > index 6fbd231..0460d83 100644
>> >> >> > > > --- a/trunk/config.h
>> >> >> > > > +++ b/trunk/config.h
>> >> >> > > > @@ -59,6 +59,7 @@ enum config_enum {
>> >> >> > > > CONFIG_AUTH_MODE,
>> >> >> > > > CONFIG_CA_FILE,
>> >> >> > > > CONFIG_CA_DIR,
>> >> >> > > > + CONFIG_INTERACTIVE_STDIN,
>> >> >> > > > LAST_CONFIG
>> >> >> > > > };
>> >> >> > > >
>> >> >> > > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
>> >> >> > > >
>> >> >> > > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
>> >> >> > > > extern void do_config(int argc, char **argv);
>> >> >> > > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
>> >> >> > > >
>> >> >> > > > extern void (*logmsg)(int priority, const char *format, ...)
>> >> >> > > > __attribute__ ((__format__ (__printf__, 2, 3)));
>> >> >> > > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
>> >> >> > > > index eaa29fa..9e548c4 100644
>> >> >> > > > --- a/trunk/vpnc.c
>> >> >> > > > +++ b/trunk/vpnc.c
>> >> >> > > > @@ -2308,14 +2308,18 @@ 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);
>> >> >> > > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
>> >> >> > > > free(prompt);
>> >> >> > > >
>> >> >> > > > + if (pass == NULL)
>> >> >> > > > + phase2_fatal(s, "failed to read password", reject);
>> >> >> > > > +
>> >> >> > > > na = new_isakmp_attribute(ap->type, NULL);
>> >> >> > > > na->u.lots.length = strlen(pass);
>> >> >> > > > na->u.lots.data = xallocc(na->u.lots.length);
>> >> >> > > > memcpy(na->u.lots.data, pass, na->u.lots.length);
>> >> >> > > > memset(pass, 0, na->u.lots.length);
>> >> >> > > > + free(pass);
>> >> >> > > > } else {
>> >> >> > > > na = new_isakmp_attribute(ap->type, NULL);
>> >> >> > > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
>> >> >> > > > --
>> >> >> > > > 1.8.1.4
>> >> >> > > >
>> >> >> > > >
>> >> >> > > > _______________________________________________
>> >> >> > > > 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/
>> >> >> >
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > 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/
>> >> >> _______________________________________________
>> >> >> 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/
>> >> >
>> >> >
>> >> > _______________________________________________
>> >> > 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/
>> >> _______________________________________________
>> >> 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/
>> >
>> >
>> > _______________________________________________
>> > 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/
>> _______________________________________________
>> 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/
>
>
> _______________________________________________
> 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/
_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
dcbw at redhat
Dec 13, 2013, 1:58 PM
Post #12 of 20 (7483 views)
Permalink
On Fri, 2013-12-13 at 22:11 +0200, Alon Bar-Lev wrote:
> On Fri, Dec 13, 2013 at 10:05 PM, Dan Williams <dcbw@redhat.com> wrote:
> > On Fri, 2013-12-13 at 21:06 +0200, Alon Bar-Lev wrote:
> >> On Fri, Dec 13, 2013 at 9:03 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> > On Fri, 2013-12-13 at 20:41 +0200, Alon Bar-Lev wrote:
> >> >> On Fri, Dec 13, 2013 at 8:36 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> >> > On Fri, 2013-12-13 at 18:59 +0200, Alon Bar-Lev wrote:
> >> >> >> On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> >> >> >
> >> >> >> > On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
> >> >> >> > > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
> >> >> >> > > > getpass() tries to use /dev/tty by default, which requests input directly
> >> >> >> > > > from the user, which prevents a controlling process from writing the input
> >> >> >> > > > to stdin. Add a config option to always listen for input on stdin and
> >> >> >> > > > implement a replacement for getpass() that always reads from stdin. The
> >> >> >> > > > config option performs no TTY operations on stdin (eg, unlike getpass()
> >> >> >> > > > it does not disable echo) so the config option should only if vpnc is
> >> >> >> > > > spawned from another program and pipes are used for communication.
> >> >> >> > > > ---
> >> >> >> > > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
> >> >> >> > > > trunk/config.h | 2 ++
> >> >> >> > > > trunk/vpnc.c | 6 +++++-
> >> >> >> > > > 3 files changed, 40 insertions(+), 2 deletions(-)
> >> >> >> > > >
> >> >> >> > > > diff --git a/trunk/config.c b/trunk/config.c
> >> >> >> > > > index e02a66f..fa7798b 100644
> >> >> >> > > > --- a/trunk/config.c
> >> >> >> > > > +++ b/trunk/config.c
> >> >> >> > > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
> >> >> >> > > > return buf;
> >> >> >> > > > }
> >> >> >> > > >
> >> >> >> > > > +char *vpnc_getpass (const char *prompt, int always_stdin)
> >> >> >> > > > +{
> >> >> >> > > > + char *buf = NULL;
> >> >> >> > > > + size_t llen = 0;
> >> >> >> > > > +
> >> >> >> > > > + /* Standard getpass() tries to open the TTY first so if the caller
> >> >> >> > > > + * always wants to use stdin, roll our own naive implementation.
> >> >> >> > > > + */
> >> >> >> > > > +
> >> >> >> > > > + if (!always_stdin) {
> >> >> >> > > > + buf = getpass(prompt);
> >> >> >> > > > + return buf ? strdup (buf) : NULL;
> >> >> >> > > > + }
> >> >> >> > > > +
> >> >> >> > > > + fprintf(stderr, "%s", prompt);
> >> >> >> > > > + setbuf(stdin, NULL);
> >> >> >> > > > + buf = vpnc_getline(&llen, stdin);
> >> >> >> > > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
> >> >> >> > > > + /* Remove the newline. */
> >> >> >> > > > + buf[llen - 1] = '\0';
> >> >> >> > > > + }
> >> >> >> > > > +
> >> >> >> > > > + return buf;
> >> >> >> > > > +}
> >> >> >> > >
> >> >> >> > > Hi Dan,
> >> >> >> > > we started talking about it, but then I did not went ahead.
> >> >> >> > > As far as I understand, the target is to:
> >> >> >> > > - use getpass() when a user types password on tty;
> >> >> >> > > - use vpnc_getline() when password is arriving from stdin.
> >> >> >> > > Or I'm missing some other useful case?
> >> >> >> >
> >> >> >> > IIRC the other use-case I was thinking of, which caused me to create the
> >> >> >> > flag, was for a command-line program that wanted to wrap vpnc and
> >> >> >> > deliver config via stdin, but still allow the user to type the password
> >> >> >> > into the TTY. GUI programs obviously wouldn't do this, and my use-cases
> >> >> >> > (NetworkManager-vpnc) would do that, but I thought it might be useful.
> >> >> >> >
> >> >> >> > I also didn't want to change any existing behavior that other users
> >> >> >> > might depend on.
> >> >> >> >
> >> >> >> > However, I agree that just using isatty() would be simpler, so if you
> >> >> >> > prefer that I'm happy to do that instead, and a config option could
> >> >> >> > always be added later. What do you think?
> >> >> >>
> >> >> >>
> >> >> >> I still think that [1] provides a superset solution for this and I
> >> >> >> already use it to feed vpnc from graphical UI in my case it is
> >> >> >> kdialog.
> >> >> >
> >> >> > I didn't take this type of approach because it requires another process.
> >> >> > In my case there's already a process controlling vpnc's lifetime and
> >> >> > writing configuration to stdin, and that process knows more details
> >> >> > about everything required. With your patch, I would require a third
> >> >> > process that would have to communicate back to the first process to get
> >> >> > additional details, which seems sub-optimal.
> >> >> >
> >> >> > While not really suitable for my uses, I certainly see how your approach
> >> >> > is valid for other use-cases and I have no problem with both approaches
> >> >> > co-existing. For example, if "Password program" were not specified,
> >> >> > then the isatty() behavior could kick in, otherwise the password program
> >> >> > would be spawned.
> >> >> >
> >> >> > Does that sound OK?
> >> >>
> >> >> Why won't 'cat' as program will do the trick?
> >> >
> >> > Theoretically it might (though you'd lose the 'prompt' infromation), but
> >> > I would rather not spawn yet another process when one already running
> >> > and perfectly capable of doing what's required. It's a couple LoC to
> >> > replace getpass(), and in my opinion the patch is pretty simple, the
> >> > benefit outweights the additional LoC, especially if we use isatty()
> >> > instead of the config option as Antonio suggests.
> >> >
> >> > But again, these two mechanisms are perfectly capable of co-existing.
> >> >
> >> > Dan
> >>
> >> No need for two mechanism to be applied if one is superset of the other.
> >> There is no theory in software only practical solutions, if executing
> >> cat one time solves the issue for you there is no reason to have a
> >> specific solution for your problem.
> >
> > I tried out the patch and modified NetworkManager-vpnc to
> > use /usr/bin/cat. There are three problems:
> >
> > 1) I'd have to include code to find /usr/bin/cat, /usr/bin/local/cat,
> > etc. This is not a huge problem.
> >
> > 2) Obviously, 'cat' does not accept arguments, and passing anything like
> > "Password required for dcbw@1.2.3.4:" causes cat to exit because that is
> > not a file cat can echo
> >
> > 3) When does 'cat' know to quit, so that vpnc can continue with the
> > connection?
> >
> > Yes, I could write a small, custom program like 'cat' that does handle
> > the prompts, and accepts a special "terminate" character from
> > NetworkManager-vpnc that would cause it to quit so vpnc could continue.
> > But that's a lot more work than few more lines in vpnc.
> >
> > Otherwise, you could modify your patch to do some parsing and terminate
> > the child process when a \n or \r was received (since responses are not
> > going to be multi-line), but that's more involved in vpnc.
> >
> > But writing my own tool, or using cat is still a third unecessary
> > process that causes (IMHO) more complications than they are worth, for
> > my use-case at least. I do understand that it works well for your
> > use-case, and that's great.
>
> Do you mean write a complete program like:
> ---
> #!/bin/sh
> read line
> echo "${line}"
> ---
>
> OK, too complex.

This script ignores the prompt, which is critical information that the
controlling process needs to know. But echoing the prompt in the script
(so that the vpnc-controlling process can read it and eventually present
it to the user) would also echo the prompt back to vpnc over the pipe
since stdout is duped to the pipe. Yes, the child process could write
the prompt to stderr, but it's starting to get a lot less clear what's
going on, who's reading from what, and who's writing to where. There
are now three points of interaction (NM-vpnc to vpnc, vpnc to helper,
helper to NM-vpnc) instead of a single one (NM-vpnc to vpnc).

I don't think the current patch is a complete superset of the patch I
posted, at least without some additional changes. But again, I don't
believe the two approaches are mutually exclusive, and I'm happy to
rebase my patch on top of yours if Antonio wants me to.

Dan

> Thanks!
>
> >
> > I'm happy to rebase my patch on top of your patch if Antonio wants that.
> >
> > Dan
> >
> >> But up to maintainer to decide.
> >>
> >> >
> >> >> >
> >> >> > Dan
> >> >> >
> >> >> >> [1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html
> >> >> >>
> >> >> >> >
> >> >> >> >
> >> >> >> > Dan
> >> >> >> >
> >> >> >> > > My question is:
> >> >> >> > > do we really need to introduce a new command line flag?
> >> >> >> > > Can we insted autodetect inside vpnc_getpass():
> >> >> >> > > always_stdin = ! isatty(fileno(stdin));
> >> >> >> > >
> >> >> >> > > Best Regards,
> >> >> >> > > Antonio
> >> >> >> > >
> >> >> >> > > > +
> >> >> >> > > > static void config_deobfuscate(int obfuscated, int clear)
> >> >> >> > > > {
> >> >> >> > > > int ret, len = 0;
> >> >> >> > > > @@ -470,6 +495,13 @@ static const struct config_names_s {
> >> >> >> > > > "Don't ask anything, exit on missing options",
> >> >> >> > > > NULL
> >> >> >> > > > }, {
> >> >> >> > > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
> >> >> >> > > > + "--interactive-stdin",
> >> >> >> > > > + "InteractiveStdin",
> >> >> >> > > > + NULL,
> >> >> >> > > > + "Always look for interactive input on stdin instead of the TTY",
> >> >> >> > > > + NULL
> >> >> >> > > > + }, {
> >> >> >> > > > CONFIG_AUTH_MODE, 1, 1,
> >> >> >> > > > "--auth-mode",
> >> >> >> > > > "IKE Authmode ",
> >> >> >> > > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
> >> >> >> > > > switch (i) {
> >> >> >> > > > case CONFIG_IPSEC_SECRET:
> >> >> >> > > > case CONFIG_XAUTH_PASSWORD:
> >> >> >> > > > - s = strdup(getpass(""));
> >> >> >> > > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
> >> >> >> > > > break;
> >> >> >> > > > case CONFIG_IPSEC_GATEWAY:
> >> >> >> > > > case CONFIG_IPSEC_ID:
> >> >> >> > > > diff --git a/trunk/config.h b/trunk/config.h
> >> >> >> > > > index 6fbd231..0460d83 100644
> >> >> >> > > > --- a/trunk/config.h
> >> >> >> > > > +++ b/trunk/config.h
> >> >> >> > > > @@ -59,6 +59,7 @@ enum config_enum {
> >> >> >> > > > CONFIG_AUTH_MODE,
> >> >> >> > > > CONFIG_CA_FILE,
> >> >> >> > > > CONFIG_CA_DIR,
> >> >> >> > > > + CONFIG_INTERACTIVE_STDIN,
> >> >> >> > > > LAST_CONFIG
> >> >> >> > > > };
> >> >> >> > > >
> >> >> >> > > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
> >> >> >> > > >
> >> >> >> > > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
> >> >> >> > > > extern void do_config(int argc, char **argv);
> >> >> >> > > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
> >> >> >> > > >
> >> >> >> > > > extern void (*logmsg)(int priority, const char *format, ...)
> >> >> >> > > > __attribute__ ((__format__ (__printf__, 2, 3)));
> >> >> >> > > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
> >> >> >> > > > index eaa29fa..9e548c4 100644
> >> >> >> > > > --- a/trunk/vpnc.c
> >> >> >> > > > +++ b/trunk/vpnc.c
> >> >> >> > > > @@ -2308,14 +2308,18 @@ 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);
> >> >> >> > > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
> >> >> >> > > > free(prompt);
> >> >> >> > > >
> >> >> >> > > > + if (pass == NULL)
> >> >> >> > > > + phase2_fatal(s, "failed to read password", reject);
> >> >> >> > > > +
> >> >> >> > > > na = new_isakmp_attribute(ap->type, NULL);
> >> >> >> > > > na->u.lots.length = strlen(pass);
> >> >> >> > > > na->u.lots.data = xallocc(na->u.lots.length);
> >> >> >> > > > memcpy(na->u.lots.data, pass, na->u.lots.length);
> >> >> >> > > > memset(pass, 0, na->u.lots.length);
> >> >> >> > > > + free(pass);
> >> >> >> > > > } else {
> >> >> >> > > > na = new_isakmp_attribute(ap->type, NULL);
> >> >> >> > > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
> >> >> >> > > > --
> >> >> >> > > > 1.8.1.4
> >> >> >> > > >
> >> >> >> > > >
> >> >> >> > > > _______________________________________________
> >> >> >> > > > 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/
> >> >> >> >
> >> >> >> >
> >> >> >> > _______________________________________________
> >> >> >> > 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/
> >> >> >> _______________________________________________
> >> >> >> 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/
> >> >> >
> >> >> >
> >> >> > _______________________________________________
> >> >> > 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/
> >> >> _______________________________________________
> >> >> 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/
> >> >
> >> >
> >> > _______________________________________________
> >> > 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/
> >> _______________________________________________
> >> 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/
> >
> >
> > _______________________________________________
> > 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/
> _______________________________________________
> 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/


_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Dec 14, 2013, 12:32 AM
Post #13 of 20 (7490 views)
Permalink
On Fri, Dec 13, 2013 at 11:58 PM, Dan Williams <dcbw@redhat.com> wrote:
> On Fri, 2013-12-13 at 22:11 +0200, Alon Bar-Lev wrote:
>> On Fri, Dec 13, 2013 at 10:05 PM, Dan Williams <dcbw@redhat.com> wrote:
>> > On Fri, 2013-12-13 at 21:06 +0200, Alon Bar-Lev wrote:
>> >> On Fri, Dec 13, 2013 at 9:03 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> > On Fri, 2013-12-13 at 20:41 +0200, Alon Bar-Lev wrote:
>> >> >> On Fri, Dec 13, 2013 at 8:36 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> >> > On Fri, 2013-12-13 at 18:59 +0200, Alon Bar-Lev wrote:
>> >> >> >> On Fri, Dec 13, 2013 at 6:46 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> >> >> >
>> >> >> >> > On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
>> >> >> >> > > On Fri, Jul 12, 2013 at 6:22 AM, Dan Williams <dcbw@redhat.com> wrote:
>> >> >> >> > > > getpass() tries to use /dev/tty by default, which requests input directly
>> >> >> >> > > > from the user, which prevents a controlling process from writing the input
>> >> >> >> > > > to stdin. Add a config option to always listen for input on stdin and
>> >> >> >> > > > implement a replacement for getpass() that always reads from stdin. The
>> >> >> >> > > > config option performs no TTY operations on stdin (eg, unlike getpass()
>> >> >> >> > > > it does not disable echo) so the config option should only if vpnc is
>> >> >> >> > > > spawned from another program and pipes are used for communication.
>> >> >> >> > > > ---
>> >> >> >> > > > trunk/config.c | 34 +++++++++++++++++++++++++++++++++-
>> >> >> >> > > > trunk/config.h | 2 ++
>> >> >> >> > > > trunk/vpnc.c | 6 +++++-
>> >> >> >> > > > 3 files changed, 40 insertions(+), 2 deletions(-)
>> >> >> >> > > >
>> >> >> >> > > > diff --git a/trunk/config.c b/trunk/config.c
>> >> >> >> > > > index e02a66f..fa7798b 100644
>> >> >> >> > > > --- a/trunk/config.c
>> >> >> >> > > > +++ b/trunk/config.c
>> >> >> >> > > > @@ -128,6 +128,31 @@ static char *vpnc_getline(size_t *out_size, FILE *stream)
>> >> >> >> > > > return buf;
>> >> >> >> > > > }
>> >> >> >> > > >
>> >> >> >> > > > +char *vpnc_getpass (const char *prompt, int always_stdin)
>> >> >> >> > > > +{
>> >> >> >> > > > + char *buf = NULL;
>> >> >> >> > > > + size_t llen = 0;
>> >> >> >> > > > +
>> >> >> >> > > > + /* Standard getpass() tries to open the TTY first so if the caller
>> >> >> >> > > > + * always wants to use stdin, roll our own naive implementation.
>> >> >> >> > > > + */
>> >> >> >> > > > +
>> >> >> >> > > > + if (!always_stdin) {
>> >> >> >> > > > + buf = getpass(prompt);
>> >> >> >> > > > + return buf ? strdup (buf) : NULL;
>> >> >> >> > > > + }
>> >> >> >> > > > +
>> >> >> >> > > > + fprintf(stderr, "%s", prompt);
>> >> >> >> > > > + setbuf(stdin, NULL);
>> >> >> >> > > > + buf = vpnc_getline(&llen, stdin);
>> >> >> >> > > > + if (buf != NULL && llen > 0 && buf[llen - 1] == '\n') {
>> >> >> >> > > > + /* Remove the newline. */
>> >> >> >> > > > + buf[llen - 1] = '\0';
>> >> >> >> > > > + }
>> >> >> >> > > > +
>> >> >> >> > > > + return buf;
>> >> >> >> > > > +}
>> >> >> >> > >
>> >> >> >> > > Hi Dan,
>> >> >> >> > > we started talking about it, but then I did not went ahead.
>> >> >> >> > > As far as I understand, the target is to:
>> >> >> >> > > - use getpass() when a user types password on tty;
>> >> >> >> > > - use vpnc_getline() when password is arriving from stdin.
>> >> >> >> > > Or I'm missing some other useful case?
>> >> >> >> >
>> >> >> >> > IIRC the other use-case I was thinking of, which caused me to create the
>> >> >> >> > flag, was for a command-line program that wanted to wrap vpnc and
>> >> >> >> > deliver config via stdin, but still allow the user to type the password
>> >> >> >> > into the TTY. GUI programs obviously wouldn't do this, and my use-cases
>> >> >> >> > (NetworkManager-vpnc) would do that, but I thought it might be useful.
>> >> >> >> >
>> >> >> >> > I also didn't want to change any existing behavior that other users
>> >> >> >> > might depend on.
>> >> >> >> >
>> >> >> >> > However, I agree that just using isatty() would be simpler, so if you
>> >> >> >> > prefer that I'm happy to do that instead, and a config option could
>> >> >> >> > always be added later. What do you think?
>> >> >> >>
>> >> >> >>
>> >> >> >> I still think that [1] provides a superset solution for this and I
>> >> >> >> already use it to feed vpnc from graphical UI in my case it is
>> >> >> >> kdialog.
>> >> >> >
>> >> >> > I didn't take this type of approach because it requires another process.
>> >> >> > In my case there's already a process controlling vpnc's lifetime and
>> >> >> > writing configuration to stdin, and that process knows more details
>> >> >> > about everything required. With your patch, I would require a third
>> >> >> > process that would have to communicate back to the first process to get
>> >> >> > additional details, which seems sub-optimal.
>> >> >> >
>> >> >> > While not really suitable for my uses, I certainly see how your approach
>> >> >> > is valid for other use-cases and I have no problem with both approaches
>> >> >> > co-existing. For example, if "Password program" were not specified,
>> >> >> > then the isatty() behavior could kick in, otherwise the password program
>> >> >> > would be spawned.
>> >> >> >
>> >> >> > Does that sound OK?
>> >> >>
>> >> >> Why won't 'cat' as program will do the trick?
>> >> >
>> >> > Theoretically it might (though you'd lose the 'prompt' infromation), but
>> >> > I would rather not spawn yet another process when one already running
>> >> > and perfectly capable of doing what's required. It's a couple LoC to
>> >> > replace getpass(), and in my opinion the patch is pretty simple, the
>> >> > benefit outweights the additional LoC, especially if we use isatty()
>> >> > instead of the config option as Antonio suggests.
>> >> >
>> >> > But again, these two mechanisms are perfectly capable of co-existing.
>> >> >
>> >> > Dan
>> >>
>> >> No need for two mechanism to be applied if one is superset of the other.
>> >> There is no theory in software only practical solutions, if executing
>> >> cat one time solves the issue for you there is no reason to have a
>> >> specific solution for your problem.
>> >
>> > I tried out the patch and modified NetworkManager-vpnc to
>> > use /usr/bin/cat. There are three problems:
>> >
>> > 1) I'd have to include code to find /usr/bin/cat, /usr/bin/local/cat,
>> > etc. This is not a huge problem.
>> >
>> > 2) Obviously, 'cat' does not accept arguments, and passing anything like
>> > "Password required for dcbw@1.2.3.4:" causes cat to exit because that is
>> > not a file cat can echo
>> >
>> > 3) When does 'cat' know to quit, so that vpnc can continue with the
>> > connection?
>> >
>> > Yes, I could write a small, custom program like 'cat' that does handle
>> > the prompts, and accepts a special "terminate" character from
>> > NetworkManager-vpnc that would cause it to quit so vpnc could continue.
>> > But that's a lot more work than few more lines in vpnc.
>> >
>> > Otherwise, you could modify your patch to do some parsing and terminate
>> > the child process when a \n or \r was received (since responses are not
>> > going to be multi-line), but that's more involved in vpnc.
>> >
>> > But writing my own tool, or using cat is still a third unecessary
>> > process that causes (IMHO) more complications than they are worth, for
>> > my use-case at least. I do understand that it works well for your
>> > use-case, and that's great.
>>
>> Do you mean write a complete program like:
>> ---
>> #!/bin/sh
>> read line
>> echo "${line}"
>> ---
>>
>> OK, too complex.
>
> This script ignores the prompt, which is critical information that the
> controlling process needs to know. But echoing the prompt in the script
> (so that the vpnc-controlling process can read it and eventually present
> it to the user) would also echo the prompt back to vpnc over the pipe
> since stdout is duped to the pipe. Yes, the child process could write
> the prompt to stderr, but it's starting to get a lot less clear what's
> going on, who's reading from what, and who's writing to where. There
> are now three points of interaction (NM-vpnc to vpnc, vpnc to helper,
> helper to NM-vpnc) instead of a single one (NM-vpnc to vpnc).
>
> I don't think the current patch is a complete superset of the patch I
> posted, at least without some additional changes. But again, I don't
> believe the two approaches are mutually exclusive, and I'm happy to
> rebase my patch on top of yours if Antonio wants me to.

We can alter the program patch to read password from alternate file
descriptor, say 5, so it can communicate with stdin/stdout/stderr of
the original process. This should solve the issue you raised.

>
> Dan
>
>> Thanks!
>>
>> >
>> > I'm happy to rebase my patch on top of your patch if Antonio wants that.
>> >
>> > Dan
>> >
>> >> But up to maintainer to decide.
>> >>
>> >> >
>> >> >> >
>> >> >> > Dan
>> >> >> >
>> >> >> >> [1] http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-March/003906.html
>> >> >> >>
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > Dan
>> >> >> >> >
>> >> >> >> > > My question is:
>> >> >> >> > > do we really need to introduce a new command line flag?
>> >> >> >> > > Can we insted autodetect inside vpnc_getpass():
>> >> >> >> > > always_stdin = ! isatty(fileno(stdin));
>> >> >> >> > >
>> >> >> >> > > Best Regards,
>> >> >> >> > > Antonio
>> >> >> >> > >
>> >> >> >> > > > +
>> >> >> >> > > > static void config_deobfuscate(int obfuscated, int clear)
>> >> >> >> > > > {
>> >> >> >> > > > int ret, len = 0;
>> >> >> >> > > > @@ -470,6 +495,13 @@ static const struct config_names_s {
>> >> >> >> > > > "Don't ask anything, exit on missing options",
>> >> >> >> > > > NULL
>> >> >> >> > > > }, {
>> >> >> >> > > > + CONFIG_INTERACTIVE_STDIN, 0, 1,
>> >> >> >> > > > + "--interactive-stdin",
>> >> >> >> > > > + "InteractiveStdin",
>> >> >> >> > > > + NULL,
>> >> >> >> > > > + "Always look for interactive input on stdin instead of the TTY",
>> >> >> >> > > > + NULL
>> >> >> >> > > > + }, {
>> >> >> >> > > > CONFIG_AUTH_MODE, 1, 1,
>> >> >> >> > > > "--auth-mode",
>> >> >> >> > > > "IKE Authmode ",
>> >> >> >> > > > @@ -839,7 +871,7 @@ void do_config(int argc, char **argv)
>> >> >> >> > > > switch (i) {
>> >> >> >> > > > case CONFIG_IPSEC_SECRET:
>> >> >> >> > > > case CONFIG_XAUTH_PASSWORD:
>> >> >> >> > > > - s = strdup(getpass(""));
>> >> >> >> > > > + s = vpnc_getpass("", !!config[CONFIG_INTERACTIVE_STDIN]);
>> >> >> >> > > > break;
>> >> >> >> > > > case CONFIG_IPSEC_GATEWAY:
>> >> >> >> > > > case CONFIG_IPSEC_ID:
>> >> >> >> > > > diff --git a/trunk/config.h b/trunk/config.h
>> >> >> >> > > > index 6fbd231..0460d83 100644
>> >> >> >> > > > --- a/trunk/config.h
>> >> >> >> > > > +++ b/trunk/config.h
>> >> >> >> > > > @@ -59,6 +59,7 @@ enum config_enum {
>> >> >> >> > > > CONFIG_AUTH_MODE,
>> >> >> >> > > > CONFIG_CA_FILE,
>> >> >> >> > > > CONFIG_CA_DIR,
>> >> >> >> > > > + CONFIG_INTERACTIVE_STDIN,
>> >> >> >> > > > LAST_CONFIG
>> >> >> >> > > > };
>> >> >> >> > > >
>> >> >> >> > > > @@ -131,6 +132,7 @@ extern uint16_t opt_udpencapport;
>> >> >> >> > > >
>> >> >> >> > > > extern void hex_dump(const char *str, const void *data, ssize_t len, const struct debug_strings *decode);
>> >> >> >> > > > extern void do_config(int argc, char **argv);
>> >> >> >> > > > +extern char *vpnc_getpass(const char *prompt, int always_stdin);
>> >> >> >> > > >
>> >> >> >> > > > extern void (*logmsg)(int priority, const char *format, ...)
>> >> >> >> > > > __attribute__ ((__format__ (__printf__, 2, 3)));
>> >> >> >> > > > diff --git a/trunk/vpnc.c b/trunk/vpnc.c
>> >> >> >> > > > index eaa29fa..9e548c4 100644
>> >> >> >> > > > --- a/trunk/vpnc.c
>> >> >> >> > > > +++ b/trunk/vpnc.c
>> >> >> >> > > > @@ -2308,14 +2308,18 @@ 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);
>> >> >> >> > > > + pass = vpnc_getpass(prompt, !!config[CONFIG_INTERACTIVE_STDIN]);
>> >> >> >> > > > free(prompt);
>> >> >> >> > > >
>> >> >> >> > > > + if (pass == NULL)
>> >> >> >> > > > + phase2_fatal(s, "failed to read password", reject);
>> >> >> >> > > > +
>> >> >> >> > > > na = new_isakmp_attribute(ap->type, NULL);
>> >> >> >> > > > na->u.lots.length = strlen(pass);
>> >> >> >> > > > na->u.lots.data = xallocc(na->u.lots.length);
>> >> >> >> > > > memcpy(na->u.lots.data, pass, na->u.lots.length);
>> >> >> >> > > > memset(pass, 0, na->u.lots.length);
>> >> >> >> > > > + free(pass);
>> >> >> >> > > > } else {
>> >> >> >> > > > na = new_isakmp_attribute(ap->type, NULL);
>> >> >> >> > > > na->u.lots.length = strlen(config[CONFIG_XAUTH_PASSWORD]);
>> >> >> >> > > > --
>> >> >> >> > > > 1.8.1.4
>> >> >> >> > > >
>> >> >> >> > > >
>> >> >> >> > > > _______________________________________________
>> >> >> >> > > > 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/
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > _______________________________________________
>> >> >> >> > 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/
>> >> >> >> _______________________________________________
>> >> >> >> 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/
>> >> >> >
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > 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/
>> >> >> _______________________________________________
>> >> >> 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/
>> >> >
>> >> >
>> >> > _______________________________________________
>> >> > 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/
>> >> _______________________________________________
>> >> 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/
>> >
>> >
>> > _______________________________________________
>> > 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/
>> _______________________________________________
>> 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/
>
>
> _______________________________________________
> 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/
_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
borneo.antonio at gmail
Dec 14, 2013, 7:12 PM
Post #14 of 20 (7495 views)
Permalink
On Sat, Dec 14, 2013 at 12:46 AM, Dan Williams <dcbw@redhat.com> wrote:
> On Sat, 2013-11-30 at 19:29 +0800, Antonio Borneo wrote:
>> Hi Dan,
>> we started talking about it, but then I did not went ahead.
>> As far as I understand, the target is to:
>> - use getpass() when a user types password on tty;
>> - use vpnc_getline() when password is arriving from stdin.
>> Or I'm missing some other useful case?
>
> IIRC the other use-case I was thinking of, which caused me to create the
> flag, was for a command-line program that wanted to wrap vpnc and
> deliver config via stdin, but still allow the user to type the password
> into the TTY. GUI programs obviously wouldn't do this, and my use-cases
> (NetworkManager-vpnc) would do that, but I thought it might be useful.

Hummm... it's me that made confusion ...

While checking your patch I was also checking the code in openconnect,
which has already get rid of obsolete getpass().
Openconnect implementation of getpass() explicitely reads from stdin
after setting no-ECHO.
This is different from original getpass(), that instead reads from /dev/tty.

I incorrectly though that also vpnc always reads from stdin, so I
suggested to detect if stdin is a tty.
Let's forget my proposal.

Going back to your need to input passwords from stdin.
You correctly list the cases:
- standalone vpnc: password through console prompt.
In this case stdin and /dev/tty are same.

- NM-vpnc or other GUI.
/dev/tty is not necesarily accessible to user.
Using stdin is fine.

- command line wrapper
Could be good to have input on /dev/tty, but we have no evidence it
is used today.

> I also didn't want to change any existing behavior that other users
> might depend on.

Correct, but I don't want to add additional command line flags if
nobody use them.
What about simply replacing getpass() with a version that reads from stdin?
It would fix your problem and vpnc would get rid of the obsolete function.
Then, if someone needs to explicitly use /dev/tty, this would trigger
a specific patch with associated command line option.
Comments are welcome.

In attachment a patch that replaces getpass().
Would be possible for you to test it in your use case?

Antonio

Attached Files:

Re: [PATCH 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
borneo.antonio at gmail
Dec 14, 2013, 8:05 PM
Post #15 of 20 (7479 views)
Permalink
Hi Alon,

I want to remove obsolete getpass(), so I'm proposing a replacement
with a version that satisfies also Dan's requirement.
It will avoid a new command line flag if not strictly required by other users.

Your patch adds a different functionality, I don't see need to mix it
with above modification.
What I would prefer is a sequence of smaller patches:
- replace getpass()
- patch that removes static buffer in vpnc_getpass() using malloc(),
check if vpnc_getpass() returns error and then exit.
- patch to call external helper program

Can be used a different flag name instead of "--password-program" ?
I would like to re-use, later on, the exact same flag to get passwords
through other external helpers, e.g. libstoken or liboath.
The word "program" in the flag would not match the future use I plan.
I'm checking the flag "--token-mode" of openconnect, where you can
pass "rsa" for libstoken or "totp" for liboath.
What about "--password-generator" or "--password-helper"?
Today you can just pass the full path of an external executable helper.
Tomorrow, can be passed a keyword, like "rsa" or "totp", to use a library.

Antonio
_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Dec 15, 2013, 1:47 AM
Post #16 of 20 (7485 views)
Permalink
On Sun, Dec 15, 2013 at 6:05 AM, Antonio Borneo
<borneo.antonio@gmail.com> wrote:
> Hi Alon,
>
> I want to remove obsolete getpass(), so I'm proposing a replacement
> with a version that satisfies also Dan's requirement.
> It will avoid a new command line flag if not strictly required by other users.

There is a reason why a password is read directly from tty, it avoids
other programs to be able to intercept it.

Any other behavior should be implemented using 'helper'... just like
in the case we are discussing.

But you decision.

> Can be used a different flag name instead of "--password-program" ?
> I would like to re-use, later on, the exact same flag to get passwords
> through other external helpers, e.g. libstoken or liboath.
> The word "program" in the flag would not match the future use I plan.
> I'm checking the flag "--token-mode" of openconnect, where you can
> pass "rsa" for libstoken or "totp" for liboath.
> What about "--password-generator" or "--password-helper"?
> Today you can just pass the full path of an external executable helper.
> Tomorrow, can be passed a keyword, like "rsa" or "totp", to use a library.

I do not have strong opinion on the actual name, state whatever you like.

From above I understand you want to pass parameter to the program, I
will workout this it, although it should not be that difficult to wrap
any executable in a simple script passing static parameters. The
problem with adding parameters is that I need somehow parse them to
argv[n] within the vpnc code or use the unsafe system().

Regards,
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/
Re: [PATCH 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
borneo.antonio at gmail
Dec 15, 2013, 3:31 AM
Post #17 of 20 (7474 views)
Permalink
On Sun, Dec 15, 2013 at 5:47 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> From above I understand you want to pass parameter to the program, I
> will workout this it, although it should not be that difficult to wrap
> any executable in a simple script passing static parameters. The
> problem with adding parameters is that I need somehow parse them to
> argv[n] within the vpnc code or use the unsafe system().

No, it's not an issue with parameters.
My idea is to specify either a keyword (like "rsa") or a real path
(like "/usr/local/bin/prog_name").
Depending on the value, vpnc should either run an internal function or
execute the external program.
With your patch vpnc will support the external program. Later on would
be easy to add new functionalities.
In such case, the name "--password-program" would not be appropriate.
"--password-generator" ?

Antonio
_______________________________________________
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 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Dec 15, 2013, 3:56 AM
Post #18 of 20 (7484 views)
Permalink
On Sun, Dec 15, 2013 at 1:31 PM, Antonio Borneo
<borneo.antonio@gmail.com> wrote:
> On Sun, Dec 15, 2013 at 5:47 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>> From above I understand you want to pass parameter to the program, I
>> will workout this it, although it should not be that difficult to wrap
>> any executable in a simple script passing static parameters. The
>> problem with adding parameters is that I need somehow parse them to
>> argv[n] within the vpnc code or use the unsafe system().
>
> No, it's not an issue with parameters.
> My idea is to specify either a keyword (like "rsa") or a real path
> (like "/usr/local/bin/prog_name").
> Depending on the value, vpnc should either run an internal function or
> execute the external program.
> With your patch vpnc will support the external program. Later on would
> be easy to add new functionalities.
> In such case, the name "--password-program" would not be appropriate.
> "--password-generator" ?

Hmmm... this is confusing....

I suggest two parameters:

--password-helper-type=
--password-helper=

Or as one:

--password-helper=type:string

Example:

--password-helper=prog:/usr/bin/ssh-askpass

However, can you please explain why you want to add functionality into
vpnc core? why not just delegate the handling of these devices into
external process?

BTW: I already use linotp and rsa securid in this mode... this is why
I wrote this patch.

Regards,
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/
Re: [PATCH 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
borneo.antonio at gmail
Dec 15, 2013, 4:42 AM
Post #19 of 20 (7479 views)
Permalink
On Sun, Dec 15, 2013 at 7:56 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> On Sun, Dec 15, 2013 at 1:31 PM, Antonio Borneo
> <borneo.antonio@gmail.com> wrote:
>> On Sun, Dec 15, 2013 at 5:47 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>>> From above I understand you want to pass parameter to the program, I
>>> will workout this it, although it should not be that difficult to wrap
>>> any executable in a simple script passing static parameters. The
>>> problem with adding parameters is that I need somehow parse them to
>>> argv[n] within the vpnc code or use the unsafe system().
>>
>> No, it's not an issue with parameters.
>> My idea is to specify either a keyword (like "rsa") or a real path
>> (like "/usr/local/bin/prog_name").
>> Depending on the value, vpnc should either run an internal function or
>> execute the external program.
>> With your patch vpnc will support the external program. Later on would
>> be easy to add new functionalities.
>> In such case, the name "--password-program" would not be appropriate.
>> "--password-generator" ?
>
> Hmmm... this is confusing....

Not so much, in my opinion.
--password-helper=/usr/bin/ssh-askpass
to run an external application (full path), or
--password-helper=keyword
to execute an internal helper, if "keyword" is recognized.

> I suggest two parameters:

I'm just trying to avoid proliferation of command line parameters, at
least when I can avoid them.

I do not see immediate needs to integrate any helper in vpnc. I'm much
more in favor of external application; easier to debug and maintain.
But I want keep this option open.

Antonio

> --password-helper-type=
> --password-helper=
>
> Or as one:
>
> --password-helper=type:string
>
> Example:
>
> --password-helper=prog:/usr/bin/ssh-askpass
>
> However, can you please explain why you want to add functionality into
> vpnc core? why not just delegate the handling of these devices into
> external process?
>
> BTW: I already use linotp and rsa securid in this mode... this is why
> I wrote this patch.
>
> Regards,
> 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/
Re: [PATCH 2/2] use custom getpass() implementation if caller wants to use stdin [ In reply to ]
alon.barlev at gmail
Dec 15, 2013, 4:54 AM
Post #20 of 20 (7476 views)
Permalink
On Sun, Dec 15, 2013 at 2:42 PM, Antonio Borneo
<borneo.antonio@gmail.com> wrote:
> On Sun, Dec 15, 2013 at 7:56 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>> On Sun, Dec 15, 2013 at 1:31 PM, Antonio Borneo
>> <borneo.antonio@gmail.com> wrote:
>>> On Sun, Dec 15, 2013 at 5:47 PM, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
>>>> From above I understand you want to pass parameter to the program, I
>>>> will workout this it, although it should not be that difficult to wrap
>>>> any executable in a simple script passing static parameters. The
>>>> problem with adding parameters is that I need somehow parse them to
>>>> argv[n] within the vpnc code or use the unsafe system().
>>>
>>> No, it's not an issue with parameters.
>>> My idea is to specify either a keyword (like "rsa") or a real path
>>> (like "/usr/local/bin/prog_name").
>>> Depending on the value, vpnc should either run an internal function or
>>> execute the external program.
>>> With your patch vpnc will support the external program. Later on would
>>> be easy to add new functionalities.
>>> In such case, the name "--password-program" would not be appropriate.
>>> "--password-generator" ?
>>
>> Hmmm... this is confusing....
>
> Not so much, in my opinion.
> --password-helper=/usr/bin/ssh-askpass
> to run an external application (full path), or
> --password-helper=keyword
> to execute an internal helper, if "keyword" is recognized.
>
>> I suggest two parameters:
>
> I'm just trying to avoid proliferation of command line parameters, at
> least when I can avoid them.
>
> I do not see immediate needs to integrate any helper in vpnc. I'm much
> more in favor of external application; easier to debug and maintain.
> But I want keep this option open.

I resent original patch with rename to password-helper. However, I
usually trying to avoid implicit meaning of parameters... but this is
your project.

BTW: while you are responsive, what about all the other patches I sent
required for running vpnc as non privileged user?

>
> Antonio
>
>> --password-helper-type=
>> --password-helper=
>>
>> Or as one:
>>
>> --password-helper=type:string
>>
>> Example:
>>
>> --password-helper=prog:/usr/bin/ssh-askpass
>>
>> However, can you please explain why you want to add functionality into
>> vpnc core? why not just delegate the handling of these devices into
>> external process?
>>
>> BTW: I already use linotp and rsa securid in this mode... this is why
>> I wrote this patch.
>>
>> Regards,
>> 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/

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal