Use key derivation to provide secret keys

Rather than asking people deploying h to provide new secret keys for
every distinct use of a secret in the application, use an HMAC-based key
derivation to generate these keys.

This simplifies deployment, and provides stronger guarantees of security
in the face of lazy configurations (such as setting all secrets the
same).

Capability URLs generated previously by the `h.notification` package
will continue to work as of this change due to a fallback option when
deserializing tokens. However, it is important to note that setting up
such a fallback for the session secrets is infeasible, and so all
current sessions will be invalidated by the deployment of this change.

Lastly, this change ensures that in the event of a misconfiguration
where a secret key is not provided, the application will issue a warning
and generate a transient key (from the OS PRNG).

.travis.yml

@@ -16,9 +16,9 @@ script:
   - make test
   - make lint
   - hypothesis extension development.ini chrome http://localhost
-  - SESSION_SECRET=foo hypothesis extension production.ini chrome https://hypothes.is chrome-extension://notarealkey/public
+  - hypothesis extension production.ini chrome https://hypothes.is chrome-extension://notarealkey/public
   - hypothesis extension development.ini firefox http://localhost
-  - SESSION_SECRET=foo hypothesis extension production.ini firefox https://hypothes.is resource://notarealkey/hypothesis/data
+  - hypothesis extension production.ini firefox https://hypothes.is resource://notarealkey/hypothesis/data
 notifications:
   irc:
     channels:

README.rst

@@ -140,7 +140,7 @@ Or, to load the assets from within the extension::
 
 To build an extension with a feature flag enabled use the environment variable::
 
-    FEATURE_NOTIFICATION=true SESSION_SECRET=foo \
+    FEATURE_NOTIFICATION=true \
     hypothesis extension production.ini chrome \
     https://hypothes.is chrome-extension://extensionid/public
 
@@ -200,7 +200,7 @@ The following shell environment variables are supported:
 - ``DATABASE_URL`` in the format used by Heroku
 - ``ELASTICSEARCH_INDEX`` the Elasticsearch index for annotation storage
 - ``MAIL_DEFAULT_SENDER`` a sender address for outbound mail
-- ``SESSION_SECRET`` a unique string secret for cookie validation
+- ``SECRET_KEY`` a unique string secret
 
 Customized embedding
 --------------------

development.ini

@@ -39,7 +39,8 @@ pyramid.includes:
 # production settings file.
 h.client_id: nosuchid
 h.client_secret: nosuchsecret
-session.secret: notverysecretafterall
+
+secret_key: notverysecretafterall
 
 sqlalchemy.url: sqlite:///h.db
 

h/app.py

@@ -1,12 +1,17 @@
 # -*- coding: utf-8 -*-
 """The main h application."""
 import functools
+import logging
+import os
 
 from pyramid.config import Configurator
 from pyramid.renderers import JSON
 from pyramid.wsgi import wsgiapp2
 
 from .auth import acl_authz, remote_authn, session_authn
+from .security import derive_key
+
+log = logging.getLogger(__name__)
 
 
 def strip_vhm(view):
@@ -83,10 +88,31 @@ def create_api(settings):
     return config.make_wsgi_app()
 
 
+def missing_secrets(settings):
+    missing = {}
+
+    if 'secret_key' not in settings:
+        log.warn('No secret key provided: using transient key. Please '
+                 'configure the secret_key setting or the SECRET_KEY '
+                 'environment variable!')
+        missing['secret_key'] = os.urandom(64)
+
+    # If the redis session secret hasn't been set explicitly, derive it from
+    # the global secret key.
+    if 'redis.sessions.secret' not in settings:
+        secret = settings.get('secret_key')
+        if secret is None:
+            secret = missing['secret_key']
+        missing['redis.sessions.secret'] = derive_key(secret, 'h.session')
+
+    return missing
+
+
 def main(global_config, **settings):
     """Create the h application with all the awesomeness that is configured."""
     from h import config
     environ_config = config.settings_from_environment()
     settings.update(environ_config)  # from environment variables
     settings.update(global_config)   # from paste [DEFAULT] + command line
+    settings.update(missing_secrets(settings))
     return create_app(settings)

h/config.py

@@ -1,9 +1,12 @@
 import os
+import logging
 import re
 import urlparse
 
 from pyramid.settings import asbool
 
+log = logging.getLogger(__name__)
+
 
 def settings_from_environment():
     settings = {}
@@ -15,8 +18,8 @@ def settings_from_environment():
     _setup_features(settings)
     _setup_nsqd(settings)
     _setup_redis(settings)
+    _setup_secrets(settings)
     _setup_client(settings)
-    _setup_sessions(settings)
     _setup_statsd(settings)
     _setup_websocket(settings)
 
@@ -141,10 +144,13 @@ def _setup_client(settings):
         settings['h.client_secret'] = os.environ['CLIENT_SECRET']
 
 
-def _setup_sessions(settings):
-    if 'SESSION_SECRET' in os.environ:
-        settings['session.secret'] = os.environ['SESSION_SECRET']
-        settings['redis.sessions.secret'] = os.environ['SESSION_SECRET']
+def _setup_secrets(settings):
+    if 'SECRET_KEY' in os.environ:
+        settings['secret_key'] = os.environ['SECRET_KEY']
+    elif 'SESSION_SECRET' in os.environ:
+        log.warn('Found deprecated SESSION_SECRET environment variable. '
+                 'Please use SECRET_KEY instead!')
+        settings['secret_key'] = os.environ['SESSION_SECRET']
 
 
 def _setup_statsd(settings):

h/notification/__init__.py

@@ -1,6 +1,31 @@
 # -*- coding: utf-8 -*-
 from webob.cookies import SignedSerializer
 
+from ..security import derive_key
+
+
+class FallbackSerializer(object):
+    """
+    A message serializer/deserializer which can try a number of serializers in
+    turn. For backwards compatibility only.
+    """
+
+    def __init__(self, serializers):
+        if not len(serializers) > 0:
+            raise ValueError('you must provide at least one serializer')
+        self.serializers = serializers
+
+    def dumps(self, appstruct):
+        return self.serializers[0].dumps(appstruct)
+
+    def loads(self, bstruct):
+        for s in self.serializers[:-1]:
+            try:
+                return s.loads(bstruct)
+            except ValueError:
+                continue
+        return self.serializers[-1].loads(bstruct)
+
 
 def includeme(config):
     config.include('.types')
@@ -10,9 +35,15 @@ def includeme(config):
     config.include('.reply_template')
     config.include('.views')
 
-    # We use the shared session secret, but salt it with the namespace
-    # 'h.notification' -- only messages serialized with this salt will
-    # authenticate on deserialization.
-    secret = config.registry.settings['session.secret']
-    serializer = SignedSerializer(secret, 'h.notification')
+    secret = config.registry.settings['secret_key']
+    derived = derive_key(secret, 'h.notification')
+
+    old_serializer = SignedSerializer(secret, 'h.notification')
+    new_serializer = SignedSerializer(derived, None)
+
+    # Create all new notification tokens with the new serializer, but, for now,
+    # allow ones created with the old serializer to deserialize correctly.
+    #
+    # bw compat -- remove after an acceptable changeover period.
+    serializer = FallbackSerializer([new_serializer, old_serializer])
     config.registry.notification_serializer = serializer

h/session.py

@@ -1,8 +1,7 @@
 # -*- coding: utf-8 -*-
-import os
-
 from pyramid.session import SignedCookieSessionFactory
 
+from .security import derive_key
 
 def model(request):
     session = {k: v for k, v in request.session.items() if k[0] != '_'}
@@ -34,23 +33,11 @@ def set_csrf_token(request, response):
         response.set_cookie('XSRF-TOKEN', csrft)
 
 
-def session_factory_from_settings(settings, prefix='session.'):
-    """Return a session factory from the provided settings."""
-    secret_key = '{}secret'.format(prefix)
-    secret = settings.get(secret_key)
-    if secret is None:
-        # Get 32 bytes (256 bits) from a secure source (urandom) as a secret.
-        # Pyramid will add a salt to this. The salt and the secret together
-        # will still be less than the, and therefore right zero-padded to,
-        # 1024-bit block size of the default hash algorithm, sha512. However,
-        # 256 bits of random should be more than enough for session secrets.
-        secret = os.urandom(32)
-
-    return SignedCookieSessionFactory(secret)
-
-
 def includeme(config):
     registry = config.registry
     settings = registry.settings
-    session_factory = session_factory_from_settings(settings)
+
+    session_secret = derive_key(settings['secret_key'], 'h.session')
+    session_factory = SignedCookieSessionFactory(session_secret)
+
     config.set_session_factory(session_factory)

h/test/app_test.py

@@ -1,28 +1,49 @@
 # -*- coding: utf-8 -*-
-from mock import call, patch
+from mock import patch
 
 from h import app, config
 
 
 @patch('h.config.settings_from_environment')
 @patch('h.app.create_app')
-def test_global_config_precence(create_app, settings_from_environment):
-    base_config = {
+def test_global_settings_precedence(create_app, settings_from_environment):
+    base_settings = {
         'foo': 'bar',
     }
-    env_config = {
+    env_settings = {
         'foo': 'override',
         'booz': 'baz',
     }
-    global_config = {
-        'booz': 'override',
-    }
-    expected_config = {
-        'foo': 'override',
+    global_settings = {
         'booz': 'override',
     }
 
-    settings_from_environment.return_value = env_config
-    app.main(global_config, **base_config)
+    settings_from_environment.return_value = env_settings
+    app.main(global_settings, **base_settings)
     assert config.settings_from_environment.call_count == 1
-    assert app.create_app.mock_calls == [call(expected_config)]
+
+    args, kwargs = app.create_app.call_args
+    result = args[0]
+    assert result['foo'] == 'override'
+    assert result['booz'] == 'override'
+
+
+def test_missing_secrets_generates_secret_key():
+    result = app.missing_secrets({})
+
+    assert 'secret_key' in result
+    assert 'redis.sessions.secret' in result
+
+
+def test_missing_secrets_doesnt_override_secret_key():
+    result = app.missing_secrets({'secret_key': 'foo'})
+
+    assert 'secret_key' not in result
+    assert 'redis.sessions.secret' in result
+
+
+def test_missing_secrets_doesnt_override_redis_sesssions_secret():
+    result = app.missing_secrets({'redis.sessions.secret': 'foo'})
+
+    assert 'secret_key' in result
+    assert 'redis.sessions.secret' not in result

h/test/config_test.py

@@ -180,12 +180,22 @@ def test_client_credentials_environment():
 
 
 @patch.dict(os.environ)
-def test_session_secret_environment():
+def test_secret_key_environment():
+    os.environ['SECRET_KEY'] = 's3kr1t'
+
+    actual_config = settings_from_environment()
+    expected_config = {
+        'secret_key': 's3kr1t',
+    }
+    assert actual_config == expected_config
+
+
+@patch.dict(os.environ)
+def test_session_secret_environment():  # bw compat
     os.environ['SESSION_SECRET'] = 's3kr1t'
 
     actual_config = settings_from_environment()
     expected_config = {
-        'session.secret': 's3kr1t',
-        'redis.sessions.secret': 's3kr1t',
+        'secret_key': 's3kr1t',
     }
     assert actual_config == expected_config

production.ini

@@ -45,9 +45,6 @@ pyramid.includes:
     pyramid_tm
 
 # Redis session configuration -- See pyramid_redis_sessions documentation
-# The session secret must be set by providing a 128 character long secrete here
-# or in the SESSION_SECRET environment variable. Without this, the application
-# will not start.
 #redis.sessions.secret:
 redis.sessions.cookie_max_age: 2592000
 redis.sessions.timeout: 604800