cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] CURLOPT_PROXYHEADER: send/replace proxy headers only

From: Maciej Puzio <mx34567_at_gmail.com>
Date: Fri, 24 Jan 2014 08:12:20 -0600

Daniel and Dan, thanks for your comments.

Dan:
> IMHO, it would be a pretty major break to change the default behaviour. There
> are undoubtedly applications out there that fall into all three categories, and
> a change would break at least one category.

I agree. Thinking aloud, perhaps the best solution would be to keep
backward-compatible behavior as currently implemented in Vijay's
patch, but issue a warning (especially in verbose mode). As I see it,
the choice is:

1. Break backward compatibility. The compatibility is with a
problematic and potentially unsafe behavior, but it is a backward
compatibility nonetheless. One can imagine that --header is used only
to set user agent, in which case there is nothing wrong sending it to
both proxy and origin. There are likely applications that do it and
that depend on it. (By the way, even with Vijay's patch, --user-agent
is always carried to both header sets, while --referer is never sent
to the proxy. Of course these can be overridden by --header and
--proxy-header.)

2. Keep behavior that is not only unsafe, but also default and
accepted silently. It doesn't take much to imagine a user who develops
and tests an application that uses curl to open a direct https
connection with some sensitive data in --header. Satisfied that the
application works well and securely, the user adds --proxy option,
perhaps because the application needs to be deployed in an environment
where proxy is required. User tests the application again and finds
that it does what it should, and so considers his/her work done. The
user reasonably expects that curl would know how to tunnel the request
through the proxy in a secure manner, and does not devote his/her time
to try to find security holes in curl. (Obviously, I was not that
user!) The moral is - if that behavior has to be unsafe and default,
at least it shouldn't be silent.

> Yes, the current behaviour is
> unclear and can cause security issues (in applications that that aren't aware
> of the behaviour), but it does work in most situations since servers will
> mostly ignore headers that aren't meant for them.

I think that this argument is for the opposing point of view, i.e. for
changing current behavior, rather than keeping it. We are talking here
about a security issue in the context where security matters. Yes,
curl currently does work - in a way that if proxy server or any party
between client and proxy does not eavesdrop, then we will be fine. But
with such an approach to security we can as well forget about SSL
altogether, send an unencrypted request, proxy it by GET, and avoid
the whole problem of two header sets that we discuss here. That's why
I am suggesting a warning, to let the user know that there is a
potential issue and how to fix it.

Daniel:
> It could also be a reason to reconsider the name. Perhaps a different one would
> make it clearer to users what the option actually does? I would assume that a
> typical user won't really see the difference when switching from http:// to https://
> or vice versa.

I thought about it too. "connect-header" would probably be too
confusing by not adequately describing its purpose (what connection
are we talking about?) Perhaps "proxy-connect-header"? We are talking
here about stuff for advanced users, so long and unfriendly name would
probably be ok. Anyway, I'll take any name, as long as the option is
there.

Thanks
Maciej

On Thu, Jan 23, 2014 at 5:49 PM, Dan Fandrich <dan_at_coneharvesters.com> wrote:
> On Thu, Jan 23, 2014 at 11:56:43PM +0100, Daniel Stenberg wrote:
>> I think this is a tricky question and I'd love to get more feedback
>> on this. The existing (before this patch) features affect headers to
>> both the server and the proxy, and since it does that we can't really
>> know which of the features existing applications want to preserve for
>> the future.
>>
>> A - they add/remove/change headers for the origin server so the proxy should
>> not get them. Like server auth or cookies.
>>
>> B - they add/remove/change headers for the proxy, so the server should not
>> get them. Like proxy auth or similar
>>
>> C - they add/remove/change headers for _both_ the proxy and the server. Like
>> user-agent or similar.
>>
>> You're clearly focusing on case (A) and I suppose we have reason to
>> suspect that is the most common use case. Is that a valid reason for
>> us to say that (B) and (C) applications need to be modified for them
>> to do right in the release with this change? I'm hesitating. Perhaps
>> it is.
>
> IMHO, it would be a pretty major break to change the default behaviour. There
> are undoubtedly applications out there that fall into all three categories, and
> a change would break at least one category. Yes, the current behaviour is
> unclear and can cause security issues (in applications that that aren't aware
> of the behaviour), but it does work in most situations since servers will
> mostly ignore headers that aren't meant for them.
>
> What about keeping the current behaviour as-is by default; i.e. if a header is
> set (with the old option), it is sent to both proxy server and destination
> server. But if the new option is set, the behaviour changes, such that headers
> set with the new option are only sent to the proxy and headers set with the
> old option are sent only to the destination server. Setting the new option to
> NULL enables the new behaviour but without sending any headers to the proxy
> server.
>
> That provides for backward compatibility as well as allowing all use cases.
>
>>>> Dan
> -------------------------------------------------------------------
> List admin: http://cool.haxx.se/list/listinfo/curl-library
> Etiquette: http://curl.haxx.se/mail/etiquette.html
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-01-24