feat: apply salt to ip hashing (#13749)

Author: miketheman

dev/environment

@@ -3,6 +3,7 @@ PATH=/opt/warehouse/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:
 
 WAREHOUSE_ENV=development
 WAREHOUSE_TOKEN=insecuretoken
+WAREHOUSE_IP_SALT="insecure himalayan pink salt"
 
 AWS_ACCESS_KEY_ID=foo
 AWS_SECRET_ACCESS_KEY=foo

tests/conftest.py

@@ -114,6 +114,14 @@ def remote_addr_hashed():
     return "6694f83c9f476da31f5df6bcc520034e7e57d421d247b9d34f49edbfc84a764c"
 
 
+@pytest.fixture
+def remote_addr_salted():
+    """
+    Output of `hashlib.sha256((remote_addr + "pepa").encode("utf8")).hexdigest()`
+    """
+    return "a69a49383d81404e4b1df297c7baa28e1cd6c4ee1495ed5d0ab165a63a147763"
+
+
 @pytest.fixture
 def jinja():
     dir_name = os.path.join(os.path.dirname(warehouse.__file__))
@@ -273,6 +281,7 @@ def app_config(database):
     settings = {
         "warehouse.prevent_esi": True,
         "warehouse.token": "insecure token",
+        "warehouse.ip_salt": "insecure salt",
         "camo.url": "http://localhost:9000/",
         "camo.key": "insecure key",
         "celery.broker_url": "amqp://",

tests/unit/test_config.py

@@ -195,6 +195,7 @@ class FakeRegistry(dict):
         def __init__(self):
             self.settings = {
                 "warehouse.token": "insecure token",
+                "warehouse.ip_salt": "insecure salt",
                 "warehouse.env": environment,
                 "camo.url": "http://camo.example.com/",
                 "pyramid.reload_assets": False,
@@ -310,7 +311,9 @@ def __init__(self):
     assert result is configurator_obj
     assert configurator_obj.set_root_factory.calls == [pretend.call(config.RootFactory)]
     assert configurator_obj.add_wsgi_middleware.calls == [
-        pretend.call(ProxyFixer, token="insecure token", num_proxies=1),
+        pretend.call(
+            ProxyFixer, token="insecure token", ip_salt="insecure salt", num_proxies=1
+        ),
         pretend.call(VhmRootRemover),
     ]
     assert configurator_obj.include.calls == (

tests/unit/utils/test_wsgi.py

@@ -34,7 +34,9 @@ def test_skips_headers(self):
         }
         start_response = pretend.stub()
 
-        resp = wsgi.ProxyFixer(app, token="1234")(environ, start_response)
+        resp = wsgi.ProxyFixer(app, token="1234", ip_salt="pepa")(
+            environ, start_response
+        )
 
         assert resp is response
         assert app.calls == [pretend.call({}, start_response)]
@@ -53,7 +55,9 @@ def test_accepts_warehouse_headers(self):
         }
         start_response = pretend.stub()
 
-        resp = wsgi.ProxyFixer(app, token="1234")(environ, start_response)
+        resp = wsgi.ProxyFixer(app, token="1234", ip_salt="pepa")(
+            environ, start_response
+        )
 
         assert resp is response
         assert app.calls == [
@@ -76,12 +80,14 @@ def test_missing_headers(self):
         environ = {"HTTP_WAREHOUSE_TOKEN": "1234"}
         start_response = pretend.stub()
 
-        resp = wsgi.ProxyFixer(app, token="1234")(environ, start_response)
+        resp = wsgi.ProxyFixer(app, token="1234", ip_salt="pepa")(
+            environ, start_response
+        )
 
         assert resp is response
         assert app.calls == [pretend.call({}, start_response)]
 
-    def test_accepts_x_forwarded_headers(self, remote_addr_hashed):
+    def test_accepts_x_forwarded_headers(self, remote_addr_salted):
         response = pretend.stub()
         app = pretend.call_recorder(lambda e, s: response)
 
@@ -93,15 +99,15 @@ def test_accepts_x_forwarded_headers(self, remote_addr_hashed):
         }
         start_response = pretend.stub()
 
-        resp = wsgi.ProxyFixer(app, token=None)(environ, start_response)
+        resp = wsgi.ProxyFixer(app, token=None, ip_salt="pepa")(environ, start_response)
 
         assert resp is response
         assert app.calls == [
             pretend.call(
                 {
                     "HTTP_SOME_OTHER_HEADER": "woop",
                     "REMOTE_ADDR": "1.2.3.4",
-                    "REMOTE_ADDR_HASHED": remote_addr_hashed,
+                    "REMOTE_ADDR_HASHED": remote_addr_salted,
                     "HTTP_HOST": "example.com",
                     "wsgi.url_scheme": "http",
                 },
@@ -116,14 +122,16 @@ def test_skips_x_forwarded_when_not_enough(self):
         environ = {"HTTP_X_FORWARDED_FOR": "1.2.3.4", "HTTP_SOME_OTHER_HEADER": "woop"}
         start_response = pretend.stub()
 
-        resp = wsgi.ProxyFixer(app, token=None, num_proxies=2)(environ, start_response)
+        resp = wsgi.ProxyFixer(app, token=None, ip_salt=None, num_proxies=2)(
+            environ, start_response
+        )
 
         assert resp is response
         assert app.calls == [
             pretend.call({"HTTP_SOME_OTHER_HEADER": "woop"}, start_response)
         ]
 
-    def test_selects_right_x_forwarded_value(self, remote_addr_hashed):
+    def test_selects_right_x_forwarded_value(self, remote_addr_salted):
         response = pretend.stub()
         app = pretend.call_recorder(lambda e, s: response)
 
@@ -135,15 +143,17 @@ def test_selects_right_x_forwarded_value(self, remote_addr_hashed):
         }
         start_response = pretend.stub()
 
-        resp = wsgi.ProxyFixer(app, token=None, num_proxies=2)(environ, start_response)
+        resp = wsgi.ProxyFixer(app, token=None, ip_salt="pepa", num_proxies=2)(
+            environ, start_response
+        )
 
         assert resp is response
         assert app.calls == [
             pretend.call(
                 {
                     "HTTP_SOME_OTHER_HEADER": "woop",
                     "REMOTE_ADDR": "1.2.3.4",
-                    "REMOTE_ADDR_HASHED": remote_addr_hashed,
+                    "REMOTE_ADDR_HASHED": remote_addr_salted,
                     "HTTP_HOST": "example.com",
                     "wsgi.url_scheme": "http",
                 },

warehouse/config.py

@@ -153,6 +153,7 @@ def configure(settings=None):
 
     # Pull in default configuration from the environment.
     maybe_set(settings, "warehouse.token", "WAREHOUSE_TOKEN")
+    maybe_set(settings, "warehouse.ip_salt", "WAREHOUSE_IP_SALT")
     maybe_set(settings, "warehouse.num_proxies", "WAREHOUSE_NUM_PROXIES", int)
     maybe_set(settings, "warehouse.domain", "WAREHOUSE_DOMAIN")
     maybe_set(settings, "forklift.domain", "FORKLIFT_DOMAIN")
@@ -671,6 +672,7 @@ def configure(settings=None):
     config.add_wsgi_middleware(
         ProxyFixer,
         token=config.registry.settings["warehouse.token"],
+        ip_salt=config.registry.settings["warehouse.ip_salt"],
         num_proxies=config.registry.settings.get("warehouse.num_proxies", 1),
     )
 

warehouse/utils/wsgi.py

@@ -41,9 +41,10 @@ def _forwarded_value(values, num_proxies):
 
 
 class ProxyFixer:
-    def __init__(self, app, token, num_proxies=1):
+    def __init__(self, app, token, ip_salt: str, num_proxies=1):
         self.app = app
         self.token = token
+        self.ip_salt = ip_salt
         self.num_proxies = num_proxies
 
     def __call__(self, environ, start_response):
@@ -73,7 +74,7 @@ def __call__(self, environ, start_response):
                 environ.get("HTTP_X_FORWARDED_FOR", ""), self.num_proxies
             )
             remote_addr_hashed = (
-                hashlib.sha256(remote_addr.encode("utf8")).hexdigest()
+                hashlib.sha256((remote_addr + self.ip_salt).encode("utf8")).hexdigest()
                 if remote_addr
                 else ""
             )