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

Proxy sending CONNECT requests for http requests #183

Open
smckee-r7 opened this issue Nov 15, 2024 · 4 comments
Open

Proxy sending CONNECT requests for http requests #183

smckee-r7 opened this issue Nov 15, 2024 · 4 comments

Comments

@smckee-r7
Copy link
Contributor

smckee-r7 commented Nov 15, 2024

Hey, while using mockttp, we noticed mockttp sending CONNECT requests for http:// requests. My understanding is CONNECT request should only be used for https:// requests? Here's a script that shows the behaviour clearly:

const mockttp = require('mockttp');

// Create server to detect CONNECT method requests
const server = require('http').createServer((req, res) => {
  res.writeHead(200, { 'Content-Type': 'text/plain' });
  res.end('Default request');
});
server.on('connect', (req, socket) => {
  console.log('CONNECT Request received');
  socket.end();
});
server.listen(3011, () => console.log('Server listening'));

// Create mockttp passthrough proxy that forwards to our CONNECT detection server
(async () => {
  const https = await mockttp.generateCACertificate()
  const externalproxy = mockttp.getLocal({ https });
  await externalproxy.start(3010);

  console.log(`Mockttp Proxy server running on port ${externalproxy.port}`);
  externalproxy.forAnyRequest().thenPassThrough({
    ignoreHostHttpsErrors: true,
    proxyConfig: { proxyUrl: `http://127.0.0.1:3011` }
  });
})();

Both these curl request trigger a CONNECT request:
curl -k --proxy http://127.0.0.1:3010 http://demo.testfire.net/
curl -k --proxy http://127.0.0.1:3010 https://demo.testfire.net/

When we don't proxy via mockttp, we only see a CONNECT request for https://demo.testfire.net which is what I'd expect:
curl -k --proxy http://127.0.0.1:3011 http://demo.testfire.net/
curl -k --proxy http://127.0.0.1:3011 https://demo.testfire.net/

My question would be is there a reason mockttp behaves like this because it seems unexpected that mockttp would send CONNECT requests for http requests?

@pimterry
Copy link
Member

CONNECT is only required for HTTPS, but it's perfectly valid for HTTP too (or actually any other protocol at all - it's just a generic tunnel) and it's generally preferable to use it for everything since there's no downside and it keeps the tunnelling approach consistent. There are also plenty of proxies who only support CONNECT (who don't accept direct requests for other URLs) so it's better for compatibility.

Is there a specific reason you don't want this to happen?

@smckee-r7
Copy link
Contributor Author

Thanks for the quick response, it would be useful to have an option to disable this CONNECT request for http request, as we know of at least one application where the CONNECT request is used to flag if the connection is secure or not

@pimterry
Copy link
Member

That's interesting. What application?

To be honest, I'm not sure it's a good idea - that just sounds like a bug in the application (it's making a security assumption that's not correct at all here) that needs fixing on their side. There's plenty of other software that expects all proxies to support CONNECT too. In my experience, it's the standard approach. Wikipedia also says:

The most common form of HTTP tunneling is the standardized HTTP CONNECT method.

and Cloudflare's very first example in their intro to HTTP proxying is actually plain HTTP via CONNECT: https://blog.cloudflare.com/a-primer-on-proxies/#http-1-1-and-connect. It's super common and standard.

We could migrate away from that, but it's likely to cause far more problems in the other direction I expect.

@smckee-r7
Copy link
Contributor Author

smckee-r7 commented Nov 15, 2024

Thanks for all the great information and explanation, I really do appreciate it.

It's an internal application and we’d have to discuss with the maintainers of it on why it works this way, but based on what you’ve said it looks like something that should most likely be changed in that applications implementation. So we’ll follow that up with them next week, but thanks again for the help! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants