Skip to content

Commit

Permalink
feat: support SSRF check on useHttpClientNext = true (#96)
Browse files Browse the repository at this point in the history
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
  - Introduced a new workflow for publishing any commit.
- Added SSRF protection configurations and a new HTTP client instance in
`config.default.js`.

- **Bug Fixes**
- Updated Node.js version requirements to `>=14.20.0` in workflow and
`package.json`.

- **Documentation**
- Updated contributors and license sections in `README.md` and
`README.zh-CN.md`.

- **Refactor**
- Renamed `safeCurl` function to `safeCurlForApplication` and
`safeCurlForContext`.

- **Tests**
- Added new test cases for SSRF protection configurations in
`ssrf.test.js`.

- **Chores**
  - Updated `egg` dependency to version `^3.26.0`.
  - Removed `git-contributor` dependency.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
fengmk2 committed Jul 1, 2024
1 parent 3f225df commit 1d6bfff
Show file tree
Hide file tree
Showing 12 changed files with 109 additions and 50 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ jobs:
uses: node-modules/github-actions/.github/workflows/node-test.yml@master
with:
os: 'ubuntu-latest, macos-latest, windows-latest'
version: '14, 16, 18, 20, 22'
version: '14.20.0, 14, 16, 18, 20, 22'
secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
23 changes: 23 additions & 0 deletions .github/workflows/pkg.pr.new.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Publish Any Commit
on: [push, pull_request]

jobs:
build:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4

- run: corepack enable
- uses: actions/setup-node@v4
with:
node-version: 20

- name: Install dependencies
run: npm install

- name: Build
run: npm run prepublishOnly --if-present

- run: npx pkg-pr-new publish
17 changes: 5 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -551,19 +551,12 @@ exports.security = {

- Forbid `trace` `track` http methods.

<!-- GITCONTRIBUTOR_START -->

## Contributors

|[<img src="https://avatars.githubusercontent.com/u/985607?v=4" width="100px;"/><br/><sub><b>dead-horse</b></sub>](https://github.com/dead-horse)<br/>|[<img src="https://avatars.githubusercontent.com/u/156269?v=4" width="100px;"/><br/><sub><b>fengmk2</b></sub>](https://github.com/fengmk2)<br/>|[<img src="https://avatars.githubusercontent.com/u/893152?v=4" width="100px;"/><br/><sub><b>jtyjty99999</b></sub>](https://github.com/jtyjty99999)<br/>|[<img src="https://avatars.githubusercontent.com/u/360661?v=4" width="100px;"/><br/><sub><b>popomore</b></sub>](https://github.com/popomore)<br/>|[<img src="https://avatars.githubusercontent.com/u/456108?v=4" width="100px;"/><br/><sub><b>shaoshuai0102</b></sub>](https://github.com/shaoshuai0102)<br/>|[<img src="https://avatars.githubusercontent.com/u/5856440?v=4" width="100px;"/><br/><sub><b>whxaxes</b></sub>](https://github.com/whxaxes)<br/>|
| :---: | :---: | :---: | :---: | :---: | :---: |
|[<img src="https://avatars.githubusercontent.com/u/227713?v=4" width="100px;"/><br/><sub><b>atian25</b></sub>](https://github.com/atian25)<br/>|[<img src="https://avatars.githubusercontent.com/u/19343?v=4" width="100px;"/><br/><sub><b>ai</b></sub>](https://github.com/ai)<br/>|[<img src="https://avatars.githubusercontent.com/u/4996660?v=4" width="100px;"/><br/><sub><b>Anemone95</b></sub>](https://github.com/Anemone95)<br/>|[<img src="https://avatars.githubusercontent.com/u/7298095?v=4" width="100px;"/><br/><sub><b>guoshencheng</b></sub>](https://github.com/guoshencheng)<br/>|[<img src="https://avatars.githubusercontent.com/u/27910496?v=4" width="100px;"/><br/><sub><b>p0sec</b></sub>](https://github.com/p0sec)<br/>|[<img src="https://avatars.githubusercontent.com/u/5009418?v=4" width="100px;"/><br/><sub><b>pusongyang</b></sub>](https://github.com/pusongyang)<br/>|
[<img src="https://avatars.githubusercontent.com/u/9857273?v=4" width="100px;"/><br/><sub><b>ShadyZOZ</b></sub>](https://github.com/ShadyZOZ)<br/>|[<img src="https://avatars.githubusercontent.com/u/5064777?v=4" width="100px;"/><br/><sub><b>viko16</b></sub>](https://github.com/viko16)<br/>|[<img src="https://avatars.githubusercontent.com/u/12656301?v=4" width="100px;"/><br/><sub><b>brizer</b></sub>](https://github.com/brizer)<br/>|[<img src="https://avatars.githubusercontent.com/u/18703255?v=4" width="100px;"/><br/><sub><b>damujiangr</b></sub>](https://github.com/damujiangr)<br/>|[<img src="https://avatars.githubusercontent.com/u/7480584?v=4" width="100px;"/><br/><sub><b>EliYao</b></sub>](https://github.com/EliYao)<br/>
## License

This project follows the git-contributor [spec](https://github.com/xudafeng/git-contributor), auto updated at `Wed May 10 2023 16:36:13 GMT+0800`.
[MIT](https://github.com/eggjs/egg-security/blob/master/LICENSE)

<!-- GITCONTRIBUTOR_END -->
## Contributors

## License
[![Contributors](https://contrib.rocks/image?repo=eggjs/egg-security)](https://github.com/eggjs/egg-security/graphs/contributors)

[MIT](https://github.com/eggjs/egg-security/blob/master/LICENSE)
Made with [contributors-img](https://contrib.rocks).
17 changes: 5 additions & 12 deletions README.zh-CN.md
Original file line number Diff line number Diff line change
Expand Up @@ -426,19 +426,12 @@ console.log(cmd);
* crossdomain.xml robots.txt 支持,默认都不加,系统可自行加,需要咨询项目安全工程师
* 禁止 trace track 两种类型请求

<!-- GITCONTRIBUTOR_START -->
## License

## Contributors

|[<img src="https://avatars.githubusercontent.com/u/985607?v=4" width="100px;"/><br/><sub><b>dead-horse</b></sub>](https://github.com/dead-horse)<br/>|[<img src="https://avatars.githubusercontent.com/u/156269?v=4" width="100px;"/><br/><sub><b>fengmk2</b></sub>](https://github.com/fengmk2)<br/>|[<img src="https://avatars.githubusercontent.com/u/893152?v=4" width="100px;"/><br/><sub><b>jtyjty99999</b></sub>](https://github.com/jtyjty99999)<br/>|[<img src="https://avatars.githubusercontent.com/u/360661?v=4" width="100px;"/><br/><sub><b>popomore</b></sub>](https://github.com/popomore)<br/>|[<img src="https://avatars.githubusercontent.com/u/456108?v=4" width="100px;"/><br/><sub><b>shaoshuai0102</b></sub>](https://github.com/shaoshuai0102)<br/>|[<img src="https://avatars.githubusercontent.com/u/5856440?v=4" width="100px;"/><br/><sub><b>whxaxes</b></sub>](https://github.com/whxaxes)<br/>|
| :---: | :---: | :---: | :---: | :---: | :---: |
|[<img src="https://avatars.githubusercontent.com/u/227713?v=4" width="100px;"/><br/><sub><b>atian25</b></sub>](https://github.com/atian25)<br/>|[<img src="https://avatars.githubusercontent.com/u/19343?v=4" width="100px;"/><br/><sub><b>ai</b></sub>](https://github.com/ai)<br/>|[<img src="https://avatars.githubusercontent.com/u/4996660?v=4" width="100px;"/><br/><sub><b>Anemone95</b></sub>](https://github.com/Anemone95)<br/>|[<img src="https://avatars.githubusercontent.com/u/7298095?v=4" width="100px;"/><br/><sub><b>guoshencheng</b></sub>](https://github.com/guoshencheng)<br/>|[<img src="https://avatars.githubusercontent.com/u/27910496?v=4" width="100px;"/><br/><sub><b>p0sec</b></sub>](https://github.com/p0sec)<br/>|[<img src="https://avatars.githubusercontent.com/u/5009418?v=4" width="100px;"/><br/><sub><b>pusongyang</b></sub>](https://github.com/pusongyang)<br/>|
[<img src="https://avatars.githubusercontent.com/u/9857273?v=4" width="100px;"/><br/><sub><b>ShadyZOZ</b></sub>](https://github.com/ShadyZOZ)<br/>|[<img src="https://avatars.githubusercontent.com/u/5064777?v=4" width="100px;"/><br/><sub><b>viko16</b></sub>](https://github.com/viko16)<br/>|[<img src="https://avatars.githubusercontent.com/u/12656301?v=4" width="100px;"/><br/><sub><b>brizer</b></sub>](https://github.com/brizer)<br/>|[<img src="https://avatars.githubusercontent.com/u/18703255?v=4" width="100px;"/><br/><sub><b>damujiangr</b></sub>](https://github.com/damujiangr)<br/>|[<img src="https://avatars.githubusercontent.com/u/7480584?v=4" width="100px;"/><br/><sub><b>EliYao</b></sub>](https://github.com/EliYao)<br/>

This project follows the git-contributor [spec](https://github.com/xudafeng/git-contributor), auto updated at `Wed May 10 2023 16:36:13 GMT+0800`.
[MIT](https://github.com/eggjs/egg-security/blob/master/LICENSE)¬

<!-- GITCONTRIBUTOR_END -->
## Contributors

## License¬
[![Contributors](https://contrib.rocks/image?repo=eggjs/egg-security)](https://github.com/eggjs/egg-security/graphs/contributors)

[MIT](https://github.com/eggjs/egg-security/blob/master/LICENSE)¬
Made with [contributors-img](https://contrib.rocks).
6 changes: 2 additions & 4 deletions app/extend/agent.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
'use strict';

const safeCurl = require('../../lib/extend/safe_curl');
const { safeCurlForApplication } = require('../../lib/extend/safe_curl');

module.exports = {
safeCurl,
safeCurl: safeCurlForApplication,
};
6 changes: 2 additions & 4 deletions app/extend/application.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';

const safeCurl = require('../../lib/extend/safe_curl');
const { safeCurlForApplication } = require('../../lib/extend/safe_curl');

const INPUT_CSRF = '\r\n<input type="hidden" name="_csrf" value="{{ctx.csrf}}" /></form>';

Expand Down Expand Up @@ -33,4 +31,4 @@ exports.injectHijackingDefense = function injectHijackingDefense(tmplStr) {
return INJECTION_DEFENSE + tmplStr + INJECTION_DEFENSE;
};

exports.safeCurl = safeCurl;
exports.safeCurl = safeCurlForApplication;
4 changes: 2 additions & 2 deletions app/extend/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const debug = require('node:util').debuglog('egg-security:context');
const { nanoid } = require('nanoid/non-secure');
const Tokens = require('csrf');
const safeCurl = require('../../lib/extend/safe_curl');
const { safeCurlForContext } = require('../../lib/extend/safe_curl');
const utils = require('../../lib/utils');

const tokens = new Tokens();
Expand Down Expand Up @@ -228,5 +228,5 @@ module.exports = {
}
},

safeCurl,
safeCurl: safeCurlForContext,
};
29 changes: 22 additions & 7 deletions lib/extend/safe_curl.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
'use strict';
const SSRF_HTTPCLIENT = Symbol('SSRF_HTTPCLIENT');

/**
* safe curl with ssrf protect
* @param {String} url request url
* @param {Object} options request options
* @return {Promise} response
*/
module.exports = function safeCurl(url, options = {}) {
const config = this.config || this.app.config;
if (config.security.ssrf && config.security.ssrf.checkAddress) {
options.checkAddress = config.security.ssrf.checkAddress;
exports.safeCurlForApplication = function safeCurlForApplication(url, options = {}) {
const app = this;
const ssrfConfig = app.config.security.ssrf;
if (ssrfConfig?.checkAddress) {
options.checkAddress = ssrfConfig.checkAddress;
} else {
this.logger.warn('[egg-security] please configure `config.security.ssrf` first');
app.logger.warn('[egg-security] please configure `config.security.ssrf` first');
}

return this.curl(url, options);
if (app.config.httpclient.useHttpClientNext && ssrfConfig?.checkAddress) {
// use the new httpClient init with checkAddress
if (!app[SSRF_HTTPCLIENT]) {
app[SSRF_HTTPCLIENT] = app.createHttpClient({
checkAddress: ssrfConfig.checkAddress,
});
}
return app[SSRF_HTTPCLIENT].request(url, options);
}

return app.curl(url, options);
};

exports.safeCurlForContext = function safeCurlForContext(url, options = {}) {
return this.app.safeCurl(url, options);
};
12 changes: 5 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"name": "egg-security",
"version": "3.3.1",
"engines": {
"node": ">=14.20.0"
},
"description": "security plugin in egg framework",
"eggPlugin": {
"name": "security",
Expand Down Expand Up @@ -33,26 +36,21 @@
"devDependencies": {
"beautify-benchmark": "^0.2.4",
"benchmark": "^2.1.4",
"egg": "^3.15.0",
"egg": "^3.26.0",
"egg-bin": "^6.4.0",
"egg-mock": "^5.10.6",
"egg-view-nunjucks": "^2.3.0",
"eslint": "^8.40.0",
"eslint-config-egg": "^12.2.1",
"git-contributor": "*",
"spy": "^1.0.0",
"supertest": "^6.3.3"
},
"engines": {
"node": ">=14.17.0"
},
"scripts": {
"lint": "eslint .",
"test": "npm run lint -- --fix && npm run test-local",
"test-local": "egg-bin test",
"cov": "egg-bin cov",
"ci": "npm run lint && npm run cov",
"contributor": "git-contributor"
"ci": "npm run lint && npm run cov"
},
"repository": {
"type": "git",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
exports.security = {
ssrf: {
ipBlackList: [
'10.0.0.0/8',
'127.0.0.1',
'0.0.0.0/32',
],
checkAddress(ip) {
return ip !== '127.0.0.2';
},
},
};

exports.httpclient = {
useHttpClientNext: true,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "ssrf-ip-check-address-useHttpClientNext"
}
24 changes: 23 additions & 1 deletion test/ssrf.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,27 @@ describe('test/ssrf.test.js', () => {
});
});

describe('checkAddress with useHttpClientNext = true', () => {
before(() => {
app = mm.app({ baseDir: 'apps/ssrf-check-address-useHttpClientNext' });
return app.ready();
});

it('should safeCurl work', async () => {
const urls = [
'https://127.0.0.2/foo',
'https://www.google.com/foo',
];
mm.data(dns, 'lookup', '127.0.0.2');
const ctx = app.createAnonymousContext();
for (const url of urls) {
await checkIllegalAddressError(app, url);
await checkIllegalAddressError(app.agent, url);
await checkIllegalAddressError(ctx, url);
}
});
});

describe('ipExceptionList', () => {
before(() => {
app = mm.app({ baseDir: 'apps/ssrf-ip-exception-list' });
Expand Down Expand Up @@ -157,6 +178,7 @@ async function checkIllegalAddressError(instance, url) {
await instance.safeCurl(url);
throw new Error('should not execute');
} catch (err) {
assert(err.name === 'IllegalAddressError');
assert.equal(err.name, 'IllegalAddressError');
assert.match(err.message, /illegal address/);
}
}

0 comments on commit 1d6bfff

Please sign in to comment.