Re: improving the report-vuln script
Hi Antoine,
I just have pushed your changes (and only the minor changes, but not
all).
On Fri, Mar 31, 2017 at 08:45:01AM -0400, Antoine Beaupré wrote:
> On 2017-03-31 06:07:08, Salvatore Bonaccorso wrote:
> > Hi Antoine,
> >
> > At the top of the this mail: fist of all thanks for your work on the
> > report-vuln script. I use it extensively in it's current form in a two
> > staged workflow. 1. generate a template for reportbug, then feed it
> > int via reportbug -i ... and do some cleanups before sending the mail.
>
> Interesting! I couldn't figure out how to use reportbug
> productively... surely it would be nice to have a better tip for this in
> the file itself...
I usually did (and I agree there are some redundant steps
unfortunately), generate template with bin/repourt-vuln, then report
-i /tmp/generated.template --src $pkg -V $version -s '$src: CVE-...'.
It was/is not optimal.
> > On Thu, Mar 30, 2017 at 03:04:39PM -0400, Antoine Beaupré wrote:
>
> [...]
>
> >> --affected AFFECTED affected version (default: unspecified)
> >
> > This one might be a bit confusing: Would you change it to --version?
> > Does this makes sense? or is --version more confusing because
> > interpreted as to show version of report-vuln? So we might leave it as
> > --affected.
>
> I found --version confusing, especially since there was already
> "include_version" in the code (which was even more confusing, because it
> actually meant "include blanks".
>
> Furthermore, --version, by convention, shows the script version number
> and users may not expect it to actually run the script (a bit like
> --help).
Agree. Let's leave it as --affected.
> > cc=True please. The default should be to always X-Debbugs-CC
> > team@security.debian.org and
> > secure-testing-team@lists.alioth.debian.org (this is was reportbug
> > does if you report a bug with tag security). This is important for
> > the security team's work.
>
> I agree. I just avoided changing policy for the first pass. This is the
> second reason why I started to patch the script. (The first reason being
> that I didn't understand what was going on with the blanks stuff. :) )
JFTR. If you use reportbug this *is* actually the behaviour, so
actually no policy-change in that sense. Whoever uses reportbug, and
adds tag 'security' to the report, then the X-Debbugs-CC to the two
lists is added automatically:
https://sources.debian.net/src/reportbug/7.1.5/bin/reportbug/#L2085
>
> >> vuln_suff = 'y'
> >> cve_suff = ''
> >> time_w = 'was'
> >> @@ -124,8 +125,13 @@
> >> time_w = 'were'
> >>
> >> header = '''Package: %s\n''' % (pkg)
> >
> > While we are at it: Can we change this to Source field. In the
> > security tracker we track affected sources. so I as well use reportbug
> > --src $sourcepackage when reporting bugs. Having the report-vuln do
> > the "right" (well biased view ...) thing, would be great!
> >
> > -> header = '''Source: %s\n''' % (pkg)
> >
> > *But*: If some other security team member (as most consumers of the
> > script at the moment) do not agree, we can leave it.
> >
> > Another option: have an optional argument, if --src is passed then the
> > header is constructed as
> >
> > header = '''Source: %s\n''' % (pkg)
> >
> > and when --src not passed, then defaults to
> >
> > header = '''Package: %s\n''' % (pkg)
>
> I also thought about this: I thought we could have a binary "--source /
> --package" flag (or "--source / --no-source", anyways like we use the
> NegateAction now) that would default to source, changing, again,
> policy. As it turns out, the second test I did on this package, for
> guacamole-client, *was* binary package specific, so "source" was not
> technically correct, and could have introduced significant confusion. So
> I left the current setting, but it would make perfect sense to have this
> changeable as well.
Again, the secrutiy-tracker is based on source-packages so I would
think having this as default for report-vuln would make sense. But we
can leave it as default to 'Package:'. Do you have capacity to
implement the feature with --src to change the header? Otherwise I
will look into it.
> >> - if include_version:
> >> - header += 'Version: FILLINAFFECTEDVERSION\n'
> >> + if affected is None:
> >> + if blanks:
> >> + header += "Version: FILLINAFFECTEDVERSION\n"
> >> + else:
> >> + header += "Version: %s\n" % affected
> >> + if cc and len(cclist) > 0:
> >> + header += "X-Debbugs-CC: %s\n" % " ".join(cclist)
> >
> > As discussed: might it be better to word --affected as --version?
> > (redundant disucssion here, always asked above).
>
> i'd vote for "affected" as it's unambiguous. we could then also have
> "--fixed" if we so desire. ;)
Well --fixed would be wrong :). It is the found version triaged were
the vulnerability present ;-). Ok I agree with you --version is
unclear, so let's stick with your --affected.
> [...]
>
> >> + parser.add_argument('--no-cc', '--cc', dest='cc', action=NegateAction,
> >> + help='add X-Debbugs-CC header to')
> >
> > The default should be here to result in a always X-Debbugs-CC to
> > team@security.debian.org,secure-testing-team@lists.alioth.debian.org
> > unless someone specifies another cc-list or deliberately choosed
> > --no-cc.
>
> as you noticed, this is simply a matter of reversing the --cc / --no-cc
> flags to change the default.
Yes noticed only later that this is the right approach to change that.
> PS: I noticed you cc'd submit@bugs.debian.org, was that intentional? If
> so, what's the issue number that was generated?
Well initially I wanted to open a wishlist bug for security-tracker
but I failed to add the Package header ;-). But now all pushed, so no
need for it.
Regards,
Salvatore
> PPS: I wonder what you think of the NegateAction. It's something I wrote
> for another project, inspired by some random stuff I found on stack
> overflow, IIRC. I wonder where/if I should publish this more
> officially..
Hmm, cannot really comment on this, sorry.
Reply to: