Skip to content

Commit

Permalink
fix: send correct SNI for proxy connections (nodejs#2939)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrros95 authored and KhafraDev committed Mar 14, 2024
1 parent 3ebdad8 commit 11f505f
Show file tree
Hide file tree
Showing 11 changed files with 174 additions and 142 deletions.
1 change: 1 addition & 0 deletions docs/docs/api/Errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { errors } from 'undici'
| `ResponseContentLengthMismatchError` | `UND_ERR_RES_CONTENT_LENGTH_MISMATCH` | response body does not match content-length header |
| `InformationalError` | `UND_ERR_INFO` | expected error with reason |
| `ResponseExceededMaxSizeError` | `UND_ERR_RES_EXCEEDED_MAX_SIZE` | response body exceed the max size allowed |
| `SecureProxyConnectionError` | `UND_ERR_PRX_TLS` | tls connection to a proxy failed |

### `SocketError`

Expand Down
13 changes: 12 additions & 1 deletion lib/core/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,16 @@ class RequestRetryError extends UndiciError {
}
}

class SecureProxyConnectionError extends UndiciError {
constructor (cause, message, options) {
super(message, { cause, ...(options ?? {}) })
this.name = 'SecureProxyConnectionError'
this.message = message || 'Secure Proxy Connection failed'
this.code = 'UND_ERR_PRX_TLS'
this.cause = cause
}
}

module.exports = {
AbortError,
HTTPParserError,
Expand All @@ -216,5 +226,6 @@ module.exports = {
ResponseContentLengthMismatchError,
BalancedPoolMissingUpstreamError,
ResponseExceededMaxSizeError,
RequestRetryError
RequestRetryError,
SecureProxyConnectionError
}
5 changes: 3 additions & 2 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class Request {
bodyTimeout,
reset,
throwOnError,
expectContinue
expectContinue,
servername
}, handler) {
if (typeof path !== 'string') {
throw new InvalidArgumentError('path must be a string')
Expand Down Expand Up @@ -181,7 +182,7 @@ class Request {

validateHandler(handler, method, upgrade)

this.servername = getServerName(this.host)
this.servername = servername || getServerName(this.host)

this[kHandler] = handler

Expand Down
14 changes: 10 additions & 4 deletions lib/dispatcher/proxy-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const { URL } = require('node:url')
const Agent = require('./agent')
const Pool = require('./pool')
const DispatcherBase = require('./dispatcher-base')
const { InvalidArgumentError, RequestAbortedError } = require('../core/errors')
const { InvalidArgumentError, RequestAbortedError, SecureProxyConnectionError } = require('../core/errors')
const buildConnector = require('../core/connect')

const kAgent = Symbol('proxy agent')
Expand Down Expand Up @@ -37,7 +37,7 @@ class ProxyAgent extends DispatcherBase {
}

const url = this.#getUrl(opts)
const { href, origin, port, protocol, username, password } = url
const { href, origin, port, protocol, username, password, hostname: proxyHostname } = url

this[kProxy] = { uri: href, protocol }
this[kAgent] = new Agent(opts)
Expand Down Expand Up @@ -78,7 +78,8 @@ class ProxyAgent extends DispatcherBase {
headers: {
...this[kProxyHeaders],
host: requestedHost
}
},
servername: this[kProxyTls]?.servername || proxyHostname
})
if (statusCode !== 200) {
socket.on('error', () => {}).destroy()
Expand All @@ -96,7 +97,12 @@ class ProxyAgent extends DispatcherBase {
}
this[kConnectEndpoint]({ ...opts, servername, httpSocket: socket }, callback)
} catch (err) {
callback(err)
if (err.code === 'ERR_TLS_CERT_ALTNAME_INVALID') {
// Throw a custom error to avoid loop in client.js#connect
callback(new SecureProxyConnectionError(err))
} else {
callback(err)
}
}
}
})
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
"jest": "^29.0.2",
"jsdom": "^24.0.0",
"jsfuzz": "^1.0.15",
"node-forge": "^1.3.1",
"pre-commit": "^1.2.2",
"proxy": "^2.1.1",
"snazzy": "^9.0.0",
Expand Down
17 changes: 0 additions & 17 deletions test/fixtures/client-ca-crt.pem

This file was deleted.

22 changes: 0 additions & 22 deletions test/fixtures/client-crt-2048.pem

This file was deleted.

17 changes: 0 additions & 17 deletions test/fixtures/client-crt.pem

This file was deleted.

27 changes: 0 additions & 27 deletions test/fixtures/client-key-2048.pem

This file was deleted.

27 changes: 0 additions & 27 deletions test/fixtures/client-key.pem

This file was deleted.

Loading

0 comments on commit 11f505f

Please sign in to comment.