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

Improve proxy security (#100) #134

Merged
merged 29 commits into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
84a2537
Rename proxy from index.ts to proxy.ts
toplenboren Apr 1, 2024
b82151e
Rename proxy from index.ts to proxy.ts
toplenboren Apr 1, 2024
b0ddaee
Update README.md file with usage example
toplenboren Apr 1, 2024
1a1a703
Update README.md file with issue link
toplenboren Apr 1, 2024
3c60072
Implement basic security measures in proxy.ts
toplenboren Apr 1, 2024
2819f15
Add ExceptionCaughtLocallyJS since it is a pattern in proxy.js
toplenboren Apr 1, 2024
0aec2c0
rename proxy.js back to index.js
toplenboren Apr 8, 2024
bb960b8
add tests to proxy module
toplenboren Apr 8, 2024
7873d0d
add query params parsing, proxy server configuration, martian ip block
toplenboren Apr 8, 2024
16704da
fixes on review: do not document api
toplenboren Apr 8, 2024
dcf9c3c
add failing local addr querying test
toplenboren Apr 8, 2024
df1f14b
fixes on ci: markdown capitalization fixes
toplenboren Apr 8, 2024
9bd40f4
fixes on review: shell -> sh
toplenboren Apr 8, 2024
14da548
fixes on review: from -> From
toplenboren Apr 8, 2024
d9e56be
fixes on review: fix documentation typo
toplenboren Apr 8, 2024
49e3721
fixes on review: lint code
toplenboren Apr 8, 2024
3c3cc73
fixes on review: add return types to getTestHttpServer and initTestHt…
toplenboren Apr 8, 2024
890f50b
fixes on review: add return type to createProxyServer
toplenboren Apr 8, 2024
54f263b
fixes on review: update pnpm lockfile
toplenboren Apr 19, 2024
635dc6f
fixes on review: use named export instead of default one
toplenboren Apr 19, 2024
a1a72e2
fixes on review: fix utils.ts file
toplenboren Apr 19, 2024
4ed2e6f
fixes on review: fix README.md
toplenboren Apr 19, 2024
12833a9
fixes on review: return 400 if request is bad
toplenboren Apr 19, 2024
30bcc4f
fixes on review: fix eslint errors
toplenboren Apr 19, 2024
2826a75
fixes on review: change env var name
toplenboren Apr 20, 2024
c378114
fixes on review: add timeout tests + upgrade error handling to use cu…
toplenboren Apr 20, 2024
6201c12
fixes on review: fix eslint related errors
toplenboren Apr 20, 2024
12ac8ba
fixes on review: remove documentation and introduce custom typings fo…
toplenboren Apr 21, 2024
f368010
Merge branch 'main' into toplenboren-proxy
ai Apr 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"core",
"server",
"web",
"loader-tests"
"loader-tests",
"proxy"
],
"devDependencies": {
"@logux/eslint-config": "53.0.1",
Expand Down
25 changes: 23 additions & 2 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions proxy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,23 @@
HTTP-server to proxy all RSS fetching request from web client.

User could use it to bypass censorship or to try web client before they install upcoming extension (to bypass CORS limit of web app).

## Scripts

### Start Proxy

```sh
pnpm start
# // Proxy server running on port 5284
```

## Abuse Protection

- Proxy allows only GET requests
- Proxy do not allow requests to in-cloud ip addresses like `127.0.0.1`
- Proxy allows only HTTP or HTTPS protocols
- Proxy removes cookie headers

## Test Strategy

To test proxy we emulate the real HTTP servers (end-to-end testing).
3 changes: 3 additions & 0 deletions proxy/global.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
declare module 'martian-cidr' {
export default function isMartianIP(ip: string): boolean
}
46 changes: 11 additions & 35 deletions proxy/index.ts
Original file line number Diff line number Diff line change
@@ -1,43 +1,19 @@
import { createServer } from 'node:http'
import { styleText } from 'node:util'

const server = createServer(async (req, res) => {
let url = decodeURIComponent(req.url!.slice(1))
let sent = false
import { createProxyServer } from './proxy.js'

try {
let proxy = await fetch(url, {
headers: {
...(req.headers as HeadersInit),
host: new URL(url).host
},
method: req.method
})
const PORT = 5284

res.writeHead(proxy.status, {
'Access-Control-Allow-Headers': '*',
'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE',
'Access-Control-Allow-Origin': '*',
'Content-Type': proxy.headers.get('content-type') ?? 'text/plain'
})
sent = true
res.write(await proxy.text())
res.end()
} catch (e) {
if (e instanceof Error) {
process.stderr.write(styleText('red', e.stack ?? e.message) + '\n')
if (!sent) {
res.writeHead(500, { 'Content-Type': 'text/plain' })
res.end('Internal Server Error')
}
} else if (typeof e === 'string') {
process.stderr.write(styleText('red', e) + '\n')
}
}
const IS_PRODUCTION = process.env.NODE_ENV === 'production'
const PRODUCTION_DOMAIN_SUFFIX = '.slowreader.app'

const proxy = createProxyServer({
isProduction: IS_PRODUCTION,
productionDomainSuffix: PRODUCTION_DOMAIN_SUFFIX
ai marked this conversation as resolved.
Show resolved Hide resolved
})

server.listen(5284, () => {
process.stderr.write(
styleText('green', 'Proxy server running on port 5284\n')
proxy.listen(PORT, () => {
process.stdout.write(
styleText('green', `Proxy server running on port ${PORT}`)
)
})
11 changes: 9 additions & 2 deletions proxy/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,16 @@
"private": true,
"type": "module",
"scripts": {
"start": "tsx ./index.ts"
"start": "tsx ./index.ts",
"test": "FORCE_COLOR=1 pnpm run /^test:/",
"test:coverage": "c8 pnpm bnt",
"clean:coverage": "rm -rf coverage"
},
"dependencies": {
"tsx": "4.7.3"
"martian-cidr": "2.0.3",
"tsx": "4.7.2"
},
"devDependencies": {
"c8": "9.1.0"
}
}
121 changes: 121 additions & 0 deletions proxy/proxy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import isMartianIP from 'martian-cidr'
import type http from 'node:http'
import { createServer } from 'node:http'
import { isIP } from 'node:net'
import { styleText } from 'node:util'

class BadRequestError extends Error {
constructor(message: string) {
super(message)
this.name = 'BadRequestError'
}
}

export function createProxyServer(
config: {
fetchTimeout?: number
hostnameWhitelist?: string[]
isProduction?: boolean
productionDomainSuffix?: string
silentMode?: boolean
} = {}
): http.Server {
let isProduction = config.isProduction || false
let silentMode = config.silentMode || false
let hostnameWhitelist = config.hostnameWhitelist || []
let productionDomainSuffix = config.productionDomainSuffix || ''
let fetchTimeout = config.fetchTimeout || 2500

return createServer(async (req, res) => {
let url = decodeURIComponent(req.url!.slice(1))
let sent = false

try {
// Only http or https protocols are allowed
if (!url.startsWith('http://') && !url.startsWith('https://')) {
throw new BadRequestError('Only HTTP or HTTPS are supported')
}

// Other requests are typically used to modify the data, and we do not typically need them to load RSS
if (req.method !== 'GET') {
throw new BadRequestError('Only GET requests are allowed')
}

// In production mode we only allow request from production domain
if (isProduction) {
let origin = req.headers.origin
if (!origin?.endsWith(productionDomainSuffix)) {
throw new BadRequestError('Unauthorized Origin')
}
}

let requestUrl = new URL(url)
if (!hostnameWhitelist.includes(requestUrl.hostname)) {
// Do not allow requests to addresses inside our cloud:
// 127.*
// 192.168.*,*
// https://en.wikipedia.org/wiki/Reserved_IP_addresses
if (
(isIP(requestUrl.hostname) === 4 ||
isIP(requestUrl.hostname) === 6) &&
isMartianIP(requestUrl.hostname)
) {
throw new BadRequestError('Requests to in-cloud ips are not allowed')
}
}

// Remove all cookie headers so they will not be set on proxy domain
delete req.headers.cookie
delete req.headers['set-cookie']
delete req.headers.host

let targetResponse = await fetch(url, {
headers: {
...(req.headers as HeadersInit),
host: new URL(url).host
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we write client’s IP to avoid using proxy to hide IP? There is X-Forwarded-For header for that.

Later we will add privacy mechanism for paid users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Let me take some time to research usecases and write tests :D

},
method: req.method,
signal: AbortSignal.timeout(fetchTimeout)
})

res.writeHead(targetResponse.status, {
'Access-Control-Allow-Headers': '*',
'Access-Control-Allow-Methods': 'OPTIONS, POST, GET, PUT, DELETE',
'Access-Control-Allow-Origin': req.headers.Origin || '*',
'Content-Type':
targetResponse.headers.get('content-type') ?? 'text/plain'
})
sent = true
res.write(await targetResponse.text())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to also improve a performance a little?

Why do we need to first create a full copy of the HTTP document in the proxy memory and only then send it to the user? Instead, on receiving first bites we can send them to the client. It improves performance and memory consumption.

https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Using_readable_streams

res.end()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is another nice abuse protection system. We can limit the size of response https://github.com/gridaco/cors.sh/blob/main/services/proxy.cors.sh/src/limit/payload-limit.ts

} catch (e) {
// Known errors:
if (e instanceof Error && e.name === 'TimeoutError') {
res.writeHead(400, { 'Content-Type': 'text/plain' })
res.end('Bad Request: Request was aborted due to timeout')
return
}

if (e instanceof BadRequestError) {
res.writeHead(400, { 'Content-Type': 'text/plain' })
res.end(`Bad Request: ${e.message}`)
return
}

// Unknown or internal errors:

if (!silentMode) {
if (e instanceof Error) {
process.stderr.write(styleText('red', e.stack ?? e.message) + '\n')
toplenboren marked this conversation as resolved.
Show resolved Hide resolved
} else if (typeof e === 'string') {
process.stderr.write(styleText('red', e) + '\n')
}
}

if (!sent) {
res.writeHead(500, { 'Content-Type': 'text/plain' })
res.end('Internal Server Error')
}
}
})
}
Loading
Loading