Skip to content

Commit 8b0ccfe

Browse files
authored
editorial: draft: Move example controls to its own page. (#1503)
Move the example controls from source-requirements.md to their own page and improve their formatting. These examples were never meant to be a part of the spec itself but rather as example controls that others may want to implement or that would serve as inspiration for developing their own controls. They go above and beyond the specific requirements of the source track itself. This change: 1. Moves the content to its own page, adding links to the nav bar and tracks.md page. 2. Improves the formatting of the content, making summary, intended for, and benefits a table to make it easier to parse. 3. Fills in some gaps that existed: one section didn't have a summary, etc, another was missing the 'intended for' information. 4. Updated the introduction so that it doesn't reference SLSA L4. These examples can be used as a part of L4 but they don't need to be (e.g. the testing or merge train examples). Points 3 and 4 required changing this to an 'editorial' fix instead of a 'style' fix. fixes #1465 --------- Signed-off-by: Tom Hennen <tomhennen@google.com>
1 parent e399146 commit 8b0ccfe

File tree

4 files changed

+280
-163
lines changed

4 files changed

+280
-163
lines changed

docs/_data/nav/main.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,11 @@
449449

450450
- title: Assessing source control systems
451451
url: /spec/draft/assessing-source-systems
452-
description: Guidelines for assessing source control system security.
452+
description: Guidelines for assessing source control system security.
453+
454+
- title: Example controls
455+
url: /spec/draft/source-example-controls
456+
description: This page provides examples of additional controls that organizations may want to implement as they adopt the SLSA Source track.
453457

454458
- title: Cross Track Information
455459
description: These pages describe information that crosses track boundaries.
Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
---
2+
title: "Source: Example controls"
3+
description: "This page provides examples of additional controls that
4+
organizations may want to implement as they adopt the SLSA Source track."
5+
---
6+
7+
At SLSA Source L3+ organizations are allowed and encouraged to define their own
8+
controls that go over and above specific requirements outlined by SLSA. This
9+
page provides some examples of what these additional controls could be.
10+
11+
If an organization has indicated that use of these controls is part of
12+
their repository's expectations, consumers SHOULD be able to verify that the
13+
process was followed for the revision they are consuming by examining the
14+
[summary](./source-requirements#source-verification-summary-attestation) or
15+
[source provenance](./source-requirements#source-provenance-attestations)
16+
attestations.
17+
18+
> For example: consumers can look for the related `ORG_SOURCE` properties in
19+
> the `verifiedLevels` field of the [summary
20+
> attestation](./source-requirements#source-verification-summary-attestation).
21+
22+
### Expert Code Review
23+
24+
<dl class="as-table">
25+
<dt>Summary<dd>
26+
27+
All changes to the source are pre-approved by experts.
28+
29+
<dt>Intended for<dd>
30+
31+
Enterprise repositories and mature open source projects.
32+
33+
<dt>Benefits<dd>
34+
35+
Prevents mistakes by developers unfamiliar with the area.
36+
37+
</dl>
38+
39+
#### Requirements
40+
41+
- **Code ownership**
42+
43+
Each part of the source MUST have a clearly identified set of experts.
44+
45+
- **Approvals from all relevant experts**
46+
47+
For each portion of the source modified by a change proposal, pre-approval
48+
MUST be granted by a member of the defined expert set. An approval from an
49+
actor that is a member of multiple expert groups may satisfy the
50+
requirement for all groups in which they are a member.
51+
52+
### Review Every Single Revision
53+
54+
<dl class="as-table">
55+
<dt>Summary<dd>
56+
57+
The final revision was reviewed by experts prior to submission.
58+
59+
<dt>Intended for<dd>
60+
61+
The highest-of-high-security-posture repos.
62+
63+
<dt>Benefits<dd>
64+
65+
Provides maximum chance for experts to spot problems.
66+
67+
</dl>
68+
69+
#### Requirements
70+
71+
- **Reset votes on all changes**
72+
73+
If the proposal is modified after receiving expert approval, all previously
74+
granted approvals MUST be revoked. A new approval MUST be granted from ALL
75+
required reviewers.
76+
77+
The new approval MAY be granted by an actor who approved a previous
78+
iteration.
79+
80+
### Automated testing
81+
82+
<dl class="as-table">
83+
<dt>Summary<dd>
84+
85+
The final revision was validated by automated tests.
86+
87+
<dt>Intended for<dd>
88+
89+
All organizations and repositories.
90+
91+
<dt>Benefits<dd>
92+
93+
Improves accuracy, prevents errors, and reduces human load.
94+
95+
</dl>
96+
97+
#### Requirements
98+
99+
The organization MUST configure a branch protection rule to require that only
100+
revisions with passing test results can be pointed-to by the branch.
101+
102+
Automatic tests SHOULD be executed in a trustworthy environment (see SLSA
103+
build track).
104+
105+
Results of each test (or an aggregate) MUST be collected by the change review
106+
tool and made available for verification.
107+
108+
Tests SHOULD be run against a revision created for testing by merging the topic
109+
branch (containing the proposed changes) into the target branch.
110+
111+
Use of the proposed merge commit should be preferred to using the tip of the
112+
topic branch.
113+
114+
### Every revision reachable from a branch was approved
115+
116+
<dl class="as-table">
117+
<dt>Summary<dd>
118+
119+
New revisions are created based ONLY on approved changes.
120+
121+
<dt>Intended for<dd>
122+
123+
All organizations and repositories.
124+
125+
<dt>Benefits<dd>
126+
127+
Prevents attacks that hide malicious, unreviewed commits.
128+
129+
</dl>
130+
131+
#### Context
132+
133+
In many organizations, it is normal to review only the "net difference"
134+
between the tip of the topic branch and the "best merge base", the closest
135+
shared commit between the topic and target branches computed at the time of
136+
review.
137+
138+
The topic branch may contain many commits of which not all were intended to
139+
represent a shippable state of the repository.
140+
141+
If a repository merges branches with a standard merge commit, all those
142+
unreviewed commits on the topic branch will become "reachable" from the
143+
protected branch by virtue of the multi-parent merge commit.
144+
145+
When a repo is cloned, all commits _reachable_ from the main branch are
146+
fetched and become accessible from the local checkout.
147+
148+
This combination of factors allows attacks where the victim performs a `git
149+
clone` operation followed by a `git reset --hard <unreviewed revision ID>`.
150+
151+
#### Requirements
152+
153+
- **Informed Review**
154+
155+
The reviewer is able and encouraged to make an informed decision about
156+
what they're approving. The reviewer MUST be presented with a full,
157+
meaningful content diff between the proposed revision and the
158+
previously reviewed revision.
159+
160+
It is not sufficient to indicate that a file changed without showing
161+
the contents.
162+
163+
- **Use only rebase operations on the protected branch**
164+
165+
Require a squash merge strategy for the protected branch.
166+
167+
To guarantee that only commits representing reviewed diffs are cloned,
168+
the SCS MUST rebase (or "squash") the reviewed diff into a single new
169+
commit (the "squashed" commit) that has only a single parent (the
170+
revision previously pointed-to by the protected branch). This is
171+
different than a standard merge commit strategy which would cause all
172+
the user-contributed commits to become reachable from the protected
173+
branch via the second parent.
174+
175+
It is not acceptable to replay the sequence of commits from the topic
176+
branch onto the protected branch. The intent is to reduce the accepted
177+
changes to the exact diffs that were reviewed. Constituent commits of
178+
the topic branch may or may not have been reviewed on an individual
179+
basis, and should not become reachable from the protected branch.
180+
181+
### Immutable Change Discussion
182+
183+
<dl class="as-table">
184+
<dt>Summary<dd>
185+
186+
The discussion around a change is preserved and immutable.
187+
188+
<dt>Intended for<dd>
189+
190+
Large orgs, or where discussion is vital to change management.
191+
192+
<dt>Benefits<dd>
193+
194+
Enables future education, forensics, and security auditing.
195+
196+
</dl>
197+
198+
#### Requirements
199+
200+
The SCS SHOULD record a description of the proposed change and all discussions
201+
/ commentary related to it.
202+
203+
The SCS MUST link this discussion to the revision itself. This is regularly
204+
done via commit metadata.
205+
206+
All collected content SHOULD be made immutable if the change is accepted. It
207+
SHOULD NOT be possible to edit the discussion around a revision after it has
208+
been accepted.
209+
210+
### Merge trains
211+
212+
<dl class="as-table">
213+
<dt>Summary<dd>
214+
215+
A buffer branch (or "train") collects a certain number of approved changes
216+
before merging into the protected branch.
217+
218+
<dt>Intended for<dd>
219+
220+
Large organizations with high-velocity repositories where the protected branch
221+
needs to remain stable for longer periods.
222+
223+
<dt>Benefits<dd>
224+
225+
Allows more time for human and automatic code review by stabilizing the
226+
protected branch.
227+
228+
</dl>
229+
230+
#### Requirements
231+
232+
Large organizations must keep the number of updates to key protected branches
233+
under certain limits to allow time for code review to happen. For example, if
234+
a team tries to merge 60 change requests per hour into the `main` branch, the
235+
tip of the `main` branch would only be stable for about 1 minute. This would
236+
leave only 1 minute for a new diff to be both generated and reviewed before
237+
it becomes stale again.
238+
239+
The normal way to work in this environment is to create a buffer branch
240+
(sometimes called a "train") to collect a certain number of approved changes.
241+
In this model, when a change is approved for submission to the protected
242+
branch, it is added to the train branch instead. After a certain amount of
243+
time, the train branch will be merged into the protected branch. If there are
244+
problems detected with the contents on the train branch, it's normal for the
245+
whole train to be abandoned and a new train to be formed. Approved changes
246+
will be re-applied to a new train in this scenario.
247+
248+
The key benefit to this approach is that the protected branch remains stable
249+
for longer, allowing more time for human and automatic code review. A key
250+
downside to this approach is that organizations will not know the final
251+
revision ID that represents a change until the entire train process completes.
252+
253+
A change review process will now be associated with multiple distinct
254+
revisions.
255+
256+
- ID 1: The revision which was reviewed before concluding the change review
257+
process. It represents the ideal state of the protected branch applying
258+
only this proposed change.
259+
- ID 2: The revision created when the change is applied to the train branch.
260+
It represents the state of the protected branch _after other changes have
261+
been applied_.
262+
263+
It is important to note that no human or automatic review will have the chance
264+
to pre-approve ID2. This will appear to violate any organization policies that
265+
require pre-approval of changes before submission. The SCS and the
266+
organization MUST protect this process in the same way they protect other
267+
artifact build pipelines.
268+
269+
At a minimum the SCS MUST issue an attestation that the revision ID generated
270+
by a merged train is identical ("MERGESAME" in git terminology) to the state
271+
of the protected branch after applying each approved changeset in sequence.
272+
No other content may be added or removed during this process.

0 commit comments

Comments
 (0)