Thank you Dave for the suggestion.
Please find the attached updated patch to make HSTS by default disabled and
conditional based on flag.
Regards,
Ganesh Jaybhay
On Mon, Oct 19, 2020 at 5:38 PM Dave Page <dp...@pgadmin.org> wrote:
> Hi
>
> On Mon, Oct 19, 2020 at 1:01 PM Ganesh Jaybhay <
> ganesh.jayb...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Please find the attached patch to fix the below security issues:
>>
>> - Host Header Injection - Added ALLOWED_HOSTS list to limit host
>> address
>> - Lack of Content Security Policy (CSP) - Added security header
>> - Lack of Protection Mechanisms - HSTS - Added security header
>> - Lack of Cookie Attribute – Secure : Kept as False as secure limits
>> cookies to HTTPS traffic only.
>> - Information Disclosure – Web Server / Development Framework
>> VersionDescription: Kept as hard coded 'Python' instead of exposing
>> wsgi/python/gunicorn version info.
>>
>> Please review and let me know if I have missed anything.
>>
>
> I took a very quick look at this, and one thing that immediately stood out
> is that HSTS should definitely not be enabled by default. That can make
> dev/test/redeploy extremely difficult.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EDB: http://www.enterprisedb.com
>
>
diff --git a/Dockerfile b/Dockerfile
index 38c1310..3f5d504 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -81,7 +81,8 @@ RUN apk add --no-cache \
flask_gravatar \
flask_migrate \
simplejson \
- cryptography
+ cryptography \
+ netaddr
# Copy the docs from the local tree. Explicitly remove any existing builds that
# may be present
@@ -177,6 +178,7 @@ RUN ln -sf /usr/lib/libpq.so.5.12 /usr/lib/libpq.so.5
# Copy the runner script
COPY pkg/docker/run_pgadmin.py /pgadmin4
+COPY pkg/docker/gunicorn_config.py /pgadmin4
COPY pkg/docker/entrypoint.sh /entrypoint.sh
# Precompile and optimize python code to save time and space on startup
diff --git a/pkg/docker/entrypoint.sh b/pkg/docker/entrypoint.sh
index 5a482c7..93d809f 100755
--- a/pkg/docker/entrypoint.sh
+++ b/pkg/docker/entrypoint.sh
@@ -58,7 +58,7 @@ TIMEOUT=$(cd /pgadmin4 && python -c 'import config; print(config.SESSION_EXPIRAT
# Using --threads to have multi-threaded single-process worker
if [ ! -z ${PGADMIN_ENABLE_TLS} ]; then
- exec gunicorn --timeout ${TIMEOUT} --bind ${PGADMIN_LISTEN_ADDRESS:-[::]}:${PGADMIN_LISTEN_PORT:-443} -w 1 --threads ${GUNICORN_THREADS:-25} --access-logfile ${GUNICORN_ACCESS_LOGFILE:--} --keyfile /certs/server.key --certfile /certs/server.cert run_pgadmin:app
+ exec gunicorn --timeout ${TIMEOUT} --bind ${PGADMIN_LISTEN_ADDRESS:-[::]}:${PGADMIN_LISTEN_PORT:-443} -w 1 --threads ${GUNICORN_THREADS:-25} --access-logfile ${GUNICORN_ACCESS_LOGFILE:--} --keyfile /certs/server.key --certfile /certs/server.cert -c gunicorn_config.py run_pgadmin:app
else
- exec gunicorn --timeout ${TIMEOUT} --bind ${PGADMIN_LISTEN_ADDRESS:-[::]}:${PGADMIN_LISTEN_PORT:-80} -w 1 --threads ${GUNICORN_THREADS:-25} --access-logfile ${GUNICORN_ACCESS_LOGFILE:--} run_pgadmin:app
+ exec gunicorn --timeout ${TIMEOUT} --bind ${PGADMIN_LISTEN_ADDRESS:-[::]}:${PGADMIN_LISTEN_PORT:-80} -w 1 --threads ${GUNICORN_THREADS:-25} --access-logfile ${GUNICORN_ACCESS_LOGFILE:--} -c gunicorn_config.py run_pgadmin:app
fi
diff --git a/pkg/docker/gunicorn_config.py b/pkg/docker/gunicorn_config.py
new file mode 100644
index 0000000..513c889
--- /dev/null
+++ b/pkg/docker/gunicorn_config.py
@@ -0,0 +1,2 @@
+import gunicorn
+gunicorn.SERVER_SOFTWARE = 'Python'
diff --git a/requirements.txt b/requirements.txt
index a5815a3..dbb0083 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -41,4 +41,5 @@ Flask-Security-Too>=3.0.0
bcrypt<=3.1.7
cryptography<=3.0
sshtunnel>=0.1.5
+netaddr==0.8.0
ldap3>=2.5.1
diff --git a/web/config.py b/web/config.py
index 702e73f..b893e35 100644
--- a/web/config.py
+++ b/web/config.py
@@ -143,12 +143,57 @@ DEFAULT_SERVER = '127.0.0.1'
# environment by the runtime
DEFAULT_SERVER_PORT = 5050
+# This param is used to validate ALLOWED_HOSTS for the application
+# This will be used to avoid Host Header Injection attack
+# For how to set ALLOWED_HOSTS see netaddr library
+# For more details https://netaddr.readthedocs.io/en/latest/tutorial_03.html
+# e.g. ALLOWED_HOSTS = ['192.0.2.0/28', '::192.0.2.0/124']
+# ALLOWED_HOSTS = ['225.0.0.0/8', '226.0.0.0/7', '228.0.0.0/6']
+# ALLOWED_HOSTS = ['127.0.0.1', '192.168.0.1']
+# if ALLOWED_HOSTS= [] then it will accept all ips (and application will be
+# vulnerable to Host Header Injection attack)
+ALLOWED_HOSTS = []
+
+# This param is used to override the default web server information about
+# the web technology and the frameworks being used in the application
+# An attacker could use this information to fingerprint underlying operating
+# system and research known exploits for the specific version of
+# software in use
+WEB_SERVER = 'Python'
+
# Enable X-Frame-Option protection.
# Set to one of "SAMEORIGIN", "ALLOW-FROM origin" or "" to disable.
# Note that "DENY" is NOT supported (and will be silently ignored).
# See https://tools.ietf.org/html/rfc7034 for more info.
X_FRAME_OPTIONS = "SAMEORIGIN"
+# The Content-Security-Policy header allows you to restrict how resources
+# such as JavaScript, CSS, or pretty much anything that the browser loads.
+# see https://content-security-policy.com/#source_list for more info
+# e.g. "default-src https: data: 'unsafe-inline' 'unsafe-eval';"
+CONTENT_SECURITY_POLICY = "default-src http: data: blob: 'unsafe-inline' " \
+ "'unsafe-eval';"
+
+# STRICT_TRANSPORT_SECURITY_ENABLED when set to True will set the
+# Strict-Transport-Security header
+STRICT_TRANSPORT_SECURITY_ENABLED = False
+
+# The Strict-Transport-Security header tells the browser to convert all HTTP
+# requests to HTTPS, preventing man-in-the-middle (MITM) attacks.
+# e.g. 'max-age=31536000; includeSubDomains'
+STRICT_TRANSPORT_SECURITY = "max-age=31536000; includeSubDomains"
+
+# The X-Content-Type-Options header forces the browser to honor the response
+# content type instead of trying to detect it, which can be abused to
+# generate a cross-site scripting (XSS) attack.
+# e.g. nosniff
+X_CONTENT_TYPE_OPTIONS = "nosniff"
+
+# The browser will try to prevent reflected XSS attacks by not loading the
+# page if the request contains something that looks like JavaScript and the
+# response contains the same data. e.g. '1; mode=block'
+X_XSS_PROTECTION = "1; mode=block"
+
# Hashing algorithm used for password storage
SECURITY_PASSWORD_HASH = 'pbkdf2_sha512'
@@ -421,12 +466,14 @@ ON_DEMAND_RECORD_COUNT = 1000
SHOW_GRAVATAR_IMAGE = True
##########################################################################
-# Set cookie path
+# Set cookie path and options
##########################################################################
COOKIE_DEFAULT_PATH = '/'
COOKIE_DEFAULT_DOMAIN = None
SESSION_COOKIE_DOMAIN = None
SESSION_COOKIE_SAMESITE = 'Lax'
+SESSION_COOKIE_SECURE = False
+SESSION_COOKIE_HTTPONLY = True
#########################################################################
# Skip storing session in files and cache for specific paths
diff --git a/web/pgadmin/__init__.py b/web/pgadmin/__init__.py
index 275f365..59cab62 100644
--- a/web/pgadmin/__init__.py
+++ b/web/pgadmin/__init__.py
@@ -12,6 +12,7 @@ such as setup of logging, dynamic loading of modules etc."""
import logging
import os
import sys
+import re
from types import MethodType
from collections import defaultdict
from importlib import import_module
@@ -19,11 +20,13 @@ from importlib import import_module
from flask import Flask, abort, request, current_app, session, url_for
from werkzeug.exceptions import HTTPException
from flask_babelex import Babel, gettext
+from flask_babelex import gettext as _
from flask_login import user_logged_in, user_logged_out
from flask_mail import Mail
from flask_paranoid import Paranoid
from flask_security import Security, SQLAlchemyUserDatastore, current_user
from flask_security.utils import login_user, logout_user
+from netaddr import IPSet
from werkzeug.datastructures import ImmutableDict
from werkzeug.local import LocalProxy
from werkzeug.utils import find_modules
@@ -36,9 +39,10 @@ from pgadmin.utils.session import create_session_interface, pga_unauthorised
from pgadmin.utils.versioned_template_loader import VersionedTemplateLoader
from datetime import timedelta
from pgadmin.setup import get_version, set_version
-from pgadmin.utils.ajax import internal_server_error
+from pgadmin.utils.ajax import internal_server_error, make_json_response
from pgadmin.utils.csrf import pgCSRFProtect
from pgadmin import authenticate
+from pgadmin.utils.security_headers import SecurityHeaders
winreg = None
if os.name == 'nt':
@@ -658,6 +662,36 @@ def create_app(app_name=None):
request.endpoint not in ('security.login', 'security.logout'):
logout_user()
+ @app.before_request
+ def limit_host_addr():
+ """
+ This function validate the hosts from ALLOWED_HOSTS before allowing
+ HTTP request to avoid Host Header Injection attack
+ :return: None/JSON response with 403 HTTP status code
+ """
+ client_host = str(request.host).split(':')[0]
+ valid = True
+ allowed_hosts = config.ALLOWED_HOSTS
+
+ if len(allowed_hosts) != 0:
+ regex = re.compile(
+ r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}(?:/\d{1,2}|)')
+ # Create separate list for ip addresses and host names
+ ip_set = list(filter(lambda ip: regex.match(ip), allowed_hosts))
+ host_set = list(filter(lambda ip: not regex.match(ip),
+ allowed_hosts))
+ is_ip = regex.match(client_host)
+ if is_ip:
+ valid = IPSet(ip_set).__contains__(client_host)
+ else:
+ valid = host_set.__contains__(client_host)
+
+ if not valid:
+ return make_json_response(
+ status=403, success=0,
+ errormsg=_("403 FORBIDDEN")
+ )
+
@app.after_request
def after_request(response):
if 'key' in request.args:
@@ -667,13 +701,12 @@ def create_app(app_name=None):
domain['domain'] = config.COOKIE_DEFAULT_DOMAIN
response.set_cookie('PGADMIN_INT_KEY', value=request.args['key'],
path=config.COOKIE_DEFAULT_PATH,
+ secure=config.SESSION_COOKIE_SECURE,
+ httponly=config.SESSION_COOKIE_HTTPONLY,
+ samesite=config.SESSION_COOKIE_SAMESITE,
**domain)
- # X-Frame-Options for security
- if config.X_FRAME_OPTIONS != "" and \
- config.X_FRAME_OPTIONS.lower() != "deny":
- response.headers["X-Frame-Options"] = config.X_FRAME_OPTIONS
-
+ SecurityHeaders.set_response_headers(response)
return response
##########################################################################
diff --git a/web/pgadmin/browser/__init__.py b/web/pgadmin/browser/__init__.py
index 5375e94..6ed138b 100644
--- a/web/pgadmin/browser/__init__.py
+++ b/web/pgadmin/browser/__init__.py
@@ -697,6 +697,9 @@ def index():
response.set_cookie("PGADMIN_LANGUAGE", value=language,
path=config.COOKIE_DEFAULT_PATH,
+ secure=config.SESSION_COOKIE_SECURE,
+ httponly=config.SESSION_COOKIE_HTTPONLY,
+ samesite=config.SESSION_COOKIE_SAMESITE,
**domain)
return response
diff --git a/web/pgadmin/preferences/__init__.py b/web/pgadmin/preferences/__init__.py
index db11d1d..d05c1c8 100644
--- a/web/pgadmin/preferences/__init__.py
+++ b/web/pgadmin/preferences/__init__.py
@@ -232,6 +232,9 @@ def save(pid):
setattr(session, 'PGADMIN_LANGUAGE', language)
response.set_cookie("PGADMIN_LANGUAGE", value=language,
path=config.COOKIE_DEFAULT_PATH,
+ secure=config.SESSION_COOKIE_SECURE,
+ httponly=config.SESSION_COOKIE_HTTPONLY,
+ samesite=config.SESSION_COOKIE_SAMESITE,
**domain)
return response
diff --git a/web/pgadmin/utils/security_headers.py b/web/pgadmin/utils/security_headers.py
new file mode 100644
index 0000000..d85b5eb
--- /dev/null
+++ b/web/pgadmin/utils/security_headers.py
@@ -0,0 +1,41 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2020, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+#########################################################################
+
+import config
+
+
+class SecurityHeaders:
+
+ @staticmethod
+ def set_response_headers(response):
+ """set response security headers"""
+
+ params_dict = {
+ 'CONTENT_SECURITY_POLICY': 'Content-Security-Policy',
+ 'X_CONTENT_TYPE_OPTIONS': 'X-Content-Type-Options',
+ 'X_XSS_PROTECTION': 'X-XSS-Protection',
+ 'WEB_SERVER': 'Server',
+ }
+
+ # X-Frame-Options for security
+ if config.X_FRAME_OPTIONS != "" and \
+ config.X_FRAME_OPTIONS.lower() != "deny":
+ response.headers["X-Frame-Options"] = config.X_FRAME_OPTIONS
+
+ # Strict-Transport-Security
+ if config.STRICT_TRANSPORT_SECURITY_ENABLED and \
+ config.STRICT_TRANSPORT_SECURITY != "":
+ response.headers["Strict-Transport-Security"] = \
+ config.STRICT_TRANSPORT_SECURITY
+
+ # add other security options
+ for key in params_dict:
+ if key in config.__dict__ and config.__dict__[key] != "" \
+ and config.__dict__[key] is not None:
+ response.headers[params_dict[key]] = config.__dict__[key]
diff --git a/web/pgadmin/utils/session.py b/web/pgadmin/utils/session.py
index b8af3d4..62e03ba 100644
--- a/web/pgadmin/utils/session.py
+++ b/web/pgadmin/utils/session.py
@@ -311,7 +311,11 @@ class ManagedSessionInterface(SessionInterface):
response.set_cookie(
app.session_cookie_name,
'%s!%s' % (session.sid, session.hmac_digest),
- expires=cookie_exp, httponly=True, domain=domain
+ expires=cookie_exp,
+ secure=config.SESSION_COOKIE_SECURE,
+ httponly=config.SESSION_COOKIE_HTTPONLY,
+ samesite=config.SESSION_COOKIE_SAMESITE,
+ domain=domain
)
