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

Added revoked dependency threat #347

Merged
merged 12 commits into from
Apr 13, 2022
Merged

Added revoked dependency threat #347

merged 12 commits into from
Apr 13, 2022

Conversation

ThomasOwens
Copy link
Contributor

Addresses the discussion from slsa-framework/slsaMissing Threat: Revoked Dependency #338 to add out-of-scope threat of unavailable dependencies.

This does not update the ../../images/supply-chain-threats.svg image. I did want to propose the update there, but editing SVGs is not my strong-suit, so I was hoping to get feedback on the easier part to see if that image even needed to be updated.

@netlify
Copy link

netlify bot commented Apr 4, 2022

Deploy Preview for slsa ready!

Name Link
🔨 Latest commit 1f6433c
🔍 Latest deploy log https://app.netlify.com/sites/slsa/deploys/6256ee3c80fd740008858911
😎 Deploy Preview https://deploy-preview-347--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 settings.

Addresses the discussion from
slsa-framework/slsaMissing Threat: Revoked Dependency #338 to add
out-of-scope threat of unavailabel dependencies.

Signed-off-by: Thomas Owens <thomas.j.owens@gmail.com>
@ThomasOwens
Copy link
Contributor Author

I updated this PR to add the DCO sign-off. If there's any guidance or advice on this and if/how to update supply-chain-threats.svg, that would be appreciated.

@MarkLodato
Copy link
Member

Thanks! Regarding the diagram, it seems to me that we have two dimensions here:

  • Location of threat
  • Type of threat: integrity (tampering) vs availability

In this light, I feel that (H) and (I) share same location but for integrity vs availability, respectively. Similarly, "Delete the code" would fall into availability at location (A). Does that align with your thinking? Any thoughts on how to update the diagram to incorporate that concept?

As for actual editing, we can do so via Figma: https://github.com/slsa-framework/slsa/tree/main/resources/editable-diagrams

@ThomasOwens
Copy link
Contributor Author

@MarkLodato

I don't think so. I don't think "A" is involved at all, at least with how I'm visualizing this. I'm picturing looking at this diagram from the perspective of someone in a development organization.

"A" would be my source, my repositories. Perhaps there are threats around a malicious internal user who destroys code in the product repositories or some other internal repositories.

My newly added "I" is something that happens at an external development organization. Perhaps the deletion isn't fully malicious with respect to their reason for doing it, but it has a domino effect on my build and package.

I see "Source Threats" and "Build Threats" as internal threats around things I can control - my people, my processes, my tools. "Dependency Threats" are around external threats and I may need to add controls or mitigations, because I can't stop the author of an NPM package from making 0.1.0 point to an empty set of files or removing 0.1.0 entirely from the external repositories.

@MarkLodato
Copy link
Member

@ThomasOwens That's a good point. In general, it's a bit awkward that (E) is recursively defined as (A)-(H). I was thinking that your (E) is someone else's (H), but I see now that (E) is probably a better fit.

What do you think about about calling it (E), with two different rows in the table (one integrity, one availability)? I'm a bit hesitant to add more complexity to the diagram.

@ThomasOwens
Copy link
Contributor Author

@ThomasOwens That's a good point. In general, it's a bit awkward that (E) is recursively defined as (A)-(H). I was thinking that your (E) is someone else's (H), but I see now that (E) is probably a better fit.

What do you think about about calling it (E), with two different rows in the table (one integrity, one availability)? I'm a bit hesitant to add more complexity to the diagram.

This approach makes sense to me. The only issue that I have is around the phrasing "bad dependency". Not a fan of the word "bad". Not sure what's better, though. For me, it comes down to evaluating the benefits vs costs/risks of using the dependency.

I can make some updates and update this PR.

@MarkLodato
Copy link
Member

The only issue that I have is around the phrasing "bad dependency". Not a fan of the word "bad". Not sure what's better, though. For me, it comes down to evaluating the benefits vs costs/risks of using the dependency.

Agreed. Maybe "Risky dependency"?

@ThomasOwens
Copy link
Contributor Author

@MarkLodato

I really like this approach. I suspect there may be a couple of other prime examples of dependency risks, but I doubt it would grow to more than 4 or 5, tops.

Need to check on the formatting of the table and make the change in the SVG from "bad" to "risky", but I wanted to get some initial feedback quickly. I'll look at using Figma for updating the SVG next.

@MarkLodato MarkLodato changed the title Added revoked dependency threa. Added revoked dependency threat Apr 12, 2022
@MarkLodato
Copy link
Member

Looks great! Here's the preview: https://deploy-preview-347--slsa.netlify.app/spec/v0.1/threats

Updating the diagram would be most appreciated! If you run into any trouble, let me know.

@ThomasOwens
Copy link
Contributor Author

ThomasOwens commented Apr 12, 2022

@MarkLodato I've updated the diagram, but I can't figure out how to export to SVG in Figma. I made the changes to the slsa-editable-diagrams, but I can only see options to export to PDF and PNG. Is this part of a build process that I'm not seeing and making the changes to the editable file is sufficient?

@MarkLodato
Copy link
Member

If you click on the "frame" (white box), on the right-hand panel there should be an export option. But if that doesn't work, don't worry about it - I'm OK leaving the diagram as-is.

image

Signed-off-by: Thomas Owens <thomas.j.owens@gmail.com>
Signed-off-by: Thomas Owens <thomas.j.owens@gmail.com>
@ThomasOwens
Copy link
Contributor Author

@MarkLodato Should be good now, at least if you squash commits. If you don't squash-on-merge, let me know and I can clean up the commit history. I've always preferred squashing on merge and pointing a single commit to the issue, but let me know if that's not what you do here.

@MarkLodato
Copy link
Member

@ThomasOwens The diagram is a bit messed up (wrong size, transparent background) so just revert that portion of it (svg and fig) and I'll send a separate PR to fix. Sorry for the back-and-forth.

We do use the "Merge" button - I also hate the messy history but no on else seems to mind 😛

@ThomasOwens
Copy link
Contributor Author

ThomasOwens commented Apr 13, 2022

@MarkLodato All set. The lint failure is in a file I didn't touch. If I need to fix, I can.

If you do want to squash merge, that's an option on the merge button. It's in the down arrow. Makes all of my commits on this branch into a single commit before merging. I find it really helpful so reviewers can see all the history but the mainline only gets the final, approved changes in its history.

MarkLodato added a commit to MarkLodato/slsa that referenced this pull request Apr 13, 2022
Use Google Web Font (Inter, Arimo) instead of custom alternatives
(Prodigy Sans, Arial) to allow easier edits. Visually it's very similar.

Rename "bad dependency" to "risky dependency" as per slsa-framework#347.

Update Figma file to match SVG (remove drop shadow, use non-oval shape
for Build, fix alignment).

Signed-off-by: Mark Lodato <lodato@google.com>
MarkLodato added a commit to MarkLodato/slsa that referenced this pull request Apr 13, 2022
Use Google Web Font (Inter, Arimo) instead of custom alternatives
(Prodigy Sans, Arial) to allow easier edits. Visually it's very similar.

Rename "bad dependency" to "risky dependency" as per slsa-framework#347.

Update Figma file to match SVG (remove drop shadow, use non-oval shape
for Build, fix alignment).

Signed-off-by: Mark Lodato <lodato@google.com>
MarkLodato added a commit to MarkLodato/slsa that referenced this pull request Apr 13, 2022
Use Google Web Font (Inter, Arimo) instead of custom alternatives
(Prodigy Sans, Arial) to allow easier edits. Visually it's very similar.

Rename "bad dependency" to "risky dependency" as per slsa-framework#347.

Update Figma file to match SVG (remove drop shadow, use non-oval shape
for Build, fix alignment).

Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
@MarkLodato MarkLodato merged commit 1dabd28 into slsa-framework:main Apr 13, 2022
MarkLodato added a commit to MarkLodato/slsa that referenced this pull request Apr 21, 2022
Use Google Web Font (Inter, Arimo) instead of custom alternatives
(Prodigy Sans, Arial) to allow easier edits. Visually it's very similar.

Rename "bad dependency" to "risky dependency" as per slsa-framework#347.

Update Figma file to match SVG (remove drop shadow, use non-oval shape
for Build, fix alignment).

Signed-off-by: Mark Lodato <lodato@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants