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: V1.1 RC2 #1298

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

content: V1.1 RC2 #1298

wants to merge 20 commits into from

Conversation

lehors
Copy link
Member

@lehors lehors commented Mar 25, 2025

Add SLSA v1.1 Release Candidate 2 (RC2).
Rename SLSA v1.1 to v1.1 RC1 (the way it should have been in the first place!) and label it as retired.
Redirect v1.1 to v1.1-rc2.
Update current activities.

Note to reviewers: Besides checking the preview, you might want to checkout this PR and do a diff between docs/spec/v1.1-rc1 and docs/spec/v1.1-rc2 to see what has changed between the 2. RC2 shouldn't include any of the future stuff (i.e., source track, build-env). Otherwise, looking at the files changed on GitHub will show all the RC2 as new and be of little value for review.

lehors added 4 commits March 25, 2025 13:59
Add SLSA v1.1 Release Candidate 2 (RC2).
Rename SLSA v1.1 to v1.1 RC1 (the way it should have been in the first place!)
and label it as retired.
Redirect v1.1 to v1.1-rc2.
Update current activities.

Signed-off-by: Arnaud Le Hors <[email protected]>
Copy link

netlify bot commented Mar 25, 2025

Deploy Preview for slsa ready!

Name Link
🔨 Latest commit 33e0413
🔍 Latest deploy log https://app.netlify.com/sites/slsa/deploys/67eba44341ca7a0008b5daae
😎 Deploy Preview https://deploy-preview-1298--slsa.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@lehors lehors marked this pull request as draft March 25, 2025 13:45
@lehors lehors marked this pull request as ready for review March 25, 2025 14:15
Signed-off-by: Arnaud Le Hors <[email protected]>
<th>
<th>Threats from
<th>Known example
<th>How SLSA could help
Copy link
Contributor

Choose a reason for hiding this comment

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

@marcelamelara brought this up in another thread, this column currently lists some things that SLSA does, and some things SLSA doesn't do but that users could do themselves.

E.g. for threat B it says "two-person review could have caught the unauthorized change". SLSA doesn't do that! Not yet at least. Similar for 'C'.

We could address this by:

  1. Ignore it, it's been fine so far.
  2. Rename the column: "SLSA and other solutions"
  3. For B and C replace the cell text with "SLSA does not address this threat."

@marcelamelara @lehors @zachariahcox thoughts?

Copy link
Member Author

@lehors lehors Mar 26, 2025

Choose a reason for hiding this comment

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

How about: 4. Replace the cell text with "SLSA does not address this threat but <what could be done instead>" ?

@zachariahcox
Copy link
Contributor

Thanks for doing this @lehors !
Generally, I reviewed the "changed" files, but did not review the "added" files, which I presume are faithful copies from the draft folder.

As noted in slack, there are some papercut items we'd like to address before shipping 1.1 rc2.
However, due to the effort required to generate these mega changesets for jekyll, should we merge this one and deal with the copy edits after the fact?
cc: @lehors @TomHennen @adityasaky @marcelamelara who I see have started work to patch things here.

I could also believe that it's easier to patch just the draft folder and then regenerate this diff, but I thought I'd ask!

@lehors
Copy link
Member Author

lehors commented Mar 27, 2025

As noted in slack, there are some papercut items we'd like to address before shipping 1.1 rc2. However, due to the effort required to generate these mega changesets for jekyll, should we merge this one and deal with the copy edits after the fact?

No, merging this PR will effectively publish RC2. Any further changes would then have to go into an RC3. Not desirable.

I could also believe that it's easier to patch just the draft folder and then regenerate this diff, but I thought I'd ask!

The better way is indeed to make changes against the draft and once they are merged I will pick those changes to update this PR accordingly. It's pretty straightforward to do.

@TomHennen
Copy link
Contributor

FWIW I've reviewed the rest of the changes and apart from the existing identified issues and outstanding PRs I think this looks great!

lehors pushed a commit that referenced this pull request Mar 28, 2025
…ram (#1313)

This PR updates the terminology page to match the updated supply chain
diagram that replaced "package" with "distribution", to address [a
remaining
TODO](#1298 (comment))
prior to v1.1-RC2 release. Very few changes have actually been made to
the original text.

Changes:
* New definition for "distribution"
* Replaced "package model" with "distribution model" and made minor
textual updates to the section
* Add-on: Moved build environment model to the BuildEnv track spec

---------

Signed-off-by: Marcela Melara <[email protected]>
@TomHennen
Copy link
Contributor

I think this just needs one more refresh before a final look and we start waiting the 72 hours. :)

arewm added a commit to arewm/slsa that referenced this pull request Mar 28, 2025
It was raised in the review of 1.1-RC2 that build L3 does not require
provenance to be complete. Instead, it only requires the external
parameters to be complete. With this requirement, it is still sufficient
to mitigate an attack by SSH on the artifact as the verifier would be
able to ensure that the external parameters meet expectations.

ref: slsa-framework#1298 (comment)

Signed-off-by: arewm <[email protected]>
arewm added a commit to arewm/slsa that referenced this pull request Mar 28, 2025
It was raised in the review of 1.1-RC2 that build L3 does not require
provenance to be complete. Instead, it only requires the external
parameters to be complete. With this requirement, it is still sufficient
to mitigate an attack by SSH on the artifact as the verifier would be
able to ensure that the external parameters meet expectations.

ref: slsa-framework#1298 (comment)

Signed-off-by: arewm <[email protected]>
lehors pushed a commit that referenced this pull request Mar 31, 2025
…rovenance (#1315)

It was raised in the review of 1.1-RC2 that build L3 does not require
provenance to be complete. Instead, it only requires the external
parameters to be complete. With this requirement, it is still sufficient
to mitigate an attack by SSH on the artifact as the verifier would be
able to ensure that the external parameters meet expectations.

ref:
#1298 (comment)

Signed-off-by: arewm <[email protected]>

Signed-off-by: arewm <[email protected]>
Copy link
Contributor

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@TomHennen
Copy link
Contributor

TomHennen commented Mar 31, 2025

@MarkLodato provided some helpful feedback on this PR:

https://deploy-preview-1298--slsa.netlify.app/spec/v1.1-rc2/terminology

  • nit: The paragraph defines "distribution platform" but the table still says "package registry".

https://deploy-preview-1298--slsa.netlify.app/spec/v1.1-rc2/threats-overview

  • nit: "Dependency becomes unavailable" seems out of place. Maybe remove?

https://deploy-preview-1298--slsa.netlify.app/spec/v1.1-rc2/threats

  • nit: "Single actor controls multiple accounts" - the "Solution" piece is a separate paragraph here, whereas it is in the same paragraph as "Example:" elsewhere.
  • nit: "Use a robot account to submit change" - the "Solution" of "Require review for such changes." is incomplete. If you just require a single person to review, it doesn't help because that same reviewer has control over hte robot. You need either two humans to review OR have some guarantee that the robot cannot be controlled by a reviewer.
  • nit: "Rule exceptions provide vector for abuse" - missing period at end
  • nit: link to "/spec/v1.1/verifying-artifacts" should just be "verifying-artifacts"

FWIW I think addressing these would be considered 'editorial' if that helps.

Thanks Mark!

lehors pushed a commit that referenced this pull request Apr 1, 2025
Addressing @MarkLodato's [comments on
RC2](#1298 (comment)).

>
https://deploy-preview-1298--slsa.netlify.app/spec/v1.1-rc2/terminology
> * [x] nit: The paragraph defines "distribution platform" but the table
still says "package registry".

Instead of _replacing_ "Package Registry" with "Distribution Platform" I
simply added "Distribution Platform" since a Package Registry is a
specialization (at least as described) and seems worth calling out (also
because it would mess us up elsewhere when we talk about Package
Registries).

>
https://deploy-preview-1298--slsa.netlify.app/spec/v1.1-rc2/threats-overview
> * [ ] nit: "Dependency becomes unavailable" seems out of place. Maybe
remove?

I chose to leave the "Dependency becomes unavailable" bit because it
maps to the expanded "Availability
Threats"(https://deploy-preview-1298--slsa.netlify.app/spec/v1.1-rc2/threats#availability-threats)
and seems worth referencing since we have an example.

We don't currently have examples of Verification Threats but I think
that's OK. If we think it's especially important we can keep it.

> https://deploy-preview-1298--slsa.netlify.app/spec/v1.1-rc2/threats
> * [x] nit: "Single actor controls multiple accounts" - the "Solution"
piece is a separate paragraph here, whereas it is in the same paragraph
as "Example:" elsewhere.
> * [x] nit: "Use a robot account to submit change" - the "Solution" of
"Require review for such changes." is incomplete. If you just require a
single person to review, it doesn't help because that same reviewer has
control over hte robot. You need either two humans to review OR have
some guarantee that the robot cannot be controlled by a reviewer.
> * [x] nit: "Rule exceptions provide vector for abuse" - missing period
at end
> * [x] nit: link to "/spec/v1.1/verifying-artifacts" should just be
"verifying-artifacts"

---------

Signed-off-by: Tom Hennen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

5 participants