X2Go Bug report logs - #447
[PATCH] Add support for cookie based auth after initial password auth

Full log

Message #5 received at submit@bugs.x2go.org (full text, mbox, reply):

Received: (at submit) by bugs.x2go.org; 6 Mar 2014 04:40:00 +0000
From jlukens@botch.com  Thu Mar  6 05:39:59 2014
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on
	ymir.das-netzwerkteam.de
X-Spam-Level: 
X-Spam-Status: No, score=-1.6 required=5.0 tests=BAYES_00,
	FILL_THIS_FORM_FRAUD_PHISH,T_FILL_THIS_FORM_SHORT,URIBL_BLOCKED autolearn=no
	version=3.3.2
Received: from felt.botch.com (felt.botch.com [207.145.43.98])
	by ymir (Postfix) with ESMTP id 549965DA6C
	for <submit@bugs.x2go.org>; Thu,  6 Mar 2014 05:39:58 +0100 (CET)
Received: from [127.0.0.1] (unknown [192.168.254.1])
	(Authenticated sender: jlukens)
	by felt.botch.com (Postfix) with ESMTP id 2B9891AC0C5
	for <submit@bugs.x2go.org>; Wed,  5 Mar 2014 23:39:56 -0500 (EST)
Date: Wed, 5 Mar 2014 23:39:55 -0500
From: Josh Lukens <jlukens@botch.com>
To: submit@bugs.x2go.org
Message-ID: <7108013299F34AF4A05264D6EAA0B5C6@botch.com>
Subject: [PATCH] Add support for cookie based auth after initial
 password auth
X-Mailer: sparrow 1.6.4 (build 1178)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Package: x2gobroker
Version: 0.0.2.4
Severity: wishlist



---
etc/x2gobroker.conf | 30 +++++++--
x2gobroker/brokers/base_broker.py | 133 +++++++++++++++++++++-----------------
x2gobroker/defaults.py | 7 +-
x2gobroker/web/json.py | 9 +--
x2gobroker/web/plain.py | 10 ++-
x2gobroker/web/uccs.py | 4 +-
6 files changed, 112 insertions(+), 81 deletions(-)

diff --git a/etc/x2gobroker.conf b/etc/x2gobroker.conf
index 19ea93b..b8b8974 100644
--- a/etc/x2gobroker.conf
+++ b/etc/x2gobroker.conf
@@ -24,20 +24,38 @@

[global]

-# Allow unauthenticated connections? Then set check-credentials to false.
-#check-credentials = true
+# Allow unauthenticated connections? Then set both require-password and require-cookie to false.
+
+# Veriy username/password combination sent by client
+#require-password = true

# To secure server-client communication the client can start the communication
# with a pre-set, agreed on authentication ID. Set the below value to true
# to make the X2Go Session Broker require this feature
-#require-cookie-auth = false ### NOT-IN-USE-YET
+#require-cookie = false

# X2Go supports two different cookie authentication modes (static and dynamic).
-#use-static-cookie = true ### NOT-IN-USE-YET
+# Dynamic cookies send new cookie to client on every request. This could possibly
+# cause issues if a client ever tries multiple requests at the same time.
+#use-static-cookie = true
+
+# Once a client is authenticated their password is not revalidated until this
+# many seconds have elapsed from their initial authentication.
+#auth-timeout = 36000
+
+# Client cookies (both static and dynamic) must be stored as local files.
+# This is the directory where those files will be stored. Please make sure
+# the permissions are set to allow the x2go broker process to write to this directory
+#cookie-directory = '/var/log/x2gobroker/cookies'

# Every server-client communication (between X2Go Client and broker) has to be
-# accompanied by this initial authentication cookie.
-#my-cookie = <aaaavveeeerrrrryyyyylooonnnnggggssttrrriiinnnggg> ### NOT-IN-USE-YET
+# accompanied by this initial authentication cookie if require-cookie is set above.
+# This should be in the format of a UUID.
+#my-cookie = xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+
+# By default the broker will pin user sessions to the IP address from which they
+# origionally authenticate. If you would like to skip that check set this to false.
+#verify-ip = true

# X2Go Session Broker knows about two output formats: a text/plain based output
# and a text/json based output that is compatible with UCCS. The different outputs
diff --git a/x2gobroker/brokers/base_broker.py b/x2gobroker/brokers/base_broker.py
index 7fb3172..2f7c7ad 100644
--- a/x2gobroker/brokers/base_broker.py
+++ b/x2gobroker/brokers/base_broker.py
@@ -726,7 +726,7 @@ class X2GoBroker(object):
else:
return []

- def check_access(self, username='', password='', cookie=None, cookie_only=False):
+ def check_access(self, username='', password='', ip='', cookie=None):
"""\
Check if a given user with a given password may gain access to the
X2Go session broker.
@@ -735,80 +735,95 @@ class X2GoBroker(object):
@type username: C{unicode}
@param password: a password that authenticates the user against the X2Go session broker
@type password: C{unicode}
+ @param ip: the ip address of the client
+ @type ip: C{unicode}
@param cookie: an extra (static or dynamic) authentication token
@type cookie: C{unicode}
- @param cookie_only: do only check the auth_cookie, not username/password
- @type cookie_only: C{bool}

@return: returns C{True} if the authentication has been successful
- @rtype: C{bool}
+ @rtype: C{bool},C{unicode}

"""
### FOR INTRANET LOAD BALANCER WE MAY JUST ALLOW ACCESS TO EVERYONE
### This is handled through the config file, normally /etc/x2go/x2gobroker.conf

- if not self.config.get_value('global', 'check-credentials'):
+ if not self.config.get_value('global', 'require-password') and not self.config.get_value('global', 'require-cookie'):
logger_broker.debug('base_broker.X2GoBroker.check_access(): access is granted without checking credentials, prevent this in {configfile}'.format(configfile=self.config_file))
- return True
+ return True,0
elif username == 'check-credentials' and password == 'FALSE':
# this catches a validation check from the UCCS web frontend...
- return False
+ return False,0

### IMPLEMENT YOUR AUTHENTICATION LOGIC IN THE self._do_authenticate(**kwargs) METHOD
### when inheriting from the base.X2GoBroker class.
-
- access = False
- if cookie_only is False:
- access = self._do_authenticate(username=username, password=password)
- logger_broker.debug('base_broker.X2GoBroker.check_access(): result of authentication check is: {access}'.format(access=access))
- else:
- access = True
-
- ### HANDLING OF DYNAMIC AUTHENTICATION ID HASHES
-
- # using cookie authentication as extra security?
- if self.config.get_value('global', 'require-cookie-auth'):
-
- if type(cookie) is types.StringType:
- cookie = unicode(cookie)
-
- if self.config.get_value('global', 'use-static-cookie'):
-
- # evaluate access based on static authentication ID feature
- access = access and ( cookie == self.config.get_value('global', 'my-cookie') )
-
- else:
-
- # evaluate access based on dynamic authentication ID feature
- if self._dynamic_cookie_map.has_key(username):
- access = access and ( cookie == self._dynamic_cookie_map[username] )
- if access:
- self._dynamic_cookie_map[username] = uuid.uuid5(namespace=cookie, name=username)
-
+ if type(cookie) is types.StringType:
+ cookie = unicode(cookie)
+
+ if (((cookie == None) or (cookie == "")) and self.config.get_value('global', 'require-cookie')):
+ #cookie required but we did not get one - catch wrong cookie case later
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): cookie required but none given.')
+ return False, 0
+ #check if cookie sent was our preset cookie from config file
+ next_cookie = self.config.get_value('global', 'my-cookie')
+ access = (cookie == next_cookie )
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): checking if our configured cookie was submitted: {access}'.format(access=access))
+ #the require cookie but not password case falls through to returning value of access
+
+ if self.config.get_value('global', 'require-password'):
+ #using files to store persistant cookie information because global variables do not work across threads in WSGI
+ import os.path
+ cookie_directory=self.config.get_value('global', 'cookie-directory')
+ if(not os.path.isdir(cookie_directory)):
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): cookie-directory {cookie_directory} does not exist trying to craete it'.format(cookie_directory=cookie_directory))
+ try:
+ os.makedirs(cookie_directory);
+ except:
+ logger_broker.warning('base_broker.X2GoBroker.check_access(): could not create cookie-directory {cookie_directory} failing to authenticate'.format(cookie_directory=cookie_directory))
+ return False, 0
+ if access or cookie == None or cookie == "":
+ # this should be the first time we have seen this user or they are using old client so verify their passwrd
+ access = self._do_authenticate(username=username, password=password)
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): checking for valid password: {access}'.format(access=access))
+
+ if access:
+ #create new cookie for this user
+ #each user gets one or more tuples of IP, time stored as username_UUID files so they can connect from multiple sessions
+ next_cookie = str(uuid.uuid4())
+ fh = open(cookie_directory+"/"+username+"_"+next_cookie,"w")
+ fh.write('{ip} {time}'.format(ip=ip, time=time.time()))
+ fh.close()
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): Giving new cookie: {cookie} to user {username} at ip {ip}'.format(cookie=next_cookie,username=username,ip=ip))
+ else:
+ # there is a cookie but its not ours so its either wrong or subsequent password auth
+ if os.path.isfile(cookie_directory+"/"+username+"_"+cookie):
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): found valid auth key for user cookie: {usercookie}'.format(usercookie=username+"_"+cookie))
+ fh=open(cookie_directory+"/"+username+"_"+cookie,"r")
+ origip,origtime= fh.read().split()
+ fh.close()
+ os.unlink(cookie_directory+"/"+username+"_"+cookie)
+ #found cookie - make sure IP and time are good
+ if self.config.get_value('global', 'verify-ip') and (ip != origip):
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): IPs differ (new: {ip} old: {origip}) - rejecting user'.format(ip=ip,origip=origip))
+ return False, 0
+ if (time.time() - float(origtime)) > self.config.get_value('global', 'auth-timeout'):
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): Too much time elapsed since origional auth - rejecting user')
+ return False, 0
+ if self.config.get_value('global', 'use-static-cookie'):
+ #if using static cookies keep same cookie as user presented
+ next_cookie = cookie
+ else:
+ #otherwise give them new random cookie
+ next_cookie = str(uuid.uuid4())
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): Giving cookie: {cookie} to ip {ip}'.format(cookie=next_cookie, ip=ip))
+ fh = open(cookie_directory+"/"+username+"_"+next_cookie,"w")
+ fh.write('{ip} {time}'.format(ip=ip, time=origtime))
+ fh.close()
+ access = True
else:
- access = access and ( cookie == self.config.get_value('global', 'my-cookie') )
- if access:
- # generate a first uuid, initialize the dynamic authencation ID security feature
- self._dynamic_cookie_map[username] = uuid.uuid4()
-
- return access
-
- def get_next_cookie(self, username):
- """\
- Get the next expected authentication cookie for the given user name.
-
- @param username: query next authentication cookie for this user
- @type username: C{unicode}
-
- @return: returns next authentication cookie for the given username, None if no cookie has been generated, yet
- @rtype: C{unicode} or C{None}
-
- """
- try:
- return self._dynamic_cookie_map[username]
- except KeyError:
- return None
-
+ # client sent us an unknown cookie so failing auth
+ logger_broker.debug('base_broker.X2GoBroker.check_access(): User {username} from {ip} presented cookie {cookie} which is not recognized - rejecting user'.format(username=username, cookie=cookie, ip=ip))
+ return False, 0
+ return access, next_cookie

def get_remote_agent(self, profile_id, exclude_agents=[], ):
"""\
diff --git a/x2gobroker/defaults.py b/x2gobroker/defaults.py
index e65fd31..9027ed0 100644
--- a/x2gobroker/defaults.py
+++ b/x2gobroker/defaults.py
@@ -180,9 +180,12 @@ X2GOBROKER_HOME = os.path.normpath(os.path.expanduser('~{broker_uid}'.format(bro
# defaults for X2Go Sessino Broker configuration file
X2GOBROKER_CONFIG_DEFAULTS = {
'global': {
- u'check-credentials': True,
- u'require-cookie-auth': False,
+ u'require-password': True,
+ u'require-cookie': False,
u'use-static-cookie': True,
+ u'auth-timeout': 36000,
+ u'cookie-directory': '/var/log/x2gobroker/cookies',
+ u'verify-ip': True,
u'my-cookie': uuid.uuid4(),
u'enable-plain-output': True,
u'enable-json-output': True,
diff --git a/x2gobroker/web/json.py b/x2gobroker/web/json.py
index b217050..1f10b31 100644
--- a/x2gobroker/web/json.py
+++ b/x2gobroker/web/json.py
@@ -119,17 +119,14 @@ class X2GoBrokerWeb(_RequestHandler):
output = ''

logger_broker.debug ('username: {username}, password: {password}, task: {task}, profile_id: {profile_id}, cookie: {cookie}'.format(username=username, password='XXXXX', task=task, profile_id=profile_id, cookie=cookie))
- if broker_backend.check_access(username=username, password=password, cookie=cookie):
+ access, next_cookie = broker_backend.check_access(username=username, password=password, ip=ip, cookie=cookie)
+ if access:

###
### CONFIRM SUCCESSFUL AUTHENTICATION FIRST
###

- if global_config['require-cookie-auth'] and not global_config['use-static-cookie']:
-
- ### FIXME: make up a nice protocol for this, disabled for now
- #output += "AUTHID: {authid}<br />".format(authid=broker_backend.get_next_authid(username=data.user))
- pass
+ ### FIXME: find good way to pass next cookie to client - stored in next_cookie

###
### X2GO BROKER TASKS
diff --git a/x2gobroker/web/plain.py b/x2gobroker/web/plain.py
index 9d58742..254c9d2 100644
--- a/x2gobroker/web/plain.py
+++ b/x2gobroker/web/plain.py
@@ -115,17 +115,15 @@ class X2GoBrokerWeb(_RequestHandler):
output = ''

logger_broker.debug ('username: {username}, password: {password}, task: {task}, profile_id: {profile_id}, cookie: {cookie}'.format(username=username, password='XXXXX', task=task, profile_id=profile_id, cookie=cookie))
- if broker_backend.check_access(username=username, password=password, cookie=cookie):
+ access, next_cookie = broker_backend.check_access(username=username, password=password, ip=ip, cookie=cookie)
+ if access:

###
### CONFIRM SUCCESSFUL AUTHENTICATION FIRST
###

- if global_config['require-cookie-auth'] and not global_config['use-static-cookie']:
-
- ### FIXME: make up a nice protocol for this, disabled for now
- #output += "AUTHID: {authid}<br />".format(authid=broker_backend.get_next_authid(username=data.user))
- pass
+ if next_cookie != 0:
+ output += "AUTHID:{authid}\n".format(authid=next_cookie)

output += "Access granted\n"
###
diff --git a/x2gobroker/web/uccs.py b/x2gobroker/web/uccs.py
index 917704f..87dc64a 100644
--- a/x2gobroker/web/uccs.py
+++ b/x2gobroker/web/uccs.py
@@ -42,11 +42,11 @@ def credentials_validate(username, password):
# from x2gobroker.conf are available here...
broker = x2gobroker.brokers.base_broker.X2GoBroker()
broker.enable()
- access = broker.check_access(username=username, password=password)
+ access, next_cookie = broker.check_access(username=username, password=password)
# UCCS only allows email addresses for remote login 
if not access and "@" in username:
username = username.split('@')[0]
- access = broker.check_access(username=username, password=password)
+ access, next_cookie = broker.check_access(username=username, password=password)
if username == 'check-credentials' and password == 'FALSE':
username = 'anonymous'
return username, access
-- 
1.8.3.4 (Apple Git-47)

Send a report that this bug log contains spam.