enh: Cookies security (#1211)

author: corruba

configs/docker_config.py

@@ -2,6 +2,8 @@
 BIND_ADDRESS = '0.0.0.0'
 PORT = 80
 SQLALCHEMY_DATABASE_URI = 'sqlite:////data/powerdns-admin.db'
+SESSION_COOKIE_SAMESITE = 'Lax'
+CSRF_COOKIE_HTTPONLY = True
 
 legal_envvars = (
     'SECRET_KEY',
@@ -56,7 +58,9 @@ legal_envvars = (
     'LDAP_ENABLED',
     'SAML_CERT',
     'SAML_KEY',
-    'FILESYSTEM_SESSIONS_ENABLED'
+    'FILESYSTEM_SESSIONS_ENABLED',
+    'SESSION_COOKIE_SECURE',
+    'CSRF_COOKIE_SECURE',
 )
 
 legal_envvars_int = ('PORT', 'MAIL_PORT', 'SAML_METADATA_CACHE_LIFETIME')
@@ -78,7 +82,9 @@ legal_envvars_bool = (
     'SIGNUP_ENABLED',
     'LOCAL_DB_ENABLED',
     'LDAP_ENABLED',
-    'FILESYSTEM_SESSIONS_ENABLED'
+    'FILESYSTEM_SESSIONS_ENABLED',
+    'SESSION_COOKIE_SECURE',
+    'CSRF_COOKIE_SECURE',
 )
 
 # import everything from environment variables

powerdnsadmin/__init__.py

@@ -1,7 +1,6 @@
 import os
 import logging
 from flask import Flask
-from flask_seasurf import SeaSurf
 from flask_mail import Mail
 from werkzeug.middleware.proxy_fix import ProxyFix
 from flask_session import Session
@@ -33,31 +32,6 @@ def create_app(config=None):
     # Proxy
     app.wsgi_app = ProxyFix(app.wsgi_app)
 
-    # CSRF protection
-    csrf = SeaSurf(app)
-    csrf.exempt(routes.index.dyndns_checkip)
-    csrf.exempt(routes.index.dyndns_update)
-    csrf.exempt(routes.index.saml_authorized)
-    csrf.exempt(routes.api.api_login_create_zone)
-    csrf.exempt(routes.api.api_login_delete_zone)
-    csrf.exempt(routes.api.api_generate_apikey)
-    csrf.exempt(routes.api.api_delete_apikey)
-    csrf.exempt(routes.api.api_update_apikey)
-    csrf.exempt(routes.api.api_zone_subpath_forward)
-    csrf.exempt(routes.api.api_zone_forward)
-    csrf.exempt(routes.api.api_create_zone)
-    csrf.exempt(routes.api.api_create_account)
-    csrf.exempt(routes.api.api_delete_account)
-    csrf.exempt(routes.api.api_update_account)
-    csrf.exempt(routes.api.api_create_user)
-    csrf.exempt(routes.api.api_delete_user)
-    csrf.exempt(routes.api.api_update_user)
-    csrf.exempt(routes.api.api_list_account_users)
-    csrf.exempt(routes.api.api_add_account_user)
-    csrf.exempt(routes.api.api_remove_account_user)
-    csrf.exempt(routes.api.api_zone_cryptokeys)
-    csrf.exempt(routes.api.api_zone_cryptokey)
-
     # Load config from env variables if using docker
     if os.path.exists(os.path.join(app.root_path, 'docker_config.py')):
         app.config.from_object('powerdnsadmin.docker_config')
@@ -69,7 +43,7 @@ def create_app(config=None):
     if 'FLASK_CONF' in os.environ:
         app.config.from_envvar('FLASK_CONF')
 
-    # Load app sepecified configuration
+    # Load app specified configuration
     if config is not None:
         if isinstance(config, dict):
             app.config.update(config)

powerdnsadmin/default_config.py

@@ -10,6 +10,8 @@ PORT = 9191
 HSTS_ENABLED = False
 OFFLINE_MODE = False
 FILESYSTEM_SESSIONS_ENABLED = False
+SESSION_COOKIE_SAMESITE = 'Lax'
+CSRF_COOKIE_HTTPONLY = True
 
 ### DATABASE CONFIG
 SQLA_DB_USER = 'pda'

powerdnsadmin/routes/__init__.py

@@ -1,5 +1,5 @@
 from .base import (
-    login_manager, handle_bad_request, handle_unauthorized_access,
+    csrf, login_manager, handle_bad_request, handle_unauthorized_access,
     handle_access_forbidden, handle_page_not_found, handle_internal_server_error
 )
 
@@ -13,6 +13,7 @@ from .api import api_bp, apilist_bp
 
 def init_app(app):
     login_manager.init_app(app)
+    csrf.init_app(app)
 
     app.register_blueprint(index_bp)
     app.register_blueprint(user_bp)

powerdnsadmin/routes/api.py

@@ -6,6 +6,7 @@ from flask import (
 )
 from flask_login import current_user
 
+from .base import csrf
 from ..models.base import db
 from ..models import (
     User, Domain, DomainUser, Account, AccountUser, History, Setting, ApiKey,
@@ -187,6 +188,7 @@ def index():
 @api_bp.route('/pdnsadmin/zones', methods=['POST'])
 @api_basic_auth
 @api_can_create_domain
+@csrf.exempt
 def api_login_create_zone():
     pdns_api_url = Setting().get('pdns_api_url')
     pdns_api_key = Setting().get('pdns_api_key')
@@ -255,6 +257,7 @@ def api_login_list_zones():
 @api_bp.route('/pdnsadmin/zones/<string:domain_name>', methods=['DELETE'])
 @api_basic_auth
 @api_can_create_domain
+@csrf.exempt
 def api_login_delete_zone(domain_name):
     pdns_api_url = Setting().get('pdns_api_url')
     pdns_api_key = Setting().get('pdns_api_key')
@@ -310,6 +313,7 @@ def api_login_delete_zone(domain_name):
 
 @api_bp.route('/pdnsadmin/apikeys', methods=['POST'])
 @api_basic_auth
+@csrf.exempt
 def api_generate_apikey():
     data = request.get_json()
     description = None
@@ -466,6 +470,7 @@ def api_get_apikey(apikey_id):
 
 @api_bp.route('/pdnsadmin/apikeys/<int:apikey_id>', methods=['DELETE'])
 @api_basic_auth
+@csrf.exempt
 def api_delete_apikey(apikey_id):
     apikey = ApiKey.query.get(apikey_id)
 
@@ -503,6 +508,7 @@ def api_delete_apikey(apikey_id):
 
 @api_bp.route('/pdnsadmin/apikeys/<int:apikey_id>', methods=['PUT'])
 @api_basic_auth
+@csrf.exempt
 def api_update_apikey(apikey_id):
     # if role different and user is allowed to change it, update
     # if apikey domains are different and user is allowed to handle
@@ -664,6 +670,7 @@ def api_list_users(username=None):
 @api_bp.route('/pdnsadmin/users', methods=['POST'])
 @api_basic_auth
 @api_role_can('create users', allow_self=True)
+@csrf.exempt
 def api_create_user():
     """
     Create new user
@@ -737,6 +744,7 @@ def api_create_user():
 @api_bp.route('/pdnsadmin/users/<int:user_id>', methods=['PUT'])
 @api_basic_auth
 @api_role_can('update users', allow_self=True)
+@csrf.exempt
 def api_update_user(user_id):
     """
     Update existing user
@@ -809,6 +817,7 @@ def api_update_user(user_id):
 @api_bp.route('/pdnsadmin/users/<int:user_id>', methods=['DELETE'])
 @api_basic_auth
 @api_role_can('delete users')
+@csrf.exempt
 def api_delete_user(user_id):
     user = User.query.get(user_id)
     if not user:
@@ -860,6 +869,7 @@ def api_list_accounts(account_name):
 
 @api_bp.route('/pdnsadmin/accounts', methods=['POST'])
 @api_basic_auth
+@csrf.exempt
 def api_create_account():
     if current_user.role.name not in ['Administrator', 'Operator']:
         msg = "{} role cannot create accounts".format(current_user.role.name)
@@ -904,6 +914,7 @@ def api_create_account():
 @api_bp.route('/pdnsadmin/accounts/<int:account_id>', methods=['PUT'])
 @api_basic_auth
 @api_role_can('update accounts')
+@csrf.exempt
 def api_update_account(account_id):
     data = request.get_json()
     name = data['name'] if 'name' in data else None
@@ -945,6 +956,7 @@ def api_update_account(account_id):
 @api_bp.route('/pdnsadmin/accounts/<int:account_id>', methods=['DELETE'])
 @api_basic_auth
 @api_role_can('delete accounts')
+@csrf.exempt
 def api_delete_account(account_id):
     account_list = [] or Account.query.filter(Account.id == account_id).all()
     if len(account_list) == 1:
@@ -996,6 +1008,7 @@ def api_list_account_users(account_id):
     methods=['PUT'])
 @api_basic_auth
 @api_role_can('add user to account')
+@csrf.exempt
 def api_add_account_user(account_id, user_id):
     account = Account.query.get(account_id)
     if not account:
@@ -1023,6 +1036,7 @@ def api_add_account_user(account_id, user_id):
     methods=['DELETE'])
 @api_basic_auth
 @api_role_can('remove user from account')
+@csrf.exempt
 def api_remove_account_user(account_id, user_id):
     account = Account.query.get(account_id)
     if not account:
@@ -1054,6 +1068,7 @@ def api_remove_account_user(account_id, user_id):
 @apikey_auth
 @apikey_can_access_domain
 @apikey_can_configure_dnssec(http_methods=['POST'])
+@csrf.exempt
 def api_zone_cryptokeys(server_id, zone_id):
     resp = helper.forward_request()
     return resp.content, resp.status_code, resp.headers.items()
@@ -1065,6 +1080,7 @@ def api_zone_cryptokeys(server_id, zone_id):
 @apikey_auth
 @apikey_can_access_domain
 @apikey_can_configure_dnssec()
+@csrf.exempt
 def api_zone_cryptokey(server_id, zone_id, cryptokey_id):
     resp = helper.forward_request()
     return resp.content, resp.status_code, resp.headers.items()
@@ -1075,6 +1091,7 @@ def api_zone_cryptokey(server_id, zone_id, cryptokey_id):
     methods=['GET', 'POST', 'PUT', 'PATCH', 'DELETE'])
 @apikey_auth
 @apikey_can_access_domain
+@csrf.exempt
 def api_zone_subpath_forward(server_id, zone_id, subpath):
     resp = helper.forward_request()
     return resp.content, resp.status_code, resp.headers.items()
@@ -1090,6 +1107,7 @@ def api_zone_subpath_forward(server_id, zone_id, subpath):
 @callback_if_request_body_contains_key(apikey_can_configure_dnssec()(),
                                        http_methods=['PUT'],
                                        keys=['dnssec', 'nsec3param'])
+@csrf.exempt
 def api_zone_forward(server_id, zone_id):
     resp = helper.forward_request()
     if not Setting().get('bg_domain_updates'):
@@ -1127,6 +1145,7 @@ def api_zone_forward(server_id, zone_id):
 @api_bp.route('/servers/<path:subpath>', methods=['GET', 'PUT'])
 @apikey_auth
 @apikey_is_admin
+@csrf.exempt
 def api_server_sub_forward(subpath):
     resp = helper.forward_request()
     return resp.content, resp.status_code, resp.headers.items()
@@ -1135,6 +1154,7 @@ def api_server_sub_forward(subpath):
 @api_bp.route('/servers/<string:server_id>/zones', methods=['POST'])
 @apikey_auth
 @apikey_can_create_domain
+@csrf.exempt
 def api_create_zone(server_id):
     resp = helper.forward_request()
 

powerdnsadmin/routes/base.py

@@ -1,9 +1,13 @@
 import base64
+
 from flask import render_template, url_for, redirect, session, request, current_app
 from flask_login import LoginManager
+from flask_seasurf import SeaSurf
 
 from ..models.user import User
 
+
+csrf = SeaSurf()
 login_manager = LoginManager()
 
 

powerdnsadmin/routes/index.py

@@ -10,7 +10,7 @@ from yaml import Loader, load
 from flask import Blueprint, render_template, make_response, url_for, current_app, g, session, request, redirect, abort
 from flask_login import login_user, logout_user, login_required, current_user
 
-from .base import login_manager
+from .base import csrf, login_manager
 from ..lib import utils
 from ..decorators import dyndns_login_required
 from ..models.base import db
@@ -763,6 +763,7 @@ def resend_confirmation_email():
 
 
 @index_bp.route('/nic/checkip.html', methods=['GET', 'POST'])
+@csrf.exempt
 def dyndns_checkip():
     # This route covers the default ddclient 'web' setting for the checkip service
     return render_template('dyndns.html',
@@ -771,6 +772,7 @@ def dyndns_checkip():
 
 
 @index_bp.route('/nic/update', methods=['GET', 'POST'])
+@csrf.exempt
 @dyndns_login_required
 def dyndns_update():
     # dyndns protocol response codes in use are:
@@ -961,6 +963,7 @@ def saml_metadata():
 
 
 @index_bp.route('/saml/authorized', methods=['GET', 'POST'])
+@csrf.exempt
 def saml_authorized():
     errors = []
     if not current_app.config.get('SAML_ENABLED'):

requirements.txt

@@ -18,7 +18,7 @@ pytz==2020.1
 cssmin==0.2.0
 jsmin==3.0.0
 Authlib==0.15
-Flask-SeaSurf==0.2.2
+Flask-SeaSurf==1.1.1
 bravado-core==5.17.0
 lima==0.5
 pytest==6.1.1