Re: [PATCH] ConnStateData flexible transport support
On 29/04/2014 9:12 a.m., Alex Rousskov wrote: On 04/28/2014 07:10 AM, Amos Jeffries wrote: * ssl-bump transforms the transportVersion from whatever it was previously (usually HTTP or HTTPS) to HTTPS. +/// the transport protocol currently being spoken on this connection +AnyP::ProtocolVersion transportVersion; If I recall our earlier discussions correctly, transport is TCP, UDP, and such while HTTP and HTTPS are transfer protocols. It sounds like you want to use the new data member for transfer, not transport protocols. If yes, the member should be renamed accordingly. Also, if this is often about the protocol name rather than just its version, let's remove Version from the data member name (PortCfg already uses that approach). Finally, the patch does not actually use the version part of the transportVersion data member AFAICT. Perhaps the data member type should be changed from ProtocolVersion to ProtocolType? I'm fine with transferProtocol or just protocol (although that may get as common as conn has become). Whichever suits you. The version is not used yet only because ssl-bump does not need it (AFAIK. possibly if ssl-bump identified the client is using HTTP/0.9 inside the CONNECT tunnel it may want to do special things, but we don't today). The 2.0 code needs the version to identify several key logic decisions, like which parser to allocate/use on reads(2), how to manage the pipeline, what to do with 10x messages etc. -debugs(33, 5, HERE converting clientConnection to SSL); +debugs(33, 5, converting clientConnection to SSL); To reduce merge conflicts, please do not remove HERE from debugging lines that otherwise do not need to be changed. Okay. This variable can be altered whenever necessary to cause an on-wire protocol change. Altering the data member does not cause an on-wire protocol change in the patched code AFAICT. Perhaps you meant that the data member should always reflect the current wire protocol? Yes. However I was planning to use it to decode which Packer was allocated. So it is both a reflection of the transfer protocol and a cause of how messages converted to bytes. I am fine with documenting it as a reflection though. Amos
Re: [PATCH] ConnStateData flexible transport support
On 04/29/2014 06:03 AM, Amos Jeffries wrote: I'm fine with transferProtocol or just protocol (although that may get as common as conn has become). Whichever suits you. Let's avoid protocol for the reasons you mentioned and use transferProtocol. This variable can be altered whenever necessary to cause an on-wire protocol change. Altering the data member does not cause an on-wire protocol change in the patched code AFAICT. Perhaps you meant that the data member should always reflect the current wire protocol? Yes. However I was planning to use it to decode which Packer was allocated. Sure, and the new data member is already used for other things. To the extent possible, the description should focus on the meaning, not use, especially when the use cases are diverse. Thank you, Alex.
[PATCH] ConnStateData flexible transport support
We are quickly approaching a time when a client connection can freely
migrate between protocols or versions of protocols. Already we have
ssl-bump which can switch a connection from HTTP to HTTPS. I am
expecting switching HTTP-HTTPS via Upgrade, and HTTP/1-HTTP/2 via
magic, Upgrade, or ALPN.
Based on ssl-bump experience with switchedToHttps() and the pain that
can be predicted when there are several permutations of such accessors
to test against I am proposing the attached patch.
* Add a transportVersion member to ConnStateData which holds the current
protocol to be used over the clientConnection socket. This variable can
be altered whenever necessary to cause an on-wire protocol change. New
connections default to the protocol signalled in the http(s)_port directive.
* ssl-bump transforms the transportVersion from whatever it was
previously (usually HTTP or HTTPS) to HTTPS.
- Also updated ssl-bump to set the traffic type flag tunnelSslBumped on
non-intercept traffic, which can be assumed to be a CONNECT request.
* transparent and reverse-proxy URL reconstruction is updated to use the
new member instead of the http(s)_port protocol= setting. This removes
edge conditions where the URL reconstructor needs to figure out ssl-bump
existence.
Christos,
I would like some help with two ssl-bump related things if you have the
time spare:
1) testing the new prepareTransparentURL reconstruction works on
ssl-bumped traffic.
2) finding switchedToHttps() usage that can be replaced.
When ssl-bump is operating we should now always have one of these
conditions being true:
a) ConnStateData::port-flags.tunnelSslBump
b) ConnStateData::port-intercepted()
ConnStateData::transportVersion.protocol==AnyP::PROTO_HTTPS
The second one is low-priority even if this patch gets added. We will
find them later anyway as the code polishing progresses.
Amos
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2014-04-28 10:28:00 +
+++ src/client_side.cc 2014-04-28 10:28:59 +
@@ -2099,94 +2099,92 @@
const bool switchedToHttps = conn-switchedToHttps();
const bool tryHostHeader = vhost || switchedToHttps;
if (tryHostHeader (host = mime_get_header(req_hdr, Host)) != NULL) {
debugs(33, 5, ACCEL VHOST REWRITE: vhost= host + vport=
vport);
char thost[256];
if (vport 0) {
thost[0] = '\0';
char *t = NULL;
if (host[strlen(host)] != ']' (t = strrchr(host,':')) != NULL) {
strncpy(thost, host, (t-host));
snprintf(thost+(t-host), sizeof(thost)-(t-host), :%d, vport);
host = thost;
} else if (!t) {
snprintf(thost, sizeof(thost), %s:%d,host, vport);
host = thost;
}
} // else nothing to alter port-wise.
int url_sz = strlen(url) + 32 + Config.appendDomainLen +
strlen(host);
http-uri = (char *)xcalloc(url_sz, 1);
-const char *protocol = switchedToHttps ?
- https :
AnyP::UriScheme(conn-port-transport.protocol).c_str();
-snprintf(http-uri, url_sz, %s://%s%s, protocol, host, url);
+snprintf(http-uri, url_sz, %s://%s%s,
AnyP::UriScheme(conn-transportVersion.protocol).c_str(), 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);
int url_sz = strlen(url) + 32 + Config.appendDomainLen +
strlen(conn-port-defaultsite);
http-uri = (char *)xcalloc(url_sz, 1);
char vportStr[32];
vportStr[0] = '\0';
if (vport 0) {
snprintf(vportStr, sizeof(vportStr),:%d,vport);
}
snprintf(http-uri, url_sz, %s://%s%s%s,
- AnyP::UriScheme(conn-port-transport.protocol).c_str(),
conn-port-defaultsite, vportStr, url);
+ AnyP::UriScheme(conn-transportVersion.protocol).c_str(),
conn-port-defaultsite, vportStr, url);
debugs(33, 5, ACCEL DEFAULTSITE REWRITE: ' http-uri ');
} else if (vport 0 /* (!vhost || no Host:) */) {
debugs(33, 5, ACCEL VPORT REWRITE: http_port IP + vport= vport);
/* Put the local socket IP address as the hostname, with whatever
vport we found */
int url_sz = strlen(url) + 32 + Config.appendDomainLen;
http-uri = (char *)xcalloc(url_sz, 1);
http-getConn()-clientConnection-local.toHostStr(ipbuf,MAX_IPSTRLEN);
snprintf(http-uri, url_sz, %s://%s:%d%s,
- AnyP::UriScheme(conn-port-transport.protocol).c_str(),
+ AnyP::UriScheme(conn-transportVersion.protocol).c_str(),
ipbuf, vport, url);
debugs(33, 5, ACCEL VPORT REWRITE: ' http-uri ');
}
}
static void
Re: [PATCH] ConnStateData flexible transport support
On 04/28/2014 07:10 AM, Amos Jeffries wrote: * ssl-bump transforms the transportVersion from whatever it was previously (usually HTTP or HTTPS) to HTTPS. +/// the transport protocol currently being spoken on this connection +AnyP::ProtocolVersion transportVersion; If I recall our earlier discussions correctly, transport is TCP, UDP, and such while HTTP and HTTPS are transfer protocols. It sounds like you want to use the new data member for transfer, not transport protocols. If yes, the member should be renamed accordingly. Also, if this is often about the protocol name rather than just its version, let's remove Version from the data member name (PortCfg already uses that approach). Finally, the patch does not actually use the version part of the transportVersion data member AFAICT. Perhaps the data member type should be changed from ProtocolVersion to ProtocolType? -debugs(33, 5, HERE converting clientConnection to SSL); +debugs(33, 5, converting clientConnection to SSL); To reduce merge conflicts, please do not remove HERE from debugging lines that otherwise do not need to be changed. This variable can be altered whenever necessary to cause an on-wire protocol change. Altering the data member does not cause an on-wire protocol change in the patched code AFAICT. Perhaps you meant that the data member should always reflect the current wire protocol? Cheers, Alex.
