Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

content: refactor threat diagram and add overview #1057

Merged
merged 20 commits into from
Jun 5, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions docs/spec/v1.1/threats.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ The producer of the software intentionally produces code that harms the
consumer, or the producer otherwise uses practices that are not deserving of the
consumer's trust.

Threats in this category likely *cannot* be mitigated through controls placed
during the authoring/reviewing process, in contrast with (B).

<details><summary>Software producer intentionally submits bad code</summary>

*Threat:* Software producer intentionally submits "bad" code, following all
Expand All @@ -88,16 +91,17 @@ auditing may discourage this from happening.

</details>

**TODO:** More producer threats?
**TODO:** More producer threats? Perhaps the attack to xz where a malicious
contributor gained enhanced privileges through social engineering?

### (B) Authoring & reviewing

An adversary introduces a change through the official source control management
interface without any special administrator privileges.

The threats in this category are theoretically mitigated by code review or some
other quality controls. Contrast this with (A), where such controls are
ineffective.
Threats in this category *can* in theory be mitigated by code review or some
other quality controls during teh authoring/reviewing process. Contrast this
marcelamelara marked this conversation as resolved.
Show resolved Hide resolved
with (A), where such controls are likely ineffective.

**TODO:** Is the split between (A) and (B) clear and valuable?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this split is valuable, though I think the distinction between "producer submits bad code" and "authoring", i.e. someone introduces a code change through official interfaces. Is this about first- vs. third-party changes? A distinction I often draw between producer/entity threats and authoring threats is that producer threats are often tied credential compromises, while authoring is about the intentional introduction of bad/malicious code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was not my intent, though I think your description is more in line with what @david-a-wheeler was saying. So we need to align on the model.

I rewrote the paragraph above to hopefully better explain the difference, and add a corresponding paragraph to (A). Does that help?

The reason why I think this is a useful split is because it's grouping by mitigation. In (A) code review won't help, in (B) it will to some degree.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think the distinction between (A) and (B) is a little clearer now, thanks!

The part about the producer practices in (A) is still a bit unclear, though. The first paragraph says "or the producer otherwise uses practices that are not deserving of the consumer's trust", while the threat description below says "Software producer intentionally submits "bad" code, following all proper processes". I think this is why I thought earlier that (A) was about bad first-party code, but it sounds like (A) rather covers producer threats that aren't related to code changes. In that case, I can see compromised producer credentials fitting as a threat category under (A).

That said, I'm fine with merging the current iteration of threats (A) and (B) with possible TODOs for further details and/or examples on the distinction between the two.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I agree it's still a bit fuzzy. I moved the TODO from (B) up to (A) and expanded it a bit to talk about compromised developer credentials, which I agree is unclear where it should go.

I'll submit now and we can hopefully improve over time :)

Thanks for the help!


Expand Down Expand Up @@ -167,7 +171,7 @@ such exceptions.

</details>

#### (A2) Evade code review requirements
#### (B2) Evade code review requirements

<details><summary>Modify code after review</summary>

Expand Down Expand Up @@ -252,7 +256,7 @@ does not accept this because the version X is not considered reviewed.

</details>

#### (A3) Render code review ineffective
#### (B3) Render code review ineffective

<details><summary>Collude with another trusted person</summary>

Expand Down Expand Up @@ -708,6 +712,13 @@ This is the most direct threat because it is the easiest to pull off. If there
are no mitigations for this threat, then (D) and (E) are often indistinguishable
from this threat.

**TODO:** We need to define "official source control repository". Its meaning is
not obvious. The gist is that each package theoretically has some "official" or
"canonical" repository from which is "should" be built, and the attack here is
MarkLodato marked this conversation as resolved.
Show resolved Hide resolved
that you either build from a different source repository or otherwise do
something that doesn't reflect that source repository. But we need to nail down
this concept.

<details><summary>Build with untrusted CI/CD <span>(expectations)</span></summary>

*Threat:* Build using an unofficial CI/CD pipeline that does not build in the
Expand Down