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 > > [email protected] and > > [email protected] (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 > > [email protected],[email protected] > > 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 [email protected], 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.
