* [PATCH 1/2] bugreport: generate config safelist based on docs
2020-06-24 1:28 [PATCH 0/2] bugreport: report configs from safelist Emily Shaffer
@ 2020-06-24 1:28 ` Emily Shaffer
2020-06-24 4:35 ` Junio C Hamano
2020-06-24 1:28 ` [PATCH 2/2] bugreport: add config values from safelist Emily Shaffer
1 sibling, 1 reply; 5+ messages in thread
From: Emily Shaffer @ 2020-06-24 1:28 UTC (permalink / raw)
To: git; +Cc: Emily Shaffer, Martin Ågren, Johannes Schindelin
Add a new step to the build to generate a safelist of git-config
variables which are appropriate to include in the output of
git-bugreport. New variables can be added to the safelist by annotating
their documentation in Documentation/config with the "annotate" macro,
which is a no-op in AsciiDoc and AsciiDoctor.
Some configs are private in nature, and can contain remote URLs,
passwords, or other sensitive information. In the event that a user
doesn't notice their information while reviewing a bugreport, that user
may leak their credentials to other individuals, mailing lists, or bug
tracking tools inadvertently. Heuristic blocklisting of configuration
keys is imperfect and prone to false negatives; given the nature of the
information which can be leaked, a safelist is more reliable.
However, it's possible that in some situations, an organization may be
less concerned with privacy of things like remote URLs and branch names,
and more concerned with ease of diagnosis for their support staff. In
those cases, it may make more sense for that organization to modify the
code to use a blocklist. To that end, we should try to mark configs which
are definitely safe, and configs which are definitely unsafe, and leave
blank configs which are somewhere in between. To mark a config as safe,
add "annotate:bugreport[include]" to the corresponding line in the
config documentation; to mark it as unsafe, add
"annotate:bugreport[exclude]" instead.
Generating bugreport-config-safelist.h at build time by grepping the
documentation for this new macro helps us prevent staleness. The macro
itself is a no-op and should not alter the appearance of the
documentation in either AsciiDoc or AsciiDoctor, confirmable by running:
cd Documentation
./doc-diff --asciidoctor HEAD^ HEAD
./doc-diff --asciidoc HEAD^ HEAD
Diffing the rendered HTML shows that only inline comments were added,
which shouldn't be a problem.
Additionally, add annotations to the sendemail config documentation in
order to demonstrate a proof of concept.
Helped-by: Martin Ågren <martin.agren@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
.gitignore | 1 +
Documentation/asciidoc.conf | 9 ++++
Documentation/asciidoctor-extensions.rb | 7 ++++
Documentation/config/sendemail.txt | 56 ++++++++++++-------------
Makefile | 7 ++++
generate-bugreport-config-safelist.sh | 18 ++++++++
6 files changed, 70 insertions(+), 28 deletions(-)
create mode 100755 generate-bugreport-config-safelist.sh
diff --git a/.gitignore b/.gitignore
index ee509a2ad2..0177839ae8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -191,6 +191,7 @@
/gitweb/static/gitweb.min.*
/config-list.h
/command-list.h
+/bugreport-config-safelist.h
*.tar.gz
*.dsc
*.deb
diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf
index 3e4c13971b..fd225e30cc 100644
--- a/Documentation/asciidoc.conf
+++ b/Documentation/asciidoc.conf
@@ -6,9 +6,14 @@
#
# Show Git link as: <command>(<section>); if section is defined, else just show
# the command.
+#
+# The annotate macro does nothing as far as rendering is
+# concerned -- we just grep for it in the sources to populate
+# things like the bugreport safelist.
[macros]
(?su)[\\]?(?P<name>linkgit):(?P<target>\S*?)\[(?P<attrlist>.*?)\]=
+(?su)[\\]?(?P<name>annotate):(?P<target>\S*?)\[(?P<attrlist>.*?)\]=
[attributes]
asterisk=*
@@ -28,6 +33,8 @@ ifdef::backend-docbook[]
{0#<citerefentry>}
{0#<refentrytitle>{target}</refentrytitle><manvolnum>{0}</manvolnum>}
{0#</citerefentry>}
+[annotate-inlinemacro]
+{0#}
endif::backend-docbook[]
ifdef::backend-docbook[]
@@ -75,4 +82,6 @@ ifdef::backend-xhtml11[]
git-relative-html-prefix=
[linkgit-inlinemacro]
<a href="{git-relative-html-prefix}{target}.html">{target}{0?({0})}</a>
+[annotate-inlinemacro]
+<!-- -->
endif::backend-xhtml11[]
diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index d906a00803..03c80af0e5 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -39,10 +39,17 @@ module Git
output
end
end
+
+ class AnnotateProcessor < Asciidoctor::Extensions::InlineMacroProcessor
+ def process(parent, target, attrs)
+ ""
+ end
+ end
end
end
Asciidoctor::Extensions.register do
inline_macro Git::Documentation::LinkGitProcessor, :linkgit
postprocessor Git::Documentation::DocumentPostProcessor
+ inline_macro Git::Documentation::AnnotateProcessor, :annotate
end
diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt
index 0006faf800..fe27473e44 100644
--- a/Documentation/config/sendemail.txt
+++ b/Documentation/config/sendemail.txt
@@ -4,7 +4,7 @@ sendemail.identity::
values in the 'sendemail' section. The default identity is
the value of `sendemail.identity`.
-sendemail.smtpEncryption::
+sendemail.smtpEncryption annotate:bugreport[include] ::
See linkgit:git-send-email[1] for description. Note that this
setting is not subject to the 'identity' mechanism.
@@ -15,7 +15,7 @@ sendemail.smtpsslcertpath::
Path to ca-certificates (either a directory or a single file).
Set it to an empty string to disable certificate verification.
-sendemail.<identity>.*::
+sendemail.<identity>.* annotate:bugreport[exclude] ::
Identity-specific versions of the 'sendemail.*' parameters
found below, taking precedence over those when this
identity is selected, through either the command-line or
@@ -23,41 +23,41 @@ sendemail.<identity>.*::
sendemail.aliasesFile::
sendemail.aliasFileType::
-sendemail.annotate::
-sendemail.bcc::
-sendemail.cc::
-sendemail.ccCmd::
-sendemail.chainReplyTo::
-sendemail.confirm::
-sendemail.envelopeSender::
-sendemail.from::
-sendemail.multiEdit::
-sendemail.signedoffbycc::
-sendemail.smtpPass::
-sendemail.suppresscc::
-sendemail.suppressFrom::
-sendemail.to::
-sendemail.tocmd::
-sendemail.smtpDomain::
-sendemail.smtpServer::
-sendemail.smtpServerPort::
-sendemail.smtpServerOption::
-sendemail.smtpUser::
-sendemail.thread::
-sendemail.transferEncoding::
-sendemail.validate::
-sendemail.xmailer::
+sendemail.annotate annotate:bugreport[include] ::
+sendemail.bcc annotate:bugreport[include] ::
+sendemail.cc annotate:bugreport[include] ::
+sendemail.ccCmd annotate:bugreport[include] ::
+sendemail.chainReplyTo annotate:bugreport[include] ::
+sendemail.confirm annotate:bugreport[include] ::
+sendemail.envelopeSender annotate:bugreport[include] ::
+sendemail.from annotate:bugreport[include] ::
+sendemail.multiEdit annotate:bugreport[include] ::
+sendemail.signedoffbycc annotate:bugreport[include] ::
+sendemail.smtpPass annotate:bugreport[exclude] ::
+sendemail.suppresscc annotate:bugreport[include] ::
+sendemail.suppressFrom annotate:bugreport[include] ::
+sendemail.to annotate:bugreport[include] ::
+sendemail.tocmd annotate:bugreport[include] ::
+sendemail.smtpDomain annotate:bugreport[include] ::
+sendemail.smtpServer annotate:bugreport[include] ::
+sendemail.smtpServerPort annotate:bugreport[include] ::
+sendemail.smtpServerOption annotate:bugreport[include] ::
+sendemail.smtpUser annotate:bugreport[exclude] ::
+sendemail.thread annotate:bugreport[include] ::
+sendemail.transferEncoding annotate:bugreport[include] ::
+sendemail.validate annotate:bugreport[include] ::
+sendemail.xmailer annotate:bugreport[include] ::
See linkgit:git-send-email[1] for description.
sendemail.signedoffcc (deprecated)::
Deprecated alias for `sendemail.signedoffbycc`.
-sendemail.smtpBatchSize::
+sendemail.smtpBatchSize annotate:bugreport[include] ::
Number of messages to be sent per connection, after that a relogin
will happen. If the value is 0 or undefined, send all messages in
one connection.
See also the `--batch-size` option of linkgit:git-send-email[1].
-sendemail.smtpReloginDelay::
+sendemail.smtpReloginDelay annotate:bugreport[include] ::
Seconds wait before reconnecting to smtp server.
See also the `--relogin-delay` option of linkgit:git-send-email[1].
diff --git a/Makefile b/Makefile
index 372139f1f2..11d4029003 100644
--- a/Makefile
+++ b/Makefile
@@ -810,6 +810,7 @@ VCSSVN_LIB = vcs-svn/lib.a
GENERATED_H += config-list.h
GENERATED_H += command-list.h
+GENERATED_H += bugreport-config-safelist.h
LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
$(FIND) . \
@@ -2164,6 +2165,12 @@ command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Doc
$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \
command-list.txt >$@+ && mv $@+ $@
+bugreport-config-safelist.h: generate-bugreport-config-safelist.sh
+
+bugreport-config-safelist.h: Documentation/config/*.txt
+ $(QUIET_GEN)$(SHELL_PATH) ./generate-bugreport-config-safelist.sh \
+ >$@+ && mv $@+ $@
+
SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\
diff --git a/generate-bugreport-config-safelist.sh b/generate-bugreport-config-safelist.sh
new file mode 100755
index 0000000000..5bf1f4ad82
--- /dev/null
+++ b/generate-bugreport-config-safelist.sh
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+cat <<"EOF"
+/* Automatically generated by bugreport-generate-config-safelist.sh */
+
+
+static const char *bugreport_config_safelist[] = {
+EOF
+
+# cat all regular files in Documentation/config
+find Documentation/config -type f -exec cat {} \; |
+# print the command name which matches the annotate-bugreport macro
+sed -n 's/^\([^ ]*\) *annotate:bugreport\[include\].* ::$/ "\1",/p' |
+ sort
+
+cat <<"EOF"
+};
+EOF
--
2.27.0.111.gc72c7da667-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] bugreport: add config values from safelist
2020-06-24 1:28 [PATCH 0/2] bugreport: report configs from safelist Emily Shaffer
2020-06-24 1:28 ` [PATCH 1/2] bugreport: generate config safelist based on docs Emily Shaffer
@ 2020-06-24 1:28 ` Emily Shaffer
1 sibling, 0 replies; 5+ messages in thread
From: Emily Shaffer @ 2020-06-24 1:28 UTC (permalink / raw)
To: git; +Cc: Emily Shaffer
Teach bugreport to gather the values of config options which are present
in 'bugreport-config-safelist.h', and show their origin scope.
Many config options are sensitive, and many Git add-ons use config
options which git-core does not know about; it is better only to gather
config options which we know to be safe, rather than excluding options
which we know to be unsafe.
Rather than including the path to someone's config, which can reveal
filesystem layout and project names, just name the scope (e.g. system,
global, local) of the config source.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
Documentation/git-bugreport.txt | 1 +
Makefile | 2 ++
bugreport.c | 28 ++++++++++++++++++++++++++++
3 files changed, 31 insertions(+)
diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index 66e88c2e31..827063d69b 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -30,6 +30,7 @@ The following information is captured automatically:
- Compiler-specific info string
- A list of enabled hooks
- $SHELL
+ - Selected config values
This tool is invoked via the typical Git setup process, which means that in some
cases, it might not be able to launch - for example, if a relevant config file
diff --git a/Makefile b/Makefile
index 11d4029003..a88f918c77 100644
--- a/Makefile
+++ b/Makefile
@@ -2132,6 +2132,8 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
help.sp help.s help.o: command-list.h
+bugreport.sp bugreport.s bugreport.o: bugreport-config-safelist.h
+
builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
diff --git a/bugreport.c b/bugreport.c
index 28f4568b01..f988939bba 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -4,6 +4,8 @@
#include "help.h"
#include "compat/compiler.h"
#include "run-command.h"
+#include "config.h"
+#include "bugreport-config-safelist.h"
static void get_system_info(struct strbuf *sys_info)
@@ -86,6 +88,29 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit)
strbuf_addf(hook_info, "%s\n", hook[i]);
}
+static void get_safelisted_config(struct strbuf *config_info)
+{
+ size_t idx;
+ struct string_list_item *it = NULL;
+ struct key_value_info *kv_info = NULL;
+
+ for (idx = 0; idx < ARRAY_SIZE(bugreport_config_safelist); idx++) {
+ const struct string_list *list =
+ git_config_get_value_multi(bugreport_config_safelist[idx]);
+
+ if (!list)
+ continue;
+
+ strbuf_addf(config_info, "%s:\n", bugreport_config_safelist[idx]);
+ for_each_string_list_item(it, list) {
+ kv_info = it->util;
+ strbuf_addf(config_info, " %s (%s)\n", it->string,
+ kv_info ? config_scope_name(kv_info->scope)
+ : _("source unknown"));
+ }
+ }
+}
+
static const char * const bugreport_usage[] = {
N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
NULL
@@ -172,6 +197,9 @@ int cmd_main(int argc, const char **argv)
get_header(&buffer, _("Enabled Hooks"));
get_populated_hooks(&buffer, nongit_ok);
+ get_header(&buffer, _("Safelisted Config Info"));
+ get_safelisted_config(&buffer);
+
/* fopen doesn't offer us an O_EXCL alternative, except with glibc. */
report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);
--
2.27.0.111.gc72c7da667-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread