The web proxy API has a set of policies on which projects can use which hostnames, which are checked when creating a proxy. I just noticed that the update API supports renaming an existing proxy, but that code path does not check the domain policy.
Description
Description
Details
Details
Related Changes in Gerrit:
| Subject | Author | Repo | Branch | Lines +/- | |
|---|---|---|---|---|---|
| dynamicproxy: Do not allow renaming proxies | Majavah | operations/puppet | production | +18 -21 |
Event Timeline
Comment Actions
The rename functionality is broken regardless, since it doesn't update DNS records either. So I guess the best this functionality could be used to delete arbitrary A/AAAA records under wmcloud.org. Let's just get rid of it:
diff --git a/modules/dynamicproxy/files/api/invisible-unicorn.py b/modules/dynamicproxy/files/api/invisible-unicorn.py index b760f45b99..2cdcec20aa 100644 --- a/modules/dynamicproxy/files/api/invisible-unicorn.py +++ b/modules/dynamicproxy/files/api/invisible-unicorn.py @@ -147,15 +147,10 @@ class RedisStore: print("Adding new key: %s " % key) self.update_route(route) - def update_route(self, route: Route, old_domain=None): + def update_route(self, route: Route): key = "frontend:" + route.domain backends = [backend.url for backend in route.backends] - - pipeline = self.redis.pipeline() - if old_domain: - # When domains get renamed, kill old one too - pipeline.delete("frontend:" + old_domain) - pipeline.delete(key).sadd(key, *backends).execute() + self.redis.pipeline().delete(key).sadd(key, *backends).execute() class Dns: @@ -607,25 +602,27 @@ def update_mapping(project_id, domain): data = flask.request.get_json(True) + # If a domain is specified in the background, validate that it + # is not changing. We no longer support renaming an existing proxy, + # so there's no real need to require clients to pass a domain in the + # body, but + # * we used to support that, and + # * for client libraries (like go-cloudvps), it is simpler to pass the same + # body structure for requests to create and update proxies, + # so we support passing one and throw an error if the request is trying to + # rename the proxy to avoid confusion. + if data.get("domain") and route.domain != data["domain"]: + return flask.jsonify({"error": "Can't rename a proxy"}), 400 + if ( - "domain" not in data - or "backends" not in data + "backends" not in data or not isinstance(data["backends"], list) + or not all(isinstance(entry, str) for entry in data["backends"]) ): - return ( - "Valid JSON but invalid format. Needs domain string and backends array", - 400, - ) + return flask.jsonify({"error": "'backends' is missing or invalid"}), 400 - new_domain = data["domain"] - if not is_valid_domain(new_domain): - return "Invalid domain", 400 backend_urls = data["backends"] - if route.domain != new_domain: - route.domain = new_domain - db.session.add(route) - # Not the most efficient, but I'm sitting in an airplane and this is the simplest from here route.backends.delete() for backend_url in backend_urls: @@ -633,7 +630,7 @@ def update_mapping(project_id, domain): db.session.add(route) db.session.commit() - redis_store.update_route(route, old_domain=domain) + redis_store.update_route(route) return "OK", 200