diff --git a/config/galaxy.ini.sample b/config/galaxy.ini.sample index b72f7b1569d..a72084bcb7a 100644 --- a/config/galaxy.ini.sample +++ b/config/galaxy.ini.sample @@ -978,7 +978,7 @@ use_interactive = True # Galaxy encodes various internal values when these values will be output in # some format (for example, in a URL or cookie). You should set a key to be # used by the algorithm that encodes and decodes these values. It can be any -# string. +# string up to 448 bits long. # One simple way to generate a value for this is with the shell command: # python -c 'import time; print time.time()' | md5sum | cut -f 1 -d ' ' #id_secret = USING THE DEFAULT IS NOT SECURE! diff --git a/lib/galaxy/web/security/__init__.py b/lib/galaxy/web/security/__init__.py index 9689a51dfd5..cb50d37bcb5 100644 --- a/lib/galaxy/web/security/__init__.py +++ b/lib/galaxy/web/security/__init__.py @@ -3,14 +3,21 @@ import os import os.path import logging -import galaxy.exceptions - from Crypto.Cipher import Blowfish from Crypto.Util.randpool import RandomPool from Crypto.Util import number +import galaxy.exceptions + +from galaxy.util import smart_str + log = logging.getLogger(__name__) +MAXIMUM_ID_SECRET_BITS = 448 +MAXIMUM_ID_SECRET_LENGTH = MAXIMUM_ID_SECRET_BITS / 8 +KIND_TOO_LONG_MESSAGE = "Galaxy coding error, keep encryption 'kinds' smaller to utilize more bites of randomness from id_secret values." + + if os.path.exists("/dev/urandom"): # We have urandom, use it as the source of random data random_fd = os.open("/dev/urandom", os.O_RDONLY) @@ -37,7 +44,8 @@ else: class SecurityHelper(object): def __init__(self, **config): - self.id_secret = config['id_secret'] + id_secret = config['id_secret'] + self.id_secret = id_secret self.id_cipher = Blowfish.new(self.id_secret) per_kind_id_secret_base = config.get('per_kind_id_secret_base', self.id_secret) @@ -127,4 +135,15 @@ class _cipher_cache(collections.defaultdict): self.secret_base = secret_base def __missing__(self, key): - return Blowfish.new(self.secret_base + "__" + key) + assert len(key) < 15, KIND_TOO_LONG_MESSAGE + secret = self.secret_base + "__" + key + return Blowfish.new(_last_bits(secret)) + + +def _last_bits(secret): + """We append the kind at the end, so just use the bits at the end. + """ + last_bits = smart_str(secret) + if len(last_bits) > MAXIMUM_ID_SECRET_LENGTH: + last_bits = last_bits[-MAXIMUM_ID_SECRET_LENGTH:] + return last_bits diff --git a/test/unit/test_security_helper.py b/test/unit/test_security_helper.py index b58d8fefdf5..6e39cfc8637 100644 --- a/test/unit/test_security_helper.py +++ b/test/unit/test_security_helper.py @@ -1,3 +1,5 @@ +# -*- coding: utf-8 -*- + from galaxy.web import security @@ -5,6 +7,64 @@ test_helper_1 = security.SecurityHelper(id_secret="sec1") test_helper_2 = security.SecurityHelper(id_secret="sec2") +def test_maximum_length_handling_ascii(): + # Test that id secrets can be up to 56 characters long. + longest_id_secret = "m" * security.MAXIMUM_ID_SECRET_LENGTH + helper = security.SecurityHelper(id_secret=longest_id_secret) + helper.encode_id(1) + + # Test that security helper will catch if the id secret is too long. + threw_exception = False + longer_id_secret = "m" * (security.MAXIMUM_ID_SECRET_LENGTH + 1) + try: + security.SecurityHelper(id_secret=longer_id_secret) + except Exception: + threw_exception = True + + assert threw_exception + + # Test that different kinds produce different keys even when id secret + # is very long. + e11 = helper.encode_id(1, kind="moo") + e12 = helper.encode_id(1, kind="moo2") + + assert e11 != e12 + + # Test that long kinds are rejected because it uses up "too much" randomness + # from id_secret values. This isn't a strict requirement up but lets just enforce + # the best practice. + assertion_error_raised = False + try: + helper.encode_id(1, kind="this is a really long kind") + except AssertionError: + assertion_error_raised = True + + assert assertion_error_raised + + +def test_maximum_length_handling_nonascii(): + longest_id_secret = "◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎" + helper = security.SecurityHelper(id_secret=longest_id_secret) + helper.encode_id(1) + + # Test that security helper will catch if the id secret is too long. + threw_exception = False + longer_id_secret = "◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎◎" + try: + security.SecurityHelper(id_secret=longer_id_secret) + except Exception: + threw_exception = True + + assert threw_exception + + # Test that different kinds produce different keys even when id secret + # is very long. + e11 = helper.encode_id(1, kind="moo") + e12 = helper.encode_id(1, kind="moo2") + + assert e11 != e12 + + def test_encode_decode(): # Different ids are encoded differently assert test_helper_1.encode_id(1) != test_helper_1.encode_id(2)