Validation of session.secret setting
authorKarl O. Pinc <kop@meme.com>
Sat, 17 Nov 2018 07:53:21 +0000 (01:53 -0600)
committerKarl O. Pinc <kop@meme.com>
Sat, 17 Nov 2018 08:15:47 +0000 (02:15 -0600)
README.rst
examples/development.ini
examples/pgwui.ini
src/pgwui_server/__init__.py
tests/test___init__.py

index 5db39bfd9a4ab27df8b54fe0f90bda32ea3afb4d..1d33b23c9ca363d180b09d1c1987fd1ee17e0be5 100644 (file)
@@ -248,6 +248,11 @@ installed in the virtual environment::
 
   pgwui_venv/bin/pip install pyramid_debugtoolbar
 
+Rudimentary validation of the ``pyramid_beaker`` ``session.secret``
+setting's value may be turned off.  To do this change the
+``pgwui.validate_hmac`` setting to ``False``.  Having validation off
+in production is not recommended.
+
 
 Complete Documentation
 ----------------------
index 923cf509ed2e6dd77eb585341c58a54bd154452a..07f896b743d05c531ff98493c7b074f5bacbf94f 100644 (file)
@@ -63,6 +63,13 @@ pgwui.dry_run = False
 #   logout = /logout
 #   upload = /upload
 
+# Settings validation
+
+# Whether or not to validate the session.secret setting.
+# session.secret must be valid to detect Cross-Site Request Forgery (CSRF)
+# vulnerabilties.  Validation is on by default.
+pgwui.validate_hmac = False
+
 
 #
 # Pyramid configuration
index 4e8b87005d2824a81fdbf9dbf83b8d75fd432b35..7d00b3d16b23b88fb4c550e0a999b35138357d1c 100644 (file)
@@ -61,6 +61,16 @@ pgwui.dry_run = False
 #   logout = /logmeout
 #   upload = /put-in
 
+
+
+# Settings validation
+
+# Whether or not to validate the session.secret setting.
+# session.secret must be valid to detect Cross-Site Request Forgery (CSRF)
+# vulnerabilties.  Validation is on by default.
+# pgwui.validate_hmac = True
+
+
 #
 # Pyramid configuration
 # https://docs.pylonsproject.org/projects/pyramid/en/latest/narr/environment.html
index a0758e9e97a51a0924a1f4ebfe7770f96be3efe0..eb8ce80e1699ebfec211c22239593be38e708d1f 100644 (file)
@@ -22,6 +22,7 @@
 '''Provide a way to configure PGWUI.
 '''
 
+from ast import literal_eval
 from pyramid.config import Configurator
 
 # Constants
@@ -34,8 +35,12 @@ SETTINGS = set(
      'dry_run',
      'route_prefix',
      'routes',
+     'validate_hmac',
      ])
 
+# Required length of HMAC value
+HMAC_LEN = 40
+
 
 # Exceptions
 class Error(Exception):
@@ -49,6 +54,22 @@ class UnknownSettingKeyError(Error):
         super().__init__('Unknown PGWUI setting: {}'.format(key))
 
 
+class BadHMACError(Error):
+    pass
+
+
+class NoHMACError(BadHMACError):
+    def __init__(self):
+        super().__init__('Missing session.secret configuration')
+
+
+class HMACLengthError(BadHMACError):
+    def __init__(self):
+        super().__init__(
+            'The session.secret value is not {} characters in length'
+            .format(HMAC_LEN))
+
+
 # Functions
 
 def abort_on_bad_setting(key):
@@ -59,11 +80,31 @@ def abort_on_bad_setting(key):
             raise UnknownSettingKeyError(key)
 
 
+def do_validate_hmac(settings):
+    '''True unless the user has specificly rejected hmac validation
+    '''
+    return ('pgwui.validate_hmac' not in settings
+            or literal_eval(settings['pgwui.validate_hmac']))
+
+
+def validate_hmac(settings):
+    '''Unless otherwise requested, validate the session.secret setting'''
+    if not do_validate_hmac(settings):
+        return
+
+    if 'session.secret' not in settings:
+        raise NoHMACError()
+
+    if len(settings['session.secret']) != HMAC_LEN:
+        raise HMACLengthError()
+
+
 def validate_settings(settings):
     '''Be sure all settings validate
     '''
     for key in settings.keys():
         abort_on_bad_setting(key)
+    validate_hmac(settings)
 
 
 def parse_assignments(lines):
index 6bc9aa1722f4cac1125e28dea5e0551102c60506..09aa885a687d4462f5c682c03b8b164dcea62d1f 100644 (file)
@@ -23,6 +23,13 @@ import pytest
 import pgwui_server.__init__ as pgwui_server_init
 
 
+# Constants
+
+TEST_SETTINGS = {
+    'pgwui.validate_hmac': 'False',
+}
+
+
 # Use contextlib.AbstractContextManager for Python >= 3.6
 class MockConfigurator():
     def __init__(self, **kwargs):
@@ -69,6 +76,60 @@ def test_abort_on_bad_setting_good():
     pgwui_server_init.abort_on_bad_setting('pgwui.pg_host')
 
 
+# do_validate_hmac()
+
+def test_do_validate_hmac_none():
+    '''pgwui.validate_hmac defaults to True'''
+    assert pgwui_server_init.do_validate_hmac({}) is True
+
+
+def test_do_validate_hmac_True():
+    '''Require hmac validation when pgwui.validate_hmac is True'''
+    result = pgwui_server_init.do_validate_hmac(
+        {'pgwui.validate_hmac': 'True'})
+    assert result is True
+
+
+def test_do_validate_hmac_False():
+    '''No hmac validation when pgwui.validate_hmac is False'''
+    result = pgwui_server_init.do_validate_hmac(
+        {'pgwui.validate_hmac': 'False'})
+    assert result is False
+
+
+# validate_hmac()
+
+def test_validate_hmac_unvalidated(monkeypatch):
+    '''No error is raised when hmac validation is off'''
+    monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac',
+                        lambda *args: False)
+    pgwui_server_init.validate_hmac({})
+
+
+def test_validate_hmac_success(monkeypatch):
+    '''No error is raised when hmac is validated an the right length'''
+    monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac',
+                        lambda *args: True)
+    pgwui_server_init.validate_hmac(
+        {'session.secret': 'x' * pgwui_server_init.HMAC_LEN})
+
+
+def test_validate_hmac_missing(monkeypatch):
+    '''Raise error when hmac is validated and missing'''
+    monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac',
+                        lambda *args: True)
+    with pytest.raises(pgwui_server_init.NoHMACError):
+        pgwui_server_init.validate_hmac({})
+
+
+def test_validate_hmac_length(monkeypatch):
+    '''Raise error when hmac is validated and the wrong length'''
+    monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac',
+                        lambda *args: True)
+    with pytest.raises(pgwui_server_init.HMACLengthError):
+        pgwui_server_init.validate_hmac({'session.secret': ''})
+
+
 # validate_settings()
 
 def test_validate_settings(monkeypatch):
@@ -82,6 +143,8 @@ def test_validate_settings(monkeypatch):
 
     monkeypatch.setattr(pgwui_server_init, 'abort_on_bad_setting',
                         mock_abort_on_bad_setting)
+    monkeypatch.setattr(pgwui_server_init, 'validate_hmac',
+                        lambda *args: None)
     settings = {'key1': 'value1',
                 'key2': 'value2'}
 
@@ -165,7 +228,7 @@ def test_main(monkeypatch):
 # Integration tests
 def test_main_integrated():
     '''Does not raise errors or warnings'''
-    pgwui_server_init.main({})
+    pgwui_server_init.main({}, **TEST_SETTINGS)
 
 
 # Functional tests