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

Fix an issue with NTLM authentication #727

Closed
wants to merge 1 commit into from

Conversation

mburbea
Copy link

@mburbea mburbea commented Oct 31, 2014

NTLM authentication sends a challenge and response which translate to www-authenticate fields which are concated into a single string. When you send the response back proxied you need to split the string and send it back as an array. You will still need to create a keepalive agent to use this. So I used the library agentkeepalive like so..

To actually use this code a relevant code snippet might be:

var httpProxy = require('http-proxy');
var Agent = require('agentkeepalive');

var agent =  new Agent({
  maxSockets: 100,
  keepAlive :true,
  maxFreeSockets: 10,
  keepAliveMsecs:1000,
  timeout: 60000,
  keepAliveTimeout: 30000 // free socket keepalive for 30 seconds
});
var proxy = httpProxy.createProxyServer({ target:myserver,agent:agent});

/*

NTLM authentication sends a challenge and response which translate to www-authenticate fields which are concated into a single string. When you send the response back proxied you need to split the string and send it back as an array. You will still need to create a keepalive agent to use this.
@jonnii
Copy link

jonnii commented Oct 31, 2014

👍

@indexzero
Copy link
Contributor

There should be a way to do this without injecting this logic here. Perhaps exposing a way to mutate headers before they are written?

@jcrugzz
Copy link
Contributor

jcrugzz commented Oct 31, 2014

@mburbea you can listen for the proxyReq event and modify/add headers before the request is sent.

@mburbea
Copy link
Author

mburbea commented Nov 1, 2014

Perhaps I could, but I don't know if this is the best way to resolve this. Proxying NTLM is a very common scenario in corporate settings. This is not some oddball case and most other proxies can actually handle this scenario just find. It is also extremely hard to find documentation on this, and other people who use this tool and need to proxy ntlm would have a much easier time if this was added and certainly documentation about this case would be great. The fact that out of the box, keepalive wasn't configured was actually alarming (without using the agent but using this patch you will get a 400 bad-request on the challenge part of ntlm). I think that the issue in that the two request headers www-authenticate are mangled into a single string when received but need to be sent back to the proxied header as an array of the components. There might be other such mangling. Just like x-headers are supported, I'd be ok with a "ntlm" switch something that makes it obvious to use without being something entirely custom.

@mburbea
Copy link
Author

mburbea commented Nov 6, 2014

How exactly could I go about doing this with proxyReq? I can't seem to figure it out and proxyRes I assume is too late in the pipeline to be relevant.

@jcrugzz
Copy link
Contributor

jcrugzz commented Nov 6, 2014

@mburbea I realized you do want proxyRes as the event is emitted right before we go through the function you have modified in the PR. So while not elegant, the solution would be as follows.

var httpProxy = require('http-proxy');
var Agent = require('agentkeepalive');

var agent =  new Agent({
  maxSockets: 100,
  keepAlive: true,
  maxFreeSockets: 10,
  keepAliveMsecs:1000,
  timeout: 60000,
  keepAliveTimeout: 30000 // free socket keepalive for 30 seconds
});

var proxy = httpProxy.createProxy({ target: 'http://whatever.com', agent: agent);

// Modify headers of the request before it gets sent
proxy.on('proxyRes', function (proxyRes) {
  var key = 'www-authenticate';
  proxyRes.headers[key] = proxyRes.headers[key] && proxyRes.headers[key].split(',');
});

require('http').createServer(function (req, res) {
  proxy.web(req, res);
}).listen(3000);

The rest is taken care of by the exact function you modified since you were able to touch the headers before hand. I will still consider your PR if this use case is large enough, I just dislike the custom logic bit as it seems niche to me.

@mburbea
Copy link
Author

mburbea commented Nov 6, 2014

Thanks that does indeed work. I was able to visit my proxied site.
NTLM-SSPI authentication is not very niche in corporate intranet/internal app development. For now, I will happily switch to using this in my proxying code, but I would like to remove it as it a hack as you said. #362 is an outstanding issue is actually where I got the logic for the code change and the second half of the puzzle (requiring to switch to an agent that supports keepalive).

@jcrugzz
Copy link
Contributor

jcrugzz commented Nov 7, 2014

@mburbea I still believe there should be no agent by default but we should be clear in the documentation that this disables keepalive and to suggest using this module for that particular case. I'd definitely take a PR for the documentation 😄

@robertlevy
Copy link

+1

@jcrugzz jcrugzz mentioned this pull request Oct 30, 2015
@jcrugzz
Copy link
Contributor

jcrugzz commented Oct 30, 2015

This shouldn't be handled in http-proxy. Adding my example to the examples folder as we speak

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

Successfully merging this pull request may close these issues.

5 participants