Skip to content

Commit

Permalink
feat: add request header filtering
Browse files Browse the repository at this point in the history
  • Loading branch information
iamtmrobinson committed Apr 20, 2020
1 parent 89380fd commit 7773d92
Show file tree
Hide file tree
Showing 12 changed files with 233 additions and 6 deletions.
27 changes: 27 additions & 0 deletions README.md
Expand Up @@ -344,6 +344,33 @@ docker run --restart=always \
snyk/broker:github-com
```

#### Types of Filtering

##### By Header

Add a validation block with the following key/values:

| Key | Value | Value Type | Example |
|-|-|-|-|
| header | The name of the header you wish to filter on. If this is defined then the named header must explicitly exist on the request otherwise it will be blocked | String | `accept` |
| values | The header value must match one of the defined strings | Array\<String\> | `["application/vnd.github.v4.sha"]` |

For example, to only allow the SHA Media Type accept header for requests to the GitHub Commits API you would add the following:

```
{
"method": "GET",
"path": "/repos/:name/:repo/commits/:ref",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}",
"valid": [
{
"header": "accept",
"values": ["application/vnd.github.v4.sha"]
}
]
}
```

### Mounting Secrets
Sometime it is required to load sensitive configurations (GitHub/Snyk's token) from a file instead from environment variables. Broker is using [dotenv](https://www.npmjs.com/package/dotenv) to load the config, so the process is relatively simple:
* Create a file named `.env` and put your sensitive config there:
Expand Down
8 changes: 7 additions & 1 deletion client-templates/github-com/accept.json.sample
Expand Up @@ -1631,7 +1631,13 @@
"//": "get the details of the commit to determine its SHA",
"method": "GET",
"path": "/repos/:name/:repo/commits/:ref",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}",
"valid": [
{
"header": "accept",
"values": ["application/vnd.github.v4.sha"]
}
]
},
{
"//": "get a list of all the refs to match find whether an existing PR is open",
Expand Down
8 changes: 7 additions & 1 deletion client-templates/github-enterprise/accept.json.sample
Expand Up @@ -669,7 +669,13 @@
"//": "get the details of the commit to determine its SHA",
"method": "GET",
"path": "/repos/:name/:repo/commits/:ref",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}",
"valid": [
{
"header": "accept",
"values": ["application/vnd.github.v4.sha"]
}
]
},
{
"//": "get a list of all the refs to match find whether an existing PR is open",
Expand Down
23 changes: 23 additions & 0 deletions lib/filters/index.js
Expand Up @@ -8,6 +8,22 @@ const authHeader = require('../auth-header');
const tryJSONParse = require('../try-json-parse');
const logger = require('../log');

const validateHeaders = (headerFilters, requestHeaders = []) => {
for (let filter of headerFilters) {
const headerValue = requestHeaders[filter.header];

if (!headerValue) {
return false;
}

if (!filter.values.includes(headerValue)) {
return false;
}
}

return true;
};

// reads config that defines
module.exports = ruleSource => {
let rules = [];
Expand Down Expand Up @@ -40,6 +56,7 @@ module.exports = ruleSource => {
const bodyFilters = valid.filter(v => !!v.path && !v.regex);
const bodyRegexFilters = valid.filter(v => !!v.path && !!v.regex);
const queryFilters = valid.filter(v => !!v.queryParam);
const headerFilters = valid.filter(v => !!v.header);

// now track if there's any values that we need to interpolate later
const fromConfig = {};
Expand Down Expand Up @@ -139,6 +156,12 @@ module.exports = ruleSource => {
}
}

if (headerFilters.length) {
if (!validateHeaders(headerFilters, req.headers)) {
return false;
}
}

logger.debug({ path: entryPath, origin, url, querystring }, 'rule matched');

querystring = (querystring) ? `?${querystring}` : '';
Expand Down
14 changes: 13 additions & 1 deletion test/fixtures/accept/github.json
Expand Up @@ -14,6 +14,18 @@
"method": "GET",
"path": "/repos/:name/:repo/contents/:path*/docs/*",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
}
},
{
"//": "get the details of the commit to determine its SHA",
"method": "GET",
"path": "/repos/:name/:repo/commits/:ref",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}",
"valid": [
{
"header": "accept",
"values": ["application/vnd.github.v4.sha"]
}
]
}
]
}
14 changes: 13 additions & 1 deletion test/fixtures/client/filters-https.json
Expand Up @@ -47,8 +47,20 @@
"path": "/echo-query",
"method": "GET",
"origin": "https://localhost:${originPort}"
}
},

{
"//": "Block on headers",
"path": "/echo-param-protected/:param",
"method": "GET",
"origin": "http://localhost:${originPort}",
"valid": [
{
"header": "accept",
"values": ["valid.accept.header"]
}
]
}
],
"public": [
{
Expand Down
22 changes: 22 additions & 0 deletions test/fixtures/client/filters.json
Expand Up @@ -94,6 +94,18 @@
"path": "/long/nested/partially/encoded%2Fpath%2Fto%2Ffile.ext",
"method": "GET",
"origin": "http://localhost:${originPort}"
},

{
"path": "/echo-param-protected/:param",
"method": "GET",
"origin": "http://localhost:${originPort}",
"valid": [
{
"header": "accept",
"values": ["valid.accept.header"]
}
]
}
],
"public": [
Expand Down Expand Up @@ -143,6 +155,16 @@
"values": ["please"]
}
]
},
{
"path": "/echo-param-protected/:param",
"method": "GET",
"valid": [
{
"header": "accept",
"values": ["valid.accept.header"]
}
]
}
]
}
11 changes: 11 additions & 0 deletions test/fixtures/relay.json
Expand Up @@ -98,5 +98,16 @@
"scheme": "token",
"token": "1234"
}
},
{
"//": "accept header whitelist",
"method": "GET",
"path": "/accept-header",
"valid": [
{
"header": "accept",
"values": ["allowed.header"]
}
]
}
]
14 changes: 13 additions & 1 deletion test/fixtures/server/filters.json
Expand Up @@ -33,9 +33,21 @@
{
"path": "/test-blob/*",
"method": "GET",
"origin": "http://localhost:${originPort}"
},

{
"//": "Block on headers",
"path": "/echo-param-protected/:param",
"method": "GET",
"origin": "http://localhost:${originPort}",
"valid": [
{
"header": "accept",
"values": ["valid.accept.header"]
}
]
}

],
"public": [
{
Expand Down
20 changes: 19 additions & 1 deletion test/functional/client-server.test.js
Expand Up @@ -37,7 +37,7 @@ test('proxy requests originating from behind the broker client', t => {
// wait for the client to successfully connect to the server and identify itself
server.io.once('connection', socket => {
socket.once('identify', clientData => {
t.plan(12);
t.plan(14);

t.test('successfully broker POST', t => {
const url = `http://localhost:${clientPort}/echo-body`;
Expand Down Expand Up @@ -137,6 +137,24 @@ test('proxy requests originating from behind the broker client', t => {
});
});

t.test('allow request for valid url with valid accept header', t => {
const url = `http://localhost:${clientPort}/echo-param-protected/xyz`;
request({ url, method: 'get', headers: { ACCEPT: 'valid.accept.header', accept: 'valid.accept.header' } }, (err, res) => {
t.equal(res.statusCode, 200, '200 statusCode');
t.equal(res.body, 'xyz', 'body brokered');
t.end();
});
});

t.test('block request for valid url with invalid accept header', t => {
const invalidAcceptHeader = 'invalid.accept.header';
const url = `http://localhost:${clientPort}/echo-param-protected/xyz`;
request({ url, method: 'get', headers: { ACCEPT: invalidAcceptHeader, accept: invalidAcceptHeader } }, (err, res) => {
t.equal(res.statusCode, 401, '401 statusCode');
t.end();
});
});

// this validates that the broker *server* sends to the correct broker token
// header to the echo-server
t.test('broker ID is included in headers from server to private', t => {
Expand Down
74 changes: 74 additions & 0 deletions test/unit/filters.test.js
Expand Up @@ -435,6 +435,80 @@ test('Filter on query and body', t => {
});
});

test('Filter on headers', t => {
t.plan(3);

t.test('should block if the provided header does not match those specified in the whitelist', (t) => {
const filter = Filters(require(__dirname + '/../fixtures/relay.json'));

filter({
url: '/accept-header',
method: 'GET',
headers: {
accept: 'unlisted.header'
}
}, (error, res) => {
t.equal(error.message, 'blocked', 'has been blocked');
t.equal(res, undefined, 'no follow allowed');
});

t.end();
});

t.test('should block if the whitelist specifies a required header but no matching header key is provided', (t) => {
const filter = Filters(require(__dirname + '/../fixtures/relay.json'));

filter({
url: '/accept-header',
method: 'GET',
}, (error, res) => {
t.equal(error.message, 'blocked', 'has been blocked');
t.equal(res, undefined, 'no follow allowed');
});

t.end();
});

t.test('For GitHub', (t) => {
const ruleSource = require(__dirname + '/../fixtures/accept/github.json');
const filter = Filters(ruleSource.private);

t.plan(2);

t.test('should allow the sha media type header when requesting a branch SHA to prevent patch information being returned', (t) => {
const url ='/repos/owner/repo-name/commits/master';

filter({
url,
method: 'GET',
headers: {
accept: 'application/vnd.github.v4.sha'
}
}, (error, res) => {
t.equal(error, null, 'no error');
t.isLike(res.url, url, 'contains expected path');
});

t.end();
});

t.test('should block the cryptographer header when requesting a branch SHA to prevent patch information being returned', (t) => {
filter({
url: '/repos/owner/repo-name/commits/master',
method: 'GET',
headers: {
accept: 'application/vnd.github.cryptographer-preview'
}
}, (error, res) => {
t.equal(error.message, 'blocked', 'has been blocked');
t.equal(res, undefined, 'no follow allowed');
});

t.end();
});
});
});

test('filter with auth', t => {
const filter = Filters(require(__dirname + '/../fixtures/relay.json'));

Expand Down
4 changes: 4 additions & 0 deletions test/utils.js
Expand Up @@ -52,6 +52,10 @@ echoServer.get('/echo-param/:param', (req, res) => {
res.send(req.params.param);
});

echoServer.get('/echo-param-protected/:param', (req, res) => {
res.send(req.params.param);
});

echoServer.post('/echo-body/:param?', (req, res) => {
const contentType = req.get('Content-Type');
if (contentType) {
Expand Down

0 comments on commit 7773d92

Please sign in to comment.