-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: support safeCurl for SSRF protection #32
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
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 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
This file contains 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,7 @@ | ||
'use strict'; | ||
|
||
const utils = require('./lib/utils'); | ||
|
||
module.exports = agent => { | ||
utils.processSSRFConfig(agent.config.security.ssrf); | ||
}; |
This file contains 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 |
---|---|---|
@@ -1,10 +1,13 @@ | ||
'use strict'; | ||
|
||
const safeRedirect = require('./lib/safe_redirect'); | ||
const utils = require('./lib/utils'); | ||
|
||
module.exports = app => { | ||
app.config.coreMiddleware.push('securities'); | ||
|
||
// patch response.redirect | ||
safeRedirect(app); | ||
|
||
utils.processSSRFConfig(app.config.security.ssrf); | ||
}; |
This file contains 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,7 @@ | ||
'use strict'; | ||
|
||
const safeCurl = require('../../lib/extend/safe_curl'); | ||
|
||
module.exports = { | ||
safeCurl, | ||
}; |
This file contains 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
This file contains 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
This file contains 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
This file contains 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,18 @@ | ||
'use strict'; | ||
|
||
/** | ||
* 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; | ||
} else { | ||
this.logger.warn('[egg-security] please configure `config.security.ssrf` first'); | ||
} | ||
|
||
return this.curl(url, options); | ||
}; |
This file contains 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
This file contains 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,50 @@ | ||
'use strict'; | ||
|
||
const ip = require('ip'); | ||
const Benchmark = require('benchmark'); | ||
const benchmarks = require('beautify-benchmark'); | ||
const suite = new Benchmark.Suite(); | ||
|
||
const parsed1 = ip.cidrSubnet('10.0.0.0/8'); | ||
const parsed2 = ip.cidrSubnet('0.0.0.0/32'); | ||
|
||
console.log('10.0.0.0/8 contains 10.255.168.1', parsed1.contains('10.255.168.1')); | ||
console.log('10.0.0.0/8 contains 11.255.168.1', parsed1.contains('11.255.168.1')); | ||
console.log('0.0.0.0/32 contains 0.0.0.0', parsed2.contains('0.0.0.0')); | ||
console.log('0.0.0.0/32 contains 0.0.0.1', parsed2.contains('0.0.0.1')); | ||
|
||
suite | ||
|
||
.add('10.0.0/8 match', () => { | ||
parsed1.contains('10.255.168.1'); | ||
}) | ||
.add('10.0.0/8 not match', () => { | ||
parsed1.contains('11.255.168.1'); | ||
}) | ||
.add('0.0.0/32 match', () => { | ||
parsed1.contains('0.0.0.0'); | ||
}) | ||
.add('0.0.0/32 not match', () => { | ||
parsed1.contains('0.0.0.1'); | ||
}) | ||
.on('cycle', event => { | ||
benchmarks.add(event.target); | ||
}) | ||
.on('start', event => { | ||
console.log('\n ip.cidrsubnet().contains() Benchmark\n node version: %s, date: %s\n Starting...', | ||
process.version, Date()); | ||
}) | ||
.on('complete', () => { | ||
benchmarks.log(); | ||
}) | ||
.run({ 'async': false }); | ||
|
||
// ip.cidrsubnet().contains() Benchmark | ||
// node version: v8.9.1, date: Tue Mar 27 2018 12:04:41 GMT+0800 (CST) | ||
// Starting... | ||
// 4 tests completed. | ||
|
||
// 10.0.0/8 match x 338,567 ops/sec ±2.98% (84 runs sampled) | ||
// 10.0.0/8 not match x 315,822 ops/sec ±5.29% (81 runs sampled) | ||
// 0.0.0/32 match x 366,250 ops/sec ±4.47% (78 runs sampled) | ||
// 0.0.0/32 not match x 370,959 ops/sec ±4.23% (82 runs sampled) |
14 changes: 14 additions & 0 deletions
14
test/fixtures/apps/ssrf-check-address/config/config.default.js
This file contains 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,14 @@ | ||
'use strict'; | ||
|
||
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'; | ||
}, | ||
}, | ||
}; |
This file contains 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,3 @@ | ||
{ | ||
"name": "ssrf-ip-check-address" | ||
} |
11 changes: 11 additions & 0 deletions
11
test/fixtures/apps/ssrf-ip-black-list/config/config.default.js
This file contains 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,11 @@ | ||
'use strict'; | ||
|
||
exports.security = { | ||
ssrf: { | ||
ipBlackList: [ | ||
'10.0.0.0/8', | ||
'127.0.0.1', | ||
'0.0.0.0/32', | ||
], | ||
}, | ||
}; |
This file contains 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,3 @@ | ||
{ | ||
"name": "ssrf-ip-black-list" | ||
} |
This file contains 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,98 @@ | ||
'use strict'; | ||
|
||
const mm = require('egg-mock'); | ||
const dns = require('dns'); | ||
const assert = require('assert'); | ||
|
||
let app; | ||
describe('test/ssrf.test.js', function() { | ||
afterEach(mm.restore); | ||
|
||
describe('no ssrf config', () => { | ||
before(() => { | ||
app = mm.app({ baseDir: 'apps/csrf' }); | ||
return app.ready(); | ||
}); | ||
|
||
it('should safeCurl work', async () => { | ||
const ctx = app.createAnonymousContext(); | ||
const url = 'https://127.0.0.1'; | ||
mm.data(app, 'curl', 'response'); | ||
mm.data(app.agent, 'curl', 'response'); | ||
mm.data(ctx, 'curl', 'response'); | ||
|
||
let count = 0; | ||
function mockWarn(msg) { | ||
count++; | ||
assert(msg === '[egg-security] please configure `config.security.ssrf` first'); | ||
} | ||
|
||
mm(app.logger, 'warn', mockWarn); | ||
mm(app.agent.logger, 'warn', mockWarn); | ||
mm(ctx.logger, 'warn', mockWarn); | ||
|
||
const r1 = await app.safeCurl(url); | ||
const r2 = await app.agent.safeCurl(url); | ||
const r3 = await ctx.safeCurl(url); | ||
assert(r1 === 'response'); | ||
assert(r2 === 'response'); | ||
assert(r3 === 'response'); | ||
assert(count === 3); | ||
}); | ||
}); | ||
|
||
|
||
describe('ipBlackList', () => { | ||
before(() => { | ||
app = mm.app({ baseDir: 'apps/ssrf-ip-black-list' }); | ||
return app.ready(); | ||
}); | ||
|
||
it('should safeCurl work', async () => { | ||
const urls = [ | ||
'https://127.0.0.1/foo', | ||
'http://10.1.2.3/foo?bar=1', | ||
'https://0.0.0.0/', | ||
'https://www.google.com/', | ||
]; | ||
mm.data(dns, 'lookup', '127.0.0.1'); | ||
const ctx = app.createAnonymousContext(); | ||
|
||
for (const url of urls) { | ||
await checkIllegalAddressError(app, url); | ||
await checkIllegalAddressError(app.agent, url); | ||
await checkIllegalAddressError(ctx, url); | ||
} | ||
}); | ||
}); | ||
|
||
describe('checkAddress', () => { | ||
before(() => { | ||
app = mm.app({ baseDir: 'apps/ssrf-check-address' }); | ||
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); | ||
} | ||
}); | ||
}); | ||
}); | ||
|
||
async function checkIllegalAddressError(instance, url) { | ||
try { | ||
await instance.safeCurl(url); | ||
throw new Error('should not execute'); | ||
} catch (err) { | ||
assert(err.name === 'IllegalAddressError'); | ||
} | ||
} |
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.
加个 benchmark
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.
加个 lru?活跃的 ip 应该是有限的。
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.
先不加 lru 吧,相较于做一次 http 请求来说,这个性能算是可以接受的,现在也不会对 curl 造成影响。