Markus Schaber wrote:
> Julian Foad wrote:
>> "specify a password ARG (insecure: on many systems,
>> other users can read the command-line arguments)"
>
> I fully agree with your concerns about vague warnings. But in my eyes,
> it is also important to point out the alternatives, so that the users
> have an easy way to use them.
and GBG wrote (to me):
> It still leaves a question --- why? And ppl might think, oh, not in linux,
> I should be safe (remembering that svn is cross platform).
> How about something like: (insecure on any OS that hosts other users) ?
We want to say, very briefly (the command-line help isn't the place for a full
explanation):
- that using this option might be insecure;
- enough of a clue about how/why so user can make an informed choice or know
what questions to ask if they want to look for more information.
In the interests of getting something in place, I have just committed the
following help text in r1612230:
--password ARG : specify a password ARG (caution: on many operating
systems, other users will be able to see this)
(I used "caution:" as I think "insecure" is too context-dependent and
judgemental.)
Improvements are welcome, of course.
Still TODO:
* write a fuller explanation in The Book
* add a '--password-file' option
* (maybe) blank out the --password argument after reading it, when possible
I don't plan to work on The Book, or on blanking; contributions are welcome.
I looked at committing a password-file option. A version based on Markus
Schaber's recent patches is attached. (It's his patch, minus the
password-env-var option, plus a test suite fix.)
Looking at how the 'rsync' program describes its '--password-file' option:$ man
rsync
--password-file
This option allows you to provide a password in a file for accessing
an rsync daemon. The file must not be world readable. It should
contain just the password as the first line of the file (all other
lines are ignored).
Two things we might want to do, that rsync does:
1. Read only the first line of the file, up to but not including a newline.
"Not including the newline" is the important part here, I think: the patch, as
is, assumes that any newline in the file is part of the password, which in a
typical case then fails to work.
(The first thing I tried was "echo my-password > foo; svn --password-file=foo
...".)
2. It says the file must not be world-readable. That seems a sensible
precaution. I think we should do that too, on operating systems
where that makes sense.
Thoughts?
- Julian
Add a new global option '--password-file'.
TODO (noted from reading about rsync's --password-file option):
* Read only the first line of the file, up to but not including a newline.
* The file must not be world readable.
Patch by: schabi
me
* subversion/svn/svn.c
(svn_cl__longopt_t, svn_cl__global_options): Add opt_auth_password_file.
(svn_cl__options): Add the '--password-file' option.
(sub_main): Add opt_auth_password_file. Check for multiple use of stdin.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Update the expected output accordingly.
--This line, and those below, will be ignored--
Index: subversion/svn/svn.c
===================================================================
--- subversion/svn/svn.c (revision 1612230)
+++ subversion/svn/svn.c (working copy)
@@ -65,12 +65,13 @@
/* Add an identifier here for long options that don't have a short
option. Options that have both long and short options should just
use the short option letter as identifier. */
typedef enum svn_cl__longopt_t {
opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
+ opt_auth_password_file,
opt_auth_username,
opt_autoprops,
opt_changelist,
opt_config_dir,
opt_config_options,
/* diff options */
@@ -192,12 +193,14 @@ const apr_getopt_option_t svn_cl__option
{"show-updates", 'u', 0, N_("display update information")},
{"username", opt_auth_username, 1, N_("specify a username ARG")},
{"password", opt_auth_password, 1,
N_("specify a password ARG (caution: on many operating\n"
" "
"systems, other users will be able to see this)")},
+ {"password-file", opt_auth_password_file, 1,
+ N_("read a password from the specified file ARG")},
{"extensions", 'x', 1,
N_("Specify differencing options for external diff or\n"
" "
"internal diff or blame. Default: '-u'. Options are\n"
" "
"separated by spaces. Internal diff and blame take:\n"
@@ -423,13 +426,14 @@ const apr_getopt_option_t svn_cl__option
/* Options that apply to all commands. (While not every command may
currently require authentication or be interactive, allowing every
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_file,
+ opt_no_auth_cache, opt_non_interactive,
opt_force_interactive, opt_trust_server_cert, opt_config_dir,
opt_config_options, 0
};
/* Options for giving a log message. (Some of these also have other uses.)
*/
@@ -2018,22 +2022,37 @@ sub_main(int *exit_code, int argc, const
break;
case 'F':
/* We read the raw file content here. We will convert it to UTF-8
* later (if it's a log/lock message or an svn:* prop value),
* according to the value of the '--encoding' option. */
SVN_ERR(svn_utf_cstring_to_utf8(&utf8_opt_arg, opt_arg, pool));
+ if (strcmp(utf8_opt_arg, "-") == 0)
+ {
+ if (reading_file_from_stdin)
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("stdin ('-') must not be specified "
+ "more than once"));
+ reading_file_from_stdin = TRUE;
+ }
SVN_ERR(svn_stringbuf_from_file2(&(opt_state.filedata),
utf8_opt_arg, pool));
- reading_file_from_stdin = (strcmp(utf8_opt_arg, "-") == 0);
dash_F_arg = utf8_opt_arg;
break;
case opt_targets:
{
svn_stringbuf_t *buffer, *buffer_utf8;
SVN_ERR(svn_utf_cstring_to_utf8(&utf8_opt_arg, opt_arg, pool));
+ if (strcmp(utf8_opt_arg, "-") == 0)
+ {
+ if (reading_file_from_stdin)
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("stdin ('-') must not be specified "
+ "more than once"));
+ reading_file_from_stdin = TRUE;
+ }
SVN_ERR(svn_stringbuf_from_file2(&buffer, utf8_opt_arg, pool));
SVN_ERR(svn_utf_stringbuf_to_utf8(&buffer_utf8, buffer, pool));
opt_state.targets = svn_cstring_split(buffer_utf8->data, "\n\r",
TRUE, pool);
}
break;
@@ -2097,12 +2116,30 @@ sub_main(int *exit_code, int argc, const
opt_arg, pool));
break;
case opt_auth_password:
SVN_ERR(svn_utf_cstring_to_utf8(&opt_state.auth_password,
opt_arg, pool));
break;
+ case opt_auth_password_file:
+ {
+ svn_stringbuf_t *buffer, *buffer_utf8;
+
+ SVN_ERR(svn_utf_cstring_to_utf8(&utf8_opt_arg, opt_arg, pool));
+ if (strcmp(utf8_opt_arg, "-") == 0)
+ {
+ if (reading_file_from_stdin)
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("stdin ('-') must not be specified "
+ "more than once"));
+ reading_file_from_stdin = TRUE;
+ }
+ SVN_ERR(svn_stringbuf_from_file2(&buffer, utf8_opt_arg, pool));
+ SVN_ERR(svn_utf_stringbuf_to_utf8(&buffer_utf8, buffer, pool));
+ opt_state.auth_password = buffer_utf8->data;
+ }
+ break;
case opt_encoding:
opt_state.encoding = apr_pstrdup(pool, opt_arg);
break;
case opt_xml:
opt_state.xml = TRUE;
break;
Index: subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
===================================================================
--- subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
(revision 1612234)
+++ subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
(working copy)
@@ -114,12 +114,13 @@ Valid options:
--search-and ARG : combine ARG with the previous search pattern
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-file ARG : read a password from the specified file 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)
--force-interactive : do interactive prompting even if standard input
is not a terminal device
--trust-server-cert : accept SSL server certificates from unknown
@@ -198,12 +199,13 @@ Valid options:
(shorthand: 'p', 'mc', 'tc', 'mf', 'tf', 'e', 'l')
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-file ARG : read a password from the specified file 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)
--force-interactive : do interactive prompting even if standard input
is not a terminal device
--trust-server-cert : accept SSL server certificates from unknown
