[squid-dev] [PATCH] squid SSL subsystem did not initialized correctly
Amos Jeffries
squid3 at treenet.co.nz
Mon Aug 10 14:12:36 UTC 2015
On 10/08/2015 11:29 p.m., Tsantilas Christos wrote:
> On 08/06/2015 02:55 PM, Amos Jeffries wrote:
>> On 6/08/2015 9:54 p.m., Tsantilas Christos wrote:
>>> Hi all,
>>>
>>> Currently SSL subsystem did not initialized correctly in squid
>>> trunk.
>>> This is because of the Security::ProxyOutgoingConfig.encryptTransport
>>> which is always false so the client SSL CTX object never builds. As a
>>> result squid may not start if SSL is configured. I am attaching a small
>>> patch I am using in my squid trees to work with SSL.
>>
>> This always-enabled code is not compatible with the possible admin
>> configuration:
>>
>> tls_outgoing_options disable
>>
>>
>> Can you please try this instead:
>>
>> Security::PeerOptions::parse(const char *token)
>> {
>> if (strncmp(token, "disable", 7) == 0) {
>> clear();
>> + return;
>> } else if (strncmp(token, "cert=", 5) == 0) {
>> ...
>> } else {
>> debugs(3, DBG_CRITICAL, "ERROR: Unknown TLS option '" << ...
>> + return;
>> }
>> +
>> + encryptTransport = true;
>> }
>>
>>
>> If that works you can go through and also remove uses of
>> "secure.encryptTransport = true" from adaptation/ServiceConfig.cc and
>> cache_cf.cc where it is set next to a call to secure.parse()
>> ... but not the other one where it is set to always-on for https_port.
>
> This is will not work, because it is not required for someone to
> configure any of the sslproxy options for the SSL client to work.
> Squid can always work with the default options.
Did you test it?
The default squid.conf parser always sets "tls_outgoing_options
tls-min-version=1.0". Which should auto-enable DIRECT outgoing, then
explicit disable is required to turn off again.
http_port ... protocol=HTTPS and https_port forces
"encryptTransport=true;" explicitly based on the expected protocol. So
it is either enabled by the parse() call when TLS options are used, or
forced on anyway later when the protocol is validated.
icaps:// services also explicitly set "encryptTransport=true;"
explicitly based on 's' in the service URI scheme.
The cache_peer requires a minimum of "ssl" option to be configured. And
calls parse(). I see that simple case is passing "" token which gets
reported as unknown option.
With the attached patch TLS should be:
* default-on for all https_port, icaps:// services, and outgoing
https:// traffic.
* manually enabled on cache_peer and http_port.
* manually disabled on outgoing https:// traffic.
>
> The Security::ProxyOutgoingConfig.encryptTransport = true must be always
> true unless the the SSL client is disabled.
Yes. And the default config should see to that happening. Which is why I
asked if you could try the change.
>
> In previous squid releases it was not possible to disable SSL client,
> but now looks that this is can be done using the
> "tls_outgoing_options disable"
Yes, that is new in Squid-4. Along with some small non-OpenSSL HTTPS
support (not much yet, but growing).
>
> Maybe we need to add a parameter to Security::PeerOptions constructor,
> to define if the SSL is enabled by default (for example in the case of
> ProxyOutgoingConfig) or not (for example in HTTP ports configuration).
>
That would be messy because ProxyOutgoingConfig is a global and the
others are all explicitly constructed.
Amos
-------------- next part --------------
=== modified file 'src/adaptation/ServiceConfig.cc'
--- src/adaptation/ServiceConfig.cc 2015-05-06 14:58:01 +0000
+++ src/adaptation/ServiceConfig.cc 2015-08-10 12:49:30 +0000
@@ -119,41 +119,40 @@
else if (strcmp(name, "uri") == 0)
grokked = grokkedUri = grokUri(value);
else if (strcmp(name, "ipv6") == 0) {
grokked = grokBool(ipv6, name, value);
if (grokked && ipv6 && !Ip::EnableIpv6)
debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: IPv6 is disabled. ICAP service option ignored.");
} else if (strcmp(name, "max-conn") == 0)
grokked = grokLong(maxConn, name, value);
else if (strcmp(name, "on-overload") == 0) {
grokked = grokOnOverload(onOverload, value);
onOverloadSet = true;
} else if (strncmp(name, "ssl", 3) == 0 || strncmp(name, "tls-", 4) == 0) {
#if !USE_OPENSSL
debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: adaptation option '" << name << "' requires --with-openssl. ICAP service option ignored.");
#else
// name prefix is "ssl" or "tls-"
std::string tmp = name + (name[0] == 's' ? 3 : 4);
tmp += "=";
tmp += value;
secure.parse(tmp.c_str());
- secure.encryptTransport = true;
grokked = true;
#endif
} else
grokked = grokExtension(name, value);
if (!grokked)
return false;
}
// set default on-overload value if needed
if (!onOverloadSet)
onOverload = bypass ? srvBypass : srvWait;
// is the service URI set?
if (!grokkedUri) {
debugs(3, DBG_CRITICAL, cfg_filename << ':' << config_lineno << ": " <<
"No \"uri\" option in adaptation service definition");
return false;
}
=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc 2015-08-04 21:04:09 +0000
+++ src/cache_cf.cc 2015-08-10 12:47:04 +0000
@@ -2176,45 +2176,43 @@
} else if (!strncmp(token, "max-conn=", 9)) {
p->max_conn = xatoi(token + 9);
} else if (!strncmp(token, "standby=", 8)) {
p->standby.limit = xatoi(token + 8);
} else if (!strcmp(token, "originserver")) {
p->options.originserver = true;
} else if (!strncmp(token, "name=", 5)) {
safe_free(p->name);
if (token[5])
p->name = xstrdup(token + 5);
} else if (!strncmp(token, "forceddomain=", 13)) {
safe_free(p->domain);
if (token[13])
p->domain = xstrdup(token + 13);
} else if (strncmp(token, "ssl", 3) == 0) {
#if !USE_OPENSSL
debugs(0, DBG_CRITICAL, "WARNING: cache_peer option '" << token << "' requires --with-openssl");
#else
- p->secure.encryptTransport = true;
p->secure.parse(token+3);
#endif
} else if (strncmp(token, "tls-", 4) == 0) {
- p->secure.encryptTransport = true;
p->secure.parse(token+4);
} else if (strcmp(token, "front-end-https") == 0) {
p->front_end_https = 1;
} else if (strcmp(token, "front-end-https=on") == 0) {
p->front_end_https = 1;
} else if (strcmp(token, "front-end-https=auto") == 0) {
p->front_end_https = 2;
} else if (strcmp(token, "connection-auth=off") == 0) {
p->connection_auth = 0;
} else if (strcmp(token, "connection-auth") == 0) {
p->connection_auth = 1;
} else if (strcmp(token, "connection-auth=on") == 0) {
p->connection_auth = 1;
} else if (strcmp(token, "connection-auth=auto") == 0) {
p->connection_auth = 2;
} else if (token[0] == '#') {
// start of a text comment. stop reading this line.
break;
} else {
debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "ERROR: Ignoring unknown cache_peer option '" << token << "'");
=== modified file 'src/security/PeerOptions.cc'
--- src/security/PeerOptions.cc 2015-07-19 08:33:29 +0000
+++ src/security/PeerOptions.cc 2015-08-10 12:46:13 +0000
@@ -25,79 +25,91 @@
Security::PeerOptions::PeerOptions(const Security::PeerOptions &p) :
certFile(p.certFile),
privateKeyFile(p.privateKeyFile),
sslOptions(p.sslOptions),
caFile(p.caFile),
caDir(p.caDir),
crlFile(p.crlFile),
sslCipher(p.sslCipher),
sslFlags(p.sslFlags),
sslDomain(p.sslDomain),
parsedOptions(p.parsedOptions),
parsedFlags(p.parsedFlags),
sslVersion(p.sslVersion),
encryptTransport(p.encryptTransport)
{
}
void
Security::PeerOptions::parse(const char *token)
{
+ if (!*token) {
+ // config says just "ssl" or "tls" (or "tls-")
+ encryptTransport = true;
+ return;
+ }
+
if (strncmp(token, "disable", 7) == 0) {
clear();
- } else if (strncmp(token, "cert=", 5) == 0) {
+ return;
+ }
+
+ if (strncmp(token, "cert=", 5) == 0) {
certFile = SBuf(token + 5);
if (privateKeyFile.isEmpty())
privateKeyFile = certFile;
} else if (strncmp(token, "key=", 4) == 0) {
privateKeyFile = SBuf(token + 4);
if (certFile.isEmpty()) {
debugs(3, DBG_PARSE_NOTE(1), "WARNING: cert= option needs to be set before key= is used.");
certFile = privateKeyFile;
}
} else if (strncmp(token, "version=", 8) == 0) {
debugs(0, DBG_PARSE_NOTE(1), "UPGRADE WARNING: SSL version= is deprecated. Use options= to limit protocols instead.");
sslVersion = xatoi(token + 8);
} else if (strncmp(token, "min-version=", 12) == 0) {
tlsMinVersion = SBuf(token + 12);
} else if (strncmp(token, "options=", 8) == 0) {
sslOptions = SBuf(token + 8);
parsedOptions = parseOptions();
} else if (strncmp(token, "cipher=", 7) == 0) {
sslCipher = SBuf(token + 7);
} else if (strncmp(token, "cafile=", 7) == 0) {
caFile = SBuf(token + 7);
} else if (strncmp(token, "capath=", 7) == 0) {
caDir = SBuf(token + 7);
} else if (strncmp(token, "crlfile=", 8) == 0) {
crlFile = SBuf(token + 8);
} else if (strncmp(token, "flags=", 6) == 0) {
if (parsedFlags != 0) {
debugs(3, DBG_PARSE_NOTE(1), "WARNING: Overwriting flags=" << sslFlags << " with " << SBuf(token + 6));
}
sslFlags = SBuf(token + 6);
parsedFlags = parseFlags();
} else if (strncmp(token, "domain=", 7) == 0) {
sslDomain = SBuf(token + 7);
} else {
debugs(3, DBG_CRITICAL, "ERROR: Unknown TLS option '" << token << "'");
+ return;
}
+
+ encryptTransport = true;
}
void
Security::PeerOptions::dumpCfg(Packable *p, const char *pfx) const
{
if (!encryptTransport) {
p->appendf(" %sdisable", pfx);
return; // no other settings are relevant
}
if (!certFile.isEmpty())
p->appendf(" %scert=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(certFile));
if (!privateKeyFile.isEmpty() && privateKeyFile != certFile)
p->appendf(" %skey=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(privateKeyFile));
if (!sslOptions.isEmpty())
p->appendf(" %soptions=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(sslOptions));
if (!sslCipher.isEmpty())
More information about the squid-dev
mailing list