From 2ee4a7bcaaff2c9c3035cb5fe81664fb0aaa92b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20G=C3=B3mez=20Garc=C3=ADa?= Date: Mon, 28 Jun 2021 13:05:48 +0200 Subject: [PATCH] Several fixes: * Added tunnel & guacaomle "fake" authid for next security * Fixed proxy detection & use --- server/src/uds/REST/methods/tunnel.py | 6 ++- server/src/uds/auths/IP/authenticator.py | 22 ++++----- .../src/uds/auths/InternalDB/authenticator.py | 43 +++++++++++++--- .../src/uds/core/util/middleware/request.py | 49 ++++++++++++++----- server/src/uds/dispatchers/guacamole/urls.py | 2 + server/src/uds/dispatchers/guacamole/views.py | 6 +++ server/src/uds/models/transport.py | 1 - .../uds/services/Proxmox/client/__init__.py | 30 +++++++++--- 8 files changed, 120 insertions(+), 39 deletions(-) diff --git a/server/src/uds/REST/methods/tunnel.py b/server/src/uds/REST/methods/tunnel.py index c8e30ec76..a69a3585b 100644 --- a/server/src/uds/REST/methods/tunnel.py +++ b/server/src/uds/REST/methods/tunnel.py @@ -62,12 +62,16 @@ class Tunnel(Handler): if ( not isTrustedSource(self._request.ip) - or len(self._args) != 2 + or len(self._args) not in (2, 3) or len(self._args[0]) != 48 ): # Invalid requests raise AccessDenied() + # If args is 3, the last one is the authId + authId = self._args[2][:48] + # TODO: Check auth Id + # Try to get ticket from DB try: user, userService, host, port, extra = models.TicketStore.get_for_tunnel( diff --git a/server/src/uds/auths/IP/authenticator.py b/server/src/uds/auths/IP/authenticator.py index aa13dcec6..81e070082 100644 --- a/server/src/uds/auths/IP/authenticator.py +++ b/server/src/uds/auths/IP/authenticator.py @@ -48,15 +48,16 @@ logger = logging.getLogger(__name__) class IPAuth(auths.Authenticator): - #acceptProxy = gui.CheckBoxField( - # label=_('Accept proxy'), - # order=3, - # tooltip=_( - # 'If checked, requests via proxy will get FORWARDED ip address' - # ' (take care with this bein checked, can take internal IP addresses from internet)' - # ), - # tab=gui.ADVANCED_TAB - #) + acceptProxy = gui.CheckBoxField( + label=_('Accept proxy'), + defvalue=gui.FALSE, + order=3, + tooltip=_( + 'If checked, requests via proxy will get FORWARDED ip address' + ' (take care with this bein checked, can take internal IP addresses from internet)' + ), + tab=gui.ADVANCED_TAB + ) typeName = _('IP Authenticator') typeType = 'IPAuth' @@ -71,8 +72,7 @@ class IPAuth(auths.Authenticator): blockUserOnLoginFailures = False def getIp(self) -> str: - # ip = getRequest().ip_proxy if self.acceptProxy.isTrue() else getRequest().ip # pylint: disable=maybe-no-member - ip = getRequest().ip # Proxy is identified by UDS + ip = getRequest().ip_proxy if self.acceptProxy.isTrue() else getRequest().ip # pylint: disable=maybe-no-member logger.debug('Client IP: %s', ip) return ip diff --git a/server/src/uds/auths/InternalDB/authenticator.py b/server/src/uds/auths/InternalDB/authenticator.py index cf8569ff7..303400e0b 100644 --- a/server/src/uds/auths/InternalDB/authenticator.py +++ b/server/src/uds/auths/InternalDB/authenticator.py @@ -55,7 +55,9 @@ logger = logging.getLogger(__name__) class InternalDBAuth(auths.Authenticator): typeName = _('Internal Database') typeType = 'InternalDBAuth' - typeDescription = _('Internal dabasase authenticator. Doesn\'t use external sources') + typeDescription = _( + 'Internal dabasase authenticator. Doesn\'t use external sources' + ) iconFile = 'auth.png' # If we need to enter the password for this user @@ -64,15 +66,40 @@ class InternalDBAuth(auths.Authenticator): # This is the only internal source isExternalSource = False - differentForEachHost = gui.CheckBoxField(label=_('Different user for each host'), order=1, tooltip=_('If checked, each host will have a different user name'), defvalue="false", rdonly=True, tab=gui.ADVANCED_TAB) - reverseDns = gui.CheckBoxField(label=_('Reverse DNS'), order=2, tooltip=_('If checked, the host will be reversed dns'), defvalue="false", rdonly=True, tab=gui.ADVANCED_TAB) - acceptProxy = gui.CheckBoxField(label=_('Accept proxy'), order=3, tooltip=_('If checked, requests via proxy will get FORWARDED ip address (take care with this bein checked, can take internal IP addresses from internet)'), tab=gui.ADVANCED_TAB) + differentForEachHost = gui.CheckBoxField( + label=_('Different user for each host'), + order=1, + tooltip=_('If checked, each host will have a different user name'), + defvalue="false", + rdonly=True, + tab=gui.ADVANCED_TAB, + ) + reverseDns = gui.CheckBoxField( + label=_('Reverse DNS'), + order=2, + tooltip=_('If checked, the host will be reversed dns'), + defvalue="false", + rdonly=True, + tab=gui.ADVANCED_TAB, + ) + acceptProxy = gui.CheckBoxField( + label=_('Accept proxy'), + order=3, + tooltip=_( + 'If checked, requests via proxy will get FORWARDED ip address (take care with this bein checked, can take internal IP addresses from internet)' + ), + tab=gui.ADVANCED_TAB, + ) def getIp(self) -> str: - ip = getRequest().ip_proxy if self.acceptProxy.isTrue() else getRequest().ip # pylint: disable=maybe-no-member + ip = ( + getRequest().ip_proxy if self.acceptProxy.isTrue() else getRequest().ip + ) # pylint: disable=maybe-no-member if self.reverseDns.isTrue(): try: - return str(dns.resolver.query(dns.reversename.from_address(ip), 'PTR')[0]) + return str( + dns.resolver.query(dns.reversename.from_address(ip), 'PTR')[0] + ) except Exception: pass return ip @@ -100,7 +127,9 @@ class InternalDBAuth(auths.Authenticator): return username - def authenticate(self, username: str, credentials: str, groupsManager: 'auths.GroupsManager') -> bool: + def authenticate( + self, username: str, credentials: str, groupsManager: 'auths.GroupsManager' + ) -> bool: logger.debug('Username: %s, Password: %s', username, credentials) dbAuth = self.dbAuthenticator() try: diff --git a/server/src/uds/core/util/middleware/request.py b/server/src/uds/core/util/middleware/request.py index b44e7f7fd..16b1bbab0 100644 --- a/server/src/uds/core/util/middleware/request.py +++ b/server/src/uds/core/util/middleware/request.py @@ -40,7 +40,12 @@ from django.utils import timezone from uds.core.util import os_detector as OsDetector from uds.core.util.config import GlobalConfig from uds.core.auths.auth import EXPIRY_KEY, ROOT_ID, USER_KEY, getRootUser, webLogout -from uds.core.util.request import setRequest, delCurrentRequest, cleanOldRequests, ExtendedHttpRequest +from uds.core.util.request import ( + setRequest, + delCurrentRequest, + cleanOldRequests, + ExtendedHttpRequest, +) from uds.models import User logger = logging.getLogger(__name__) @@ -124,20 +129,38 @@ class GlobalRequestMiddleware: except Exception: request.ip = '' # No remote addr?? ... - try: - proxies = request.META.get('HTTP_X_FORWARDED_FOR', '').split(",") - request.ip_proxy = proxies[0] + # X-FORWARDED-FOR: CLIENT, FAR_PROXY, PROXY, NEAR_PROXY, NGINX + # We will accept only 2 proxies, the last ones + proxies = list( + reversed( + [ + i.split('%')[0] + for i in request.META.get('HTTP_X_FORWARDED_FOR', '').split(",") + ] + ) + ) + proxies = list(reversed(['172.27.0.8', '172.27.0.128', '172.27.0.1'])) + # proxies = list(reversed(['172.27.0.12', '172.27.0.1'])) + # proxies = list(reversed(['172.27.0.12'])) + request.ip = '' - if not request.ip or behind_proxy: - # Request.IP will be None in case of nginx & gunicorn - # Some load balancers may include "domains with a %" on x-forwarded for, - request.ip = request.ip_proxy.split('%')[0] # Stores the ip + logger.debug('Detected proxies: %s', proxies) - # will raise "list out of range", leaving ip_proxy = proxy in case of no other proxy apart of nginx - # Also, behind_proxy must be activated to work correctly (security concerns) - request.ip_proxy = proxies[1].strip() if behind_proxy else request.ip - except Exception: - request.ip_proxy = request.ip + # Request.IP will be None in case of nginx & gunicorn using sockets, as we do + if not request.ip: + request.ip = proxies[0] # Stores the ip + proxies = proxies[1:] # Remove from proxies list + + logger.debug('Proxies: %s', proxies) + + request.ip_proxy = proxies[0] if proxies and proxies[0] else request.ip + + if behind_proxy: + request.ip = request.ip_proxy + request.ip_proxy = proxies[1] if len(proxies) > 1 else request.ip + logger.debug('Behind a proxy is active') + + logger.debug('ip: %s, ip_proxy: %s', request.ip, request.ip_proxy) @staticmethod def getUser(request: ExtendedHttpRequest) -> None: diff --git a/server/src/uds/dispatchers/guacamole/urls.py b/server/src/uds/dispatchers/guacamole/urls.py index 900740253..5ff867709 100644 --- a/server/src/uds/dispatchers/guacamole/urls.py +++ b/server/src/uds/dispatchers/guacamole/urls.py @@ -38,4 +38,6 @@ urlpatterns = [ url(r'^guacamole/(?P.+)$', guacamole, name='dispatcher.guacamole'), # New path url(r'^uds/guacamole/(?P.+)$', guacamole, name='dispatcher.guacamole'), + # Authenticated path + url(r'^uds/guacamole/auth/(?P.+)/(?P.+)$', guacamole, name='dispatcher.guacamole'), ] diff --git a/server/src/uds/dispatchers/guacamole/views.py b/server/src/uds/dispatchers/guacamole/views.py index 01a165c1c..ab67242a5 100644 --- a/server/src/uds/dispatchers/guacamole/views.py +++ b/server/src/uds/dispatchers/guacamole/views.py @@ -83,3 +83,9 @@ def guacamole(request: HttpRequest, tunnelId: str) -> HttpResponse: return HttpResponse(ERROR, content_type=CONTENT_TYPE) return HttpResponse(response, content_type=CONTENT_TYPE) + +@auth.trustedSourceRequired +def guacamole_authenticated(request: HttpRequest, authId: str, tunnelId: str) -> HttpResponse: + authId = authId[:48] + # TODO: Check the authId validity + return guacamole(request, tunnelId) \ No newline at end of file diff --git a/server/src/uds/models/transport.py b/server/src/uds/models/transport.py index b76f618a2..f21053538 100644 --- a/server/src/uds/models/transport.py +++ b/server/src/uds/models/transport.py @@ -131,7 +131,6 @@ class Transport(ManagedObjectModel, TaggingMixin): return self.networks.filter(net_start__lte=ip, net_end__gte=ip).count() == 0 def validForOs(self, os: str) -> bool: - logger.debug('Checkin if os "%s" is in "%s"', os, self.allowed_oss) if not self.allowed_oss or os in self.allowed_oss.split(','): return True return False diff --git a/server/src/uds/services/Proxmox/client/__init__.py b/server/src/uds/services/Proxmox/client/__init__.py index 24a4e974a..c24948fd5 100644 --- a/server/src/uds/services/Proxmox/client/__init__.py +++ b/server/src/uds/services/Proxmox/client/__init__.py @@ -198,7 +198,11 @@ class ProxmoxClient: ) logger.debug( - 'DELETE result to %s: %s -- %s -- %s', path, result.status_code, result.content, result.headers + 'DELETE result to %s: %s -- %s -- %s', + path, + result.status_code, + result.content, + result.headers, ) return ProxmoxClient.checkError(result) @@ -390,7 +394,9 @@ class ProxmoxClient: self, vmId: int, node: typing.Optional[str] = None, purge: bool = True ) -> types.UPID: node = node or self.getVmInfo(vmId).node - return types.UPID.fromDict(self._delete('nodes/{}/qemu/{}?purge=1'.format(node, vmId))) + return types.UPID.fromDict( + self._delete('nodes/{}/qemu/{}?purge=1'.format(node, vmId)) + ) @ensureConected def getTask(self, node: str, upid: str) -> types.TaskStatus: @@ -426,13 +432,19 @@ class ProxmoxClient: return sorted(result, key=lambda x: '{}{}'.format(x.node, x.name)) @ensureConected - @allowCache('vmip', CACHE_INFO_DURATION, cachingArgs=[1, 2], cachingKWArgs=['vmId', 'poolId'], cachingKeyFnc=cachingKeyHelper) + @allowCache( + 'vmip', + CACHE_INFO_DURATION, + cachingArgs=[1, 2], + cachingKWArgs=['vmId', 'poolId'], + cachingKeyFnc=cachingKeyHelper, + ) def getVMPoolInfo(self, vmId: int, poolId: str, **kwargs) -> types.VMInfo: # try to locate machine in pool node = None if poolId: try: - for i in self._get(f'pools/{poolId}')['data']['members']: + for i in self._get(f'pools/{poolId}')['data']['members']: try: if i['vmid'] == vmId: node = i['node'] @@ -445,7 +457,13 @@ class ProxmoxClient: return self.getVmInfo(vmId, node) @ensureConected - @allowCache('vmin', CACHE_INFO_DURATION, cachingArgs=[1, 2], cachingKWArgs=['vmId', 'node'], cachingKeyFnc=cachingKeyHelper) + @allowCache( + 'vmin', + CACHE_INFO_DURATION, + cachingArgs=[1, 2], + cachingKWArgs=['vmId', 'node'], + cachingKeyFnc=cachingKeyHelper, + ) def getVmInfo( self, vmId: int, node: typing.Optional[str] = None, **kwargs ) -> types.VMInfo: @@ -557,7 +575,7 @@ class ProxmoxClient: self, node: typing.Union[None, str, typing.Iterable[str]] = None, content: typing.Optional[str] = None, - **kwargs + **kwargs, ) -> typing.List[types.StorageInfo]: """We use a list for storage instead of an iterator, so we can cache it...""" nodeList: typing.Iterable[str]