Improve description of what is considered a security issue #147035
Conversation
This patch improves the description of what the LLVM project considers a security issue, and what not. This patch is based on the RFC discussion in https://discourse.llvm.org/t/improving-documentation-of-what-is-considered-a-security-issue-in-llvm/86714
|
@llvm/llvm-security-group |
llvm/docs/Security.rst
Outdated
| making these parts of the code securable, and maintain these security | ||
| properties over time. In all cases the LLVM Security Response Group should be consulted, | ||
| making these parts of the code securable, and maintain these security properties | ||
| over time. In all cases the LLVM Security Response Group should be consulted, |
There was a problem hiding this comment.
Could be worth stating how best to consult the Security Response Group over possible changes. I can think of 2 likely ones given the information available in https://llvm.org/docs/Security.html#discussion-medium
- Join the Monthly public call.
- File a Github issue.
I'd expect that any changes to the security-sensitive area would need a discussion with the community via RFC. One way of communication could be just writing an RFC directly. I think I'd prefer that the Security Response Group would be consulted first and to write the RFC though.
There was a problem hiding this comment.
That does make sense. For now, I just ended up adding a link from "should be consulted" to the https://llvm.org/docs/Security.html#discussion-medium section.
Agreed, I also expect most changes requiring an RFC.
I'm personally OK with not requiring the security response group to be consulted first, but if more people think it's best to explicitly recommend to first consult with the security response group, I can add that.
| Security Response Group might agree or disagree and will explain its rationale | ||
| in the report, as well as update this document through the above process. | ||
|
|
||
| * Code generation: most miscompilations are not security sensitive. However, a |
There was a problem hiding this comment.
Could be worth going into a bit more detail here. Otherwise I expect something like, miscompile causes a segfault leading to denial of service attack. I think we would be looking for something systemic that affected multiple programs in a predictable way that an attacker could exploit.
I've found it difficult to think of concrete examples:
- Major mitigation bypasses
- Breaking security properties of certain attributes like
cmse_nonsecure_call
There was a problem hiding this comment.
Yes, it would be nice if we could be more concrete in the description here.
Given we've seen very few (I can think of one) such cases, I find it hard to describe very clearly what would be considered a security-sensitive code generation bug and what wouldn't. I think I'd prefer to leave it a bit more vague. If the security group does end up getting quite a few code generation-related reports, we could use those example and our experience with analyzing them to refine the text further?
There was a problem hiding this comment.
I'm inclined to lean into the existence (or credible plausibility of existance) of a reproducer being the key distinguisher here, i.e. leave the text as is.
Even in "obviously" security-implicated features like compiler hardening techniques, most software bugs realistically will not reduce the security of the customer's executable beyond a handwave-y DoS argument. (Yes, I'm aware there's an A in CIA, but in my opinion the security community has gone way overboard in classifying every potential process crash as a CVE, and I don't think I'm alone in this assessment.)
I've also seen cases (not in LLVM) where a "ordinary" miscompilation bug was systematically reproducible in security-critical code, such that it necessitated a security response despite being seemingly benign.
I think it's still a good idea for issues @smithp35 mentioned to be run by us, because there is an elevated risk that they could lead to an actual security vulnerability, but at the same time I don't want to give security researchers the impression that these sections of the code must automatically entitle them to a CVE they can slap on their CVs.
There was a problem hiding this comment.
Thanks for the interesting discussion on this!
My opinion is that we probably should publish the text as is, and iterate/improve it, if needed, later, based on further experience.
| * The following parts of the run-time libraries are explicitly not considered | ||
| security-sensitive: | ||
|
|
||
| * parts of the run-time libraries that are not meant to be included in |
There was a problem hiding this comment.
We might be able to be more specific. As well as (most of) the sanitizers, I expect instrumentation for profiling or coverage would be included. I think that leaves language runtime like builtins, the scudo allocator and other security feature runtimes which would be in scope.
For the sanitizers I think it is only parts of ubsan. Helpfully; ubsan has this paragraph which says that traps-only and minimal-runtime is suitable for production https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#security-considerations
There was a problem hiding this comment.
I was thinking that there is value in sticking with "meant to be included in production binaries" and not specify that further in detail, as otherwise, we might end up having problems with the list becoming outdated over time...
If other people would also prefer to make the list more explicit, I'm happy to add more specifics though.
There was a problem hiding this comment.
I'm generally fine with the wording as-is, but we may want to add @smithp35 as an example of a "gotcha" that developers should consider.
That being said, as I sit here trying to come up with a wording, I can't come up with anything that doesn't sound excessively stilted.
There was a problem hiding this comment.
We could add more examples to the "For example, ...." sentence. But I'm not sure it would add enough value to offset the "cost" of making the documentation a bit longer to read through...
I guess one way to change the sentence would be "For example, most sanitizers are not considered security-sensitive as they are meant to be used during development only, not in production. Profiling and coverage-related run-time code is similarly meant for development only".
(I haven't tried adding words to explain that some features of ubsan are considered suitable for production)
Overall, I'm still in favour of keeping the text as simple as it is. Of course, we can always adapt/improve it based on future experience.
| @@ -204,10 +205,9 @@ The LLVM Security Policy may be changed by majority vote of the LLVM Security Re | |||
| What is considered a security issue? | |||
There was a problem hiding this comment.
I wonder if it would be helpful to add a concise tl;dr here?
**tl;dr**: Generally speaking, most security issues will be focused on
_a bug or deficiency in LLVM tooling_ that leads to an increased attack
surface in _artifacts produced by said tooling_. Moreover, a buffer
overflow in well-defined usage of libcxx is very likely to be a security
issue, as this defect may lead to vulnerabilities in well-formed code that
uses libcxx. An arbitrary buffer overflow in Clang's frontend is less likely
to be considered a security issue, as the most likely outcome is that it
leads to a Clang crash, which most often just leads to a build failure.
Admittedly, "artifacts" isn't as precise as I'd like it to be, but maybe a bit of iteration on ^ can help us set the tone for this section?
There was a problem hiding this comment.
IMHO this is too specific for a tl;dr section.
I'm also not wild about calling out artifacts, since I wouldn't consider LLVM runtimes to be "artifacts" of LLVM but part of it (putting aside the obvious technicality of LLVM being used to build LLVM), yet we very clearly mention that LLVM runtimes are in scope if they're intended to be consumed by downstream users.
I'd focus on the intended user since I see that as the main distinguisher throughout this document, something along the lines of "issues with LLVM that affect developers who consciously chose to use LLVM tooling to generate binaries are considered out-of-scope, issues with LLVM which affect the end users of said developer are in scope."
There was a problem hiding this comment.
Yeah, I'm not strongly of the opinion that a tldr is necessary, so I'm also content if we'd rather go without.
In any case, I do agree that intended user is a much better focus than 'artifacts' - thanks for that. :)
There was a problem hiding this comment.
I quite like the sentence that @wphuhn-intel suggested above as a general principle: "issues with LLVM that affect developers who consciously chose to use LLVM tooling to generate binaries are considered out-of-scope, issues with LLVM which affect the end users of said developer are in scope." That being said, I'm not sure if it would correctly cover all the different cases we do document further in detail.
I'd prefer to land this PR without a tl;dr, as is.
We can then see what the feedback is on the this version of the documentation, and add a well-thought-through tl;dr if the feedback is clear that that would be a major improvement.
| Security Response Group might agree or disagree and will explain its rationale | ||
| in the report, as well as update this document through the above process. | ||
|
|
||
| * Code generation: most miscompilations are not security sensitive. However, a |
There was a problem hiding this comment.
I'm inclined to lean into the existence (or credible plausibility of existance) of a reproducer being the key distinguisher here, i.e. leave the text as is.
Even in "obviously" security-implicated features like compiler hardening techniques, most software bugs realistically will not reduce the security of the customer's executable beyond a handwave-y DoS argument. (Yes, I'm aware there's an A in CIA, but in my opinion the security community has gone way overboard in classifying every potential process crash as a CVE, and I don't think I'm alone in this assessment.)
I've also seen cases (not in LLVM) where a "ordinary" miscompilation bug was systematically reproducible in security-critical code, such that it necessitated a security response despite being seemingly benign.
I think it's still a good idea for issues @smithp35 mentioned to be run by us, because there is an elevated risk that they could lead to an actual security vulnerability, but at the same time I don't want to give security researchers the impression that these sections of the code must automatically entitle them to a CVE they can slap on their CVs.
| @@ -204,10 +205,9 @@ The LLVM Security Policy may be changed by majority vote of the LLVM Security Re | |||
| What is considered a security issue? | |||
There was a problem hiding this comment.
IMHO this is too specific for a tl;dr section.
I'm also not wild about calling out artifacts, since I wouldn't consider LLVM runtimes to be "artifacts" of LLVM but part of it (putting aside the obvious technicality of LLVM being used to build LLVM), yet we very clearly mention that LLVM runtimes are in scope if they're intended to be consumed by downstream users.
I'd focus on the intended user since I see that as the main distinguisher throughout this document, something along the lines of "issues with LLVM that affect developers who consciously chose to use LLVM tooling to generate binaries are considered out-of-scope, issues with LLVM which affect the end users of said developer are in scope."
| * The following parts of the run-time libraries are explicitly not considered | ||
| security-sensitive: | ||
|
|
||
| * parts of the run-time libraries that are not meant to be included in |
There was a problem hiding this comment.
I'm generally fine with the wording as-is, but we may want to add @smithp35 as an example of a "gotcha" that developers should consider.
That being said, as I sit here trying to come up with a wording, I can't come up with anything that doesn't sound excessively stilted.
|
There are 4 explicit approvals on this PR. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/16216 Here is the relevant piece of the build log for the reference |
| * Code generation: most miscompilations are not security sensitive. However, a | ||
| miscompilation where there are clear indications that it can result in the | ||
| produced binary becoming significantly easier to exploit could be considered | ||
| security sensitive, and should be reported to the security response group. |
There was a problem hiding this comment.
I'd like the wording for this point to be more defensive. Something along these lines:
However, a miscompilation that a) affects widely used real-world code, b) that is compiled in a typical production configuration, c) which does not exhibit undefined behavior, and d) makes the resulting binary exploitable, could be considered security sensitive, and should be reported to the security response group.
The main thing this adds is that it must affect real-world code to even be up for consideration. Otherwise it is too easy to come with with plausible hypotheticals.
If something does not affect real-world code, then coordinated disclosure is obviously unnecessary.
This patch improves the description of what the LLVM project considers a security issue, and what not.
This patch is based on the RFC discussion in
https://discourse.llvm.org/t/improving-documentation-of-what-is-considered-a-security-issue-in-llvm/86714