Fix HTTPS for non-Let's Encrypt certificates #313
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -343,7 +343,7 @@ export class Redbird { | |
| opts?: any; | ||
| } = { | ||
| SNICallback: async (hostname: string, cb: (err: any, ctx?: any) => void) => { | ||
| if (this.opts?.letsencrypt && !certs[hostname]) { | ||
|
There was a problem hiding this comment. This change affects HTTPS behavior when
Author
There was a problem hiding this comment. When a custom certificate is provided, I don't believe that checking for certs in this way is useful or relevant. I have tested this code in my team's own project and it only worked when it simply used the cert I provided without interference here. |
||
| if (!this.opts?.letsencrypt?.path) { | ||
| console.error('Missing certificate path for Lets Encrypt'); | ||
| return cb(new Error('No certs for hostname ' + hostname)); | ||
|
|
@@ -376,8 +376,6 @@ export class Redbird { | |
| console.error('Error getting LetsEncrypt certificates', err); | ||
| return cb(err); | ||
| } | ||
| } | ||
|
|
||
| if (cb) { | ||
|
|
||
This file contains hidden or 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,29 @@ | ||
| -----BEGIN CERTIFICATE----- | ||
| MIIFCTCCAvGgAwIBAgIUJUC0eDhJha4VRcTWhqS3LuAPkkwwDQYJKoZIhvcNAQEL | ||
| BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTI2MDQwMTE0NDczOVoXDTM2MDMy | ||
| OTE0NDczOVowFDESMBAGA1UEAwwJbG9jYWxob3N0MIICIjANBgkqhkiG9w0BAQEF | ||
| AAOCAg8AMIICCgKCAgEAshfjprzv2RMEwSRo2AWm69DhoWT425SVxNGIuIwq1lQ7 | ||
| 9kNRrFN3hTUTE42roTv8BtqbIOQzTK8ajbYVMZpH0sMiGLIOSTAVKltLLvn9FcCO | ||
| fyzl5qwzqOWZMSJ4l8lNeCFBa9WtIUpQiBHMqxYIOyal06puthbs20aiecacpd3i | ||
| Ydmf3wxecSpmDHxdguLtRl8g8cdFDkKvnFmNWFhb+2lP1t7Y5d4t9g0JZtbUwF2t | ||
| rCCYxaLhl91lruDEzO60PvqimqJLcRJhs3kvcgYQ9rO4ZmV3kON6xA+2PWLGi0MP | ||
| fqyHKj3TUTFDu9FowHHN/Qb9dwVJ2eLbcSJGOAwC+nWVobaAR2VH2Ee4wabFKntt | ||
| BKO6PcUCiCaYSWK9hYT6m//CLi+weLoRLjP08e0iaEWZVDshRk1b1moWsCpPlV19 | ||
| kHevztPONVncT0/42eqZJXLjqd8F9lb91TGuTjw/AjS4R++vLivreseETxAaj5vY | ||
| YHa1mEeb9YHOBaswP4zX6DDjWBCKlq3EqETl1Ie54PEgrpZcMb2FepXJ7HHWYb2C | ||
| aAG1fMahNtUNsjDOu+yNVx7zq7gtXr8bDBy1vNw7A1m5Ql7cD6FZyvVvJH9hv1Ap | ||
| NhmtpgwWgYSzJqBmOyLKp5Uv+qaqIfAlHCXBYjyvXNOPmhkqGX5CH+Ify/MMNycC | ||
| AwEAAaNTMFEwHQYDVR0OBBYEFNp1+0JAA7lH4l43E+foG2SL0rmmMB8GA1UdIwQY | ||
| MBaAFNp1+0JAA7lH4l43E+foG2SL0rmmMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZI | ||
| hvcNAQELBQADggIBAFgnLsMLPWNXdi2V2PQ4xrekZQHy0oaQjIXKgRhtCPpKN0Ew | ||
| XYet2N9YPIuM3NqAeCQ2mmjwTbKt98ahVZmeYBUH6HifLN3DpNUApugYAHeFrkkk | ||
| ls2cBoV0aOeXX4tkMfpkOIZF+Tkhu+zS27Q0dGJCRkwXd4E3wN0nOU1o8NmTH6C9 | ||
| +NZ1/i9LEUZSTCFa7VUKVAS9bGS+yMAmxB78kURvW9eS8X7nYQ5KQaPhizTw5ncl | ||
| jMn7rRZydf+OrMyK0jyrPYKsRdtp6mX1RNS41JLQqWMaZQAOzKUuHPyEJ36L+fj4 | ||
| oIKAIRpWe/UmXL+YOt6G36gyY/Qow8mKBxg0HgFa27IEkPJWkyVK9LxarTg4+zDW | ||
| S2L7ZysJFz6Gm2NF+mliV5ARaZ2gTQGA94oyaMYqhyAOUjMfMeWRcjWIDLEduJVI | ||
| +KiAQqGLQtHol2b9bA8VHAXd/y3ODqB/qSyNTFeeWheB+81SuDgx5k1AfuQcDOKr | ||
| jMiEddY0jqphmqbzGTaCPGAfDoRRWtEnNlVFNMalBowUSI1g7n/r2sdVJjwtNc/6 | ||
| C397wFRharWeNO5NphHW85CO7zB1S5vhn4zoKcg2Gs+XQORwa2YGzdD86pG5zI9B | ||
| 3hCWHP3gx+aMiU/gSEuEsi0BrAnlYf38wLuwBPKnCLpzJ4zVgyAXetLBT/zE | ||
| -----END CERTIFICATE----- |
This file contains hidden or 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,20 +1,3 @@ | ||
| import { readFileSync } from "fs"; | ||
|
|
||
| export const certificate = readFileSync(__dirname + '/test_crt.pem'); |
This file contains hidden or 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,52 @@ | ||
| -----BEGIN PRIVATE KEY----- | ||
| MIIJQwIBADANBgkqhkiG9w0BAQEFAASCCS0wggkpAgEAAoICAQCyF+OmvO/ZEwTB | ||
| JGjYBabr0OGhZPjblJXE0Yi4jCrWVDv2Q1GsU3eFNRMTjauhO/wG2psg5DNMrxqN | ||
| thUxmkfSwyIYsg5JMBUqW0su+f0VwI5/LOXmrDOo5ZkxIniXyU14IUFr1a0hSlCI | ||
| EcyrFgg7JqXTqm62FuzbRqJ5xpyl3eJh2Z/fDF5xKmYMfF2C4u1GXyDxx0UOQq+c | ||
| WY1YWFv7aU/W3tjl3i32DQlm1tTAXa2sIJjFouGX3WWu4MTM7rQ++qKaoktxEmGz | ||
| eS9yBhD2s7hmZXeQ43rED7Y9YsaLQw9+rIcqPdNRMUO70WjAcc39Bv13BUnZ4ttx | ||
| IkY4DAL6dZWhtoBHZUfYR7jBpsUqe20Eo7o9xQKIJphJYr2FhPqb/8IuL7B4uhEu | ||
| M/Tx7SJoRZlUOyFGTVvWahawKk+VXX2Qd6/O0841WdxPT/jZ6pklcuOp3wX2Vv3V | ||
| Ma5OPD8CNLhH768uK+t6x4RPEBqPm9hgdrWYR5v1gc4FqzA/jNfoMONYEIqWrcSo | ||
| ROXUh7ng8SCullwxvYV6lcnscdZhvYJoAbV8xqE21Q2yMM677I1XHvOruC1evxsM | ||
| HLW83DsDWblCXtwPoVnK9W8kf2G/UCk2Ga2mDBaBhLMmoGY7IsqnlS/6pqoh8CUc | ||
| JcFiPK9c04+aGSoZfkIf4h/L8ww3JwIDAQABAoICAA2lkS8fYdB3Y1Et6ijJGhbb | ||
| Q6B7g8RG5ckWQ0zc7l7/emhiLszh/BHr7lSWJQiVKtIXO0IXqnfd7Rt4NrYj1XNP | ||
| IS3Vd9nGdcC9DFnpRxCJ8YFjEXgN48sb7BODScyO9nQ8aO5x8H/RP+WFWgxI6Z2N | ||
| 7+QN0C9jOogjW/zkqe+qo0r00AoMwRayQiEDAKL9PGOJk5UwIovRGVk4ok4BC1KM | ||
| qdUHVriA83J1lXq/ZVmBdCokUoCTSbuHROAgxVCgAMjYqZzoaP6DnMZenjLPFMqx | ||
| HKUgCyIoaqmObw9tVVVDillRfO2d0JnZ1amVOaeRfbbXXvtmHz6AFZx2nP0OrCwG | ||
| Npy5mgzypYLwk9rnYxn2uGvQq0d4vn/bOfWNouY5wHFtMdRzqyz5XUAlDNaVlvWe | ||
| rMQ7yNOVdN3VUAq3JUQkNIy9lHiVsZ2dfILuBuqdL37DGlRMS8qRX6UO8NNIdGs3 | ||
| j8u6dyk7x3i6lUzJJqhgNaTYkugIKPPdP2s8cD5oAlfrhhrBdt9KFnWhJbNf87S6 | ||
| m9kssmtPA3cCcglVh2FH3WdCrMU711WMsyD7wCQ5EvWoFi7kSG4D61zkf5ewaV5x | ||
| cKf+o/HrDmzBv3VccROEzIc9PfmB4ZByxEzqrk0Lo2+hd6Fjn1AZiEIjSH0W8A/0 | ||
| yyG3ErP1N/XSaiCXoSQNAoIBAQD0fQXms76RS9eIsWeiFCbuz6IC4yvSd7p87mly | ||
| y59YCIEKsPYoMdVNTDsFRPV3BG6nzdXCoGIaPeIQWFoJBNZRqt/KL3vQtmoOXt22 | ||
| RYtmN+cLM2o7djDVzrgaJQNJfh3uyRE9iH/DG7hXQZbdSKGgzs4KpnzFsKDpjBEu | ||
| ZrWgJkgs55npoKMJY1L+seMI+expPFykA/kq0QdIb2gxFLCrAXCo4+B/1zQECN10 | ||
| OOMRNXTD/3k5MpGqpIgS08T3sKHC8o4Jk5QRkdAHSpgri5zRe3M+lmgVwEOSOiDM | ||
| 3q5TqsCaCyinJ5FLECWOoUEqNDpRhv7Cmr6ykf+zJ5436CnFAoIBAQC6epBFFmMU | ||
| d4fIutd0YEeJK2ZGC7d/jOYDVLGVme43k3Keuu7sczuStHf8pPvQBHSkUMWxWtGJ | ||
| uAMDNOl2rxwi4AiUmoh/p5A1KS2sBONEX34qtJjLwJArYmzYg719JmxoUpp9ao0/ | ||
| PBRdkjLsIN7se4cQRyFtd9tTguPXM1qdkLPrJXFaIPaLczmdKZ1EyQEpf9skoirj | ||
| paaS8yMJegsdgq1/gjAzPws4hzhZ8GNhUjq0rlw44WjgqVliHSxfgO6zyC+ixAbg | ||
| OGIZK+Nqhjh2Q+QIca6zFGOFQKyGtjZ5D8JvhpIFBT9xFSngOtj0PnpX1KK1iKZP | ||
| vLcDwlJF+Wf7AoIBAQDjqeQr3ybOn+QVirVpDQDwEnLX43i0JP+IRG33ErvPuyOG | ||
| tCCUb0skDp9LGdrFuF/ebGLoOHcB3qw68jrs+Q8lv0tjN/JA+QwrU5V7nKjhR58j | ||
| I/6BXl69SiDPX7X+4qy25Y7jUM8//Pkcb5wvdOnlF45wBnZblE1TDTYWklgAyIUf | ||
| I4+EKSyImm/3cehrPGcz25ycsoJ8p2bzwaIxSgX1R4MEEibEhL4QH70v86UmKjQQ | ||
| Kz5Tz84rK8j2VzKsP1NyeHFhY4ZXEgO4NnMa4Yfm3eED4AUOdBm3+d7/R0Dn8jIl | ||
| f8ycUtEOJ/wBJBHNXhMMRfvHdVesax3DU1oWz17NAoIBAQCN4Ui5N/JElTk7cWUS | ||
| R1rGM74J10pTApnSuPKwd0C3CjlwKZw1TOtrkGGmWMS6nVk917LCYO4CUX4BR5XF | ||
| 1i+bHUeKARWPwqbGu8DjG4jq8Zjl1yhWBfnq+GUGSx22BbD/orGmcJZtIUIBe4XB | ||
| ALV7yfSWXvhA+KtoG7NMfecY8QrjA0b+IwxSR2XzfMEKn8CBmaTbd4MYV2WoctMa | ||
| l7+zQUTOOfiAVRwtczxdubdzSYQfCnZiVKZeIJtAOiATvd5f7RTJS0pL+LoymqDZ | ||
| 4Ks+2CK8hfo8iGu769wYVrSVYXc3MXUR2s9NM6P71epR0uN0xfTSF9kp29SHSKce | ||
| VEdXAoIBAE1lqIfZy/cvr0P9hge4aFXZTVFQquxroEG+AKuKGTohDyNKENwODVBQ | ||
| xD4J6hGS9GcMLJrCXHXHaqkL/0Dbdznd9bviZlnW3l9u6lg7IO2q3xzff9yCiEjs | ||
| TBhWXHH7Y5QxbotXL52e/iEwLl9NfdWZ1CXFuLXND2ev8yoqBDjnrCKagMKTP6It | ||
| QgC7Ac/3Z1EuSE9W4ArREPUia0lnYlocgvRXT8mMMzKyKQA8b+r5//FP/M+4ED3S | ||
| pXffjDey9Bj/BmMbVrx9fQqOOd/4BkRrEP99dAF4+oJwAAr2LHQKMhtkKHaVQJMl | ||
| wbC+UnAOlDS/ZZ9RqLi61XaIMcH+H0c= | ||
| -----END PRIVATE KEY----- |
This file contains hidden or 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,29 +1,3 @@ | ||
| import { readFileSync } from "fs"; | ||
|
|
||
| export const key = readFileSync(__dirname + '/test_key.pem'); |
This file contains hidden or 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,133 @@ | ||
| import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; | ||
| import http, { Server } from 'http'; | ||
| import https from 'https'; | ||
| import { Redbird } from '../lib/index.js'; | ||
| import { certificate, key } from './fixtures/index.js'; | ||
|
|
||
| const proxyPort = 8085; | ||
| const sslPort = 8445; | ||
| const testPort = 54681; | ||
|
|
||
| function makeHttpRequest(options) { | ||
| return new Promise<{ status: number; headers: any; data: string }>((resolve, reject) => { | ||
| const req = http.request(options, (res) => { | ||
| let data = ''; | ||
| res.on('data', (chunk) => { | ||
| data += chunk; | ||
| }); | ||
| res.on('end', () => { | ||
| resolve({ status: res.statusCode!, headers: res.headers, data }); | ||
| }); | ||
| }); | ||
| req.on('error', (err) => { | ||
| console.error('ERROR', err); | ||
| reject(err); | ||
| }); | ||
| req.end(); | ||
| }); | ||
| } | ||
|
|
||
| function makeHttpsRequest(options) { | ||
| return new Promise<{ status: number; headers: any; data: string }>((resolve, reject) => { | ||
| options.rejectUnauthorized = false; // For self-signed certificates | ||
| const req = https.request(options, (res) => { | ||
| let data = ''; | ||
| res.on('data', (chunk) => { | ||
| data += chunk; | ||
| }); | ||
| res.on('end', () => { | ||
| resolve({ status: res.statusCode!, headers: res.headers, data }); | ||
| }); | ||
| }); | ||
| req.on('error', (err) => { | ||
| reject(err); | ||
| }); | ||
| req.end(); | ||
| }); | ||
| } | ||
|
|
||
| const responseMessage = 'Hello from target server'; | ||
|
|
||
| describe("HTTPS with custom certificates", () => { | ||
| let proxy: Redbird; | ||
| let targetServer: Server; | ||
|
|
||
| beforeAll(async () => { | ||
| targetServer = http.createServer((req, res) => { | ||
| res.writeHead(200, { 'Content-Type': 'text/plain' }); | ||
| res.end(responseMessage); | ||
| }); | ||
|
|
||
| await new Promise((resolve) => { | ||
| targetServer.listen(testPort, () => { | ||
| resolve(null); | ||
| }); | ||
| }); | ||
|
|
||
| proxy = new Redbird({ | ||
| port: proxyPort, | ||
| ssl: { | ||
| port: sslPort, | ||
| key: key, | ||
| cert: certificate, | ||
| }, | ||
| }); | ||
|
|
||
| await proxy.register({ | ||
| src: 'example.com', | ||
| target: `http://localhost:${testPort}`, | ||
| }); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await proxy.close(); | ||
| await new Promise((resolve) => targetServer.close(() => resolve(null))); | ||
| }); | ||
|
|
||
| it('should proxy HTTPS requests to the correct target', async () => { | ||
| const options = { | ||
| hostname: 'localhost', | ||
| port: sslPort, | ||
| path: '/', | ||
| method: 'GET', | ||
| headers: { | ||
| Host: 'example.com', | ||
| }, | ||
| }; | ||
|
|
||
| const response = await makeHttpsRequest(options); | ||
| expect(response.status).toBe(200); | ||
| expect(response.data).toBe(responseMessage); | ||
| }); | ||
|
|
||
| it('should fail for HTTPS requests to a non-configured host', async () => { | ||
| const options = { | ||
| hostname: 'localhost', | ||
| port: sslPort, | ||
| path: '/', | ||
| method: 'GET', | ||
| headers: { | ||
| Host: 'not-configured.example.com', | ||
| }, | ||
| }; | ||
|
|
||
| const response = await makeHttpsRequest(options); | ||
| expect(response.status).toBe(404); | ||
| }); | ||
|
|
||
| it('should proxy HTTP requests to the correct target', async () => { | ||
| const options = { | ||
| hostname: 'localhost', | ||
| port: proxyPort, | ||
| path: '/', | ||
| method: 'GET', | ||
| headers: { | ||
| Host: 'example.com', | ||
| }, | ||
| }; | ||
|
|
||
| const response = await makeHttpRequest(options); | ||
| expect(response.status).toBe(200); | ||
| expect(response.data).toBe(responseMessage); | ||
| }) | ||
| }); |
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.
The
!certs[hostname]check treatscerts[host] = void 0(used elsewhere to explicitly mean “use default certificates”) the same as “no entry for this hostname”. Ifthis.opts.letsencryptis enabled globally, hostnames intentionally configured to use the default cert will still enter the Let’s Encrypt loading path and may error. Consider distinguishing “missing” vs “present but undefined” (e.g.,!(hostname in certs)/!Object.hasOwn(certs, hostname)) before attempting Let’s Encrypt.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.
This might be useful, but my intention with my PR was to be a pretty minimal fix. So I kept the existing conditional and added the single check for letsencrypt