oops, just took my "debug" version of src/openvpn/ssl.c ...

Of course output of string length and especially password in log file is
not intended ;-)

Joerg

-------- Original-Nachricht --------
Betreff: Re: [Openvpn-devel] [PATCH] Allow inlining of --auth-user-pass
Von: max.mus...@kaffeeschluerfer.com
An: Davide Brini <dave...@gmx.com>
Kopie (CC): openvpn-devel@lists.sourceforge.net
Datum: Sa 05 Okt 2013 15:48:42 CEST

> Hi Davide,
> nice idea. But I think I found two small bugs:
> First, it won't compile in a config w/o ENABLE_CLIENT_CR defined (there is no 
> "sc_info").
> Second, if I'm not mistaken, name and password are not copied correctly:
> If # of charachters is (pos - prev), the last charakter is at [pos - prev -1] 
> and to terminate the string you should set [pos - prev] to '\0'.
> My suggestion (not sure, if the first one works with management enabled, only 
> tested w/o).
> Fix in src/openvpn/misc.c (add #ifdef):
> [...]
> +
> +  if ( ( !streq(options->auth_user_pass_file, "stdin") || 
> options->auth_user_pass_file_inline) && options->auth_nocache)
> +    msg (M_USAGE, "Cannot use --auth-nocache with credentials from file");
> +
> +#ifdef ENABLE_CLIENT_CR
> +  if ( ( !streq(options->auth_user_pass_file, "stdin") || 
> options->auth_user_pass_file_inline) && options->sc_info.challenge_text)
> +    msg (M_USAGE, "Credentials cannot be in a file if using 
> --static-challenge");
> +#endif
> +
> +  if (options->auth_user_pass_file_inline)
> +    {
> +      int n_inlined = 0;
> [...]
> in src/openvpn/ssl.c yo should use [pos - prev] instead of [pos - prev +1]
> [...]
> +                {
> +                  strncpy(inlined_username, prev, pos - prev);
> +                  inlined_username[pos - prev] = '\0';
> +                  msg (M_INFO, "Using inlined username: %s (POS=%d 
> PREV=%d)", 
> inlined_username, pos, prev);
> +                }
> +              else
> +                {
> +                  strncpy(inlined_password, prev, pos - prev);
> +                  inlined_password[pos - prev] = '\0';
> +                  msg (M_INFO, "Using inlined password: 
> ...**%s**",inlined_password);
> +                }
> [...]
> 
> I hope it's o.k. just to give the idea but not post full patches again.
> 
> Regards
> Joerg
> *Gesendet:* Donnerstag, 03. Oktober 2013 um 10:30 Uhr
> *Von:* "Davide Brini" <dave...@gmx.com>
> *An:* openvpn-devel@lists.sourceforge.net
> *Betreff:* Re: [Openvpn-devel] [PATCH] Allow inlining of --auth-user-pass
> (previous version contained a leftover debug line left by mistake, sorry)
> 
> This patch allows inlining of the --auth-user-pass directive, so it is now
> possible to do
> 
> <auth-user-pass>
> myusername
> mypassword
> </auth-user-pass>
> 
> or supply just the username, eg
> 
> <auth-user-pass>
> myusername
> </auth-user-pass>
> 
> (in this case the user is prompted for the password only).
> The most changed files are options.c (sanity check of inlined credentials)
> and ssl.c (actual parsing of the inlined credentials).
> 
> Udates to the documentation will be provided in a separate patch if and when
> the present patch is accepted.
> 
> As discussed on IRC, for the time being the non-inlined syntax
> 
> auth-user-pass [up]
> 
> is still supported and [up] is expected to contain username and password
> on two lines.
> 
> Signed-off-by: Davide Brini <dave...@gmx.com>
> ---
> src/openvpn/init.c | 6 +--
> src/openvpn/misc.c | 2 +-
> src/openvpn/options.c | 56 ++++++++++++++++++++++++----
> src/openvpn/options.h | 2 +
> src/openvpn/ssl.c | 98 +++++++++++++++++++++++++++++++++++++------------
> src/openvpn/ssl.h | 2 +-
> 6 files changed, 131 insertions(+), 35 deletions(-)
> 
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 031fb20..b11ae8a 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -400,12 +400,12 @@ init_query_passwords (struct context *c)
> 
> #if P2MP
> /* Auth user/pass input */
> - if (c->options.auth_user_pass_file)
> + if (c->options.auth_user_pass_file || c->options.auth_user_pass_file_inline)
> {
> #ifdef ENABLE_CLIENT_CR
> - auth_user_pass_setup (c->options.auth_user_pass_file, &c->options.sc_info);
> + auth_user_pass_setup (c->options.auth_user_pass_file, 
> c->options.auth_user_pass_file_inline, &c->options.sc_info);
> #else
> - auth_user_pass_setup (c->options.auth_user_pass_file, NULL);
> + auth_user_pass_setup (c->options.auth_user_pass_file, 
> c->options.auth_user_pass_file_inline, NULL);
> #endif
> }
> #endif
> diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
> index 4688444..05bc2f0 100644
> --- a/src/openvpn/misc.c
> +++ b/src/openvpn/misc.c
> @@ -1036,7 +1036,7 @@ get_user_pass_cr (struct user_pass *up,
> 
> if (!up->defined)
> {
> - const bool from_stdin = (!auth_file || !strcmp (auth_file, "stdin"));
> + const bool from_stdin = (!auth_file || streq (auth_file, "stdin"));
> 
> if (flags & GET_USER_PASS_PREVIOUS_CREDS_FAILED)
> msg (M_WARN, "Note: previous '%s' credentials failed", prefix);
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 4d0271f..6f2ea66 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -1233,6 +1233,7 @@ show_p2mp_parms (const struct options *o)
> SHOW_BOOL (client);
> SHOW_BOOL (pull);
> SHOW_STR (auth_user_pass_file);
> + SHOW_STR (auth_user_pass_file_inline);
> 
> gc_free (&gc);
> }
> @@ -2080,7 +2081,7 @@ options_postprocess_verify_ce (const struct options 
> *options, const struct conne
> if (options->ifconfig_ipv6_local && !options->tun_ipv6 )
> msg (M_INFO, "Warning: --ifconfig-ipv6 without --tun-ipv6 will not do IPv6");
> 
> - if (options->auth_user_pass_file)
> + if ( options->auth_user_pass_file || options->auth_user_pass_file_inline )
> msg (M_USAGE, "--auth-user-pass cannot be used with --mode server (it should 
> be 
> used on the client side only)");
> if (options->ccd_exclusive && !options->client_config_dir)
> msg (M_USAGE, "--ccd-exclusive must be used with --client-config-dir");
> @@ -2274,7 +2275,7 @@ options_postprocess_verify_ce (const struct options 
> *options, const struct conne
> if (sum == 0)
> {
> #if P2MP
> - if (!options->auth_user_pass_file)
> + if (!options->auth_user_pass_file && !options->auth_user_pass_file_inline)
> #endif
> msg (M_USAGE, "No client-side authentication method is specified. You must 
> use 
> either --cert/--key, --pkcs12, or --auth-user-pass");
> }
> @@ -2350,8 +2351,42 @@ options_postprocess_verify_ce (const struct options 
> *options, const struct conne
> #endif /* ENABLE_SSL */
> 
> #if P2MP
> - if (options->auth_user_pass_file && !options->pull)
> + if ( ( options->auth_user_pass_file || options->auth_user_pass_file_inline) 
> && 
> !options->pull)
> msg (M_USAGE, "--auth-user-pass requires --pull");
> +
> + if ( ( !streq(options->auth_user_pass_file, "stdin") || 
> options->auth_user_pass_file_inline) && options->auth_nocache)
> + msg (M_USAGE, "Cannot use --auth-nocache with credentials from file");
> +
> + if ( ( !streq(options->auth_user_pass_file, "stdin") || 
> options->auth_user_pass_file_inline) && options->sc_info.challenge_text)
> + msg (M_USAGE, "Credentials cannot be in a file if using 
> --static-challenge");
> +
> + if (options->auth_user_pass_file_inline)
> + {
> + int n_inlined = 0;
> + const char *pos = options->auth_user_pass_file_inline;
> + const char *prev = pos;
> +
> + if ( strlen(pos) == 0 )
> + msg (M_USAGE, "Invalid format for inlined --auth-user-pass");
> +
> + while( (pos = strchr(pos, '\n')) != NULL )
> + {
> + n_inlined++;
> +
> + if (n_inlined > 2)
> + msg (M_USAGE, "Too many lines in inlined --auth-user-pass");
> +
> + if ( pos - prev > USER_PASS_LEN - 1 )
> + msg (M_USAGE, "Line too long in inlined --auth-user-pass");
> +
> + pos++;
> + prev = pos;
> + }
> +
> + if ( (n_inlined == 0) || (*prev != '\0') )
> + msg (M_USAGE, "Invalid format for inlined --auth-user-pass");
> +
> + }
> #endif
> 
> uninit_options (&defaults);
> @@ -2719,9 +2754,10 @@ options_postprocess_filechecks (struct options 
> *options)
> "--management user/password file");
> #endif /* ENABLE_MANAGEMENT */
> #if P2MP
> - errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN,
> - options->auth_user_pass_file, R_OK,
> - "--auth-user-pass");
> + if (!streq(options->auth_user_pass_file, INLINE_FILE_TAG))
> + errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN,
> + options->auth_user_pass_file, R_OK,
> + "--auth-user-pass");
> #endif /* P2MP */
> 
> /* ** System related ** */
> @@ -5894,7 +5930,12 @@ add_option (struct options *options,
> VERIFY_PERMISSION (OPT_P_GENERAL);
> if (p[1])
> {
> - options->auth_user_pass_file = p[1];
> + options->auth_user_pass_file = p[1];
> + if (streq (p[1], INLINE_FILE_TAG) && p[2])
> + {
> + options->auth_user_pass_file = "stdin";
> + options->auth_user_pass_file_inline = p[2];
> + }
> }
> else
> options->auth_user_pass_file = "stdin";
> @@ -6591,6 +6632,7 @@ add_option (struct options *options,
> else if (streq (p[0], "auth-nocache"))
> {
> VERIFY_PERMISSION (OPT_P_GENERAL);
> + options->auth_nocache = true;
> ssl_set_auth_nocache ();
> }
> else if (streq (p[0], "auth-token") && p[1])
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index 8e0e367..5ae733a 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -292,6 +292,7 @@ struct options
> bool up_delay;
> bool up_restart;
> bool daemon;
> + bool auth_nocache;
> 
> int remap_sigusr1;
> 
> @@ -460,6 +461,7 @@ struct options
> bool pull; /* client pull of config options from server */
> int push_continuation;
> const char *auth_user_pass_file;
> + const char *auth_user_pass_file_inline;
> struct options_pre_pull *pre_pull;
> 
> int server_poll_timeout;
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 69f77f3..aa249f1 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -354,36 +354,88 @@ static char *auth_challenge; /* GLOBAL */
> #endif
> 
> void
> -auth_user_pass_setup (const char *auth_file, const struct 
> static_challenge_info 
> *sci)
> +auth_user_pass_setup (const char *auth_file, const char *auth_file_inline, 
> const struct static_challenge_info *sci)
> {
> +
> + int extra_flags = 0;
> + int n_inlined = 0;
> + char inlined_username[USER_PASS_LEN];
> + char inlined_password[USER_PASS_LEN];
> +
> auth_user_pass_enabled = true;
> +
> if (!auth_user_pass.defined)
> {
> + if (auth_file && streq (auth_file, "stdin") && auth_file_inline)
> + {
> + /* check how much is inlined. */
> + const char *pos = auth_file_inline;
> + const char *prev = pos;
> +
> + while( (pos = strchr(pos, '\n')) != NULL)
> + {
> + n_inlined++;
> +
> + if (n_inlined == 1)
> + {
> + strncpy(inlined_username, prev, pos - prev);
> + inlined_username[pos - prev + 1] = '\0';
> + msg (M_INFO, "Using inlined username: %s", inlined_username);
> + }
> + else
> + {
> + strncpy(inlined_password, prev, pos - prev);
> + inlined_password[pos - prev + 1] = '\0';
> + msg (M_INFO, "Using inlined password: ...");
> + }
> +
> + pos++;
> + prev = pos;
> + }
> +
> + if (n_inlined == 1)
> + extra_flags = GET_USER_PASS_PASSWORD_ONLY;
> + }
> +
> + if (n_inlined == 2)
> + {
> + strcpy(auth_user_pass.username, inlined_username);
> + strcpy(auth_user_pass.password, inlined_password);
> + auth_user_pass.defined = true;
> + }
> + else
> + {
> #if AUTO_USERID
> - get_user_pass_auto_userid (&auth_user_pass, auth_file);
> + get_user_pass_auto_userid (&auth_user_pass, auth_file);
> #else
> # ifdef ENABLE_CLIENT_CR
> - if (auth_challenge) /* dynamic challenge/response */
> - get_user_pass_cr (&auth_user_pass,
> - auth_file,
> - UP_TYPE_AUTH,
> - 
> GET_USER_PASS_MANAGEMENT|GET_USER_PASS_SENSITIVE|GET_USER_PASS_DYNAMIC_CHALLENGE,
> - auth_challenge);
> - else if (sci) /* static challenge response */
> - {
> - int flags = 
> GET_USER_PASS_MANAGEMENT|GET_USER_PASS_SENSITIVE|GET_USER_PASS_STATIC_CHALLENGE;
> - if (sci->flags & SC_ECHO)
> - flags |= GET_USER_PASS_STATIC_CHALLENGE_ECHO;
> - get_user_pass_cr (&auth_user_pass,
> - auth_file,
> - UP_TYPE_AUTH,
> - flags,
> - sci->challenge_text);
> - }
> - else
> + if (auth_challenge) /* dynamic challenge/response */
> + get_user_pass_cr (&auth_user_pass,
> + auth_file,
> + UP_TYPE_AUTH,
> + 
> GET_USER_PASS_MANAGEMENT|GET_USER_PASS_SENSITIVE|GET_USER_PASS_DYNAMIC_CHALLENGE|extra_flags,
> + auth_challenge);
> + else if (sci) /* static challenge response */
> + {
> + int flags = 
> GET_USER_PASS_MANAGEMENT|GET_USER_PASS_SENSITIVE|GET_USER_PASS_STATIC_CHALLENGE|extra_flags;
> + if (sci->flags & SC_ECHO)
> + flags |= GET_USER_PASS_STATIC_CHALLENGE_ECHO;
> +
> + get_user_pass_cr (&auth_user_pass,
> + auth_file,
> + UP_TYPE_AUTH,
> + flags,
> + sci->challenge_text);
> + }
> + else
> # endif
> - get_user_pass (&auth_user_pass, auth_file, UP_TYPE_AUTH, 
> GET_USER_PASS_MANAGEMENT|GET_USER_PASS_SENSITIVE);
> + get_user_pass (&auth_user_pass, auth_file, UP_TYPE_AUTH, 
> GET_USER_PASS_MANAGEMENT|GET_USER_PASS_SENSITIVE|extra_flags);
> #endif
> + }
> +
> + if (n_inlined == 1)
> + strcpy(auth_user_pass.username, inlined_username);
> +
> }
> }
> 
> @@ -1892,9 +1944,9 @@ key_method_2_write (struct buffer *buf, struct 
> tls_session 
> *session)
> if (auth_user_pass_enabled)
> {
> #ifdef ENABLE_CLIENT_CR
> - auth_user_pass_setup (NULL, session->opt->sci);
> + auth_user_pass_setup (NULL, NULL, session->opt->sci);
> #else
> - auth_user_pass_setup (NULL, NULL);
> + auth_user_pass_setup (NULL, NULL, NULL);
> #endif
> if (!write_string (buf, auth_user_pass.username, -1))
> goto error;
> diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
> index cd7cae2..c7554aa 100644
> --- a/src/openvpn/ssl.h
> +++ b/src/openvpn/ssl.h
> @@ -388,7 +388,7 @@ void pem_password_setup (const char *auth_file);
> * Setup authentication username and password. If auth_file is given, use the
> * credentials stored in the file.
> */
> -void auth_user_pass_setup (const char *auth_file, const struct 
> static_challenge_info *sc_info);
> +void auth_user_pass_setup (const char *auth_file, const char 
> *auth_file_inline, 
> const struct static_challenge_info *sc_info);
> 
> /*
> * Ensure that no caching is performed on authentication information
> --
> 1.7.10.4------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk_______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
> 
> 
> 
> ------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk
> 
> 
> 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
> 


Reply via email to