Date:   Thu, 2 Jun 2022 17:34:48 +0200
From:   Vegard Nossum <vegard.nossum@...cle.com>
To:     Willy Tarreau <w@....eu>
Cc:     Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, Amit Shah <aams@...zon.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Gustavo A . R . Silva" <gustavoars@...nel.org>,
        Jiri Kosina <jkosina@...e.cz>,
        Kees Cook <keescook@...omium.org>,
        Laura Abbott <labbott@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Solar Designer <solar@...nwall.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Thorsten Leemhuis <linux@...mhuis.info>,
        Tyler Hicks <tyhicks@...onical.com>,
        Will Deacon <will@...nel.org>
Subject: Re: [PATCH] Documentation/security-bugs: overhaul


On 6/1/22 05:12, Willy Tarreau wrote:
> On Wed, Jun 01, 2022 at 01:03:09AM +0200, Vegard Nossum wrote:
>> The current instructions for reporting security vulnerabilities in the
>> kernel are not clear enough, in particular the process of disclosure
>> and requesting CVEs, and what the roles of the different lists are and
>> how exactly to report to each of them.
>>
>> Let's give this document an overhaul. Goals are stated as a comment at
>> the top of the document itself (these will not appear in the rendered
>> document).
> 
> Thanks for working on this, I'm having a few comments below.

Thanks for the comments, I'll send an updated v2 shortly, just wanted to
address individual comments.

>> +Linux kernel security team at security@...nel.org, henceforth "the
>> +security list". This is a closed list of trusted developers who will
>> +help verify the bug report and develop a patch.
> 
> + "in case none was already proposed".

Done.

> There's indeed recently a tendency on the list to get a lot of first-time
> reports from individuals showing that some work was done trying to make
> some code parts fail, but no effort was made to try to figure how these
> ought to be addressed, and that costs a lot of time, because once the
> analysis was done, the person who knows best about the problem and how
> to fix it is the reporter, and we must absolutely encourage that the
> work is finished and a candidate patch is proposed. Usually, returning
> reporters propose patches, so I think they understand the value in doing
> the work properly, which makes me think that we just don't make that
> obvious enough from the rules. It's also possible that many reporters
> are not used to working with OSS projects and imagine it's inappropriate
> of them to propose a fix. But the question that's the most commonly
> asked on the list is "do you have a patch for this?".
> 
> Overall it seems that reporters are willing to do their best but that
> it's never easy to engage in such a bug disclosure process which may
> involve short times, and that it can be stressful for the reporters
> who forget to do a lot of the parts they would do for more regular
> bugs.

I think there's also an argument for encouraging reports whether the
person has a patch or not, presumably it is better to know about an
issue so it can be prioritized against all other known issues, no?

In any case, I have amended the text like this:

+Reporters are encouraged to propose patches, participate in the
+discussions of a fix, and test patches.

>> +While the security list is closed, the security team may bring in
>> +extra help from the relevant maintainers to understand and fix the
>> +security vulnerability.
>> +
>> +Note that the main interest of the kernel security list is in getting
>> +bugs fixed; CVE assignment, disclosure to distributions, and public
>> +disclosure happens on different lists with different people.
> 
> I think it's also important to explain that sometimes some patches may
> be merged ASAP to plug a hole and let them flow to stable branches,
> while letting the reporter deal with the full disclosure once they
> consider that enough time has elapsed. The recent "dirty pipe" fix was
> one of the best examples of a report that went smoothly and allowed the
> reporter to work on a nice description of the problem:
> 
>     https://dirtypipe.cm4all.com/
> 
> There's always this gray area between getting a fix merged and disclosing
> all the details of the bug. A bug indeed becomes public once the fix is
> merged, but that doesn't mean that all the details are shared, so the
> reporter still has the opportunity to write their story about it. The
> reporter must only understand that it's only a matter of time between
> the merge of a fix and the moment someone will understand how to exploit
> the bug and publish about it.

I've changed the bit above to address Jon's comments as well:

+Note that the main interest of the kernel security list is in getting
+bugs fixed and getting patches reviewed, tested, and merged; CVE
+assignment, disclosure to distributions, and public disclosure happen
+on different lists with different people, as described below.

We could maybe add something like this to the "Disclosure" section of
"Contacting the security list":

+Please note that although a fix is public, there may still be value
+in withholding the details of its security relevance and/or how to exploit
+it for another while; see below for when and how to properly disclose the
+security impact of your findings.

I'm not totally happy with the wording here, probably because I don't
really agree that patches should be published ahead of disclosure in the
first place. Anyway, happy to take other suggestions as long as we can
keep it short.

>> +Here is a quick overview of the various lists:
>> +
>> +.. list-table::
[snip]
> While this part renders well on your HTML version, I'm afraid it's totally
> unreadable here in the text version, and this document is supposed to be
> the one reporters will use. I suggest to try again with an item list
> instead of a table.

Jon had a good way to do this that looks good in both text and HTML.

>> +**Disclosure.** The security list prefers to merge fixes into the
>> +appropriate public git repository as soon as they become available.
>> +However, you or an affected party may request that the patch be
>> +withheld for up to 7 calendar days from the availability of the patch,
>> +with an exceptional extension to 14 calendar days if it is agreed that
>> +the bug is critical enough to warrant more time. The only valid reason
>> +for deferring the publication of a fix is to accommodate the logistics
>> +of QA and large scale rollouts which require release coordination.
> 
> I'm still having an issue here. Originally it was something along "up
> to 5 days if there's a really compelling reason". Then it was extended
> to 7 days in order to better match the tuesday-to-thursday range. Then
> "up to 14 days" for exceptional cases (i.e. hopefully never). But the
> wording tends to make all reporters think that asking for 7 days is
> normal. We really need to make it even clearer that:
>   - the default is no embargo
> 
>   - if there is a *really compelling reason* (that has to be justified
>     by the reporter), it may reach *up to 7 days*
> 
>   - in exceptional cases (the bug-of-the-year with a risk of massive
>     exploitation requiring coordination) it may be extended to 14 days
> 
>   - "I already contacted this or that list so you must respect THEIR
>     embargo" is NOT a valid justification to withhold a fix
> 
>   - dealing with embargoes is a pain, a withheld patch risks to be
>     forgotten or confused with a previous version of the patch that
>     appeared in discussions, it's difficult to synchronize stable
>     and mainline, and the risk that it leaks by accident grows with
>     time and with the number of participants in a discussion.

I've changed this section to:

+**Disclosure.** The security list strongly prefers to have patches posted
+for review and testing on public mailing lists and and merged into the
+appropriate public git repository as soon as they become available.
+However, in exceptional cases, you or an affected party may request that
+the patch be withheld for some days; as a rule, the maximum is 7 days.
+Only in truly exceptional cases will the security list consider deferring
+the publication of a fix beyond this, and the only valid reason for doing
+so would be to accommodate the logistics of QA and large scale rollouts
+that require release coordination.

Note that I'm only trying to clarify the existing policy here -- I'm not
in a position to change it. There is no mention of 5 days in the current
document, which is why I've kept the wording of "the maximum is 7 days".
The justification for the current wording was given in commit
544b03da39e2d7b4961d3163976ed4bfb1fac509. If there's a consensus to
change it back to 5 days then I can incorporate that, or somebody can do
an incremental patch.

In any case, I would assume these are also things that can/should be
discussed on a per-bug basis as long as the overarching rules have been
given here. What do you think?

Thanks,


Vegard

Powered by blists - more mailing lists