On 8/12/2011 11:58 a.m., Amos Jeffries wrote:
On Wed, 07 Dec 2011 18:59:15 +0200, Tsantilas Christos wrote:
On 12/07/2011 01:19 PM, Amos Jeffries wrote:
So, in theory ssl-bump is a form of intercept not a form of
accelerator. Its use of prepareAcceleratorURL() to convert the partial
to absolute URL unconditionally sets the accel flag with a mix of side
effects. Some bad ones have been identified already.
This patch changes the flag setting, to allow ssl-bump to use the URL
preparation function without the side effects. I'm in half a mind to
make a ssl-bump specific URL preparation function, but only after this
is proven workable.
Christos: as the person who appears to have the best testing ability
for
ssl-bump can you run your tests over the resulting Squid and check that
the expected behaviours have not changed for the worse? I am fully
expecting there to be several as yet unknown places needing to add a
test of the sslBumped flag alongside testing accel flag.
Looks OK.
Also because accel and related flags are used in http authentication and
esi, I think too it is better to detach it from sslbumped requests,
where we should handle authentication with a different way...
The only objection is for the Config.onoff.ie_refresh parameter.
Look in the client_side_request.cc line 1027 inside block:
if (Config.onoff.ie_refresh) {
}
But does not look important....
Hmm, maybe. The docs around that code are incorrect "or transparent
proxy" is not one of the cases it kicks in. Only (accel && ims).
We can either drop it or need to add all of sslbump, intercept,
tproxy, and accel flags.
Since it is so screwed I think we can leave it for now and fix
separately.
Thank you for the check. I have moved on to splitting the prepare*()
function now.
On the SSL intercept case we don't have any backup host:port that I
can see easily. If it is saved somewhere please let me know.
For now the order of hot:port locating is:
- use bumped requests Host: header, else
- use CONNECT authority-URL, (http->sslHostName contains the host:port
from the CONNECT right?)
- reject with "400 Invalid Request" (RFC 2616 mandated response on
missing Host:)
I have also added a test case to the transparent intercept case to
default port-443 received traffic through this new https:// URL
emitting function. port-80 and unknown ports through the old function
emitting http:// URLs.
Amos
Here is the mk2 patch. With better re-assembly of the absolute URL for
ssl-bump and SSL interception case as well.
The only thing I'm worried about now is that strange ports on CONNECT
request being ssl-bumped might be lost with sslHostName. The same is
assumed for intercepted SSL, so its not a huge blocker, but could be a
problem for some. Ideas?
Amos
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2011-12-06 14:06:38 +0000
+++ src/client_side.cc 2011-12-08 13:22:33 +0000
@@ -1986,6 +1986,7 @@
}
}
+/// Convert a partial-URL into an absolute URL when it has been received on a
reverse-proxy (accel) configured port
static void
prepareAcceleratedURL(ConnStateData * conn, ClientHttpRequest *http, char
*url, const char *req_hdr)
{
@@ -1994,8 +1995,6 @@
char *host;
char ipbuf[MAX_IPSTRLEN];
- http->flags.accel = 1;
-
/* BUG: Squid cannot deal with '*' URLs (RFC2616 5.1.2) */
if (strncasecmp(url, "cache_object://", 15) == 0)
@@ -2035,9 +2034,7 @@
if (vport < 0)
vport = http->getConn()->clientConnection->local.GetPort();
- const bool switchedToHttps = conn->switchedToHttps();
- const bool tryHostHeader = vhost || switchedToHttps;
- if (tryHostHeader && (host = mime_get_header(req_hdr, "Host")) != NULL) {
+ if (vhost && (host = mime_get_header(req_hdr, "Host")) != NULL) {
debugs(33, 5, "ACCEL VHOST REWRITE: vhost=" << host << " + vport=" <<
vport);
char thost[256];
if (vport > 0) {
@@ -2055,9 +2052,7 @@
int url_sz = strlen(url) + 32 + Config.appendDomainLen +
strlen(host);
http->uri = (char *)xcalloc(url_sz, 1);
- const char *protocol = switchedToHttps ?
- "https" : conn->port->protocol;
- snprintf(http->uri, url_sz, "%s://%s%s", protocol, host, url);
+ snprintf(http->uri, url_sz, "%s://%s%s", conn->port->protocol, host,
url);
debugs(33, 5, "ACCEL VHOST REWRITE: '" << http->uri << "'");
} else if (conn->port->defaultsite /* && !vhost */) {
debugs(33, 5, "ACCEL DEFAULTSITE REWRITE: defaultsite=" <<
conn->port->defaultsite << " + vport=" << vport);
@@ -2085,8 +2080,9 @@
}
}
+/// Convert a partial-URL into an absolute http:// URL when it has been
intercepted from port 80
static void
-prepareTransparentURL(ConnStateData * conn, ClientHttpRequest *http, char
*url, const char *req_hdr)
+prepareInterceptedHttpURL(ConnStateData * conn, ClientHttpRequest *http, char
*url, const char *req_hdr)
{
char *host;
char ipbuf[MAX_IPSTRLEN];
@@ -2095,13 +2091,13 @@
return; /* already in good shape */
/* BUG: Squid cannot deal with '*' URLs (RFC2616 5.1.2) */
- // BUG 2976: Squid only accepts intercepted HTTP.
+ // BUG 2976: Assume http:// scheme if it gets here.
if ((host = mime_get_header(req_hdr, "Host")) != NULL) {
int url_sz = strlen(url) + 32 + Config.appendDomainLen +
strlen(host);
http->uri = (char *)xcalloc(url_sz, 1);
- snprintf(http->uri, url_sz, "http://%s%s";, /*conn->port->protocol,*/
host, url);
+ snprintf(http->uri, url_sz, "http://%s%s";, host, url);
debugs(33, 5, "TRANSPARENT HOST REWRITE: '" << http->uri <<"'");
} else {
/* Put the local socket IP address as the hostname. */
@@ -2109,12 +2105,57 @@
http->uri = (char *)xcalloc(url_sz, 1);
http->getConn()->clientConnection->local.ToHostname(ipbuf,MAX_IPSTRLEN),
snprintf(http->uri, url_sz, "http://%s:%d%s";,
- // http->getConn()->port->protocol,
ipbuf, http->getConn()->clientConnection->local.GetPort(),
url);
debugs(33, 5, "TRANSPARENT REWRITE: '" << http->uri << "'");
}
}
+/// Convert a partial-URL into an absolute https:// URL when it has been
intercepted from port 443 or a decrypted CONNECT request
+static void
+prepareInterceptedHttpsURL(ConnStateData * conn, ClientHttpRequest *http, char
*url, const char *req_hdr)
+{
+ if (*url != '/')
+ return; /* already in good shape */
+
+ /* BUG: Squid cannot deal with '*' URLs (RFC2616 5.1.2) */
+ // BUG 2976: Assume https:// scheme if it gets here.
+
+ const char *host;
+ if ((host = mime_get_header(req_hdr, "Host")) != NULL) {
+ int url_sz = strlen(url) + 32 + Config.appendDomainLen + strlen(host);
+ http->uri = (char *)xcalloc(url_sz, 1);
+ snprintf(http->uri, url_sz, "https://%s%s";, host, url);
+ debugs(33, 5, "SSL INTERCEPT HOST REWRITE: '" << http->uri << "'");
+
+ } else if(conn->sslHostName.size()>0) {
+ // Put the CONNECT request authority details (host:port) as the
hostname.
+ int url_sz = strlen(url) + 32 + Config.appendDomainLen +
conn->sslHostName.size();
+ http->uri = (char *)xcalloc(url_sz, 1);
+ snprintf(http->uri, url_sz, "https://"; SQUIDSTRINGPH "%s",
SQUIDSTRINGPRINT(conn->sslHostName), url);
+ debugs(33, 5, "SSL INTERCEPT REWRITE: '" << http->uri << "'");
+
+ } else {
+ // Intercepted traffic with no Host: is unable to be safely repaired
(yet, maybe use SNI later).
+ // RFC 2616 section 14.32 'MUST respond with a 400'
+ clientStreamNode *node = (clientStreamNode
*)http->client_stream.tail->prev->data;
+ clientReplyContext *repContext = dynamic_cast<clientReplyContext
*>(node->data.getRaw());
+ assert(repContext);
+ repContext->setReplyToError(ERR_INVALID_REQ, HTTP_BAD_REQUEST,
+ http->request->method, NULL,
+ http->getConn()->clientConnection->remote,
+ http->request,
+ NULL,
+#if USE_AUTH
+ http->getConn() != NULL &&
http->getConn()->auth_user_request != NULL ?
+ http->getConn()->auth_user_request :
http->request->auth_user_request);
+#else
+ NULL);
+#endif
+ node = (clientStreamNode *)http->client_stream.tail->data;
+ clientStreamRead(node, http, node->readBuffer);
+ }
+}
+
/**
* parseHttpRequest()
*
@@ -2264,25 +2305,48 @@
#endif
- debugs(33,5, HERE << "repare absolute URL from " <<
(csd->transparent()?"intercept":(csd->port->accel ? "accel":"")));
+ debugs(33, 5, "parseHttpRequest: Complete request received");
+
+ // XXX: crop this dump at the end of headers. No need for extras
+ debugs(11, 2, "HTTP Client " << csd->clientConnection);
+ debugs(11, 2, "HTTP Client REQUEST:\n---------\n" << (hp->buf) +
hp->req.m_start << "\n----------");
+
+
+ debugs(33,5, HERE << "Repair absolute URL from " <<
(csd->switchedToHttps()?"SSL ":"") <<
+ (csd->transparent()?"intercept":(csd->port->accel ? "accel":"")));
/* Rewrite the URL in transparent or accelerator mode */
/* NP: there are several cases to traverse here:
* - standard mode (forward proxy)
- * - transparent mode (TPROXY)
+ * - transparent mode (TPROXY port 80)
+ * - transparent mode (TPROXY port 443)
* - transparent mode with failures
- * - intercept mode (NAT)
+ * - intercept mode (NAT port 80)
+ * - intercept mode (NAT port 443)
+ * - intercept mode (SSL CONNECT requests)
* - intercept mode with failures
* - accelerator mode (reverse proxy)
* - internal URL
* - mixed combos of the above with internal URL
*/
if (csd->transparent()) {
- /* intercept or transparent mode, properly working with no failures */
- prepareTransparentURL(csd, http, url, req_hdr);
-
- } else if (csd->port->accel || csd->switchedToHttps()) {
+ /* NAT intercept or TPROXY mode, properly working with no failures */
+ unsigned short port =
http->getConn()->clientConnection->local.GetPort();
+ if (port == 80)
+ prepareInterceptedHttpURL(csd, http, url, req_hdr);
+ else if (port == 443)
+ prepareInterceptedHttpsURL(csd, http, url, req_hdr);
+ else {
+ debugs(33, 4, HERE << "Intercepted port " << port << ". Assuming
its HTTP.");
+ prepareInterceptedHttpURL(csd, http, url, req_hdr);
+ }
+ } else if (csd->port->accel) {
/* accelerator mode */
prepareAcceleratedURL(csd, http, url, req_hdr);
+ http->flags.accel = 1;
+
+ } else if (csd->switchedToHttps()) {
+ /* CONNECT 'ssl-bump' interception mode */
+ prepareInterceptedHttpsURL(csd, http, url, req_hdr);
} else if (internalCheck(url)) {
/* internal URL mode */
@@ -2299,12 +2363,6 @@
strcpy(http->uri, url);
}
- debugs(33, 5, "parseHttpRequest: Complete request received");
-
- // XXX: crop this dump at the end of headers. No need for extras
- debugs(11, 2, "HTTP Client " << csd->clientConnection);
- debugs(11, 2, "HTTP Client REQUEST:\n---------\n" << (hp->buf) +
hp->req.m_start << "\n----------");
-
result->flags.parsed_ok = 1;
xfree(url);
return result;
@@ -3593,7 +3651,7 @@
// We are going to read new request
flags.readMore = true;
- debugs(33, 5, HERE << "converting " << clientConnection << " to SSL");
+ debugs(33, 5, HERE << "converting " << clientConnection << " (for " <<
sslHostName << ") to SSL");
return getSslContextStart();
}
