From dbaf58dbfec2b8c10eacaf933b58899db488f806 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 20:15:08 +0200 Subject: [PATCH 1/9] Remove base_prefix and use SCRIPT_NAME instead This conforms with the WSGI reference (PEP 333) --- config | 6 ----- radicale/__init__.py | 57 ++++++++++++++++++++------------------------ radicale/__main__.py | 3 --- radicale/config.py | 2 -- radicale/xmlutils.py | 46 +++++++++++++++++------------------ 5 files changed, 49 insertions(+), 65 deletions(-) diff --git a/config b/config index 1cdfb75..0dd2927 100644 --- a/config +++ b/config @@ -51,12 +51,6 @@ # Reverse DNS to resolve client address in logs #dns_lookup = True -# Root URL of Radicale (starting and ending with a slash) -#base_prefix = / - -# Possibility to allow URLs cleaned by a HTTP server, without the base_prefix -#can_skip_base_prefix = False - # Message displayed in the client when a password is needed #realm = Radicale - Password Required diff --git a/radicale/__init__.py b/radicale/__init__.py index 06354d9..693a0f1 100644 --- a/radicale/__init__.py +++ b/radicale/__init__.py @@ -307,19 +307,11 @@ class Application: headers = pprint.pformat(self.headers_log(environ)) self.logger.debug("Request headers:\n%s", headers) - # Strip base_prefix from request URI - base_prefix = self.configuration.get("server", "base_prefix") - if environ["PATH_INFO"].startswith(base_prefix): - environ["PATH_INFO"] = environ["PATH_INFO"][len(base_prefix):] - elif self.configuration.get("server", "can_skip_base_prefix"): - self.logger.debug( - "Prefix already stripped from path: %s", environ["PATH_INFO"]) - else: - # Request path not starting with base_prefix, not allowed - self.logger.debug( - "Path not starting with prefix: %s", environ["PATH_INFO"]) - return response(*NOT_ALLOWED) - + # Sanitize base prefix + environ["SCRIPT_NAME"] = storage.sanitize_path( + environ.get("SCRIPT_NAME", "")).rstrip("/") + self.logger.debug("Sanitized script name: %s", environ["SCRIPT_NAME"]) + base_prefix = environ["SCRIPT_NAME"] # Sanitize request URI environ["PATH_INFO"] = storage.sanitize_path( unquote(environ["PATH_INFO"])) @@ -376,7 +368,8 @@ class Application: if is_valid_user: try: - status, headers, answer = function(environ, path, user) + status, headers, answer = function( + environ, base_prefix, path, user) except socket.timeout: return response(*REQUEST_TIMEOUT) else: @@ -423,7 +416,7 @@ class Application: content = None return content - def do_DELETE(self, environ, path, user): + def do_DELETE(self, environ, base_prefix, path, user): """Manage DELETE request.""" if not self._access(user, path, "w"): return NOT_ALLOWED @@ -438,12 +431,13 @@ class Application: # ETag precondition not verified, do not delete item return PRECONDITION_FAILED if isinstance(item, self.Collection): - answer = xmlutils.delete(path, item) + answer = xmlutils.delete(base_prefix, path, item) else: - answer = xmlutils.delete(path, item.collection, item.href) + answer = xmlutils.delete( + base_prefix, path, item.collection, item.href) return client.OK, {}, answer - def do_GET(self, environ, path, user): + def do_GET(self, environ, base_prefix, path, user): """Manage GET request.""" # Display a "Radicale works!" message if the root URL is requested if not path.strip("/"): @@ -471,12 +465,13 @@ class Application: answer = item.serialize() return client.OK, headers, answer - def do_HEAD(self, environ, path, user): + def do_HEAD(self, environ, base_prefix, path, user): """Manage HEAD request.""" - status, headers, answer = self.do_GET(environ, path, user) + status, headers, answer = self.do_GET( + environ, base_prefix, path, user) return status, headers, None - def do_MKCALENDAR(self, environ, path, user): + def do_MKCALENDAR(self, environ, base_prefix, path, user): """Manage MKCALENDAR request.""" if not self.authorized(user, path, "w"): return NOT_ALLOWED @@ -492,7 +487,7 @@ class Application: self.Collection.create_collection(path, props=props) return client.CREATED, {}, None - def do_MKCOL(self, environ, path, user): + def do_MKCOL(self, environ, base_prefix, path, user): """Manage MKCOL request.""" if not self.authorized(user, path, "w"): return NOT_ALLOWED @@ -505,7 +500,7 @@ class Application: self.Collection.create_collection(path, props=props) return client.CREATED, {}, None - def do_MOVE(self, environ, path, user): + def do_MOVE(self, environ, base_prefix, path, user): """Manage MOVE request.""" to_url = urlparse(environ["HTTP_DESTINATION"]) if to_url.netloc != environ["HTTP_HOST"]: @@ -542,7 +537,7 @@ class Application: self.Collection.move(item, to_collection, to_href) return client.CREATED, {}, None - def do_OPTIONS(self, environ, path, user): + def do_OPTIONS(self, environ, base_prefix, path, user): """Manage OPTIONS request.""" headers = { "Allow": ", ".join( @@ -550,7 +545,7 @@ class Application: "DAV": DAV_HEADERS} return client.OK, headers, None - def do_PROPFIND(self, environ, path, user): + def do_PROPFIND(self, environ, base_prefix, path, user): """Manage PROPFIND request.""" if not self._access(user, path, "r"): return NOT_ALLOWED @@ -569,13 +564,13 @@ class Application: read_items, write_items = self.collect_allowed_items(items, user) headers = {"DAV": DAV_HEADERS, "Content-Type": "text/xml"} status, answer = xmlutils.propfind( - path, content, read_items, write_items, user) + base_prefix, path, content, read_items, write_items, user) if status == client.FORBIDDEN: return NOT_ALLOWED else: return status, headers, answer - def do_PROPPATCH(self, environ, path, user): + def do_PROPPATCH(self, environ, base_prefix, path, user): """Manage PROPPATCH request.""" if not self.authorized(user, path, "w"): return NOT_ALLOWED @@ -585,10 +580,10 @@ class Application: if not isinstance(item, self.Collection): return WEBDAV_PRECONDITION_FAILED headers = {"DAV": DAV_HEADERS, "Content-Type": "text/xml"} - answer = xmlutils.proppatch(path, content, item) + answer = xmlutils.proppatch(base_prefix, path, content, item) return client.MULTI_STATUS, headers, answer - def do_PUT(self, environ, path, user): + def do_PUT(self, environ, base_prefix, path, user): """Manage PUT request.""" if not self._access(user, path, "w"): return NOT_ALLOWED @@ -640,7 +635,7 @@ class Application: headers = {"ETag": new_item.etag} return client.CREATED, headers, None - def do_REPORT(self, environ, path, user): + def do_REPORT(self, environ, base_prefix, path, user): """Manage REPORT request.""" if not self._access(user, path, "w"): return NOT_ALLOWED @@ -656,5 +651,5 @@ class Application: else: collection = item.collection headers = {"Content-Type": "text/xml"} - answer = xmlutils.report(path, content, collection) + answer = xmlutils.report(base_prefix, path, content, collection) return client.MULTI_STATUS, headers, answer diff --git a/radicale/__main__.py b/radicale/__main__.py index 8a50075..342335c 100644 --- a/radicale/__main__.py +++ b/radicale/__main__.py @@ -153,9 +153,6 @@ def serve(configuration, logger): atexit.register(cleanup) logger.info("Starting Radicale") - logger.debug( - "Base URL prefix: %s", configuration.get("server", "base_prefix")) - # Create collection servers servers = {} if configuration.getboolean("server", "ssl"): diff --git a/radicale/config.py b/radicale/config.py index 5076ffa..209c349 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -41,8 +41,6 @@ INITIAL_CONFIG = { "protocol": "PROTOCOL_SSLv23", "ciphers": "", "dns_lookup": "True", - "base_prefix": "/", - "can_skip_base_prefix": "False", "realm": "Radicale - Password Required"}, "encoding": { "request": "utf-8", diff --git a/radicale/xmlutils.py b/radicale/xmlutils.py index a75a441..c8f41ff 100644 --- a/radicale/xmlutils.py +++ b/radicale/xmlutils.py @@ -102,11 +102,9 @@ def _response(code): return "HTTP/1.1 %i %s" % (code, client.responses[code]) -def _href(collection, href): +def _href(base_prefix, href): """Return prefixed href.""" - return "%s%s" % ( - collection.configuration.get("server", "base_prefix"), - href.lstrip("/")) + return "%s%s" % (base_prefix, href) def _date_to_datetime(date_): @@ -466,7 +464,7 @@ def props_from_request(root, actions=("set", "remove")): return result -def delete(path, collection, href=None): +def delete(base_prefix, path, collection, href=None): """Read and answer DELETE requests. Read rfc4918-9.6 for info. @@ -479,7 +477,7 @@ def delete(path, collection, href=None): multistatus.append(response) href = ET.Element(_tag("D", "href")) - href.text = _href(collection, path) + href.text = _href(base_prefix, path) response.append(href) status = ET.Element(_tag("D", "status")) @@ -489,7 +487,8 @@ def delete(path, collection, href=None): return _pretty_xml(multistatus) -def propfind(path, xml_request, read_collections, write_collections, user): +def propfind(base_prefix, path, xml_request, read_collections, + write_collections, user): """Read and answer PROPFIND requests. Read rfc4918-9.1 for info. @@ -522,19 +521,19 @@ def propfind(path, xml_request, read_collections, write_collections, user): for collection in write_collections: collections.append(collection) response = _propfind_response( - path, collection, props, user, write=True) + base_prefix, path, collection, props, user, write=True) multistatus.append(response) for collection in read_collections: if collection in collections: continue response = _propfind_response( - path, collection, props, user, write=False) + base_prefix, path, collection, props, user, write=False) multistatus.append(response) return client.MULTI_STATUS, _pretty_xml(multistatus) -def _propfind_response(path, item, props, user, write=False): +def _propfind_response(base_prefix, path, item, props, user, write=False): """Build and return a PROPFIND response.""" is_collection = isinstance(item, storage.BaseCollection) if is_collection: @@ -552,7 +551,7 @@ def _propfind_response(path, item, props, user, write=False): else: uri = "/" + posixpath.join(collection.path, item.href) - href.text = _href(collection, uri) + href.text = _href(base_prefix, uri) response.append(href) propstat404 = ET.Element(_tag("D", "propstat")) @@ -574,7 +573,7 @@ def _propfind_response(path, item, props, user, write=False): element.text = item.last_modified elif tag == _tag("D", "principal-collection-set"): tag = ET.Element(_tag("D", "href")) - tag.text = _href(collection, "/") + tag.text = _href(base_prefix, "/") element.append(tag) elif (tag in (_tag("C", "calendar-user-address-set"), _tag("D", "principal-URL"), @@ -582,7 +581,7 @@ def _propfind_response(path, item, props, user, write=False): _tag("C", "calendar-home-set")) and collection.is_principal and is_collection): tag = ET.Element(_tag("D", "href")) - tag.text = _href(collection, path) + tag.text = _href(base_prefix, path) element.append(tag) elif tag == _tag("C", "supported-calendar-component-set"): human_tag = _tag_from_clark(tag) @@ -600,7 +599,7 @@ def _propfind_response(path, item, props, user, write=False): is404 = True elif tag == _tag("D", "current-user-principal"): tag = ET.Element(_tag("D", "href")) - tag.text = _href(collection, ("/%s/" % user) if user else "/") + tag.text = _href(base_prefix, ("/%s/" % user) if user else "/") element.append(tag) elif tag == _tag("D", "current-user-privilege-set"): privilege = ET.Element(_tag("D", "privilege")) @@ -720,7 +719,7 @@ def _add_propstat_to(element, tag, status_number): propstat.append(status) -def proppatch(path, xml_request, collection): +def proppatch(base_prefix, path, xml_request, collection): """Read and answer PROPPATCH requests. Read rfc4918-9.2 for info. @@ -735,7 +734,7 @@ def proppatch(path, xml_request, collection): multistatus.append(response) href = ET.Element(_tag("D", "href")) - href.text = _href(collection, path) + href.text = _href(base_prefix, path) response.append(href) for short_name in props_to_remove: @@ -748,7 +747,7 @@ def proppatch(path, xml_request, collection): return _pretty_xml(multistatus) -def report(path, xml_request, collection): +def report(base_prefix, path, xml_request, collection): """Read and answer REPORT requests. Read rfc3253-3.6 for info. @@ -765,12 +764,11 @@ def report(path, xml_request, collection): _tag("C", "calendar-multiget"), _tag("CR", "addressbook-multiget")): # Read rfc4791-7.9 for info - base_prefix = collection.configuration.get("server", "base_prefix") hreferences = set() for href_element in root.findall(_tag("D", "href")): href_path = unquote(urlparse(href_element.text).path) - if href_path.startswith(base_prefix): - hreferences.add(href_path[len(base_prefix) - 1:]) + if (href_path + "/").startswith(base_prefix + "/"): + hreferences.add(href_path[len(base_prefix):]) else: hreferences = (path,) filters = ( @@ -787,7 +785,8 @@ def report(path, xml_request, collection): # Reference is an item item = collection.get(name) if not item: - response = _item_response(hreference, found_item=False) + response = _item_response(base_prefix, hreference, + found_item=False) multistatus.append(response) continue items = [item] @@ -828,13 +827,14 @@ def report(path, xml_request, collection): uri = "/" + posixpath.join(collection.path, item.href) multistatus.append(_item_response( - uri, found_props=found_props, + base_prefix, uri, found_props=found_props, not_found_props=not_found_props, found_item=True)) return _pretty_xml(multistatus) -def _item_response(href, found_props=(), not_found_props=(), found_item=True): +def _item_response(base_prefix, href, found_props=(), not_found_props=(), + found_item=True): response = ET.Element(_tag("D", "response")) href_tag = ET.Element(_tag("D", "href")) From 664fa712783af7508edbfdc5d73cbbe1c731db94 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 20:16:23 +0200 Subject: [PATCH 2/9] Don't double unquote request URL "%2525" was transformed to "%" instead of "%25". --- radicale/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/radicale/__init__.py b/radicale/__init__.py index 693a0f1..29d67a2 100644 --- a/radicale/__init__.py +++ b/radicale/__init__.py @@ -313,8 +313,7 @@ class Application: self.logger.debug("Sanitized script name: %s", environ["SCRIPT_NAME"]) base_prefix = environ["SCRIPT_NAME"] # Sanitize request URI - environ["PATH_INFO"] = storage.sanitize_path( - unquote(environ["PATH_INFO"])) + environ["PATH_INFO"] = storage.sanitize_path(environ["PATH_INFO"]) self.logger.debug("Sanitized path: %s", environ["PATH_INFO"]) path = environ["PATH_INFO"] From 13d652b094d5276544194dff9d25cef6acd30315 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 20:17:58 +0200 Subject: [PATCH 3/9] Remove unnecessary module prefix --- radicale/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/__init__.py b/radicale/__init__.py index 29d67a2..25e0c8e 100644 --- a/radicale/__init__.py +++ b/radicale/__init__.py @@ -171,7 +171,7 @@ class RequestHandler(wsgiref.simple_server.WSGIRequestHandler): def get_environ(self): env = super().get_environ() # Parent class only tries latin1 encoding - env["PATH_INFO"] = urllib.parse.unquote(self.path.split("?", 1)[0]) + env["PATH_INFO"] = unquote(self.path.split("?", 1)[0]) return env def handle(self): From 139076faee768a226cb597d9353527ada2c10539 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 20:18:44 +0200 Subject: [PATCH 4/9] Sanitize URLs from XML requests --- radicale/xmlutils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/radicale/xmlutils.py b/radicale/xmlutils.py index c8f41ff..7e91192 100644 --- a/radicale/xmlutils.py +++ b/radicale/xmlutils.py @@ -766,7 +766,8 @@ def report(base_prefix, path, xml_request, collection): # Read rfc4791-7.9 for info hreferences = set() for href_element in root.findall(_tag("D", "href")): - href_path = unquote(urlparse(href_element.text).path) + href_path = storage.sanitize_path( + unquote(urlparse(href_element.text).path)) if (href_path + "/").startswith(base_prefix + "/"): hreferences.add(href_path[len(base_prefix):]) else: From d5b8ddd71c5b4bf8fd56e91041c5f2cd6da9a276 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 20:19:39 +0200 Subject: [PATCH 5/9] Check that name is valid in name_from_path Before it was possible craft XML requests, so that the storage backend got requests with invalid hrefs. --- radicale/xmlutils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/radicale/xmlutils.py b/radicale/xmlutils.py index 7e91192..b0c5b92 100644 --- a/radicale/xmlutils.py +++ b/radicale/xmlutils.py @@ -423,7 +423,11 @@ def name_from_path(path, collection): start = collection.path + "/" if not path.startswith(start): raise ValueError("'%s' doesn't start with '%s'" % (path, start)) - return path[len(start):].rstrip("/") + name = path[len(start):][:-1] + if name and not storage.is_safe_path_component(name): + raise ValueError("'%s' is not a component in collection '%s'" % + (path, collection.path)) + return name def props_from_request(root, actions=("set", "remove")): From 90486f33a59b4e8b0407ab64f9aeff02ef6b3f68 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 20:21:23 +0200 Subject: [PATCH 6/9] Log invalid URLs in XML requests Before the requests either failed or the invalid hreference was silently dropped. --- radicale/xmlutils.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/radicale/xmlutils.py b/radicale/xmlutils.py index b0c5b92..c9b1289 100644 --- a/radicale/xmlutils.py +++ b/radicale/xmlutils.py @@ -774,6 +774,9 @@ def report(base_prefix, path, xml_request, collection): unquote(urlparse(href_element.text).path)) if (href_path + "/").startswith(base_prefix + "/"): hreferences.add(href_path[len(base_prefix):]) + else: + collection.logger.info( + "Skipping invalid path: %s", href_path) else: hreferences = (path,) filters = ( @@ -785,7 +788,14 @@ def report(base_prefix, path, xml_request, collection): multistatus = ET.Element(_tag("D", "multistatus")) for hreference in hreferences: - name = name_from_path(hreference, collection) + try: + name = name_from_path(hreference, collection) + except ValueError: + collection.logger.info("Skipping invalid path: %s", hreference) + response = _item_response(base_prefix, hreference, + found_item=False) + multistatus.append(response) + continue if name: # Reference is an item item = collection.get(name) From 83046c80c47dce0598c4eed08792e8c6fc7bd834 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 21:10:58 +0200 Subject: [PATCH 7/9] Let reverse proxies overwrite script name Reverse proxies can overwrite the script name with the HTTP header field X-Script-Name. --- radicale/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/radicale/__init__.py b/radicale/__init__.py index 25e0c8e..d589ef7 100644 --- a/radicale/__init__.py +++ b/radicale/__init__.py @@ -307,6 +307,11 @@ class Application: headers = pprint.pformat(self.headers_log(environ)) self.logger.debug("Request headers:\n%s", headers) + # Let reverse proxies overwrite SCRIPT_NAME + if "HTTP_X_SCRIPT_NAME" in environ: + environ["SCRIPT_NAME"] = environ["HTTP_X_SCRIPT_NAME"] + self.logger.debug("Script name overwritten by client: %s", + environ["SCRIPT_NAME"]) # Sanitize base prefix environ["SCRIPT_NAME"] = storage.sanitize_path( environ.get("SCRIPT_NAME", "")).rstrip("/") From eb4b513d6311981e6bc6a0d3c6eb856bab99f8b2 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 22:01:27 +0200 Subject: [PATCH 8/9] Quote hreferences RFC 4918 states that they are URIs and RFC 3986 says that URIs must always be in percent-encoded form. --- radicale/xmlutils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/radicale/xmlutils.py b/radicale/xmlutils.py index c9b1289..c3215a5 100644 --- a/radicale/xmlutils.py +++ b/radicale/xmlutils.py @@ -31,7 +31,7 @@ import xml.etree.ElementTree as ET from collections import OrderedDict from datetime import datetime, timedelta, timezone from http import client -from urllib.parse import unquote, urlparse +from urllib.parse import quote, unquote, urlparse from . import storage @@ -104,7 +104,7 @@ def _response(code): def _href(base_prefix, href): """Return prefixed href.""" - return "%s%s" % (base_prefix, href) + return quote("%s%s" % (base_prefix, href)) def _date_to_datetime(date_): From f7435814fc0c8dcdbf1b49346a2ef64b23e6ab02 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 4 Sep 2016 22:28:34 +0200 Subject: [PATCH 9/9] Repair hreferences in REPORT response They were not extended with base_prefix. --- radicale/xmlutils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/xmlutils.py b/radicale/xmlutils.py index c3215a5..915b306 100644 --- a/radicale/xmlutils.py +++ b/radicale/xmlutils.py @@ -853,7 +853,7 @@ def _item_response(base_prefix, href, found_props=(), not_found_props=(), response = ET.Element(_tag("D", "response")) href_tag = ET.Element(_tag("D", "href")) - href_tag.text = href + href_tag.text = _href(base_prefix, href) response.append(href_tag) if found_item: