-
Notifications
You must be signed in to change notification settings - Fork 2
policy review document #8
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ac592fe
initial draft of policy review document
naugtur 38c4c09
Apply suggestions from code review
naugtur e7ee012
update formatting
leotm 5409f65
chore: Apply suggestions from code review
naugtur 3391f5b
chore: Apply suggestions from code review
naugtur a97abef
chore: more fixes to eventually squash
naugtur 981fa16
chore: fix formatting
naugtur File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
--- | ||
title: Reviewing Policy | ||
description: How to review LavaMoat Policy and its diffs | ||
--- | ||
|
||
<!-- markdownlint-disable no-inline-html --> | ||
|
||
This guide will show you how to review changes to your LavaMoat Policy File. | ||
|
||
## Why review Policy? | ||
|
||
The Policy File generated by LavaMoat is based on a scan of your codebase, identifying all the powers it uses. The initial policy, resulting from the first time you run policy generation, doesn't provide security on its own. | ||
**Instead, it's your review of the initial policy and the subsequent updates (with new or updated dependencies) that makes your application secure.** | ||
|
||
Reviewing diffs as dependencies change lets you spot suspicious packages or limit the powers you wish to allow newly added packages to use. | ||
|
||
The purpose of the initial review is twofold: | ||
|
||
1. It helps you build confidence that the current state of your app is not compromised | ||
2. You may deny powers to dependencies if you determine they are excessive - not needed for the subset of functionality your app uses. | ||
|
||
Reviewing your initial policy may seem like a lot of effort - but think of it as an _investment_ in your application's security posture. | ||
|
||
## How to review your policy? | ||
|
||
The LavaMoat Policy lists all powers that a package can use; these are the `globals` and `builtin` fields. | ||
It also lists which other packages are allowed for the current package to import. You can follow those relations to see whether a package with access to very [powerful APIs](#powerful-apis) is used by any suspicious packages as a dependency. See [Principle of Least Authority][PoLA] | ||
|
||
### What to look for when reviewing a Policy diff? | ||
|
||
The goal of reviewing the diff is to spot a malicious package being added. | ||
|
||
#### TL;DR | ||
|
||
- Check `globals` and `builtins` for new powers and investigate if you're surprised the package would need them | ||
- Check if new relationships in `packages` are pointing to packages with very [powerful APIs](#powerful-apis) (e.g. spawning child processes in Node.js) | ||
- Be aware that the identifier may change to `pkgC>actual-name` from `pkgB>pkgA>actual-name` BUT! If the package now also has totally different powers, it's likely a different package of the same name. Investigate! `npm ls actual-name` should help | ||
- When a new package is added, consider limiting its powers to what you actually use | ||
|
||
#### Best Practices for Finding Suspicious Changes | ||
|
||
First of all - you need to check if any of the packages get access to new [powerful APIs](#powerful-apis) unexpectedly. | ||
naugtur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
If a package that was supposed to only be doing basic string operations is suddenly also using `fetch` and `process.env` in your build system, you should give it a closer look or add | ||
|
||
```json | ||
"fetch": false, | ||
"process": false | ||
``` | ||
|
||
to the `globals` field for that package in `policy-override.json`. | ||
|
||
When a new dependency shows up in `packages` field of _packageA_: look up what it's pointing to and if the dependency has access to very [powerful APIs](#powerful-apis); doublecheck whether it makes sense to you that _packageA_ would need to use it. | ||
|
||
When dependency tree changes, it's possible that the dependency nesting might change - so the shortest identifier for one of the resources may now be `pkgC>actual-name`, _not_ `pkgB>pkgA>actual-name`. | ||
But there are other more nefarious reasons why that could happen. | ||
If the package now also has totally different powers or dependencies listed it's likely a different package of the same name. There can be more than one `actual-name` named package in this case. It could have been introduced as a different version or a totally different package installed from git or as a bundled dependency. | ||
|
||
Whn a new package is added, consider limiting its powers to what you actually use. | ||
|
||
### What to look for in initial review? | ||
|
||
The goal of reviewing the initial policy is to spot where packages are given powers that allow them escaping LavaMoat protections or abusing the application. | ||
|
||
The minimal viable review is to look at the `globals` and `builtins` fields of the policy file to see if any of the packages have access to unexpected [powerful APIs](#powerful-apis). | ||
|
||
A more advanced review would be to apply [Principle of Least Authority][PoLA] and add entries to policy-override.json to limit the powers of packages to what they actually need to serve your usecase. | ||
|
||
## Powerful APIs | ||
|
||
Examples of powerful APIs - not an exhaustive list: | ||
|
||
| global | builtin | description | | ||
| ----------------------------------------------------- | ------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------- | | ||
| | `child_process` and any form of `exec` or `spawn` | Allows running arbitrary commands on the host machine and is not covered | | ||
| | `fs` | Allows reading and writing files on the host machine | | ||
| `fetch`, `XMLHttpRequest`, `WebSocket`, `EventSource` | `http`, `https`, `net` | Allows making network requests | | ||
| `document` | | contains a lot of powerful APIs that can be used to manipulate the DOM, including creating iframes with unprotected globals | | ||
| `open` | | `window.open` allows opening new windows/tabs and accessing clean globals there | | ||
| `navigator` | | contains a lot of powerful APIs that can be used to fingerprint the user or control the browser | | ||
| `chrome` or `browser` | | extension APIs - should only be accessed by a package that is a helper library for cross-browser extensions | | ||
| `process` | | Allows reading and writing environment variables and other process-related operations | | ||
| | `vm` | Allows running arbitrary code in a new context | | ||
|
||
[PoLA]: https://en.wikipedia.org/wiki/Principle_of_least_privilege |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this usually comes first 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a TLDR of this paragraph - what to look for.