Ori.livneh has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/110700

Change subject: Use HMAC SHA1 with rotating key to hash IP addresses
......................................................................

Use HMAC SHA1 with rotating key to hash IP addresses

The current behavior is to generate a 'salt' (really a key) at runtime and to
use it continuously for the lifetime of the program. The lifespan of the key
and the cheapness of the hash function make it easier to attack. This change
makes EventLogging scramble IPs by generating an HMAC SHA1 with the IP address
as the message and a random byte string as the key. The key rotates every 24H.

Change-Id: Ibbaf182c2e165911b51755da51fbf1769cf67edf
---
M server/eventlogging/__init__.py
A server/eventlogging/crypto.py
M server/eventlogging/parse.py
M server/tests/__init__.py
A server/tests/test_crypto.py
M server/tests/test_parser.py
6 files changed, 109 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/EventLogging 
refs/changes/00/110700/1

diff --git a/server/eventlogging/__init__.py b/server/eventlogging/__init__.py
index 7e4c852..5de88c1 100644
--- a/server/eventlogging/__init__.py
+++ b/server/eventlogging/__init__.py
@@ -23,6 +23,7 @@
 from .parse import *
 from .schema import *
 from .streams import *
+from .crypto import *
 
 # The fact that schema validation is entrusted to a third-party module
 # is an implementation detail that a consumer of this package's API
diff --git a/server/eventlogging/crypto.py b/server/eventlogging/crypto.py
new file mode 100644
index 0000000..8cc4164
--- /dev/null
+++ b/server/eventlogging/crypto.py
@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+"""
+  eventlogging.crypto
+  ~~~~~~~~~~~~~~~~~~~
+
+  This module implements ephemeral key-hashing, used by EventLogging to
+  anonymize IP addresses.
+
+"""
+from __future__ import unicode_literals
+
+import hashlib
+import hmac
+import os
+import time
+
+
+__all__ = ('keyhasher', 'rotating_key')
+
+
+def rotating_key(size, period):
+    """Produce a random key of `size` bytes and yield it repeatedly until
+    `period` seconds expire, at which point a new key is produced.
+
+    :param size: Byte length of key.
+    :param period: Key lifetime in seconds.
+    """
+    while 1:
+        key = os.urandom(size)
+        created = time.time()
+        while (time.time() - created) <= period:
+            yield key
+
+
+def keyhasher(keys, digestmod=hashlib.sha1):
+    """Returns an HMAC function that acquires keys from an iterator."""
+    keys_iter = iter(keys)
+
+    def hash_func(msg):
+        """HMAC function bound to a key iterator and hash function."""
+        code = hmac.new(next(keys_iter), msg.encode('utf-8'), digestmod)
+        return code.hexdigest()
+
+    return hash_func
diff --git a/server/eventlogging/parse.py b/server/eventlogging/parse.py
index 5e38622..e05889a 100644
--- a/server/eventlogging/parse.py
+++ b/server/eventlogging/parse.py
@@ -32,24 +32,15 @@
 from __future__ import division, unicode_literals
 
 import calendar
-import hashlib
-import os
 import re
 import time
 import uuid
 
 from .compat import json, unquote_plus, uuid5
+from .crypto import keyhasher, rotating_key
 
 
 __all__ = ('LogParser', 'ncsa_to_epoch', 'ncsa_utcnow', 'capsule_uuid')
-
-#: Salt value for hashing IPs. Because this value is generated at
-#: runtime, IPs cannot be compared across restarts. This limitation is
-#: tolerated because it helps underscore the field's unsuitability for
-#: analytic purposes. Client IP is logged solely for detecting and
-#: grouping spam coming from a single origin so that it can be filtered
-#: out of the logs.
-salt = os.urandom(16)
 
 #: Format string (as would be passed to `strftime`) for timestamps in
 #: NCSA Common Log Format.
@@ -95,14 +86,6 @@
     return time.strftime(NCSA_FORMAT, time.gmtime())
 
 
-def hash_value(val):
-    """Produces a salted SHA1 hash of any string value.
-    :param val: String to hash.
-    """
-    hash = hashlib.sha1(val.encode('utf-8') + salt)
-    return hash.hexdigest()
-
-
 def decode_qson(qson):
     """Decodes a QSON (query-string-encoded JSON) object.
     :param qs: Query string.
@@ -110,9 +93,16 @@
     return json.loads(unquote_plus(qson.strip('?;')))
 
 
+#: A crytographic hash function for hashing client IPs. Produces HMAC SHA1
+#: hashes by using the client IP as the message and a 64-byte byte string as
+#: the key. The key is generated at runtime and is refreshed every 24 hours.
+#: It is not written anywhere. The hash value is useful for detecting spam
+#: (large volume of events sharing a common origin).
+hash_ip = keyhasher(rotating_key(size=64, period=24 * 60 * 60))
+
 #: A mapping of format specifiers to a tuple of (regexp, caster).
 format_specifiers = {
-    '%h': (r'(?P<clientIp>\S+)', hash_value),
+    '%h': (r'(?P<clientIp>\S+)', hash_ip),
     '%j': (r'(?P<capsule>\S+)', json.loads),
     '%l': (r'(?P<recvFrom>\S+)', str),
     '%n': (r'(?P<seqId>\d+)', int),
diff --git a/server/tests/__init__.py b/server/tests/__init__.py
index eb0bb27..38d8617 100644
--- a/server/tests/__init__.py
+++ b/server/tests/__init__.py
@@ -12,3 +12,4 @@
 from .test_parser import *
 from .test_schema import *
 from .test_handlers import *
+from .test_crypto import *
diff --git a/server/tests/test_crypto.py b/server/tests/test_crypto.py
new file mode 100644
index 0000000..2725093
--- /dev/null
+++ b/server/tests/test_crypto.py
@@ -0,0 +1,53 @@
+# -*- coding: utf8 -*-
+"""
+  eventlogging unit tests
+  ~~~~~~~~~~~~~~~~~~~~~~~
+
+  This module contains tests for :module:`eventlogging.crypto`.
+
+"""
+from __future__ import unicode_literals
+
+import time
+import unittest
+
+import eventlogging
+
+
+class KeyHasherTestCase(unittest.TestCase):
+    """Test case for :func:`eventlogging.keyhasher`."""
+
+    def test_hash_function(self):
+        """``keyhasher`` produces HMAC SHA1 using key iterator for keys"""
+        hash_func = eventlogging.keyhasher((b'key1', b'key2'))
+        self.assertEqual(
+            hash_func('message1'),
+            'e45a01bfebb0d5596564cc7b712b2d570041a839'
+        )
+        self.assertEqual(
+            hash_func('message2'),
+            'c8ec32b32d5bd7dc5a6a0b203f7f220bb641f52c'
+        )
+
+    def test_keys_depleted(self):
+        """``keyhasher`` raises StopIteration if key iterator is depleted."""
+        hash_func = eventlogging.keyhasher(())
+        with self.assertRaises(StopIteration):
+            hash_func('message')
+
+
+class RotatingKeyTestCase(unittest.TestCase):
+    """Test case for :func:`eventlogging.rotating_key`."""
+
+    def test_key_repeats(self):
+        """``rotating_key`` yields the same key until that key expires."""
+        key_iter = eventlogging.rotating_key(size=64, period=60)
+        self.assertEqual(next(key_iter), next(key_iter))
+
+    def test_key_expires(self):
+        """``rotating_key`` produces a new key when the old key expires."""
+        key_iter = eventlogging.rotating_key(size=64, period=0.001)
+        key1 = next(key_iter)
+        time.sleep(0.01)
+        key2 = next(key_iter)
+        self.assertNotEqual(key1, key2)
diff --git a/server/tests/test_parser.py b/server/tests/test_parser.py
index c7a7b09..df7ad8e 100644
--- a/server/tests/test_parser.py
+++ b/server/tests/test_parser.py
@@ -47,7 +47,7 @@
             'webHost': 'test.wikipedia.org',
             'seqId': 132073,
             'timestamp': 1358637398,
-            'clientIp': eventlogging.parse.hash_value('86.149.229.149'),
+            'clientIp': eventlogging.parse.hash_ip('86.149.229.149'),
             'schema': 'Generic',
             'revision': 13,
             'event': {

-- 
To view, visit https://gerrit.wikimedia.org/r/110700
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibbaf182c2e165911b51755da51fbf1769cf67edf
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/EventLogging
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to