[PATCH] issue #4375: provide --password-fd option

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] issue #4375: provide --password-fd option

William Orr
Hey,

This is my first patch to subversion, so please bear with me.

This looks to address a very commonly requested feature: providing an
alternative for automated tools to provide a password to svn via piping
it in over an fd (similar to gnupg).

One outstanding concern that I couldn't find addressed is clearing out
memory that once contained passwords (like with memset_s or
explicit_bzero). If I missed a technique for doing this that exists in
svn already, please let me know so I can update the diff.

Tested on Fedora 25 x86_64 and OpenBSD 6.1 x86_64.

Please CC me; I'm not on this list.

[[[
Introduce global opt --password-fd to allow applications to provide a
password over an already-opened file descriptor.

* subversion/include/svn_cmdline.h
  (svn_cmdline_create_auth_baton2): Add `auth_password_fd` argument
* subversion/include/svn_error_codes.h
  (SVN_ERR_IO_PIPE_READ_ERROR): Undeprecate, as now used
* subversion/libsvn_subr/cmdline.c
  (read_pass_from_fd): Add static function to get password from a file
descriptor
  (svn_cmdline_create_auth_baton2): Add `auth_password_fd` arg and
trigger read of fd if this arg is not -1
* subversion/libsvn_subr/deprecated.c:
  (svn_cmdline_create_auth_baton): Add default val of -1 when calling
`svn_cmdline_create_auth_baton2`
* subversion/svn/svn.c
  (svn_cl__longopt_t): Add `opt_auth_password_fd` longopt
  (svn_cl__global_options): Add `opt_auth_password_fd` to global options
  (sub_main): Process global option `opt_auth_password_fd` and pass it
to `svn_cmdline_create_auth_baton2`
* subversion/svnmucc/svnmucc.c
  (sub_main): Process global option `opt_auth_password_fd` and pass it
to `svn_cmdline_create_auth_baton2`
* subversion/svnrdump/svnrdump.c
  (svn_svnrdump__longopt_t): add `opt_auth_password_fd`
  (svnrdump__options): add help message for `--password-fd`
  (init_client_context): Pass `auth_password_fd` to
`svn_cmdline_create_auth_baton2`
  (sub_main): Process global option `opt_auth_password_fd` and pass it
to `init_client_context`
* subversion/svnsync/svnsync.c
  (svnsync__opt): Add `svnsync_opt_source_password_fd` and
`svnsync_opt_sync_password_fd`
  (svnsync_options): Add help messages for `--source-password-fd` and
`--sync-password-fd`
  (opt_baton_t): Add `source_password_fd` and `sync_password_fd`
  (sub_main): Process global option `--source-password-fd` and
`--sync-password-fd` and pass it to `svn_cmdline_create_auth_baton2`
invocations
* subversion/tests/cmdline/atomic-ra-revprop-change.c
  (construct_auth_baton): Pass -1 as the `auth_password_fd`
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
  (): Add new `--password-fd` option to expected output
* subversion/tests/libsvn_ra/ra-test.c
  (check_tunnel_callback_test): Pass -1 as the `auth_password_fd`
  (tunnel_callback_test): Pass -1 as the `auth_password_fd`
  (tunnel_run_checkout): Pass -1 as the `auth_password_fd`
* subversion/tests/svn_test_main.c
  (svn_test__init_auth_baton): Pass -1 as the `auth_password_fd`
* tools/client-side/svn-mergeinfo-normalizer/mergeinfo-normalizer.h
  (svn_min__opt_state_t): Add `auth_password_fd`
* tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c
  (svn_min__longopt_t) Add `opt_auth_password_fd`
  (sub_main) Process global option `--password-fd` and pass it to
`svn_cmdline_create_auth_baton2` invocations
* tools/client-side/svnconflict/svnconflict.c
  (svnconflict_opt_state_t): Add `auth_password_fd`
  (svnconflict_options): Add `--password-fd` documentation
  (svnconflict_global_options): Add `opt_auth_password_fd`
  (sub_main): Process global option `--password-fd` and pass it to
`svn_cmdline_create_auth_baton2` invocations
* tools/dev/svnmover/svnmover.c
  (sub_main): Process global option `--password-fd` and pass it to
`svn_cmdline_create_auth_baton2` invocations
]]]

svn-password-fd.patch (37K) Download Attachment
signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] issue #4375: provide --password-fd option

Stefan Sperling-9
On Sun, Sep 17, 2017 at 01:06:14AM -0700, William Orr wrote:
> Hey,
>
> This is my first patch to subversion, so please bear with me.
>
> This looks to address a very commonly requested feature: providing an
> alternative for automated tools to provide a password to svn via piping
> it in over an fd (similar to gnupg).

Hi William,

This looks good to me in principle and I support the idea.

Will the implementation of read_pass_from_fd() compile and work on Win32?
We usually defer such operations to APR to avoid portability concerns.
However, APR does not seem to offer an API which wraps fdopen().
Whenever we bypass APR and use system APIs directly we need to support
at least Unix-like systems and Win32.

We cannot change the parameter list of svn_cmdline_create_auth_baton2().
The function is already part of a release, so changing it breaks our
ABI compatibility promise. Instead, we can add a new function called
svn_cmdline_create_auth_baton3() which has the additional parameter.
The svn_cmdline_create_auth_baton2() interface can be implemented
as a wrapper around our new version of this function.

Given that this is feature intends to support non-interactive usage,
I wonder if it should depend on the --non-interactive option as well?
And maybe we could then reduce the new option to --password-from-stdin?

Some tools already use stdin for a different purpose, though.
E.g. 'svnrdump load' reads a dump file from stdin. But if we also extended
such tools with an option to read their normal input from a file, we could
make this idea work. "svnadmin load" already supports the -F (--file)
option for this purpose. Scripts would have to pass the right combination
of options: --non-interactive --password-from-stdin -F /tmp/my-dump-file
If --password-from-stdin is used without --non-interactive and without
-F then the program should error out and complain.

This idea also solves the portability question, since svn_stream_for_stdin2()
or apr_file_open_flags_stdin() could be used to read a password from stdin.
And we wouldn't need a new revision of svn_cmdline_create_auth_baton2()
either because the client could pass the password as a string, as it
does for the --password option.

> One outstanding concern that I couldn't find addressed is clearing out
> memory that once contained passwords (like with memset_s or
> explicit_bzero). If I missed a technique for doing this that exists in
> svn already, please let me know so I can update the diff.

I don't think we have an API for that either, unfortunately. However,
the same portability concerns apply. In any case, it would be great to
have such an API available in APR.

Regards,
Stefan

> Tested on Fedora 25 x86_64 and OpenBSD 6.1 x86_64.
>
> Please CC me; I'm not on this list.
>
> [[[
> Introduce global opt --password-fd to allow applications to provide a
> password over an already-opened file descriptor.
>
> * subversion/include/svn_cmdline.h
>   (svn_cmdline_create_auth_baton2): Add `auth_password_fd` argument
> * subversion/include/svn_error_codes.h
>   (SVN_ERR_IO_PIPE_READ_ERROR): Undeprecate, as now used
> * subversion/libsvn_subr/cmdline.c
>   (read_pass_from_fd): Add static function to get password from a file
> descriptor
>   (svn_cmdline_create_auth_baton2): Add `auth_password_fd` arg and
> trigger read of fd if this arg is not -1
> * subversion/libsvn_subr/deprecated.c:
>   (svn_cmdline_create_auth_baton): Add default val of -1 when calling
> `svn_cmdline_create_auth_baton2`
> * subversion/svn/svn.c
>   (svn_cl__longopt_t): Add `opt_auth_password_fd` longopt
>   (svn_cl__global_options): Add `opt_auth_password_fd` to global options
>   (sub_main): Process global option `opt_auth_password_fd` and pass it
> to `svn_cmdline_create_auth_baton2`
> * subversion/svnmucc/svnmucc.c
>   (sub_main): Process global option `opt_auth_password_fd` and pass it
> to `svn_cmdline_create_auth_baton2`
> * subversion/svnrdump/svnrdump.c
>   (svn_svnrdump__longopt_t): add `opt_auth_password_fd`
>   (svnrdump__options): add help message for `--password-fd`
>   (init_client_context): Pass `auth_password_fd` to
> `svn_cmdline_create_auth_baton2`
>   (sub_main): Process global option `opt_auth_password_fd` and pass it
> to `init_client_context`
> * subversion/svnsync/svnsync.c
>   (svnsync__opt): Add `svnsync_opt_source_password_fd` and
> `svnsync_opt_sync_password_fd`
>   (svnsync_options): Add help messages for `--source-password-fd` and
> `--sync-password-fd`
>   (opt_baton_t): Add `source_password_fd` and `sync_password_fd`
>   (sub_main): Process global option `--source-password-fd` and
> `--sync-password-fd` and pass it to `svn_cmdline_create_auth_baton2`
> invocations
> * subversion/tests/cmdline/atomic-ra-revprop-change.c
>   (construct_auth_baton): Pass -1 as the `auth_password_fd`
> * subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
>   (): Add new `--password-fd` option to expected output
> * subversion/tests/libsvn_ra/ra-test.c
>   (check_tunnel_callback_test): Pass -1 as the `auth_password_fd`
>   (tunnel_callback_test): Pass -1 as the `auth_password_fd`
>   (tunnel_run_checkout): Pass -1 as the `auth_password_fd`
> * subversion/tests/svn_test_main.c
>   (svn_test__init_auth_baton): Pass -1 as the `auth_password_fd`
> * tools/client-side/svn-mergeinfo-normalizer/mergeinfo-normalizer.h
>   (svn_min__opt_state_t): Add `auth_password_fd`
> * tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c
>   (svn_min__longopt_t) Add `opt_auth_password_fd`
>   (sub_main) Process global option `--password-fd` and pass it to
> `svn_cmdline_create_auth_baton2` invocations
> * tools/client-side/svnconflict/svnconflict.c
>   (svnconflict_opt_state_t): Add `auth_password_fd`
>   (svnconflict_options): Add `--password-fd` documentation
>   (svnconflict_global_options): Add `opt_auth_password_fd`
>   (sub_main): Process global option `--password-fd` and pass it to
> `svn_cmdline_create_auth_baton2` invocations
> * tools/dev/svnmover/svnmover.c
>   (sub_main): Process global option `--password-fd` and pass it to
> `svn_cmdline_create_auth_baton2` invocations
> ]]]

> Index: subversion/include/svn_cmdline.h
> ===================================================================
> --- subversion/include/svn_cmdline.h (revisi??n: 1808405)
> +++ subversion/include/svn_cmdline.h (copia de trabajo)
> @@ -356,6 +356,7 @@ svn_cmdline_create_auth_baton2(svn_auth_baton_t **
>                                 svn_boolean_t non_interactive,
>                                 const char *username,
>                                 const char *password,
> +                               int password_fd,
>                                 const char *config_dir,
>                                 svn_boolean_t no_auth_cache,
>                                 svn_boolean_t trust_server_cert_unknown_ca,
> Index: subversion/include/svn_error_codes.h
> ===================================================================
> --- subversion/include/svn_error_codes.h (revisi??n: 1808405)
> +++ subversion/include/svn_error_codes.h (copia de trabajo)
> @@ -296,7 +296,6 @@ SVN_ERROR_START
>               SVN_ERR_IO_CATEGORY_START + 4,
>               "Framing error in pipe protocol")
>  
> -  /** @deprecated Unused, slated for removal in the next major release. */
>    SVN_ERRDEF(SVN_ERR_IO_PIPE_READ_ERROR,
>               SVN_ERR_IO_CATEGORY_START + 5,
>               "Read error in pipe")
> Index: subversion/libsvn_subr/cmdline.c
> ===================================================================
> --- subversion/libsvn_subr/cmdline.c (revisi??n: 1808405)
> +++ subversion/libsvn_subr/cmdline.c (copia de trabajo)
> @@ -516,6 +516,39 @@ struct trust_server_cert_non_interactive_baton {
>    svn_boolean_t trust_server_cert_other_failure;
>  };
>  
> +static svn_error_t *
> +read_pass_from_fd(int fd, const char **password, apr_pool_t *pool)
> +{
> +  SVN_ERR_ASSERT(fd != -1);
> +
> +  svn_error_t *err = SVN_NO_ERROR;
> +  size_t password_size = 0;
> +  ssize_t password_len = 0;
> +  char *ret = NULL;
> +  FILE *desc = NULL;
> +  svn_stringbuf_t *password_str = NULL;
> +
> +  if (! (desc = fdopen(fd, "r")))
> +    {
> +      return svn_error_create(SVN_ERR_IO_PIPE_READ_ERROR, NULL, NULL);
> +    }
> +
> +  if ((password_len = getline(&ret, &password_size, desc)) == -1)
> +    {
> +      err = svn_error_create(SVN_ERR_IO_PIPE_READ_ERROR, NULL, NULL);
> +      goto cleanup;
> +    }
> +
> +  password_str = svn_stringbuf_create(ret, pool);
> +  svn_stringbuf_chop(password_str, 1);
> +  *password = password_str->data;
> +
> + cleanup:
> +  free(ret);
> +
> +  return err;
> +}
> +
>  /* This implements 'svn_auth_ssl_server_trust_prompt_func_t'.
>  
>     Don't actually prompt.  Instead, set *CRED_P to valid credentials
> @@ -567,6 +600,7 @@ svn_cmdline_create_auth_baton2(svn_auth_baton_t **
>                                 svn_boolean_t non_interactive,
>                                 const char *auth_username,
>                                 const char *auth_password,
> +                               int auth_password_fd,
>                                 const char *config_dir,
>                                 svn_boolean_t no_auth_cache,
>                                 svn_boolean_t trust_server_cert_unknown_ca,
> @@ -584,6 +618,7 @@ svn_cmdline_create_auth_baton2(svn_auth_baton_t **
>    svn_boolean_t store_auth_creds_val = TRUE;
>    svn_auth_provider_object_t *provider;
>    svn_cmdline_prompt_baton2_t *pb = NULL;
> +  const char *password = NULL;
>  
>    /* The whole list of registered providers */
>    apr_array_header_t *providers;
> @@ -701,8 +736,14 @@ svn_cmdline_create_auth_baton2(svn_auth_baton_t **
>    /* Build an authentication baton to give to libsvn_client. */
>    svn_auth_open(ab, providers, pool);
>  
> -  /* Place any default --username or --password credentials into the
> -     auth_baton's run-time parameter hash. */
> +  /* need to audit for places I set it to 0 */
> +  if (auth_password_fd != -1 && auth_password == NULL)
> +    {
> +      SVN_ERR(read_pass_from_fd(auth_password_fd, &password, pool));
> +    }
> +
> +  /* Place any default --username, --password or credentials read from password
> +     fd into the auth_baton's run-time parameter hash. */
>    if (auth_username)
>      svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_DEFAULT_USERNAME,
>                             auth_username);
> @@ -709,6 +750,9 @@ svn_cmdline_create_auth_baton2(svn_auth_baton_t **
>    if (auth_password)
>      svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_DEFAULT_PASSWORD,
>                             auth_password);
> +  else if (password)
> +    svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_DEFAULT_PASSWORD,
> +                           password);
>  
>    /* Same with the --non-interactive option. */
>    if (non_interactive)
> Index: subversion/libsvn_subr/deprecated.c
> ===================================================================
> --- subversion/libsvn_subr/deprecated.c (revisi??n: 1808405)
> +++ subversion/libsvn_subr/deprecated.c (copia de trabajo)
> @@ -1573,6 +1573,7 @@ svn_cmdline_create_auth_baton(svn_auth_baton_t **a
>                                                          non_interactive,
>                                                          auth_username,
>                                                          auth_password,
> +                                                        -1,
>                                                          config_dir,
>                                                          no_auth_cache,
>                                                          trust_server_cert,
> Index: subversion/svn/cl.h
> ===================================================================
> --- subversion/svn/cl.h (revisi??n: 1808405)
> +++ subversion/svn/cl.h (copia de trabajo)
> @@ -178,6 +178,7 @@ typedef struct svn_cl__opt_state_t
>    svn_boolean_t help;            /* print usage message */
>    const char *auth_username;     /* auth username */
>    const char *auth_password;     /* auth password */
> +  int auth_password_fd;          /* fd to read password from */
>    const char *extensions;        /* subprocess extension args */
>    apr_array_header_t *targets;   /* target list from file */
>    svn_boolean_t xml;             /* output in xml, e.g., "svn log --xml" */
> Index: subversion/svn/svn.c
> ===================================================================
> --- subversion/svn/svn.c (revisi??n: 1808405)
> +++ subversion/svn/svn.c (copia de trabajo)
> @@ -68,6 +68,7 @@
>     use the short option letter as identifier.  */
>  typedef enum svn_cl__longopt_t {
>    opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
> +  opt_auth_password_fd,
>    opt_auth_username,
>    opt_autoprops,
>    opt_changelist,
> @@ -200,6 +201,8 @@ const apr_getopt_option_t svn_cl__options[] =
>                      N_("specify a password ARG (caution: on many operating\n"
>                         "                             "
>                         "systems, other users will be able to see this)")},
> +  {"password-fd",   opt_auth_password_fd, 1,
> +                    N_("specify an fd to read a password from ARG")},
>    {"extensions",    'x', 1,
>                      N_("Specify differencing options for external diff or\n"
>                         "                             "
> @@ -495,7 +498,8 @@ const apr_getopt_option_t svn_cl__options[] =
>     command to take these arguments allows scripts to just pass them
>     willy-nilly to every invocation of 'svn') . */
>  const int svn_cl__global_options[] =
> -{ opt_auth_username, opt_auth_password, opt_no_auth_cache, opt_non_interactive,
> +{ opt_auth_username, opt_auth_password, opt_auth_password_fd,
> +  opt_no_auth_cache, opt_non_interactive,
>    opt_force_interactive, opt_trust_server_cert,
>    opt_trust_server_cert_failures,
>    opt_config_dir, opt_config_options, 0
> @@ -1991,6 +1995,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    opt_state.set_depth = svn_depth_unknown;
>    opt_state.accept_which = svn_cl__accept_unspecified;
>    opt_state.show_revs = svn_cl__show_revs_invalid;
> +  opt_state.auth_password_fd = -1;
>  
>    /* No args?  Show usage. */
>    if (argc <= 1)
> @@ -2251,6 +2256,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
>                                          opt_arg, pool));
>          break;
> +      case opt_auth_password_fd:
> +        SVN_ERR(svn_cstring_atoi(&opt_state.auth_password_fd, opt_arg));
> +        break;
>        case opt_encoding:
>          opt_state.encoding = apr_pstrdup(pool, opt_arg);
>          break;
> @@ -3044,6 +3052,7 @@ sub_main(int *exit_code, int argc, const char *arg
>              opt_state.non_interactive,
>              opt_state.auth_username,
>              opt_state.auth_password,
> +            opt_state.auth_password_fd,
>              opt_state.config_dir,
>              opt_state.no_auth_cache,
>              opt_state.trust_server_cert_unknown_ca,
> Index: subversion/svnbench/cl.h
> ===================================================================
> --- subversion/svnbench/cl.h (revisi??n: 1808405)
> +++ subversion/svnbench/cl.h (copia de trabajo)
> @@ -80,6 +80,7 @@ typedef struct svn_cl__opt_state_t
>    svn_boolean_t help;            /* print usage message */
>    const char *auth_username;     /* auth username */ /* UTF-8! */
>    const char *auth_password;     /* auth password */ /* UTF-8! */
> +  int auth_password_fd;          /* auth password fd */
>    apr_array_header_t *targets;   /* target list from file */ /* UTF-8! */
>    svn_boolean_t no_auth_cache;   /* do not cache authentication information */
>    svn_boolean_t stop_on_copy;    /* don't cross copies during processing */
> Index: subversion/svnbench/svnbench.c
> ===================================================================
> --- subversion/svnbench/svnbench.c (revisi??n: 1808405)
> +++ subversion/svnbench/svnbench.c (copia de trabajo)
> @@ -53,6 +53,7 @@
>     use the short option letter as identifier.  */
>  typedef enum svn_cl__longopt_t {
>    opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
> +  opt_auth_password_fd,
>    opt_auth_username,
>    opt_config_dir,
>    opt_config_options,
> @@ -112,6 +113,7 @@ const apr_getopt_option_t svn_cl__options[] =
>    {"verbose",       'v', 0, N_("print extra information")},
>    {"username",      opt_auth_username, 1, N_("specify a username ARG")},
>    {"password",      opt_auth_password, 1, N_("specify a password ARG")},
> +  {"password-fd",   opt_auth_password_fd, 1, N_("specify a password-fd ARG")},
>    {"targets",       opt_targets, 1,
>                      N_("pass contents of file ARG as additional args")},
>    {"depth",         opt_depth, 1,
> @@ -197,7 +199,8 @@ const apr_getopt_option_t svn_cl__options[] =
>     command to take these arguments allows scripts to just pass them
>     willy-nilly to every invocation of 'svn') . */
>  const int svn_cl__global_options[] =
> -{ opt_auth_username, opt_auth_password, opt_no_auth_cache, opt_non_interactive,
> +{ opt_auth_username, opt_auth_password, opt_auth_password_fd,
> +  opt_no_auth_cache, opt_non_interactive,
>    opt_trust_server_cert, opt_trust_server_cert_failures,
>    opt_config_dir, opt_config_options, 0
>  };
> @@ -420,6 +423,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    opt_state.revision_ranges =
>      apr_array_make(pool, 0, sizeof(svn_opt_revision_range_t *));
>    opt_state.depth = svn_depth_unknown;
> +  opt_state.auth_password_fd = -1;
>  
>    /* No args?  Show usage. */
>    if (argc <= 1)
> @@ -625,6 +629,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
>                                              opt_arg, pool));
>          break;
> +      case opt_auth_password_fd:
> +        SVN_ERR(svn_cstring_atoi(&opt_state.auth_password_fd, opt_arg));
> +        break;
>        case opt_stop_on_copy:
>          opt_state.stop_on_copy = TRUE;
>          break;
> @@ -929,6 +936,7 @@ sub_main(int *exit_code, int argc, const char *arg
>              opt_state.non_interactive,
>              opt_state.auth_username,
>              opt_state.auth_password,
> +            opt_state.auth_password_fd,
>              opt_state.config_dir,
>              opt_state.no_auth_cache,
>              opt_state.trust_server_cert_unknown_ca,
> Index: subversion/svnmucc/svnmucc.c
> ===================================================================
> --- subversion/svnmucc/svnmucc.c (revisi??n: 1808405)
> +++ subversion/svnmucc/svnmucc.c (copia de trabajo)
> @@ -480,7 +480,8 @@ sub_main(int *exit_code, int argc, const char *arg
>      non_interactive_opt,
>      force_interactive_opt,
>      trust_server_cert_opt,
> -    trust_server_cert_failures_opt
> +    trust_server_cert_failures_opt,
> +    password_fd_opt
>    };
>    static const apr_getopt_option_t options[] = {
>      {"message", 'm', 1, ""},
> @@ -487,6 +488,7 @@ sub_main(int *exit_code, int argc, const char *arg
>      {"file", 'F', 1, ""},
>      {"username", 'u', 1, ""},
>      {"password", 'p', 1, ""},
> +    {"password-fd", password_fd_opt, 1, ""},
>      {"root-url", 'U', 1, ""},
>      {"revision", 'r', 1, ""},
>      {"with-revprop",  with_revprop_opt, 1, ""},
> @@ -527,6 +529,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    svn_client_ctx_t *ctx;
>    struct log_message_baton lmb;
>    int i;
> +  int password_fd = -1;
>  
>    /* Check library versions */
>    SVN_ERR(check_lib_versions());
> @@ -572,6 +575,8 @@ sub_main(int *exit_code, int argc, const char *arg
>          case 'p':
>            password = apr_pstrdup(pool, arg);
>            break;
> +        case password_fd_opt:
> +          SVN_ERR(svn_cstring_atoi(&password_fd, arg));
>          case 'U':
>            SVN_ERR(svn_utf_cstring_to_utf8(&root_url, arg, pool));
>            if (! svn_path_is_url(root_url))
> @@ -729,6 +734,7 @@ sub_main(int *exit_code, int argc, const char *arg
>              non_interactive,
>              username,
>              password,
> +            password_fd,
>              config_dir,
>              no_auth_cache,
>              trust_unknown_ca,
> Index: subversion/svnrdump/svnrdump.c
> ===================================================================
> --- subversion/svnrdump/svnrdump.c (revisi??n: 1808405)
> +++ subversion/svnrdump/svnrdump.c (copia de trabajo)
> @@ -59,6 +59,7 @@ enum svn_svnrdump__longopt_t
>      opt_config_option,
>      opt_auth_username,
>      opt_auth_password,
> +    opt_auth_password_fd,
>      opt_auth_nocache,
>      opt_non_interactive,
>      opt_skip_revprop,
> @@ -73,6 +74,7 @@ enum svn_svnrdump__longopt_t
>                                     opt_config_option, \
>                                     opt_auth_username, \
>                                     opt_auth_password, \
> +                                   opt_auth_password_fd, \
>                                     opt_auth_nocache, \
>                                     opt_trust_server_cert, \
>                                     opt_trust_server_cert_failures, \
> @@ -114,6 +116,8 @@ static const apr_getopt_option_t svnrdump__options
>                        N_("specify a username ARG")},
>      {"password",      opt_auth_password, 1,
>                        N_("specify a password ARG")},
> +    {"password-fd",   opt_auth_password, 1,
> +                      N_("specify a password fd ARG")},
>      {"non-interactive", opt_non_interactive, 0,
>                        N_("do no interactive prompting (default is to prompt\n"
>                           "                             "
> @@ -294,6 +298,7 @@ init_client_context(svn_client_ctx_t **ctx_p,
>                      svn_boolean_t non_interactive,
>                      const char *username,
>                      const char *password,
> +                    int password_fd,
>                      const char *config_dir,
>                      const char *repos_url,
>                      svn_boolean_t no_auth_cache,
> @@ -366,7 +371,8 @@ init_client_context(svn_client_ctx_t **ctx_p,
>  
>    /* Default authentication providers for non-interactive use */
>    SVN_ERR(svn_cmdline_create_auth_baton2(&(ctx->auth_baton), non_interactive,
> -                                         username, password, config_dir,
> +                                         username, password, password_fd,
> +                                         config_dir,
>                                           no_auth_cache, trust_unknown_ca,
>                                           trust_cn_mismatch, trust_expired,
>                                           trust_not_yet_valid,
> @@ -760,6 +766,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    const char *config_dir = NULL;
>    const char *username = NULL;
>    const char *password = NULL;
> +  int password_fd = -1;
>    svn_boolean_t no_auth_cache = FALSE;
>    svn_boolean_t trust_unknown_ca = FALSE;
>    svn_boolean_t trust_cn_mismatch = FALSE;
> @@ -850,6 +857,8 @@ sub_main(int *exit_code, int argc, const char *arg
>          case opt_auth_password:
>            SVN_ERR(svn_utf_cstring_to_utf8(&password, opt_arg, pool));
>            break;
> +        case opt_auth_password_fd:
> +          SVN_ERR(svn_cstring_atoi(&password_fd, opt_arg));
>          case opt_auth_nocache:
>            no_auth_cache = TRUE;
>            break;
> @@ -1046,6 +1055,7 @@ sub_main(int *exit_code, int argc, const char *arg
>                                non_interactive,
>                                username,
>                                password,
> +                              password_fd,
>                                config_dir,
>                                opt_baton->url,
>                                no_auth_cache,
> Index: subversion/svnsync/svnsync.c
> ===================================================================
> --- subversion/svnsync/svnsync.c (revisi??n: 1808405)
> +++ subversion/svnsync/svnsync.c (copia de trabajo)
> @@ -59,8 +59,10 @@ enum svnsync__opt {
>    svnsync_opt_auth_password,
>    svnsync_opt_source_username,
>    svnsync_opt_source_password,
> +  svnsync_opt_source_password_fd,
>    svnsync_opt_sync_username,
>    svnsync_opt_sync_password,
> +  svnsync_opt_sync_password_fd,
>    svnsync_opt_config_dir,
>    svnsync_opt_config_options,
>    svnsync_opt_source_prop_encoding,
> @@ -84,8 +86,10 @@ enum svnsync__opt {
>                               svnsync_opt_trust_server_cert_failures_dst, \
>                               svnsync_opt_source_username, \
>                               svnsync_opt_source_password, \
> +                             svnsync_opt_source_password_fd, \
>                               svnsync_opt_sync_username, \
>                               svnsync_opt_sync_password, \
> +                             svnsync_opt_sync_password_fd, \
>                               svnsync_opt_config_dir, \
>                               svnsync_opt_config_options
>  
> @@ -240,10 +244,14 @@ static const apr_getopt_option_t svnsync_options[]
>                         N_("connect to source repository with username ARG") },
>      {"source-password", svnsync_opt_source_password, 1,
>                         N_("connect to source repository with password ARG") },
> +    {"source-password-fd", svnsync_opt_source_password_fd, 1,
> +                       N_("connect to source repository with password from fd ARG") },
>      {"sync-username",  svnsync_opt_sync_username, 1,
>                         N_("connect to sync repository with username ARG") },
>      {"sync-password",  svnsync_opt_sync_password, 1,
>                         N_("connect to sync repository with password ARG") },
> +    {"source-password-fd", svnsync_opt_sync_password_fd, 1,
> +                       N_("connect to sync repository with password from fd ARG") },
>      {"config-dir",     svnsync_opt_config_dir, 1,
>                         N_("read user configuration files from directory ARG")},
>      {"config-option",  svnsync_opt_config_options, 1,
> @@ -301,8 +309,10 @@ typedef struct opt_baton_t {
>    svn_auth_baton_t *sync_auth_baton;
>    const char *source_username;
>    const char *source_password;
> +  int source_password_fd;
>    const char *sync_username;
>    const char *sync_password;
> +  int sync_password_fd;
>    const char *config_dir;
>    apr_hash_t *config;
>    const char *source_prop_encoding;
> @@ -1973,6 +1983,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    apr_array_header_t *config_options = NULL;
>    const char *source_prop_encoding = NULL;
>    svn_boolean_t force_interactive = FALSE;
> +  int source_password_fd = -1, sync_password_fd = -1;
>  
>    /* Check library versions */
>    SVN_ERR(check_lib_versions());
> @@ -2071,6 +2082,10 @@ sub_main(int *exit_code, int argc, const char *arg
>              opt_err = svn_utf_cstring_to_utf8(&source_password, opt_arg, pool);
>              break;
>  
> +          case svnsync_opt_source_password_fd:
> +            opt_err = svn_cstring_atoi(&source_password_fd, opt_arg);
> +            break;
> +
>            case svnsync_opt_sync_username:
>              opt_err = svn_utf_cstring_to_utf8(&sync_username, opt_arg, pool);
>              break;
> @@ -2079,6 +2094,10 @@ sub_main(int *exit_code, int argc, const char *arg
>              opt_err = svn_utf_cstring_to_utf8(&sync_password, opt_arg, pool);
>              break;
>  
> +        case svnsync_opt_sync_password_fd:
> +          opt_err = svn_cstring_atoi(&sync_password_fd, opt_arg);
> +          break;
> +
>            case svnsync_opt_config_dir:
>              {
>                const char *path;
> @@ -2229,8 +2248,10 @@ sub_main(int *exit_code, int argc, const char *arg
>      }
>    opt_baton.source_username = source_username;
>    opt_baton.source_password = source_password;
> +  opt_baton.source_password_fd = source_password_fd;
>    opt_baton.sync_username = sync_username;
>    opt_baton.sync_password = sync_password;
> +  opt_baton.sync_password_fd = sync_password_fd;
>  
>    /* Disallow mixing of --steal-lock and --disable-locking. */
>    if (opt_baton.steal_lock && opt_baton.disable_locking)
> @@ -2351,6 +2372,7 @@ sub_main(int *exit_code, int argc, const char *arg
>            opt_baton.non_interactive,
>            opt_baton.source_username,
>            opt_baton.source_password,
> +          opt_baton.source_password_fd,
>            opt_baton.config_dir,
>            opt_baton.no_auth_cache,
>            opt_baton.src_trust.trust_server_cert_unknown_ca,
> @@ -2367,6 +2389,7 @@ sub_main(int *exit_code, int argc, const char *arg
>              opt_baton.non_interactive,
>              opt_baton.sync_username,
>              opt_baton.sync_password,
> +            opt_baton.sync_password_fd,
>              opt_baton.config_dir,
>              opt_baton.no_auth_cache,
>              opt_baton.dst_trust.trust_server_cert_unknown_ca,
> Index: subversion/tests/cmdline/atomic-ra-revprop-change.c
> ===================================================================
> --- subversion/tests/cmdline/atomic-ra-revprop-change.c (revisi??n: 1808405)
> +++ subversion/tests/cmdline/atomic-ra-revprop-change.c (copia de trabajo)
> @@ -60,7 +60,7 @@ construct_auth_baton(svn_auth_baton_t **auth_baton
>  {
>    SVN_ERR(svn_cmdline_create_auth_baton2(auth_baton_p,
>                                           TRUE  /* non_interactive */,
> -                                         "jrandom", "rayjandom",
> +                                         "jrandom", "rayjandom", -1,
>                                           config_dir,
>                                           TRUE  /* no_auth_cache */,
>                                           FALSE /* trust_server_cert */,
> Index: subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
> ===================================================================
> --- subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout (revisi??n: 1808405)
> +++ subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout (copia de trabajo)
> @@ -134,6 +134,7 @@ Global options:
>    --username ARG           : specify a username ARG
>    --password ARG           : specify a password ARG (caution: on many operating
>                               systems, other users will be able to see this)
> +  --password-fd ARG        : specify an fd to read a password from ARG
>    --no-auth-cache          : do not cache authentication tokens
>    --non-interactive        : do no interactive prompting (default is to prompt
>                               only if standard input is a terminal device)
> @@ -224,6 +225,7 @@ Global options:
>    --username ARG           : specify a username ARG
>    --password ARG           : specify a password ARG (caution: on many operating
>                               systems, other users will be able to see this)
> +  --password-fd ARG        : specify an fd to read a password from ARG
>    --no-auth-cache          : do not cache authentication tokens
>    --non-interactive        : do no interactive prompting (default is to prompt
>                               only if standard input is a terminal device)
> Index: subversion/tests/libsvn_ra/ra-test.c
> ===================================================================
> --- subversion/tests/libsvn_ra/ra-test.c (revisi??n: 1808405)
> +++ subversion/tests/libsvn_ra/ra-test.c (copia de trabajo)
> @@ -344,7 +344,7 @@ check_tunnel_callback_test(const svn_test_opts_t *
>    cbtable->tunnel_baton = b;
>    SVN_ERR(svn_cmdline_create_auth_baton2(&cbtable->auth_baton,
>                                           TRUE  /* non_interactive */,
> -                                         "jrandom", "rayjandom",
> +                                         "jrandom", "rayjandom", -1,
>                                           NULL,
>                                           TRUE  /* no_auth_cache */,
>                                           FALSE /* trust_server_cert */,
> @@ -387,7 +387,7 @@ tunnel_callback_test(const svn_test_opts_t *opts,
>    cbtable->tunnel_baton = b;
>    SVN_ERR(svn_cmdline_create_auth_baton2(&cbtable->auth_baton,
>                                           TRUE  /* non_interactive */,
> -                                         "jrandom", "rayjandom",
> +                                         "jrandom", "rayjandom", -1,
>                                           NULL,
>                                           TRUE  /* no_auth_cache */,
>                                           FALSE /* trust_server_cert */,
> @@ -1557,7 +1557,7 @@ tunnel_run_checkout(const svn_test_opts_t *opts,
>    cbtable->tunnel_baton = b;
>    SVN_ERR(svn_cmdline_create_auth_baton2(&cbtable->auth_baton,
>      TRUE  /* non_interactive */,
> -    "jrandom", "rayjandom",
> +    "jrandom", "rayjandom", -1,
>      NULL,
>      TRUE  /* no_auth_cache */,
>      FALSE /* trust_server_cert */,
> Index: subversion/tests/svn_test_main.c
> ===================================================================
> --- subversion/tests/svn_test_main.c (revisi??n: 1808405)
> +++ subversion/tests/svn_test_main.c (copia de trabajo)
> @@ -754,7 +754,7 @@ svn_test__init_auth_baton(svn_auth_baton_t **ab,
>  
>    SVN_ERR(svn_cmdline_create_auth_baton2(ab,
>                                           TRUE  /* non_interactive */,
> -                                         "jrandom", "rayjandom",
> +                                         "jrandom", "rayjandom", -1,
>                                           NULL,
>                                           TRUE  /* no_auth_cache */,
>                                           TRUE /* trust_server_cert_unkown_ca */,
> Index: tools/client-side/svn-mergeinfo-normalizer/mergeinfo-normalizer.h
> ===================================================================
> --- tools/client-side/svn-mergeinfo-normalizer/mergeinfo-normalizer.h (revisi??n: 1808405)
> +++ tools/client-side/svn-mergeinfo-normalizer/mergeinfo-normalizer.h (copia de trabajo)
> @@ -56,6 +56,7 @@ typedef struct svn_min__opt_state_t
>    svn_boolean_t help;            /* print usage message */
>    const char *auth_username;     /* auth username */
>    const char *auth_password;     /* auth password */
> +  int auth_password_fd;          /* auth password fd */
>    apr_array_header_t *targets;
>    svn_boolean_t no_auth_cache;   /* do not cache authentication information */
>    svn_boolean_t dry_run;         /* try operation but make no changes */
> Index: tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c
> ===================================================================
> --- tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c (revisi??n: 1808405)
> +++ tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c (copia de trabajo)
> @@ -68,6 +68,7 @@
>     use the short option letter as identifier.  */
>  typedef enum svn_min__longopt_t {
>    opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
> +  opt_auth_password_fd,
>    opt_auth_username,
>    opt_config_dir,
>    opt_config_options,
> @@ -113,6 +114,8 @@ const apr_getopt_option_t svn_min__options[] =
>                      N_("specify a password ARG (caution: on many operating\n"
>                         "                             "
>                         "systems, other users will be able to see this)")},
> +  {"password-fd",   opt_auth_password_fd, 1,
> +                    N_("specify an fd to read a password from")},
>    {"targets",       opt_targets, 1,
>                      N_("pass contents of file ARG as additional args")},
>    {"depth",         opt_depth, 1,
> @@ -419,6 +422,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    apr_hash_t *cfg_hash;
>  
>    received_opts = apr_array_make(pool, SVN_OPT_MAX_OPTIONS, sizeof(int));
> +  opt_state.auth_password_fd = -1;
>  
>    /* Check library versions */
>    SVN_ERR(check_lib_versions());
> @@ -528,6 +532,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
>                                          opt_arg, pool));
>          break;
> +      case opt_auth_password_fd:
> +        SVN_ERR(svn_cstring_atoi(&opt_state.auth_password_fd, opt_arg));
> +        break;
>        case opt_no_auth_cache:
>          opt_state.no_auth_cache = TRUE;
>          break;
> @@ -825,6 +832,7 @@ sub_main(int *exit_code, int argc, const char *arg
>              opt_state.non_interactive,
>              opt_state.auth_username,
>              opt_state.auth_password,
> +            opt_state.auth_password_fd,
>              opt_state.config_dir,
>              opt_state.no_auth_cache,
>              opt_state.trust_server_cert_unknown_ca,
> Index: tools/client-side/svnconflict/svnconflict.c
> ===================================================================
> --- tools/client-side/svnconflict/svnconflict.c (revisi??n: 1808405)
> +++ tools/client-side/svnconflict/svnconflict.c (copia de trabajo)
> @@ -60,6 +60,7 @@ typedef struct svnconflict_opt_state_t {
>    svn_boolean_t help;            /* print usage message */
>    const char *auth_username;     /* auth username */
>    const char *auth_password;     /* auth password */
> +  int auth_password_fd;          /* auth password fd */
>    const char *config_dir;        /* over-riding configuration directory */
>    apr_array_header_t *config_options; /* over-riding configuration options */
>  } svnconflict_opt_state_t;
> @@ -78,6 +79,7 @@ typedef struct svnconflict_cmd_baton_t
>     use the short option letter as identifier.  */
>  typedef enum svnconflict_longopt_t {
>    opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
> +  opt_auth_password_fd,
>    opt_auth_username,
>    opt_config_dir,
>    opt_config_options,
> @@ -96,6 +98,8 @@ static const apr_getopt_option_t svnconflict_optio
>                      N_("specify a password ARG (caution: on many operating\n"
>                         "                             "
>                         "systems, other users will be able to see this)")},
> +  {"password-fd",   opt_auth_password_fd, 1,
> +                    N_("specify an fd to read a password from ARG")},
>    {"config-dir",    opt_config_dir, 1,
>                      N_("read user configuration files from directory ARG")},
>    {"config-option", opt_config_options, 1,
> @@ -141,7 +145,8 @@ static svn_error_t * svnconflict_resolve_tree(apr_
>  
>  /* Options that apply to all commands. */
>  static const int svnconflict_global_options[] =
> -{ opt_auth_username, opt_auth_password, opt_config_dir, opt_config_options, 0 };
> +{ opt_auth_username, opt_auth_password, opt_auth_password_fd,
> +  opt_config_dir, opt_config_options, 0 };
>  
>  static const svn_opt_subcommand_desc2_t svnconflict_cmd_table[] =
>  {
> @@ -641,6 +646,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    apr_hash_t *cfg_hash;
>  
>    received_opts = apr_array_make(pool, SVN_OPT_MAX_OPTIONS, sizeof(int));
> +  opt_state.auth_password_fd = -1;
>  
>    /* Check library versions */
>    SVN_ERR(check_lib_versions());
> @@ -704,6 +710,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
>                                          opt_arg, pool));
>          break;
> +      case opt_auth_password_fd:
> +        SVN_ERR(svn_cstring_atoi(&opt_state.auth_password_fd, opt_arg));
> +        break;
>        case opt_config_dir:
>          SVN_ERR(svn_utf_cstring_to_utf8(&utf8_opt_arg, opt_arg, pool));
>          opt_state.config_dir = svn_dirent_internal_style(utf8_opt_arg, pool);
> @@ -856,6 +865,7 @@ sub_main(int *exit_code, int argc, const char *arg
>              TRUE, /* non-interactive */
>              opt_state.auth_username,
>              opt_state.auth_password,
> +            opt_state.auth_password_fd,
>              opt_state.config_dir,
>              TRUE, /* no auth cache */
>              FALSE, FALSE, FALSE, FALSE, FALSE, /* reject invalid SSL certs */
> Index: tools/dev/svnmover/svnmover.c
> ===================================================================
> --- tools/dev/svnmover/svnmover.c (revisi??n: 1808405)
> +++ tools/dev/svnmover/svnmover.c (copia de trabajo)
> @@ -4332,7 +4332,8 @@ sub_main(int *exit_code, int argc, const char *arg
>      trust_server_cert_opt,
>      trust_server_cert_failures_opt,
>      ui_opt,
> -    colour_opt
> +    colour_opt,
> +    auth_password_fd_opt
>    };
>    static const apr_getopt_option_t options[] = {
>      {"verbose", 'v', 0, ""},
> @@ -4341,6 +4342,7 @@ sub_main(int *exit_code, int argc, const char *arg
>      {"file", 'F', 1, ""},
>      {"username", 'u', 1, ""},
>      {"password", 'p', 1, ""},
> +    {"password-fd", auth_password_fd_opt, 1, ""},
>      {"root-url", 'U', 1, ""},
>      {"revision", 'r', 1, ""},
>      {"branch-id", 'B', 1, ""},
> @@ -4387,6 +4389,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    const char *log_msg;
>    svn_tristate_t coloured_output = svn_tristate_false;
>    svnmover_wc_t *wc;
> +  int password_fd = -1;
>  
>    /* Check library versions */
>    SVN_ERR(check_lib_versions());
> @@ -4431,6 +4434,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          case 'p':
>            password = apr_pstrdup(pool, arg);
>            break;
> +        case auth_password_fd_opt:
> +          password_fd = svn_cstring_atoi(&password_fd, arg);
> +          break;
>          case 'U':
>            SVN_ERR(svn_utf_cstring_to_utf8(&anchor_url, arg, pool));
>            if (! svn_path_is_url(anchor_url))
> @@ -4587,6 +4593,7 @@ sub_main(int *exit_code, int argc, const char *arg
>                                           non_interactive,
>                                           username,
>                                           password,
> +                                         password_fd,
>                                           config_dir,
>                                           no_auth_cache,
>                                           trust_unknown_ca,





Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] issue #4375: provide --password-fd option

William Orr
El 17/09/17 a las 02:08, Stefan Sperling escribió:

> On Sun, Sep 17, 2017 at 01:06:14AM -0700, William Orr wrote:
>> Hey,
>>
>> This is my first patch to subversion, so please bear with me.
>>
>> This looks to address a very commonly requested feature: providing an
>> alternative for automated tools to provide a password to svn via piping
>> it in over an fd (similar to gnupg).
>
> Hi William,
>
> This looks good to me in principle and I support the idea.
>
> Will the implementation of read_pass_from_fd() compile and work on Win32?
> We usually defer such operations to APR to avoid portability concerns.
> However, APR does not seem to offer an API which wraps fdopen().
> Whenever we bypass APR and use system APIs directly we need to support
> at least Unix-like systems and Win32.
>
> We cannot change the parameter list of svn_cmdline_create_auth_baton2().
> The function is already part of a release, so changing it breaks our
> ABI compatibility promise. Instead, we can add a new function called
> svn_cmdline_create_auth_baton3() which has the additional parameter.
> The svn_cmdline_create_auth_baton2() interface can be implemented
> as a wrapper around our new version of this function.
>
> Given that this is feature intends to support non-interactive usage,
> I wonder if it should depend on the --non-interactive option as well?
> And maybe we could then reduce the new option to --password-from-stdin?
>
> Some tools already use stdin for a different purpose, though.
> E.g. 'svnrdump load' reads a dump file from stdin. But if we also extended
> such tools with an option to read their normal input from a file, we could
> make this idea work. "svnadmin load" already supports the -F (--file)
> option for this purpose. Scripts would have to pass the right combination
> of options: --non-interactive --password-from-stdin -F /tmp/my-dump-file
> If --password-from-stdin is used without --non-interactive and without
> -F then the program should error out and complain.
>
> This idea also solves the portability question, since svn_stream_for_stdin2()
> or apr_file_open_flags_stdin() could be used to read a password from stdin.
> And we wouldn't need a new revision of svn_cmdline_create_auth_baton2()
> either because the client could pass the password as a string, as it
> does for the --password option.
>
>> One outstanding concern that I couldn't find addressed is clearing out
>> memory that once contained passwords (like with memset_s or
>> explicit_bzero). If I missed a technique for doing this that exists in
>> svn already, please let me know so I can update the diff.
>
> I don't think we have an API for that either, unfortunately. However,
> the same portability concerns apply. In any case, it would be great to
> have such an API available in APR.
>
> Regards,
> Stefan
>
>> Tested on Fedora 25 x86_64 and OpenBSD 6.1 x86_64.
>>
>> Please CC me; I'm not on this list.
>>
>> [[[
>> Introduce global opt --password-fd to allow applications to provide a
>> password over an already-opened file descriptor.
>>
>> * subversion/include/svn_cmdline.h
>>   (svn_cmdline_create_auth_baton2): Add `auth_password_fd` argument
>> * subversion/include/svn_error_codes.h
>>   (SVN_ERR_IO_PIPE_READ_ERROR): Undeprecate, as now used
>> * subversion/libsvn_subr/cmdline.c
>>   (read_pass_from_fd): Add static function to get password from a file
>> descriptor
>>   (svn_cmdline_create_auth_baton2): Add `auth_password_fd` arg and
>> trigger read of fd if this arg is not -1
>> * subversion/libsvn_subr/deprecated.c:
>>   (svn_cmdline_create_auth_baton): Add default val of -1 when calling
>> `svn_cmdline_create_auth_baton2`
>> * subversion/svn/svn.c
>>   (svn_cl__longopt_t): Add `opt_auth_password_fd` longopt
>>   (svn_cl__global_options): Add `opt_auth_password_fd` to global options
>>   (sub_main): Process global option `opt_auth_password_fd` and pass it
>> to `svn_cmdline_create_auth_baton2`
>> * subversion/svnmucc/svnmucc.c
>>   (sub_main): Process global option `opt_auth_password_fd` and pass it
>> to `svn_cmdline_create_auth_baton2`
>> * subversion/svnrdump/svnrdump.c
>>   (svn_svnrdump__longopt_t): add `opt_auth_password_fd`
>>   (svnrdump__options): add help message for `--password-fd`
>>   (init_client_context): Pass `auth_password_fd` to
>> `svn_cmdline_create_auth_baton2`
>>   (sub_main): Process global option `opt_auth_password_fd` and pass it
>> to `init_client_context`
>> * subversion/svnsync/svnsync.c
>>   (svnsync__opt): Add `svnsync_opt_source_password_fd` and
>> `svnsync_opt_sync_password_fd`
>>   (svnsync_options): Add help messages for `--source-password-fd` and
>> `--sync-password-fd`
>>   (opt_baton_t): Add `source_password_fd` and `sync_password_fd`
>>   (sub_main): Process global option `--source-password-fd` and
>> `--sync-password-fd` and pass it to `svn_cmdline_create_auth_baton2`
>> invocations
>> * subversion/tests/cmdline/atomic-ra-revprop-change.c
>>   (construct_auth_baton): Pass -1 as the `auth_password_fd`
>> * subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
>>   (): Add new `--password-fd` option to expected output
>> * subversion/tests/libsvn_ra/ra-test.c
>>   (check_tunnel_callback_test): Pass -1 as the `auth_password_fd`
>>   (tunnel_callback_test): Pass -1 as the `auth_password_fd`
>>   (tunnel_run_checkout): Pass -1 as the `auth_password_fd`
>> * subversion/tests/svn_test_main.c
>>   (svn_test__init_auth_baton): Pass -1 as the `auth_password_fd`
>> * tools/client-side/svn-mergeinfo-normalizer/mergeinfo-normalizer.h
>>   (svn_min__opt_state_t): Add `auth_password_fd`
>> * tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c
>>   (svn_min__longopt_t) Add `opt_auth_password_fd`
>>   (sub_main) Process global option `--password-fd` and pass it to
>> `svn_cmdline_create_auth_baton2` invocations
>> * tools/client-side/svnconflict/svnconflict.c
>>   (svnconflict_opt_state_t): Add `auth_password_fd`
>>   (svnconflict_options): Add `--password-fd` documentation
>>   (svnconflict_global_options): Add `opt_auth_password_fd`
>>   (sub_main): Process global option `--password-fd` and pass it to
>> `svn_cmdline_create_auth_baton2` invocations
>> * tools/dev/svnmover/svnmover.c
>>   (sub_main): Process global option `--password-fd` and pass it to
>> `svn_cmdline_create_auth_baton2` invocations
>> ]]]
>
Hey,

Thanks for the feedback. I've finally gotten the chance to update the
patch with the suggestions. Lemme know if this addresses your concerns,
and I can write up the changelog for it.

Thanks,
William Orr


svn-password-fd.patch (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] issue #4375: provide --password-fd option

Stefan Sperling-9
On Sat, Oct 07, 2017 at 03:18:11PM -0700, William Orr wrote:
> Thanks for the feedback. I've finally gotten the chance to update the
> patch with the suggestions. Lemme know if this addresses your concerns,
> and I can write up the changelog for it.
>
> Thanks,
> William Orr
>

Thanks. Some remarks inline below.

> Index: subversion/include/svn_io.h
> ===================================================================
> --- subversion/include/svn_io.h (revisi??n: 1811402)
> +++ subversion/include/svn_io.h (copia de trabajo)
> @@ -2633,6 +2633,14 @@ svn_io_file_readline(apr_file_t *file,
>                       apr_pool_t *result_pool,
>                       apr_pool_t *scratch_pool);
>  
> +/** Reads a string from stdin until a newline or EOF is found
> + *
> + * @since New in 1.9.

This should say "1.10" because our current trunk will become 1.10, not 1.9.

> + */
> +svn_error_t *
> +svn_io_stdin_readline(const char **result,
> +                      apr_pool_t *pool);

Most of our APIs now use a dual-pool idiom: A scratch pool for temporary
data, and a result pool for data which the function will return.
Can we use this idiom here?

> +
>  /** @} */
>  
>  #ifdef __cplusplus
> Index: subversion/libsvn_subr/io.c
> ===================================================================
> --- subversion/libsvn_subr/io.c (revisi??n: 1811402)
> +++ subversion/libsvn_subr/io.c (copia de trabajo)
> @@ -5440,3 +5440,19 @@ svn_io_file_readline(apr_file_t *file,
>  
>    return SVN_NO_ERROR;
>  }
> +
> +svn_error_t *
> +svn_io_stdin_readline(const char **result,
> +                       apr_pool_t *pool)
> +{
> +  svn_stringbuf_t *buf = NULL;
> +  svn_stream_t *stdin = NULL;
> +  svn_boolean_t oob = FALSE;
> +
> +  SVN_ERR(svn_stream_for_stdin2(&stdin, TRUE, pool));

The stream would be stored in the scratch_pool.

> +  SVN_ERR(svn_stream_readline(stdin, &buf, APR_EOL_STR, &oob, pool));

The stringbuf would be stored in the result_pool.

> +
> +  *result = buf->data;
> +
> +  return SVN_NO_ERROR;
> +}
> Index: subversion/svn/cl.h
> ===================================================================
> --- subversion/svn/cl.h (revisi??n: 1811402)
> +++ subversion/svn/cl.h (copia de trabajo)
> @@ -178,6 +178,7 @@ typedef struct svn_cl__opt_state_t
>    svn_boolean_t help;            /* print usage message */
>    const char *auth_username;     /* auth username */
>    const char *auth_password;     /* auth password */
> +  svn_boolean_t auth_password_from_stdin; /* fd to read password from */

The above comment still talks about a file descriptor. Update it?

>    const char *extensions;        /* subprocess extension args */
>    apr_array_header_t *targets;   /* target list from file */
>    svn_boolean_t xml;             /* output in xml, e.g., "svn log --xml" */
> Index: subversion/svn/svn.c
> ===================================================================
> --- subversion/svn/svn.c (revisi??n: 1811402)
> +++ subversion/svn/svn.c (copia de trabajo)
> @@ -68,6 +68,7 @@
>     use the short option letter as identifier.  */
>  typedef enum svn_cl__longopt_t {
>    opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
> +  opt_auth_password_from_stdin,
>    opt_auth_username,
>    opt_autoprops,
>    opt_changelist,
> @@ -200,6 +201,9 @@ const apr_getopt_option_t svn_cl__options[] =
>                      N_("specify a password ARG (caution: on many operating\n"
>                         "                             "
>                         "systems, other users will be able to see this)")},
> +  {"password-from-stdin",
> +                    opt_auth_password_from_stdin, 0,
> +                    N_("read password from stdin")},
>    {"extensions",    'x', 1,
>                      N_("Specify differencing options for external diff or\n"
>                         "                             "
> @@ -495,7 +499,8 @@ const apr_getopt_option_t svn_cl__options[] =
>     command to take these arguments allows scripts to just pass them
>     willy-nilly to every invocation of 'svn') . */
>  const int svn_cl__global_options[] =
> -{ opt_auth_username, opt_auth_password, opt_no_auth_cache, opt_non_interactive,
> +{ opt_auth_username, opt_auth_password, opt_auth_password_from_stdin,
> +  opt_no_auth_cache, opt_non_interactive,
>    opt_force_interactive, opt_trust_server_cert,
>    opt_trust_server_cert_failures,
>    opt_config_dir, opt_config_options, 0
> @@ -1959,6 +1964,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    apr_hash_t *changelists;
>    apr_hash_t *cfg_hash;
>    svn_membuf_t buf;
> +  svn_boolean_t read_pass_from_stdin = FALSE;
>  
>    received_opts = apr_array_make(pool, SVN_OPT_MAX_OPTIONS, sizeof(int));
>  
> @@ -2251,6 +2257,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
>                                          opt_arg, pool));
>          break;
> +      case opt_auth_password_from_stdin:
> +        read_pass_from_stdin = TRUE;
> +        break;
>        case opt_encoding:
>          opt_state.encoding = apr_pstrdup(pool, opt_arg);
>          break;
> @@ -2745,6 +2754,14 @@ sub_main(int *exit_code, int argc, const char *arg
>                                    "--non-interactive"));
>      }
>  
> +  /* --password-from-stdin can only be used with --non-interactive */
> +  if (read_pass_from_stdin && !opt_state.non_interactive)
> +    {
> +      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> +                              _("--password-from-stdin requires "
> +                                "--non-interactive"));
> +    }
> +
>    /* Disallow simultaneous use of both --diff-cmd and
>       --internal-diff.  */
>    if (opt_state.diff.diff_cmd && opt_state.diff.internal_diff)
> @@ -3034,6 +3051,12 @@ sub_main(int *exit_code, int argc, const char *arg
>                                     conflict_stats, pool));
>      }
>  
> +  /* Get password from stdin if necessary */
> +  if (read_pass_from_stdin)
> +    {
> +      svn_io_stdin_readline(&opt_state.auth_password, pool);

This call needs to be wrapped in SVN_ERR().
In developer builds (./configure --enable-maintainer-mode), ignoring an
'svn_error_t' ("error leak") will make the svn binary dump core upon exit.

> +    }
> +
>    /* Set up our cancellation support. */
>    svn_cl__check_cancel = svn_cmdline__setup_cancellation_handler();
>    ctx->cancel_func = svn_cl__check_cancel;
> Index: subversion/svnbench/svnbench.c
> ===================================================================
> --- subversion/svnbench/svnbench.c (revisi??n: 1811402)
> +++ subversion/svnbench/svnbench.c (copia de trabajo)
> @@ -53,6 +53,7 @@
>     use the short option letter as identifier.  */
>  typedef enum svn_cl__longopt_t {
>    opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
> +  opt_auth_password_from_stdin,
>    opt_auth_username,
>    opt_config_dir,
>    opt_config_options,
> @@ -112,6 +113,8 @@ const apr_getopt_option_t svn_cl__options[] =
>    {"verbose",       'v', 0, N_("print extra information")},
>    {"username",      opt_auth_username, 1, N_("specify a username ARG")},
>    {"password",      opt_auth_password, 1, N_("specify a password ARG")},
> +  {"password-from-stdin",
> +                    opt_auth_password_from_stdin, 0, N_("read password from stdin")},
>    {"targets",       opt_targets, 1,
>                      N_("pass contents of file ARG as additional args")},
>    {"depth",         opt_depth, 1,
> @@ -197,7 +200,8 @@ const apr_getopt_option_t svn_cl__options[] =
>     command to take these arguments allows scripts to just pass them
>     willy-nilly to every invocation of 'svn') . */
>  const int svn_cl__global_options[] =
> -{ opt_auth_username, opt_auth_password, opt_no_auth_cache, opt_non_interactive,
> +{ opt_auth_username, opt_auth_password, opt_auth_password_from_stdin,
> +  opt_no_auth_cache, opt_non_interactive,
>    opt_trust_server_cert, opt_trust_server_cert_failures,
>    opt_config_dir, opt_config_options, 0
>  };
> @@ -394,6 +398,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    apr_time_t start_time, time_taken;
>    ra_progress_baton_t ra_progress_baton = {0};
>    svn_membuf_t buf;
> +  svn_boolean_t read_pass_from_stdin = FALSE;
>  
>    received_opts = apr_array_make(pool, SVN_OPT_MAX_OPTIONS, sizeof(int));
>  
> @@ -625,6 +630,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
>                                              opt_arg, pool));
>          break;
> +      case opt_auth_password_from_stdin:
> +        read_pass_from_stdin = TRUE;
> +        break;
>        case opt_stop_on_copy:
>          opt_state.stop_on_copy = TRUE;
>          break;
> @@ -842,6 +850,14 @@ sub_main(int *exit_code, int argc, const char *arg
>                                    "--non-interactive"));
>      }
>  
> +  /* --password-from-stdin can only be used with --non-interactive */
> +  if (read_pass_from_stdin && !opt_state.non_interactive)
> +    {
> +      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> +                              _("--password-from-stdin requires "
> +                                "--non-interactive"));
> +    }
> +
>    /* Ensure that 'revision_ranges' has at least one item, and make
>       'start_revision' and 'end_revision' match that item. */
>    if (opt_state.revision_ranges->nelts == 0)
> @@ -919,6 +935,12 @@ sub_main(int *exit_code, int argc, const char *arg
>                                         pool));
>      }
>  
> +  /* Get password from stdin if necessary */
> +  if (read_pass_from_stdin)
> +    {
> +      svn_io_stdin_readline(&opt_state.auth_password, pool);

This call needs to be wrapped in SVN_ERR().

> +    }
> +
>    /* Set up our cancellation support. */
>    svn_cl__check_cancel = svn_cmdline__setup_cancellation_handler();
>    ctx->cancel_func = svn_cl__check_cancel;
> Index: subversion/svnmucc/svnmucc.c
> ===================================================================
> --- subversion/svnmucc/svnmucc.c (revisi??n: 1811402)
> +++ subversion/svnmucc/svnmucc.c (copia de trabajo)
> @@ -297,6 +297,7 @@ help(FILE *stream, apr_pool_t *pool)
>        "  -F [--file] ARG        : read log message from file ARG\n"
>        "  -u [--username] ARG    : commit the changes as username ARG\n"
>        "  -p [--password] ARG    : use ARG as the password\n"
> +      "  --password-from-stdin  : read password from stdin\n"
>        "  -U [--root-url] ARG    : interpret all action URLs relative to ARG\n"
>        "  -r [--revision] ARG    : use revision ARG as baseline for changes\n"
>        "  --with-revprop ARG     : set revision property in the following format:\n"
> @@ -480,7 +481,8 @@ sub_main(int *exit_code, int argc, const char *arg
>      non_interactive_opt,
>      force_interactive_opt,
>      trust_server_cert_opt,
> -    trust_server_cert_failures_opt
> +    trust_server_cert_failures_opt,
> +    password_from_stdin_opt
>    };
>    static const apr_getopt_option_t options[] = {
>      {"message", 'm', 1, ""},
> @@ -487,6 +489,7 @@ sub_main(int *exit_code, int argc, const char *arg
>      {"file", 'F', 1, ""},
>      {"username", 'u', 1, ""},
>      {"password", 'p', 1, ""},
> +    {"password-from-stdin", password_from_stdin_opt, 0, ""},
>      {"root-url", 'U', 1, ""},
>      {"revision", 'r', 1, ""},
>      {"with-revprop",  with_revprop_opt, 1, ""},
> @@ -527,6 +530,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    svn_client_ctx_t *ctx;
>    struct log_message_baton lmb;
>    int i;
> +  svn_boolean_t read_pass_from_stdin = FALSE;
>  
>    /* Check library versions */
>    SVN_ERR(check_lib_versions());
> @@ -572,6 +576,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          case 'p':
>            password = apr_pstrdup(pool, arg);
>            break;
> +        case password_from_stdin_opt:
> +          read_pass_from_stdin = TRUE;
> +          break;
>          case 'U':
>            SVN_ERR(svn_utf_cstring_to_utf8(&root_url, arg, pool));
>            if (! svn_path_is_url(root_url))
> @@ -672,6 +679,15 @@ sub_main(int *exit_code, int argc, const char *arg
>                                    "--non-interactive"));
>      }
>  
> +  /* --password-from-stdin can only be used with --non-interactive */
> +  if (read_pass_from_stdin && !non_interactive)
> +    {
> +      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> +                              _("--password-from-stdin requires "
> +                                "--non-interactive"));
> +    }
> +
> +
>    /* Copy the rest of our command-line arguments to an array,
>       UTF-8-ing them along the way. */
>    action_args = apr_array_make(pool, opts->argc, sizeof(const char *));
> @@ -721,6 +737,12 @@ sub_main(int *exit_code, int argc, const char *arg
>                                              "svnmucc: ", "--config-option"));
>      }
>  
> +  /* Get password from stdin if necessary */
> +  if (read_pass_from_stdin)
> +    {
> +      svn_io_stdin_readline(&password, pool);

This call needs to be wrapped in SVN_ERR().

> +    }
> +
>    SVN_ERR(svn_client_create_context2(&ctx, cfg_hash, pool));
>  
>    cfg_config = svn_hash_gets(cfg_hash, SVN_CONFIG_CATEGORY_CONFIG);
> Index: subversion/svnrdump/svnrdump.c
> ===================================================================
> --- subversion/svnrdump/svnrdump.c (revisi??n: 1811402)
> +++ subversion/svnrdump/svnrdump.c (copia de trabajo)
> @@ -59,6 +59,7 @@ enum svn_svnrdump__longopt_t
>      opt_config_option,
>      opt_auth_username,
>      opt_auth_password,
> +    opt_auth_password_from_stdin,
>      opt_auth_nocache,
>      opt_non_interactive,
>      opt_skip_revprop,
> @@ -66,7 +67,7 @@ enum svn_svnrdump__longopt_t
>      opt_incremental,
>      opt_trust_server_cert,
>      opt_trust_server_cert_failures,
> -    opt_version
> +    opt_version,

Is the comma here added on purpose? It's not strictly needed.
Since we're tyring to be C89 compatible (yes, that's old), I suspect
adding this comma could cause problems with some compilers.

>    };
>  
>  #define SVN_SVNRDUMP__BASE_OPTIONS opt_config_dir, \
> @@ -73,6 +74,7 @@ enum svn_svnrdump__longopt_t
>                                     opt_config_option, \
>                                     opt_auth_username, \
>                                     opt_auth_password, \
> +                                   opt_auth_password_from_stdin, \
>                                     opt_auth_nocache, \
>                                     opt_trust_server_cert, \
>                                     opt_trust_server_cert_failures, \
> @@ -114,6 +116,8 @@ static const apr_getopt_option_t svnrdump__options
>                        N_("specify a username ARG")},
>      {"password",      opt_auth_password, 1,
>                        N_("specify a password ARG")},
> +    {"password-from-stdin",   opt_auth_password_from_stdin, 0,
> +                      N_("read password from stdin")},
>      {"non-interactive", opt_non_interactive, 0,
>                        N_("do no interactive prompting (default is to prompt\n"
>                           "                             "
> @@ -154,6 +158,7 @@ static const apr_getopt_option_t svnrdump__options
>                         "valid certificate) and 'other' (all other not\n"
>                         "                             "
>                         "separately classified certificate errors).")},
> +    {"dumpfile", 'F', 1, N_("Read or write to a dumpfile instead of stdin/stdout")},
>      {0, 0, 0, 0}
>    };
>  
> @@ -174,6 +179,7 @@ typedef struct opt_baton_t {
>    svn_client_ctx_t *ctx;
>    svn_ra_session_t *session;
>    const char *url;
> +  const char *dumpfile;
>    svn_boolean_t help;
>    svn_boolean_t version;
>    svn_opt_revision_t start_revision;
> @@ -366,7 +372,8 @@ init_client_context(svn_client_ctx_t **ctx_p,
>  
>    /* Default authentication providers for non-interactive use */
>    SVN_ERR(svn_cmdline_create_auth_baton2(&(ctx->auth_baton), non_interactive,
> -                                         username, password, config_dir,
> +                                         username, password,
> +                                         config_dir,

Adding line wrapping here is unnecessary.

>                                           no_auth_cache, trust_unknown_ca,
>                                           trust_cn_mismatch, trust_expired,
>                                           trust_not_yet_valid,
> @@ -463,31 +470,39 @@ replay_revisions(svn_ra_session_t *session,
>                   svn_revnum_t end_revision,
>                   svn_boolean_t quiet,
>                   svn_boolean_t incremental,
> +                 const char *dumpfile,
>                   apr_pool_t *pool)
>  {
>    struct replay_baton *replay_baton;
>    const char *uuid;
> -  svn_stream_t *stdout_stream;
> +  svn_stream_t *dump_stream;

This is very nitpicky of me, but maybe call it 'output_stream'?
In any case, I agree that stdout_stream should be renamed.

>  
> -  SVN_ERR(svn_stream_for_stdout(&stdout_stream, pool));
> +  if (dumpfile)
> +    {
> +      SVN_ERR(svn_stream_open_writable(&dump_stream, dumpfile, pool, pool));
> +    }
> +  else
> +    {
> +      SVN_ERR(svn_stream_for_stdout(&dump_stream, pool));
> +    }
>  
>    replay_baton = apr_pcalloc(pool, sizeof(*replay_baton));
> -  replay_baton->stdout_stream = stdout_stream;
> +  replay_baton->stdout_stream = dump_stream;
>    replay_baton->extra_ra_session = extra_ra_session;
>    replay_baton->quiet = quiet;
>  
>    /* Write the magic header and UUID */
> -  SVN_ERR(svn_stream_printf(stdout_stream, pool,
> +  SVN_ERR(svn_stream_printf(dump_stream, pool,
>                              SVN_REPOS_DUMPFILE_MAGIC_HEADER ": %d\n\n",
>                              SVN_REPOS_DUMPFILE_FORMAT_VERSION));
>    SVN_ERR(svn_ra_get_uuid2(session, &uuid, pool));
> -  SVN_ERR(svn_stream_printf(stdout_stream, pool,
> +  SVN_ERR(svn_stream_printf(dump_stream, pool,
>                              SVN_REPOS_DUMPFILE_UUID ": %s\n\n", uuid));
>  
>    /* Fake revision 0 if necessary */
>    if (start_revision == 0)
>      {
> -      SVN_ERR(dump_revision_header(session, stdout_stream,
> +      SVN_ERR(dump_revision_header(session, dump_stream,
>                                     start_revision, pool));
>  
>        /* Revision 0 has no tree changes, so we're done. */
> @@ -506,7 +521,7 @@ replay_revisions(svn_ra_session_t *session,
>    if (!incremental)
>      {
>        SVN_ERR(dump_initial_full_revision(session, extra_ra_session,
> -                                         stdout_stream, start_revision,
> +                                         dump_stream, start_revision,
>                                           quiet, pool));
>        start_revision++;
>      }
> @@ -538,16 +553,23 @@ replay_revisions(svn_ra_session_t *session,
>  static svn_error_t *
>  load_revisions(svn_ra_session_t *session,
>                 svn_ra_session_t *aux_session,
> -               const char *url,
> +               const char *dumpfile,
>                 svn_boolean_t quiet,
>                 apr_hash_t *skip_revprops,
>                 apr_pool_t *pool)
>  {
> -  svn_stream_t *stdin_stream;
> +  svn_stream_t *dump_stream;
>  
> -  SVN_ERR(svn_stream_for_stdin2(&stdin_stream, TRUE, pool));
> +  if (dumpfile)
> +    {
> +      SVN_ERR(svn_stream_open_readonly(&dump_stream, dumpfile, pool, pool));
> +    }
> +  else
> +    {
> +      SVN_ERR(svn_stream_for_stdin2(&dump_stream, TRUE, pool));
> +    }
>  
> -  SVN_ERR(svn_rdump__load_dumpstream(stdin_stream, session, aux_session,
> +  SVN_ERR(svn_rdump__load_dumpstream(dump_stream, session, aux_session,
>                                       quiet, skip_revprops,
>                                       check_cancel, NULL, pool));
>  
> @@ -616,7 +638,8 @@ dump_cmd(apr_getopt_t *os,
>    return replay_revisions(opt_baton->session, extra_ra_session,
>                            opt_baton->start_revision.value.number,
>                            opt_baton->end_revision.value.number,
> -                          opt_baton->quiet, opt_baton->incremental, pool);
> +                          opt_baton->quiet, opt_baton->incremental,
> +                          opt_baton->dumpfile, pool);
>  }
>  
>  /* Handle the "load" subcommand.  Implements `svn_opt_subcommand_t'.  */
> @@ -630,8 +653,9 @@ load_cmd(apr_getopt_t *os,
>  
>    SVN_ERR(svn_client_open_ra_session2(&aux_session, opt_baton->url, NULL,
>                                        opt_baton->ctx, pool, pool));
> -  return load_revisions(opt_baton->session, aux_session, opt_baton->url,
> -                        opt_baton->quiet, opt_baton->skip_revprops, pool);
> +  return load_revisions(opt_baton->session, aux_session,
> +                        opt_baton->dumpfile, opt_baton->quiet,
> +                        opt_baton->skip_revprops, pool);
>  }
>  
>  /* Handle the "help" subcommand.  Implements `svn_opt_subcommand_t'.  */
> @@ -772,6 +796,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    apr_getopt_t *os;
>    apr_array_header_t *received_opts;
>    int i;
> +  svn_boolean_t read_pass_from_stdin = FALSE;
>  
>    opt_baton = apr_pcalloc(pool, sizeof(*opt_baton));
>    opt_baton->start_revision.kind = svn_opt_revision_unspecified;
> @@ -778,6 +803,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    opt_baton->end_revision.kind = svn_opt_revision_unspecified;
>    opt_baton->url = NULL;
>    opt_baton->skip_revprops = apr_hash_make(pool);
> +  opt_baton->dumpfile = NULL;
>  
>    SVN_ERR(svn_cmdline__getopt_init(&os, argc, argv, pool));
>  
> @@ -850,6 +876,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          case opt_auth_password:
>            SVN_ERR(svn_utf_cstring_to_utf8(&password, opt_arg, pool));
>            break;
> +        case opt_auth_password_from_stdin:
> +          read_pass_from_stdin = TRUE;
> +          break;
>          case opt_auth_nocache:
>            no_auth_cache = TRUE;
>            break;
> @@ -890,6 +919,11 @@ sub_main(int *exit_code, int argc, const char *arg
>                                                       opt_arg,
>                                                       "svnrdump: ",
>                                                       pool));
> +          break;
> +        case 'F':
> +          SVN_ERR(svn_utf_cstring_to_utf8(&opt_arg, opt_arg, pool));
> +          opt_baton->dumpfile = opt_arg;
> +          break;
>          }
>      }
>  
> @@ -1005,6 +1039,20 @@ sub_main(int *exit_code, int argc, const char *arg
>                                    "--non-interactive"));
>      }
>  
> +  if (read_pass_from_stdin && !non_interactive)
> +    {
> +      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> +                              _("--password-from-stdin requires "
> +                                "--non-interactive"));
> +    }
> +
> +  if (strcmp(subcommand->name, "load") == 0)
> +    {
> +      if (read_pass_from_stdin)
> +        {
> +        }

There seems to be some code missing above.

> +    }
> +
>    /* Expect one more non-option argument:  the repository URL. */
>    if (os->ind != os->argc - 1)
>      {
> @@ -1039,9 +1087,16 @@ sub_main(int *exit_code, int argc, const char *arg
>          force_interactive = (username == NULL || password == NULL);
>      }
>  
> +  /* Get password from stdin if necessary */
> +  if (read_pass_from_stdin)
> +    {
> +      svn_io_stdin_readline(&password, pool);

The above call needs to be wrapped in SVN_ERR().

> +    }
> +
>    non_interactive = !svn_cmdline__be_interactive(non_interactive,
>                                                   force_interactive);
>  
> +

Adding an empty line here is not necessary.

>    SVN_ERR(init_client_context(&(opt_baton->ctx),
>                                non_interactive,
>                                username,
> Index: subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
> ===================================================================
> --- subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout (revisi??n: 1811402)
> +++ subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout (copia de trabajo)
> @@ -134,6 +134,7 @@ Global options:
>    --username ARG           : specify a username ARG
>    --password ARG           : specify a password ARG (caution: on many operating
>                               systems, other users will be able to see this)
> +  --password-from-stdin    : read password from stdin
>    --no-auth-cache          : do not cache authentication tokens
>    --non-interactive        : do no interactive prompting (default is to prompt
>                               only if standard input is a terminal device)
> @@ -224,6 +225,7 @@ Global options:
>    --username ARG           : specify a username ARG
>    --password ARG           : specify a password ARG (caution: on many operating
>                               systems, other users will be able to see this)
> +  --password-from-stdin    : read password from stdin
>    --no-auth-cache          : do not cache authentication tokens
>    --non-interactive        : do no interactive prompting (default is to prompt
>                               only if standard input is a terminal device)
> Index: tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c
> ===================================================================
> --- tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c (revisi??n: 1811402)
> +++ tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c (copia de trabajo)
> @@ -68,6 +68,7 @@
>     use the short option letter as identifier.  */
>  typedef enum svn_min__longopt_t {
>    opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
> +  opt_auth_password_from_stdin,
>    opt_auth_username,
>    opt_config_dir,
>    opt_config_options,
> @@ -113,6 +114,9 @@ const apr_getopt_option_t svn_min__options[] =
>                      N_("specify a password ARG (caution: on many operating\n"
>                         "                             "
>                         "systems, other users will be able to see this)")},
> +  {"password-from-stdin",
> +                    opt_auth_password_from_stdin, 0,
> +                    N_("read password from stdin")},
>    {"targets",       opt_targets, 1,
>                      N_("pass contents of file ARG as additional args")},
>    {"depth",         opt_depth, 1,
> @@ -209,11 +213,11 @@ const apr_getopt_option_t svn_min__options[] =
>     command to take these arguments allows scripts to just pass them
>     willy-nilly to every invocation of 'svn') . */
>  const int svn_min__global_options[] =
> -{ opt_auth_username, opt_auth_password, opt_no_auth_cache, opt_non_interactive,
> -  opt_force_interactive, opt_trust_server_cert,
> -  opt_trust_server_cert_unknown_ca, opt_trust_server_cert_cn_mismatch,
> -  opt_trust_server_cert_expired, opt_trust_server_cert_not_yet_valid,
> -  opt_trust_server_cert_other_failure,
> +{ opt_auth_username, opt_auth_password, opt_auth_password_from_stdin,
> +  opt_no_auth_cache, opt_non_interactive, opt_force_interactive,
> +  opt_trust_server_cert, opt_trust_server_cert_unknown_ca,
> +  opt_trust_server_cert_cn_mismatch, opt_trust_server_cert_expired,
> +  opt_trust_server_cert_not_yet_valid, opt_trust_server_cert_other_failure,
>    opt_config_dir, opt_config_options, 0
>  };
>  
> @@ -417,6 +421,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    svn_boolean_t interactive_conflicts = FALSE;
>    svn_boolean_t force_interactive = FALSE;
>    apr_hash_t *cfg_hash;
> +  svn_boolean_t read_pass_from_stdin = FALSE;
>  
>    received_opts = apr_array_make(pool, SVN_OPT_MAX_OPTIONS, sizeof(int));
>  
> @@ -528,6 +533,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
>                                          opt_arg, pool));
>          break;
> +      case opt_auth_password_from_stdin:
> +        read_pass_from_stdin = TRUE;
> +        break;
>        case opt_no_auth_cache:
>          opt_state.no_auth_cache = TRUE;
>          break;
> @@ -606,6 +614,14 @@ sub_main(int *exit_code, int argc, const char *arg
>                                    opt_state.non_interactive,
>                                    force_interactive);
>  
> +  /* --password-from-stdin can only be used with --non-interactive */
> +  if (read_pass_from_stdin && !opt_state.non_interactive)
> +    {
> +      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> +                              _("--password-from-stdin requires "
> +                                "--non-interactive"));
> +    }
> +
>    /* ### This really belongs in libsvn_client.  The trouble is,
>       there's no one place there to run it from, no
>       svn_client_init().  We'd have to add it to all the public
> @@ -788,6 +804,12 @@ sub_main(int *exit_code, int argc, const char *arg
>    }
>  #endif
>  
> +  /* Get password from stdin if necessary */
> +  if (read_pass_from_stdin)
> +    {
> +      svn_io_stdin_readline(&opt_state.auth_password, pool);

The above call needs to be wrapped in SVN_ERR().

> +    }
> +
>    /* Create a client context object. */
>    command_baton.opt_state = &opt_state;
>    SVN_ERR(svn_client_create_context2(&ctx, cfg_hash, pool));
> Index: tools/client-side/svnconflict/svnconflict.c
> ===================================================================
> --- tools/client-side/svnconflict/svnconflict.c (revisi??n: 1811402)
> +++ tools/client-side/svnconflict/svnconflict.c (copia de trabajo)
> @@ -78,6 +78,7 @@ typedef struct svnconflict_cmd_baton_t
>     use the short option letter as identifier.  */
>  typedef enum svnconflict_longopt_t {
>    opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
> +  opt_auth_password_from_stdin,
>    opt_auth_username,
>    opt_config_dir,
>    opt_config_options,
> @@ -96,6 +97,9 @@ static const apr_getopt_option_t svnconflict_optio
>                      N_("specify a password ARG (caution: on many operating\n"
>                         "                             "
>                         "systems, other users will be able to see this)")},
> +  {"password-from-stdin",
> +                    opt_auth_password_from_stdin, 0,
> +                    N_("read password from stdin")},
>    {"config-dir",    opt_config_dir, 1,
>                      N_("read user configuration files from directory ARG")},
>    {"config-option", opt_config_options, 1,
> @@ -141,7 +145,8 @@ static svn_error_t * svnconflict_resolve_tree(apr_
>  
>  /* Options that apply to all commands. */
>  static const int svnconflict_global_options[] =
> -{ opt_auth_username, opt_auth_password, opt_config_dir, opt_config_options, 0 };
> +{ opt_auth_username, opt_auth_password, opt_auth_password_from_stdin,
> +  opt_config_dir, opt_config_options, 0 };
>  
>  static const svn_opt_subcommand_desc2_t svnconflict_cmd_table[] =
>  {
> @@ -639,6 +644,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    svn_auth_baton_t *ab;
>    svn_config_t *cfg_config;
>    apr_hash_t *cfg_hash;
> +  svn_boolean_t read_pass_from_stdin = FALSE;
>  
>    received_opts = apr_array_make(pool, SVN_OPT_MAX_OPTIONS, sizeof(int));
>  
> @@ -704,6 +710,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
>                                          opt_arg, pool));
>          break;
> +      case opt_auth_password_from_stdin:
> +        read_pass_from_stdin = TRUE;
> +        break;
>        case opt_config_dir:
>          SVN_ERR(svn_utf_cstring_to_utf8(&utf8_opt_arg, opt_arg, pool));
>          opt_state.config_dir = svn_dirent_internal_style(utf8_opt_arg, pool);
> @@ -845,6 +854,13 @@ sub_main(int *exit_code, int argc, const char *arg
>  
>    cfg_config = svn_hash_gets(cfg_hash, SVN_CONFIG_CATEGORY_CONFIG);
>  
> +  /* Get password from stdin if necessary */
> +  if (read_pass_from_stdin)
> +    {
> +      svn_io_stdin_readline(&opt_state.auth_password, pool);

The above call needs to be wrapped in SVN_ERR().

> +    }
> +
> +
>    /* Create a client context object. */
>    command_baton.opt_state = &opt_state;
>    SVN_ERR(svn_client_create_context2(&ctx, cfg_hash, pool));
> Index: tools/dev/svnmover/svnmover.c
> ===================================================================
> --- tools/dev/svnmover/svnmover.c (revisi??n: 1811402)
> +++ tools/dev/svnmover/svnmover.c (copia de trabajo)
> @@ -4332,7 +4332,8 @@ sub_main(int *exit_code, int argc, const char *arg
>      trust_server_cert_opt,
>      trust_server_cert_failures_opt,
>      ui_opt,
> -    colour_opt
> +    colour_opt,
> +    auth_password_from_stdin_opt
>    };
>    static const apr_getopt_option_t options[] = {
>      {"verbose", 'v', 0, ""},
> @@ -4341,6 +4342,7 @@ sub_main(int *exit_code, int argc, const char *arg
>      {"file", 'F', 1, ""},
>      {"username", 'u', 1, ""},
>      {"password", 'p', 1, ""},
> +    {"password-from-stdin", auth_password_from_stdin_opt, 1, ""},
>      {"root-url", 'U', 1, ""},
>      {"revision", 'r', 1, ""},
>      {"branch-id", 'B', 1, ""},
> @@ -4387,6 +4389,7 @@ sub_main(int *exit_code, int argc, const char *arg
>    const char *log_msg;
>    svn_tristate_t coloured_output = svn_tristate_false;
>    svnmover_wc_t *wc;
> +  svn_boolean_t read_pass_from_stdin = FALSE;
>  
>    /* Check library versions */
>    SVN_ERR(check_lib_versions());
> @@ -4431,6 +4434,9 @@ sub_main(int *exit_code, int argc, const char *arg
>          case 'p':
>            password = apr_pstrdup(pool, arg);
>            break;
> +        case auth_password_from_stdin_opt:
> +          read_pass_from_stdin = TRUE;
> +          break;
>          case 'U':
>            SVN_ERR(svn_utf_cstring_to_utf8(&anchor_url, arg, pool));
>            if (! svn_path_is_url(anchor_url))
> @@ -4553,6 +4559,13 @@ sub_main(int *exit_code, int argc, const char *arg
>                                    "--non-interactive"));
>      }
>  
> +  /* --password-from-stdin can only be used with --non-interactive */
> +  if (read_pass_from_stdin && !non_interactive)
> +    {
> +      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> +                              _("--password-from-stdin requires "
> +                                "--non-interactive"));
> +    }
>  
>    /* Now initialize the client context */
>  
> @@ -4580,6 +4593,12 @@ sub_main(int *exit_code, int argc, const char *arg
>                                              "svnmover: ", "--config-option"));
>      }
>  
> +  /* Get password from stdin if necessary */
> +  if (read_pass_from_stdin)
> +    {
> +      svn_io_stdin_readline(&password, pool);

The above call needs to be wrapped in SVN_ERR().

> +    }
> +
>    SVN_ERR(svn_client_create_context2(&ctx, cfg_hash, pool));
>  
>    cfg_config = svn_hash_gets(cfg_hash, SVN_CONFIG_CATEGORY_CONFIG);