reimplementing security fix from master merge

agent/listener/server.py

@@ -23,6 +23,7 @@
 
 # Set whether or not a request is internal or not
 import socket
+from hmac import compare_digest
 
 __VERSION__ = ncpa.__VERSION__
 __STARTED__ = datetime.datetime.now()
@@ -162,6 +163,15 @@ def is_network(ip):
         logging.debug(e)
         return False
 
+# Securely compares strings - byte string or unicode
+# Comparison is done via compare_digest() to prevent timing attacks
+# If both items evaluate to false, they match. This makes it easier to handle
+# empty strings or variables which may have "NoneType"
+def secure_compare(item1, item2):
+    item1 = '' if item1 is None else str(item1)
+    item2 = '' if item2 is None else str(item2)
+    return compare_digest(item1, item2)
+
 
 # ------------------------------
 # Authentication Wrappers
@@ -280,16 +290,17 @@ def requires_token_or_auth(f):
     def token_auth_decoration(*args, **kwargs):
         ncpa_token = listener.config['iconfig'].get('api', 'community_string')
         token = request.values.get('token', None)
+        token_valid = secure_compare(token, ncpa_token)
 
         # This is an internal call, we don't check
         if __INTERNAL__ is True:
             pass
-        elif session.get('logged', False) or token == ncpa_token:
+        elif session.get('logged', False) or token_valid:
             pass
         elif token is None:
             session['redirect'] = request.url
             return redirect(url_for('login'))
-        elif token != ncpa_token:
+        elif not token_valid:
             return error(msg='Incorrect credentials given.')
         return f(*args, **kwargs)
 
@@ -369,6 +380,9 @@ def login():
     url = session.get('redirect', None)
     token = request.values.get('token', None)
 
+    token_valid = secure_compare(token, ncpa_token)
+    token_is_admin = secure_compare(token, admin_password)
+
     template_args = { 'hide_page_links': True,
                       'message': message,
                       'url': url,
@@ -378,9 +392,9 @@ def login():
     session['message'] = None
 
     # Do actual authentication check
-    if token == ncpa_token and not admin_auth_only:
+    if not admin_auth_only and token_valid:
         session['logged'] = True
-    elif token == admin_password and admin_password is not None:
+    elif admin_password is not None and token_is_admin:
         session['logged'] = True
         session['admin_logged'] = True
 
@@ -394,10 +408,10 @@ def login():
     # Display error messages depending on what was given
     if token is not None:
         if not admin_auth_only:
-            if token != ncpa_token or token != admin_password:
+            if not token_valid and not token_is_admin:
                 template_args['error'] = 'Invalid token or password.'
         else:
-            if token == ncpa_token:
+            if token_valid:
                 template_args['error'] = 'Admin authentication only.'
             else:
                 template_args['error'] = 'Invalid password.'
@@ -414,14 +428,16 @@ def admin_login():
 
     # Admin password
     admin_password = get_config_value('listener', 'admin_password', None)
+    password = request.values.get('password', None)
+    password_valid = secure_compare(password, admin_password)
 
     message = session.get('message', None)
     template_args = { 'hide_page_links': False,
                       'message': message }
 
     session['message'] = None
 
-    if password == admin_password and admin_password is not None:
+    if admin_password is not None and password_valid:
         session['admin_logged'] = True
         return redirect(url_for('admin'))
     elif password is not None: