From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id E18A645005 for ; Mon, 19 Dec 2022 13:33:24 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DB74068B586; Mon, 19 Dec 2022 15:33:21 +0200 (EET) Received: from mail-ej1-f48.google.com (mail-ej1-f48.google.com [209.85.218.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5D8376802B0 for ; Mon, 19 Dec 2022 15:33:15 +0200 (EET) Received: by mail-ej1-f48.google.com with SMTP id kw15so21419189ejc.10 for ; Mon, 19 Dec 2022 05:33:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=cxmfCOG1xIuOwukzS9N7v3MGnFr+KjLQbsSHsjP51CM=; b=dGu9VQUCO+/4r1l15hkbgdw+tRHvw87+4BSByPNJKNi0nif/Zi8+Wggxd/NJ8AJnpB xi5dUVfp6ADA4fSydsTSUg2bkN6ZTndFuvhdMVOPLd/xlfk+ADYnRytOcGvYAm7h9qeJ SBbiD/h06RCk14raBLs/jQFtSZB5CEpPqK6lUFeoXzfUbsHiG3MWrWWCSWYmqVBt3+zn Nk2VpCqemFejICxSXzSLrYpN6Pw2+l0WFdUjWxzpmW+BHUQDqoPiBaw/WbypOVYrTzcf umFuVlEqloaYMzjfrY1y4KroSbWh7PkGU2k96q+gEs6ZSbi4Lqkz3L0XKH3GNfPv91ju F8xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cxmfCOG1xIuOwukzS9N7v3MGnFr+KjLQbsSHsjP51CM=; b=vl/mN1OpvRzKLijhlQRAhTcUhIwXgREXhsUBt1MfVJIGWncek/97MNOj2tHZuvR9R6 4TW6SL4xxjRrLtcCEyFvrGV/Odk0NfKgrPrhiFZKokIkgzMUM8f6wv8+6c13QgaIVaYm +vhTl7sPgJhWG4F09I13CRZB1wBpDpbu09q7yG3fzRl+UehvE7yB8rp3zsHVyVXWIuJt qlYG/RqKY9fT2tqGU+IVerf+ByFukY2GaEODCMSYJZhZd1K30WfzOFAVESDXQN8DSiyM zYf5Uzoj88KXCNTZl7ZRzw5LLXwFPEOcFQRbSbY/fBd7AzbrH+S+XEe1wVP9//zwtRJV O0Ig== X-Gm-Message-State: ANoB5pmS8pAawvUfqvO0HGa1vxJ+yg2hlP6vwAytxOvyJfPT4iNFswXz EnLJuTmxH9gzcbVvXosLwvM5Paahhq4LEg== X-Google-Smtp-Source: AA0mqf7LbcTMFK4grX6jLnI2isl1EHG72xGl4caFkxeB/U5wPOPoxFzsPMsjb3aMf+Ki8rM74R/pvw== X-Received: by 2002:a17:907:76d4:b0:7c0:e5c8:d439 with SMTP id kf20-20020a17090776d400b007c0e5c8d439mr36652864ejc.3.1671456794687; Mon, 19 Dec 2022 05:33:14 -0800 (PST) Received: from [192.168.178.30] (dynamic-2a01-0c22-3482-a300-c5ce-abfc-55b6-93b5.c22.pool.telefonica.de. [2a01:c22:3482:a300:c5ce:abfc:55b6:93b5]) by smtp.gmail.com with ESMTPSA id gy19-20020a170906f25300b007bf24b8f80csm4404308ejb.63.2022.12.19.05.33.13 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 19 Dec 2022 05:33:14 -0800 (PST) From: Marvin Scholz To: FFmpeg development discussions and patches Date: Mon, 19 Dec 2022 14:33:12 +0100 X-Mailer: MailMate (1.14r5898) Message-ID: In-Reply-To: <7407e74b181e4e00a7b7104fb63cf56a@huawei.com> References: <7407e74b181e4e00a7b7104fb63cf56a@huawei.com> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*) X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 19 Dec 2022, at 14:15, Wujian(Chin) wrote: > I have modified the issues. Please review it again. Thank you. > > If the protocol address contains the user name and password, The ps -ef command exposes plaintext. > The -mask_url parameter option is added to replace the protocol address in the command line with the asterisk (*). > Because other users can run the ps -ef command to view sensitive information such as the user name and password > in the protocol address, which is insecure. > > Signed-off-by: wujian_nanjing > --- > doc/ffmpeg.texi | 9 +++++++++ > doc/ffplay.texi | 8 ++++++++ > doc/ffprobe.texi | 9 +++++++++ > fftools/cmdutils.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- > fftools/cmdutils.h | 15 +++++++++++++++ > fftools/ffmpeg.c | 16 +++++++++++++--- > fftools/ffplay.c | 15 +++++++++++++-- > fftools/ffprobe.c | 18 ++++++++++++++---- > 8 files changed, 124 insertions(+), 13 deletions(-) > > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi > index 0367930..1f6cb33 100644 > --- a/doc/ffmpeg.texi > +++ b/doc/ffmpeg.texi > @@ -50,6 +50,15 @@ output files. Also do not mix options which belong to different files. All > options apply ONLY to the next input or output file and are reset between files. > > @itemize > +@item -mask_url -i @var{url} (@emph{output}) > +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. > +The -mask_url parameter option is added to replace the protocol address in the command line with the asterisk (*). > +Because other users can run the ps -ef command to view sensitive information such as the user name and password > +in the protocol address, which is insecure. > +@example > +ffmpeg -mask_url -i rtsp://username:password-ip:port/stream/test > +@end example > + > @item > To set the video bitrate of the output file to 64 kbit/s: > @example > diff --git a/doc/ffplay.texi b/doc/ffplay.texi > index 5dd860b..b40fe75 100644 > --- a/doc/ffplay.texi > +++ b/doc/ffplay.texi > @@ -120,8 +120,16 @@ sources and sinks). > Read @var{input_url}. > @end table > > +@item -mask_url -i @var{url} (@emph{output}) > +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. > +The -mask_url parameter option is added to replace the protocol address in the command line with the asterisk (*). > +Because other users can run the ps -ef command to view sensitive information such as the user name and password > +in the protocol address, which is insecure. > +@end table > + > @section Advanced options > @table @option > + > @item -stats > Print several playback statistics, in particular show the stream > duration, the codec parameters, the current position in the stream and > diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi > index 4dc9f57..33c0e7d 100644 > --- a/doc/ffprobe.texi > +++ b/doc/ffprobe.texi > @@ -89,6 +89,15 @@ Set the output printing format. > @var{writer_name} specifies the name of the writer, and > @var{writer_options} specifies the options to be passed to the writer. > > +@item -mask_url -i @var{url} (@emph{output}) > +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. > +The -mask_url parameter option is added to replace the protocol address in the command line with the asterisk (*). > +Because other users can run the ps -ef command to view sensitive information such as the user name and password > +in the protocol address, which is insecure. > +@example > +ffprobe -mask_url -i rtsp://username:password-ip:port/stream/test > +@end example > + > For example for printing the output in JSON format, specify: > @example > -print_format json > diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c > index a1de621..c35d7e1 100644 > --- a/fftools/cmdutils.c > +++ b/fftools/cmdutils.c > @@ -61,6 +61,40 @@ AVDictionary *format_opts, *codec_opts; > > int hide_banner = 0; > > +void param_masking(int argc, char **argv) { > + int i, j; > + for (i = 1; i < argc; i++) { > + char *match = strstr(argv[i], "://"); > + if (match) { > + int total = strlen(argv[i]); > + for (j = 0; j < total; j++) { > + argv[i][j] = '*'; > + } > + } > + } > +} > + > +char **copy_argv(int argc, char **argv) { > + char **argv2; > + argv2 = av_mallocz(argc * sizeof(char *)); > + if (!argv2) > + exit_program(1); > + > + for (int i = 0; i < argc; i++) { > + int length = strlen(argv[i]) + 1; > + argv2[i] = av_mallocz(length * sizeof(char *)); > + if (!argv2[i]) > + exit_program(1); > + memcpy(argv2[i], argv[i], length - 1); > + } > + return argv2; > +} > + > +void free_pp(int argc, char **argv) { > + for (int i = 0; i < argc; i++) > + av_free(argv[i]); > + av_free(argv); > +} > void uninit_opts(void) > { > av_dict_free(&swr_opts); > @@ -215,13 +249,13 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr) > if (win32_argv_utf8) { > *argc_ptr = win32_argc; > *argv_ptr = win32_argv_utf8; > - return; > + goto end; > } > > win32_argc = 0; > argv_w = CommandLineToArgvW(GetCommandLineW(), &win32_argc); > if (win32_argc <= 0 || !argv_w) > - return; > + goto end; > > /* determine the UTF-8 buffer size (including NULL-termination symbols) */ > for (i = 0; i < win32_argc; i++) > @@ -232,7 +266,7 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr) > argstr_flat = (char *)win32_argv_utf8 + sizeof(char *) * (win32_argc + 1); > if (!win32_argv_utf8) { > LocalFree(argv_w); > - return; > + goto end; > } > > for (i = 0; i < win32_argc; i++) { > @@ -243,9 +277,14 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr) > } > win32_argv_utf8[i] = NULL; > LocalFree(argv_w); > - > *argc_ptr = win32_argc; > *argv_ptr = win32_argv_utf8; > +end: > + if (*argc_ptr > 1 && !strcmp((*argv_ptr)[1], "-mask_url")) { > + (*argv_ptr)[1] = (*argv_ptr)[0]; > + (*argc_ptr)--; > + (*argv_ptr)++; > + } IIUC this means the `-mask_url` option has to be the first option passed, which seems a bit of an unfortunate requirement and is not documented at all, as far as I can see. So at least this should be clearly documented to prevent users being confused why the get an unrecognised option error when they do not pass it as the first option. > } > #else > static inline void prepare_app_arguments(int *argc_ptr, char ***argv_ptr) > diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h > index 4496221..ce4c1db 100644 > --- a/fftools/cmdutils.h > +++ b/fftools/cmdutils.h > @@ -50,6 +50,21 @@ extern AVDictionary *format_opts, *codec_opts; > extern int hide_banner; > > /** > + * Using to masking sensitive info. > + */ > +void param_masking(int argc, char **argv); > + > +/** > + * Using to copy ori argv. > + */ > +char **copy_argv(int argc, char **argv); > + > +/** > + * Free ** > + */ > +void free_pp(int argc, char **argv); > + > +/** > * Register a program-specific cleanup routine. > */ > void register_exit(void (*cb)(int ret)); > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 881d6f0..fccbde9 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -3865,9 +3865,9 @@ static int64_t getmaxrss(void) > > int main(int argc, char **argv) > { > - int ret; > + int ret, maskFlag; > BenchmarkTimeStamps ti; > - > + char **argv2; > init_dynload(); > > register_exit(ffmpeg_cleanup); > @@ -3877,15 +3877,25 @@ int main(int argc, char **argv) > av_log_set_flags(AV_LOG_SKIP_REPEATED); > parse_loglevel(argc, argv, options); > > + maskFlag = 0; > + if (argc > 1 && !strcmp(argv[1], "-mask_url")) { > + argv[1] = argv[0]; > + maskFlag = 1; > + argc--; > + argv++; > + } > #if CONFIG_AVDEVICE > avdevice_register_all(); > #endif > avformat_network_init(); > > show_banner(argc, argv, options); > + argv2 = copy_argv(argc, argv); > + if (maskFlag) > + param_masking(argc, argv); > > /* parse options and open all input/output files */ > - ret = ffmpeg_parse_options(argc, argv); > + ret = ffmpeg_parse_options(argc, argv2); > if (ret < 0) > exit_program(1); > > diff --git a/fftools/ffplay.c b/fftools/ffplay.c > index fc7e1c2..5d282f1 100644 > --- a/fftools/ffplay.c > +++ b/fftools/ffplay.c > @@ -3663,10 +3663,18 @@ void show_help_default(const char *opt, const char *arg) > /* Called from the main */ > int main(int argc, char **argv) > { > - int flags; > + int flags, maskFlag; > + char **argv2; > VideoState *is; > > init_dynload(); > + maskFlag = 0; > + if (argc > 1 && !strcmp(argv[1], "-mask_url")) { > + argv[1] = argv[0]; > + maskFlag = 1; > + argc--; > + argv++; > + } > > av_log_set_flags(AV_LOG_SKIP_REPEATED); > parse_loglevel(argc, argv, options); > @@ -3682,7 +3690,10 @@ int main(int argc, char **argv) > > show_banner(argc, argv, options); > > - parse_options(NULL, argc, argv, options, opt_input_file); > + argv2 = copy_argv(argc, argv); > + parse_options(NULL, argc, argv2, options, opt_input_file); > + if (maskFlag) > + param_masking(argc, argv); > > if (!input_filename) { > show_usage(); > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c > index d2f126d..e69f49f 100644 > --- a/fftools/ffprobe.c > +++ b/fftools/ffprobe.c > @@ -4035,9 +4035,16 @@ int main(int argc, char **argv) > WriterContext *wctx; > char *buf; > char *w_name = NULL, *w_args = NULL; > - int ret, input_ret, i; > - > + int ret, input_ret, i, maskFlag; > + char **argv2; > init_dynload(); > + maskFlag = 0; > + if (argc > 1 && !strcmp(argv[1], "-mask_url")) { > + argv[1] = argv[0]; > + maskFlag = 1; > + argc--; > + argv++; > + } > > #if HAVE_THREADS > ret = pthread_mutex_init(&log_mutex, NULL); > @@ -4056,8 +4063,10 @@ int main(int argc, char **argv) > #endif > > show_banner(argc, argv, options); > - parse_options(NULL, argc, argv, options, opt_input_file); > - > + argv2 = copy_argv(argc, argv); > + parse_options(NULL, argc, argv2, options, opt_input_file); > + if (maskFlag) > + param_masking(argc, argv); I am a bit confused how this helps for the issue it tries to solve, as for some amount of time, until this is done, it would expose the full plaintext URL still, no? > if (do_show_log) > av_log_set_callback(log_callback); > > @@ -4173,6 +4182,7 @@ end: > av_freep(&print_format); > av_freep(&read_intervals); > av_hash_freep(&hash); > + free_pp(argc, argv2); > > uninit_opts(); > for (i = 0; i < FF_ARRAY_ELEMS(sections); i++) > -- > 2.7.4 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".