From d7476abc1ec5fa929ba08a4e1a1e30c30e01f8fa Mon Sep 17 00:00:00 2001 From: ne-bbahn Date: Mon, 13 Nov 2023 14:27:41 -0800 Subject: [PATCH] reimplementing security fix from master merge --- agent/listener/server.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/agent/listener/server.py b/agent/listener/server.py index 3f7e9cc63..a6ec6d585 100755 --- a/agent/listener/server.py +++ b/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,6 +428,8 @@ 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, @@ -421,7 +437,7 @@ def admin_login(): 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: