Skip to content

Commit

Permalink
Make sure permissions on cookie files are set sanely
Browse files Browse the repository at this point in the history
Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
  • Loading branch information
puiterwijk committed May 5, 2017
1 parent e9b17d3 commit 6cf9094
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 2 deletions.
29 changes: 29 additions & 0 deletions fedora/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
.. moduleauthor:: Luke Macken <lmacken@redhat.com>
.. moduleauthor:: Toshio Kuratomi <tkuratom@redhat.com>
'''
import errno
import os
import warnings

from munch import Munch
Expand Down Expand Up @@ -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):
Expand All @@ -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
Expand Down
23 changes: 21 additions & 2 deletions fedora/client/openidbaseclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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:
Expand All @@ -312,24 +322,33 @@ 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:
return

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')

0 comments on commit 6cf9094

Please # to comment.