Hello Andreas, On Tue, 14 Jul 2020 13:57:48 +0200 Andreas Schulz <andreas.sch...@tds.fujitsu.com> wrote: > Package: squid > Version: 3.5.23-5+deb9u2.1 > Severity: important > File: /usr/sbin/squid > > Dear Maintainer, > > We installed the security update deb9u2 and learned that no more > http-access (with icap) was possible.
I am not the maintainer but I have prepared the security update for squid3 in Stretch. So far you are the only one who reported this problem. I had sent a request for testing but never received any feedback. [1] Please note that Stretch is now supported by the LTS team. We have a dedicated mailing list where you can report problems dedicated to packages in Stretch called debian-...@lists.debian.org. Could you set debug_options to ALL,9 (which should enable full debugging according to the squid wiki) and reproduce the issue again? Please attach the complete log either to this bug report or send it to me via private email directly. The patch for CVE-2019-12523 contains only one line that appears to touch icap related code in src/adaptation/icap/ModXact.cc. I have reverted this change and attached a new CVE-2019-12523.patch. Could you apply it and report back if it makes any difference? Otherwise only the debug log could help to narrow down the problem. Regards, Markus [1] https://lists.debian.org/debian-lts/2020/07/msg00018.html
From: Markus Koschany <a...@debian.org> Date: Tue, 30 Jun 2020 22:11:00 +0200 Subject: CVE-2019-12523 Origin: http://www.squid-cache.org/Versions/v4/changesets/squid-4-fbbdf75efd7a5cc244b4886a9d42ea458c5a3a73.patch --- src/HttpRequest.cc | 12 +-- src/HttpRequest.h | 4 +- src/URL.h | 4 +- src/acl/Asn.cc | 2 +- src/adaptation/ecap/MessageRep.cc | 4 +- src/client_side.cc | 2 +- src/client_side_request.cc | 4 +- src/htcp.cc | 2 +- src/icmp/net_db.cc | 2 +- src/icp_v2.cc | 2 +- src/mgr/Inquirer.cc | 2 +- src/mime.cc | 2 +- src/neighbors.cc | 2 +- src/peer_digest.cc | 2 +- src/servers/FtpServer.cc | 6 +- src/store_digest.cc | 2 +- src/tests/testHttpRequest.cc | 31 ++---- src/url.cc | 199 ++++++++++++++++++++++++++++++-------- src/urn.cc | 2 +- src/urn.h | 2 + 20 files changed, 192 insertions(+), 96 deletions(-) diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index a2c7afd..8b7ab99 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -320,13 +320,7 @@ HttpRequest::parseFirstLine(const char *start, const char *end) if (end < start) // missing URI return false; - char save = *end; - - * (char *) end = '\0'; // temp terminate URI, XXX dangerous? - - HttpRequest *tmp = urlParse(method, (char *) start, this); - - * (char *) end = save; + HttpRequest *tmp = urlParse(method, SBuf(start, size_t(end-start)), this); if (NULL == tmp) return false; @@ -540,7 +534,7 @@ HttpRequest::expectingBody(const HttpRequestMethod& unused, int64_t& theSize) co * If the request cannot be created cleanly, NULL is returned */ HttpRequest * -HttpRequest::CreateFromUrlAndMethod(char * url, const HttpRequestMethod& method) +HttpRequest::CreateFromUrlAndMethod(const SBuf & url, const HttpRequestMethod& method) { return urlParse(method, url, NULL); } @@ -551,7 +545,7 @@ HttpRequest::CreateFromUrlAndMethod(char * url, const HttpRequestMethod& method) * If the request cannot be created cleanly, NULL is returned */ HttpRequest * -HttpRequest::CreateFromUrl(char * url) +HttpRequest::CreateFromUrl(const SBuf & url) { return urlParse(Http::METHOD_GET, url, NULL); } diff --git a/src/HttpRequest.h b/src/HttpRequest.h index cfb5a46..47f3593 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -224,9 +224,9 @@ public: static void httpRequestPack(void *obj, Packer *p); - static HttpRequest * CreateFromUrlAndMethod(char * url, const HttpRequestMethod& method); + static HttpRequest * CreateFromUrlAndMethod(const SBuf & url, const HttpRequestMethod& method); - static HttpRequest * CreateFromUrl(char * url); + static HttpRequest * CreateFromUrl(const SBuf & url); ConnStateData *pinnedConnection(); diff --git a/src/URL.h b/src/URL.h index 0b75bc6..5857272 100644 --- a/src/URL.h +++ b/src/URL.h @@ -62,9 +62,9 @@ MEMPROXY_CLASS_INLINE(URL); class HttpRequest; class HttpRequestMethod; -AnyP::ProtocolType urlParseProtocol(const char *, const char *e = NULL); +AnyP::ProtocolType urlParseProtocol(const SBuf &); void urlInitialize(void); -HttpRequest *urlParse(const HttpRequestMethod&, char *, HttpRequest *request = NULL); +HttpRequest *urlParse(const HttpRequestMethod&, const SBuf & rawRequest, HttpRequest *request = NULL); const char *urlCanonical(HttpRequest *); char *urlCanonicalClean(const HttpRequest *); const char *urlCanonicalFakeHttps(const HttpRequest * request); diff --git a/src/acl/Asn.cc b/src/acl/Asn.cc index 551c182..e3ccb91 100644 --- a/src/acl/Asn.cc +++ b/src/acl/Asn.cc @@ -240,7 +240,7 @@ asnCacheStart(int as) debugs(53, 3, "AS " << as); snprintf(asres, 4096, "whois://%s/!gAS%d", Config.as_whois_server, as); asState->as_number = as; - asState->request = HttpRequest::CreateFromUrl(asres); + asState->request = HttpRequest::CreateFromUrl(SBuf(asres)); assert(asState->request != NULL); if ((e = storeGetPublic(asres, Http::METHOD_GET)) == NULL) { diff --git a/src/adaptation/ecap/MessageRep.cc b/src/adaptation/ecap/MessageRep.cc index cff44d4..60a6d28 100644 --- a/src/adaptation/ecap/MessageRep.cc +++ b/src/adaptation/ecap/MessageRep.cc @@ -210,9 +210,7 @@ Adaptation::Ecap::RequestLineRep::uri(const Area &aUri) // TODO: if method is not set, urlPath will assume it is not connect; // Can we change urlParse API to remove the method parameter? // TODO: optimize: urlPath should take constant URL buffer - char *buf = xstrdup(aUri.toString().c_str()); - const bool ok = urlParse(theMessage.method, buf, &theMessage); - xfree(buf); + const bool ok = urlParse(theMessage.method, SBuf(aUri.toString()), &theMessage); Must(ok); } diff --git a/src/client_side.cc b/src/client_side.cc index dbb75ae..b025af8 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2639,7 +2639,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c return; } - if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, method)) == NULL) { + if ((request = HttpRequest::CreateFromUrlAndMethod(SBuf(http->uri), method)) == NULL) { clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Invalid URL: " << http->uri); conn->quitAfterError(request.getRaw()); diff --git a/src/client_side_request.cc b/src/client_side_request.cc index e266ace..7a7aad4 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -325,7 +325,7 @@ clientBeginRequest(const HttpRequestMethod& method, char const *url, CSCB * stre http->uri = (char *)xcalloc(url_sz, 1); strcpy(http->uri, url); - if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, method)) == NULL) { + if ((request = HttpRequest::CreateFromUrlAndMethod(SBuf(http->uri), method)) == NULL) { debugs(85, 5, "Invalid URL: " << http->uri); return -1; } @@ -1281,7 +1281,7 @@ ClientRequestContext::clientRedirectDone(const Helper::Reply &reply) // XXX: validate the URL properly *without* generating a whole new request object right here. // XXX: the clone() should be done only AFTER we know the new URL is valid. HttpRequest *new_request = old_request->clone(); - if (urlParse(old_request->method, const_cast<char*>(urlNote), new_request)) { + if (urlParse(old_request->method, SBuf(urlNote), new_request)) { debugs(61,2, HERE << "URL-rewriter diverts URL from " << urlCanonical(old_request) << " to " << urlCanonical(new_request)); // update the new request to flag the re-writing was done on it diff --git a/src/htcp.cc b/src/htcp.cc index e033951..cddb635 100644 --- a/src/htcp.cc +++ b/src/htcp.cc @@ -747,7 +747,7 @@ htcpUnpackSpecifier(char *buf, int sz) */ method = HttpRequestMethod(s->method, NULL); - s->request = HttpRequest::CreateFromUrlAndMethod(s->uri, method == Http::METHOD_NONE ? HttpRequestMethod(Http::METHOD_GET) : method); + s->request = HttpRequest::CreateFromUrlAndMethod(SBuf(s->uri), method == Http::METHOD_NONE ? HttpRequestMethod(Http::METHOD_GET) : method); if (s->request) HTTPMSGLOCK(s->request); diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc index 8d9e1ec..f8c3bf9 100644 --- a/src/icmp/net_db.cc +++ b/src/icmp/net_db.cc @@ -1285,7 +1285,7 @@ netdbExchangeStart(void *data) uri = internalRemoteUri(p->host, p->http_port, "/squid-internal-dynamic/", "netdb"); debugs(38, 3, "netdbExchangeStart: Requesting '" << uri << "'"); assert(NULL != uri); - ex->r = HttpRequest::CreateFromUrl(uri); + ex->r = HttpRequest::CreateFromUrl(SBuf(uri)); if (NULL == ex->r) { debugs(38, DBG_IMPORTANT, "netdbExchangeStart: Bad URI " << uri); diff --git a/src/icp_v2.cc b/src/icp_v2.cc index 5b2a4ee..38ba428 100644 --- a/src/icp_v2.cc +++ b/src/icp_v2.cc @@ -438,7 +438,7 @@ icpGetRequest(char *url, int reqnum, int fd, Ip::Address &from) HttpRequest *result; - if ((result = HttpRequest::CreateFromUrl(url)) == NULL) + if ((result = HttpRequest::CreateFromUrl(SBuf(url))) == NULL) icpCreateAndSend(ICP_ERR, 0, url, reqnum, 0, fd, from); return result; diff --git a/src/mgr/Inquirer.cc b/src/mgr/Inquirer.cc index d3b020b..97a90d9 100644 --- a/src/mgr/Inquirer.cc +++ b/src/mgr/Inquirer.cc @@ -76,7 +76,7 @@ Mgr::Inquirer::start() if (strands.empty()) { LOCAL_ARRAY(char, url, MAX_URL); snprintf(url, MAX_URL, "%s", aggrAction->command().params.httpUri.termedBuf()); - HttpRequest *req = HttpRequest::CreateFromUrl(url); + HttpRequest *req = HttpRequest::CreateFromUrl(SBuf(url)); ErrorState err(ERR_INVALID_URL, Http::scNotFound, req); std::unique_ptr<HttpReply> reply(err.BuildHttpReply()); replyBuf.reset(reply->pack()); diff --git a/src/mime.cc b/src/mime.cc index a07564f..2739d07 100644 --- a/src/mime.cc +++ b/src/mime.cc @@ -407,7 +407,7 @@ MimeIcon::created(StoreEntry *newEntry) EBIT_SET(e->flags, ENTRY_SPECIAL); e->setPublicKey(); e->buffer(); - HttpRequest *r = HttpRequest::CreateFromUrl(url_); + HttpRequest *r = HttpRequest::CreateFromUrl(SBuf(url_)); if (NULL == r) fatalf("mimeLoadIcon: cannot parse internal URL: %s", url_); diff --git a/src/neighbors.cc b/src/neighbors.cc index 103c9bc..5225909 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -1398,7 +1398,7 @@ peerCountMcastPeersStart(void *data) p->in_addr.toUrl(url+7, MAX_URL -8 ); strcat(url, "/"); fake = storeCreateEntry(url, url, RequestFlags(), Http::METHOD_GET); - HttpRequest *req = HttpRequest::CreateFromUrl(url); + HttpRequest *req = HttpRequest::CreateFromUrl(SBuf(url)); psstate = new ps_state; psstate->request = req; HTTPMSGLOCK(psstate->request); diff --git a/src/peer_digest.cc b/src/peer_digest.cc index e955d4a..2497801 100644 --- a/src/peer_digest.cc +++ b/src/peer_digest.cc @@ -293,7 +293,7 @@ peerDigestRequest(PeerDigest * pd) else url = xstrdup(internalRemoteUri(p->host, p->http_port, "/squid-internal-periodic/", StoreDigestFileName)); - req = HttpRequest::CreateFromUrl(url); + req = HttpRequest::CreateFromUrl(SBuf(url)); assert(req); diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc index ea3dab7..fa83577 100644 --- a/src/servers/FtpServer.cc +++ b/src/servers/FtpServer.cc @@ -725,12 +725,10 @@ Ftp::Server::parseOneRequest(Http::ProtocolVersion &ver) const SBuf *path = (params.length() && CommandHasPathParameter(cmd)) ? ¶ms : NULL; calcUri(path); - char *newUri = xstrdup(uri.c_str()); - HttpRequest *const request = HttpRequest::CreateFromUrlAndMethod(newUri, method); + HttpRequest *const request = HttpRequest::CreateFromUrlAndMethod(uri, method); if (!request) { debugs(33, 5, "Invalid FTP URL: " << uri); uri.clear(); - safe_free(newUri); return earlyError(eekInvalidUri); } @@ -753,7 +751,7 @@ Ftp::Server::parseOneRequest(Http::ProtocolVersion &ver) http->request = request; HTTPMSGLOCK(http->request); http->req_sz = tok.parsedSize(); - http->uri = newUri; + http->uri = xstrdup(uri.c_str()); ClientSocketContext *const result = new ClientSocketContext(clientConnection, http); diff --git a/src/store_digest.cc b/src/store_digest.cc index da676bc..86bfd0f 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -420,7 +420,7 @@ storeDigestRewriteStart(void *datanotused) assert(e); sd_state.rewrite_lock = e; debugs(71, 3, "storeDigestRewrite: url: " << url << " key: " << e->getMD5Text()); - HttpRequest *req = HttpRequest::CreateFromUrl(url); + HttpRequest *req = HttpRequest::CreateFromUrl(SBuf(url)); e->mem_obj->request = req; HTTPMSGLOCK(e->mem_obj->request); /* wait for rebuild (if any) to finish */ diff --git a/src/tests/testHttpRequest.cc b/src/tests/testHttpRequest.cc index 82a7efe..8914464 100644 --- a/src/tests/testHttpRequest.cc +++ b/src/tests/testHttpRequest.cc @@ -43,47 +43,43 @@ testHttpRequest::testCreateFromUrlAndMethod() { /* vanilla url */ unsigned short expected_port; - char * url = xstrdup("http://foo:90/bar";); + SBuf url("http://foo:90/bar";); HttpRequest *aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET); expected_port = 90; + CPPUNIT_ASSERT(aRequest != nullptr); HttpRequest *nullRequest = NULL; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/bar"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme())); - CPPUNIT_ASSERT_EQUAL(String("http://foo:90/bar";), String(url)); - xfree(url); /* vanilla url, different method */ - url = xstrdup("http://foo/bar";); + url = "http://foo/bar";; aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_PUT); expected_port = 80; + CPPUNIT_ASSERT(aRequest != nullptr); CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_PUT); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/bar"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme())); - CPPUNIT_ASSERT_EQUAL(String("http://foo/bar";), String(url)); - xfree(url); /* a connect url with non-CONNECT data */ - url = xstrdup(":foo/bar"); + url = ":foo/bar"; aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_CONNECT); - xfree(url); CPPUNIT_ASSERT_EQUAL(nullRequest, aRequest); /* a CONNECT url with CONNECT data */ - url = xstrdup("foo:45"); + url = "foo:45"; aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_CONNECT); expected_port = 45; + CPPUNIT_ASSERT(aRequest != nullptr); CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_CONNECT); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String(""), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_NONE, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme())); - CPPUNIT_ASSERT_EQUAL(String("foo:45"), String(url)); - xfree(url); } /* @@ -113,11 +109,10 @@ void testHttpRequest::testIPv6HostColonBug() { unsigned short expected_port; - char * url = NULL; HttpRequest *aRequest = NULL; /* valid IPv6 address without port */ - url = xstrdup("http://[2000:800::45]/foo";); + SBuf url("http://[2000:800::45]/foo";); aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET); expected_port = 80; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); @@ -125,11 +120,9 @@ testHttpRequest::testIPv6HostColonBug() CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme())); - CPPUNIT_ASSERT_EQUAL(String("http://[2000:800::45]/foo";), String(url)); - xfree(url); /* valid IPv6 address with port */ - url = xstrdup("http://[2000:800::45]:90/foo";); + url = "http://[2000:800::45]:90/foo";; aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET); expected_port = 90; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); @@ -137,11 +130,9 @@ testHttpRequest::testIPv6HostColonBug() CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme())); - CPPUNIT_ASSERT_EQUAL(String("http://[2000:800::45]:90/foo";), String(url)); - xfree(url); /* IPv6 address as invalid (bug trigger) */ - url = xstrdup("http://2000:800::45/foo";); + url = "http://2000:800::45/foo";; aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET); expected_port = 80; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); @@ -149,8 +140,6 @@ testHttpRequest::testIPv6HostColonBug() CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme())); - CPPUNIT_ASSERT_EQUAL(String("http://2000:800::45/foo";), String(url)); - xfree(url); } void diff --git a/src/url.cc b/src/url.cc index 1122bf3..d3f7dd8 100644 --- a/src/url.cc +++ b/src/url.cc @@ -11,10 +11,12 @@ #include "squid.h" #include "globals.h" #include "HttpRequest.h" +#include "parser/Tokenizer.h" #include "rfc1738.h" #include "SquidConfig.h" #include "SquidString.h" #include "URL.h" +#include "urn.h" static HttpRequest *urlParseFinish(const HttpRequestMethod& method, const AnyP::ProtocolType protocol, @@ -23,7 +25,9 @@ static HttpRequest *urlParseFinish(const HttpRequestMethod& method, const char *const login, const int port, HttpRequest *request); -static HttpRequest *urnParse(const HttpRequestMethod& method, char *urn, HttpRequest *request); + +static HttpRequest *parseUrn(const HttpRequestMethod &method, Parser::Tokenizer &tok, HttpRequest *request); + static const char valid_hostname_chars_u[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz" @@ -37,6 +41,7 @@ static const char valid_hostname_chars[] = "[:]" ; + void urlInitialize(void) { @@ -81,13 +86,55 @@ urlInitialize(void) /* more cases? */ } +/** + * Extract the URI scheme and ':' delimiter from the given input buffer. + * + * Schemes up to 16 characters are accepted. + * + * Governed by RFC 3986 section 3.1 + */ +static AnyP::UriScheme +uriParseScheme(Parser::Tokenizer &tok) +{ + /* + * RFC 3986 section 3.1 paragraph 2: + * + * Scheme names consist of a sequence of characters beginning with a + * letter and followed by any combination of letters, digits, plus + * ("+"), period ("."), or hyphen ("-"). + * + * The underscore ("_") required to match "cache_object://" squid + * special URI scheme. + */ + static const auto schemeChars = +#if USE_HTTP_VIOLATIONS + CharacterSet("special", "_") + +#endif + CharacterSet("scheme", "+.-") + CharacterSet::ALPHA + CharacterSet::DIGIT; + + SBuf str; + if (tok.prefix(str, schemeChars, 16) && tok.skip(':') && CharacterSet::ALPHA[str.at(0)]) { + // const auto protocol = AnyP::UriScheme::FindProtocolType(str); + const auto protocol = urlParseProtocol(str); + if (protocol == AnyP::PROTO_UNKNOWN) { + // return AnyP::UriScheme(protocol, str.c_str()); + throw TextException("Unknown protocol", __FILE__, __LINE__); + } + return AnyP::UriScheme(protocol); + } + + throw TextException("invalid URI scheme", __FILE__, __LINE__); +} + + /** * urlParseProtocol() takes begin (b) and end (e) pointers, but for * backwards compatibility, e defaults to NULL, in which case we * assume b is NULL-terminated. */ + AnyP::ProtocolType -urlParseProtocol(const char *b, const char *e) +urlParseProtocol(const SBuf & protocol) //const char *b, const char *e) { /* * if e is NULL, b must be NULL terminated and we @@ -95,10 +142,8 @@ urlParseProtocol(const char *b, const char *e) * after b. */ - if (NULL == e) - e = b + strcspn(b, ":"); - - int len = e - b; + auto len = protocol.length(); + auto b = protocol.rawContent(); /* test common stuff first */ @@ -199,6 +244,13 @@ urlAppendDomain(char *host) return true; } +static const SBuf & +Asterisk() +{ + static SBuf star("*"); + return star; +} + /* * Parse a URI/URL. * @@ -221,59 +273,71 @@ urlAppendDomain(char *host) * being "end of host with implied path of /". */ HttpRequest * -urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) +urlParse(const HttpRequestMethod& method, const SBuf &rawUrl, HttpRequest *request) { - LOCAL_ARRAY(char, proto, MAX_URL); + try { + LOCAL_ARRAY(char, login, MAX_URL); LOCAL_ARRAY(char, host, MAX_URL); LOCAL_ARRAY(char, urlpath, MAX_URL); char *t = NULL; char *q = NULL; int port; - AnyP::ProtocolType protocol = AnyP::PROTO_NONE; int l; int i; const char *src; char *dst; - proto[0] = host[0] = urlpath[0] = login[0] = '\0'; + host[0] = urlpath[0] = login[0] = '\0'; - if ((l = strlen(url)) + Config.appendDomainLen > (MAX_URL - 1)) { - /* terminate so it doesn't overflow other buffers */ - *(url + (MAX_URL >> 1)) = '\0'; + if ((l = rawUrl.length()) + Config.appendDomainLen > (MAX_URL - 1)) { debugs(23, DBG_IMPORTANT, "urlParse: URL too large (" << l << " bytes)"); return NULL; } + + if ((method == Http::METHOD_OPTIONS || method == Http::METHOD_TRACE) && + Asterisk().cmp(rawUrl) == 0) { + // XXX: these methods might also occur in HTTPS traffic. Handle this better. + port = urlDefaultPort(AnyP::PROTO_HTTP); + + const SBuf& h = Asterisk(); + return urlParseFinish(method, AnyP::PROTO_HTTP, h.rawContent(), host, login, port, request); + } + + Parser::Tokenizer tok(rawUrl); + AnyP::UriScheme scheme; + if (method == Http::METHOD_CONNECT) { port = CONNECT_PORT; + // XXX: use tokenizer + auto B = tok.buf(); + const char *url = B.c_str(); + if (sscanf(url, "[%[^]]]:%d", host, &port) < 1) if (sscanf(url, "%[^:]:%d", host, &port) < 1) return NULL; - } else if ((method == Http::METHOD_OPTIONS || method == Http::METHOD_TRACE) && - strcmp(url, "*") == 0) { - protocol = AnyP::PROTO_HTTP; - port = urlDefaultPort(protocol); - return urlParseFinish(method, protocol, url, host, login, port, request); - } else if (!strncmp(url, "urn:", 4)) { - return urnParse(method, url, request); } else { - /* Parse the URL: */ - src = url; - i = 0; - /* Find first : - everything before is protocol */ - for (i = 0, dst = proto; i < l && *src != ':'; ++i, ++src, ++dst) { - *dst = *src; + scheme = uriParseScheme(tok); + + if (scheme == AnyP::PROTO_NONE) + return NULL; // invalid scheme + + if (scheme == AnyP::PROTO_URN) { + return parseUrn(method, tok, request); // throws on any error } - if (i >= l) - return NULL; - *dst = '\0'; - /* Then its :// */ - if ((i+3) > l || *src != ':' || *(src + 1) != '/' || *(src + 2) != '/') - return NULL; - i += 3; - src += 3; + // URLs then have "//" + static const SBuf doubleSlash("//"); + if (!tok.skip(doubleSlash)) + return NULL; + + auto B = tok.remaining(); + const char *url = B.c_str(); + + /* Parse the URL: */ + src = url; + i = 0; /* Then everything until first /; thats host (and port; which we'll look for here later) */ // bug 1881: If we don't get a "/" then we imply it was there @@ -314,8 +378,8 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) } *dst = '\0'; - protocol = urlParseProtocol(proto); - port = urlDefaultPort(protocol); + // port = scheme.defaultPort(); // may be reset later + port = urlDefaultPort(scheme); /* Is there any login information? (we should eventually parse it above) */ t = strrchr(host, '@'); @@ -363,7 +427,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) } // Bug 3183 sanity check: If scheme is present, host must be too. - if (protocol != AnyP::PROTO_NONE && host[0] == '\0') { + if (scheme != AnyP::PROTO_NONE && host[0] == '\0') { debugs(23, DBG_IMPORTANT, "SECURITY ALERT: Missing hostname in URL '" << url << "'. see access.log for details."); return NULL; } @@ -392,7 +456,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) } } - debugs(23, 3, "urlParse: Split URL '" << url << "' into proto='" << proto << "', host='" << host << "', port='" << port << "', path='" << urlpath << "'"); + debugs(23, 3, "urlParse: Split URL '" << rawUrl << "' into proto='" << scheme << "', host='" << host << "', port='" << port << "', path='" << urlpath << "'"); if (Config.onoff.check_hostnames && strspn(host, Config.onoff.allow_underscore ? valid_hostname_chars_u : valid_hostname_chars) != strlen(host)) { debugs(23, DBG_IMPORTANT, "urlParse: Illegal character in hostname '" << host << "'"); @@ -427,7 +491,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) #endif if (stringHasWhitespace(urlpath)) { - debugs(23, 2, "urlParse: URI has whitespace: {" << url << "}"); + debugs(23, 2, "urlParse: URI has whitespace: {" << rawUrl << "}"); switch (Config.uri_whitespace) { @@ -460,7 +524,12 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) } } - return urlParseFinish(method, protocol, urlpath, host, login, port, request); + return urlParseFinish(method, scheme, urlpath, host, login, port, request); + + } catch (...) { + debugs(23, 2, "error: " << CurrentException << " " << Raw("rawUrl", rawUrl.rawContent(), rawUrl.length())); + } + return NULL; } /** @@ -490,6 +559,51 @@ urlParseFinish(const HttpRequestMethod& method, return request; } +/** + * Governed by RFC 8141 section 2: + * + * assigned-name = "urn" ":" NID ":" NSS + * NID = (alphanum) 0*30(ldh) (alphanum) + * ldh = alphanum / "-" + * NSS = pchar *(pchar / "/") + * + * RFC 3986 Appendix D.2 defines (as deprecated): + * + * alphanum = ALPHA / DIGIT + * + * Notice that NID is exactly 2-32 characters in length. + */ +static HttpRequest* +parseUrn(const HttpRequestMethod &method, Parser::Tokenizer &tok, HttpRequest *request) +{ + static const auto nidChars = CharacterSet("NID","-") + CharacterSet::ALPHA + CharacterSet::DIGIT; + static const auto alphanum = (CharacterSet::ALPHA + CharacterSet::DIGIT).rename("alphanum"); + SBuf nid; + if (!tok.prefix(nid, nidChars, 32)) + throw TextException("NID not found", __FILE__, __LINE__); + + if (!tok.skip(':')) + throw TextException("NID too long or missing ':' delimiter", __FILE__, __LINE__); + + if (nid.length() < 2) + throw TextException("NID too short", __FILE__, __LINE__); + + if (!alphanum[nid[0]]) + throw TextException("NID prefix is not alphanumeric", __FILE__, __LINE__); + + if (!alphanum[nid[nid.length()-1]]) + throw TextException("NID suffix is not alphanumeric", __FILE__, __LINE__); + + nid.append(':').append(tok.remaining()); + if (request) { + request->initHTTP(method, AnyP::PROTO_URN, nid.rawContent()); + safe_free(request->canonical); + return request; + } + return new HttpRequest(method, AnyP::PROTO_URN, nid.rawContent()); +} + +/* static HttpRequest * urnParse(const HttpRequestMethod& method, char *urn, HttpRequest *request) { @@ -501,7 +615,7 @@ urnParse(const HttpRequestMethod& method, char *urn, HttpRequest *request) } return new HttpRequest(method, AnyP::PROTO_URN, urn + 4); -} +}*/ const char * urlCanonical(HttpRequest * request) @@ -673,7 +787,8 @@ urlMakeAbsolute(const HttpRequest * req, const char *relUrl) char *urlbuf = (char *)xmalloc(MAX_URL * sizeof(char)); if (req->url.getScheme() == AnyP::PROTO_URN) { - snprintf(urlbuf, MAX_URL, "urn:" SQUIDSTRINGPH, + snprintf(urlbuf, MAX_URL, "urn:%s:" SQUIDSTRINGPH, + req->GetHost(), SQUIDSTRINGPRINT(req->urlpath)); return (urlbuf); } diff --git a/src/urn.cc b/src/urn.cc index 487e4f2..1ff54f7 100644 --- a/src/urn.cc +++ b/src/urn.cc @@ -183,7 +183,7 @@ UrnState::createUriResRequest (String &uri) safe_free(host); safe_free(urlres); urlres = xstrdup(local_urlres); - urlres_r = HttpRequest::CreateFromUrl(urlres); + urlres_r = HttpRequest::CreateFromUrl(SBuf(urlres)); } void diff --git a/src/urn.h b/src/urn.h index 81b1e95..d54f5ff 100644 --- a/src/urn.h +++ b/src/urn.h @@ -16,5 +16,7 @@ class StoreEntry; void urnStart(HttpRequest *, StoreEntry *); +std::ostream& CurrentException(std::ostream &os); + #endif /* SQUID_URN_H_ */
signature.asc
Description: OpenPGP digital signature
