From 6cf9094e12361a0aa306752e9d9fd8bfaaf51c85 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Fri, 5 May 2017 15:45:17 +0200 Subject: [PATCH] Make sure permissions on cookie files are set sanely Signed-off-by: Patrick Uiterwijk --- fedora/client/__init__.py | 29 +++++++++++++++++++++++++++++ fedora/client/openidbaseclient.py | 23 +++++++++++++++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/fedora/client/__init__.py b/fedora/client/__init__.py index 595e6858..ab474766 100644 --- a/fedora/client/__init__.py +++ b/fedora/client/__init__.py @@ -28,6 +28,8 @@ .. moduleauthor:: Luke Macken .. moduleauthor:: Toshio Kuratomi ''' +import errno +import os import warnings from munch import Munch @@ -113,6 +115,16 @@ def __repr__(self): self.name, self.message, self.extras) +class UnsafeFileError(Exception): + def __init__(self, filename, message): + super(UnsafeFileError, self).__init__(message) + self.message = message + self.filename = filename + + def __str__(self): + return 'Unsafe file permissions! {}: {}'.format(self.filename, + self.message) + # Backwards compatibility class DictContainer(Munch): @@ -123,6 +135,23 @@ def __init__(self, *args, **kwargs): Munch.__init__(self, *args, **kwargs) +def check_file_permissions(filename, allow_notexists=False): + if os.path.islink(filename): + raise UnsafeFileError(filename, 'File is a symlink') + try: + stat = os.stat(filename) + except OSError as e: + if e.errno == errno.ENOENT and allow_notexists: + return + raise + if stat.st_uid != os.getuid(): + raise UnsafeFileError(filename, 'File not owned by current user') + if stat.st_gid != os.getgid(): + raise UnsafeFileError(filename, 'File not owner by primary group') + if stat.st_mode & 0o007: + raise UnsafeFileError(filename, 'File is world-readable') + + # We want people to be able to import fedora.client.*Client directly # pylint: disable-msg=W0611 from fedora.client.proxyclient import ProxyClient diff --git a/fedora/client/openidbaseclient.py b/fedora/client/openidbaseclient.py index ff336e64..425d9325 100644 --- a/fedora/client/openidbaseclient.py +++ b/fedora/client/openidbaseclient.py @@ -47,7 +47,11 @@ from kitchen.text.converters import to_bytes from fedora import __version__ -from fedora.client import AuthError, LoginRequiredError, ServerError +from fedora.client import (AuthError, + LoginRequiredError, + ServerError, + UnsafeFileError, + check_file_permissions) from fedora.client.openidproxyclient import ( OpenIdProxyClient, absolute_url, openid_login) @@ -182,7 +186,7 @@ def _initialize_session_cache(self): # for their password over and over. if not os.path.isdir(b_SESSION_DIR): try: - os.makedirs(b_SESSION_DIR, mode=0o755) + os.makedirs(b_SESSION_DIR, mode=0o750) except OSError as err: log.warning('Unable to create {file}: {error}'.format( file=b_SESSION_DIR, error=err)) @@ -301,6 +305,12 @@ def _load_cookies(self): if not self.cache_session: return + try: + check_file_permissions(b_SESSION_FILE, True) + except UnsafeFileError as e: + log.debug('Current sessions ignored: {}'.format(str(e))) + return + try: with self.cache_lock: with open(b_SESSION_FILE, 'rb') as f: @@ -312,8 +322,10 @@ def _load_cookies(self): except IOError: # The file doesn't exist, so create it. log.debug("Creating %s", b_SESSION_FILE) + oldmask = os.umask(0o027) with open(b_SESSION_FILE, 'wb') as f: f.write(json.dumps({}).encode('utf-8')) + os.umask(oldmask) def _save_cookies(self): if not self.cache_session: @@ -321,15 +333,22 @@ def _save_cookies(self): with self.cache_lock: try: + check_file_permissions(b_SESSION_FILE, True) with open(b_SESSION_FILE, 'rb') as f: data = json.loads(f.read().decode('utf-8')) + except UnsafeFileError as e: + log.debug('Clearing sessions: {}'.format(str(e))) + os.unlink(b_SESSION_FILE) + data = {} except Exception: log.warn("Failed to open cookie cache before saving.") data = {} + oldmask = os.umask(0o027) data[self.session_key] = self._session.cookies.items() with open(b_SESSION_FILE, 'wb') as f: f.write(json.dumps(data).encode('utf-8')) + os.umask(oldmask) __all__ = ('OpenIdBaseClient', 'requires_login')