From e965e0284789e610c0a50d20a92a82ec5c135064 Mon Sep 17 00:00:00 2001 From: Strahinja Val Markovic Date: Fri, 25 Apr 2014 11:07:08 -0700 Subject: [PATCH] Client/server communication now uses HMAC for auth This is to prevent a convoluted exploit that can trigger remote code execution. --- python/ycm/client/base_request.py | 48 ++++++++++++++++++++------ python/ycm/server/hmac_plugin.py | 57 +++++++++++++++++++++++++++++++ python/ycm/server/ycmd.py | 5 +++ python/ycm/utils.py | 10 ++++++ python/ycm/youcompleteme.py | 12 ++++--- 5 files changed, 117 insertions(+), 15 deletions(-) create mode 100644 python/ycm/server/hmac_plugin.py diff --git a/python/ycm/client/base_request.py b/python/ycm/client/base_request.py index 7f609727..c9c417f4 100644 --- a/python/ycm/client/base_request.py +++ b/python/ycm/client/base_request.py @@ -24,6 +24,7 @@ from retries import retries from requests_futures.sessions import FuturesSession from ycm.unsafe_thread_pool_executor import UnsafeThreadPoolExecutor from ycm import vimsupport +from ycm import utils from ycm.utils import ToUtf8Json from ycm.server.responses import ServerError, UnknownExtraConf @@ -31,6 +32,7 @@ _HEADERS = {'content-type': 'application/json'} _EXECUTOR = UnsafeThreadPoolExecutor( max_workers = 30 ) # Setting this to None seems to screw up the Requests/urllib3 libs. _DEFAULT_TIMEOUT_SEC = 30 +_HMAC_HEADER = 'x-ycm-hmac' class BaseRequest( object ): def __init__( self ): @@ -88,24 +90,28 @@ class BaseRequest( object ): timeout = _DEFAULT_TIMEOUT_SEC ): def SendRequest( data, handler, method, timeout ): if method == 'POST': - return BaseRequest.session.post( _BuildUri( handler ), - data = ToUtf8Json( data ), - headers = _HEADERS, - timeout = timeout ) + sent_data = ToUtf8Json( data ) + return BaseRequest.session.post( + _BuildUri( handler ), + data = sent_data, + headers = BaseRequest._ExtraHeaders( sent_data ), + timeout = timeout ) if method == 'GET': - return BaseRequest.session.get( _BuildUri( handler ), - headers = _HEADERS, - timeout = timeout ) + return BaseRequest.session.get( + _BuildUri( handler ), + headers = BaseRequest._ExtraHeaders(), + timeout = timeout ) @retries( 5, delay = 0.5, backoff = 1.5 ) def DelayedSendRequest( data, handler, method ): if method == 'POST': + sent_data = ToUtf8Json( data ) return requests.post( _BuildUri( handler ), - data = ToUtf8Json( data ), - headers = _HEADERS ) + data = sent_data, + headers = BaseRequest._ExtraHeaders( sent_data ) ) if method == 'GET': return requests.get( _BuildUri( handler ), - headers = _HEADERS ) + headers = BaseRequest._ExtraHeaders() ) if not _CheckServerIsHealthyWithCache(): return _EXECUTOR.submit( DelayedSendRequest, data, handler, method ) @@ -113,8 +119,18 @@ class BaseRequest( object ): return SendRequest( data, handler, method, timeout ) + @staticmethod + def _ExtraHeaders( request_body = None ): + if not request_body: + request_body = '' + headers = dict( _HEADERS ) + headers[ _HMAC_HEADER ] = utils.CreateHexHmac( request_body, + BaseRequest.hmac_secret ) + return headers + session = FuturesSession( executor = _EXECUTOR ) server_location = 'http://localhost:6666' + hmac_secret = '' def BuildRequestData( start_column = None, @@ -141,6 +157,7 @@ def BuildRequestData( start_column = None, def JsonFromFuture( future ): response = future.result() + _ValidateResponseObject( response ) if response.status_code == requests.codes.server_error: _RaiseExceptionForData( response.json() ) @@ -153,6 +170,13 @@ def JsonFromFuture( future ): return None +def _ValidateResponseObject( response ): + if not utils.ContentHexHmacValid( response.content, + response.headers[ _HMAC_HEADER ], + BaseRequest.hmac_secret ): + raise RuntimeError( 'Received invalid HMAC for response!' ) + return True + def _BuildUri( handler ): return urlparse.urljoin( BaseRequest.server_location, handler ) @@ -163,7 +187,9 @@ def _CheckServerIsHealthyWithCache(): global SERVER_HEALTHY def _ServerIsHealthy(): - response = requests.get( _BuildUri( 'healthy' ) ) + response = requests.get( _BuildUri( 'healthy' ), + headers = BaseRequest._ExtraHeaders() ) + _ValidateResponseObject( response ) response.raise_for_status() return response.json() diff --git a/python/ycm/server/hmac_plugin.py b/python/ycm/server/hmac_plugin.py new file mode 100644 index 00000000..b433b2b0 --- /dev/null +++ b/python/ycm/server/hmac_plugin.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python +# +# Copyright (C) 2014 Google Inc. +# +# This file is part of YouCompleteMe. +# +# YouCompleteMe is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# YouCompleteMe is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with YouCompleteMe. If not, see . + +import logging +import httplib +from bottle import request, response, abort +from ycm import utils + +_HMAC_HEADER = 'x-ycm-hmac' + +# This class implements the Bottle plugin API: +# http://bottlepy.org/docs/dev/plugindev.html +# +# We want to ensure that every request coming in has a valid HMAC set in the +# x-ycm-hmac header and that every response coming out sets such a valid header. +# This is to prevent security issues with possible remote code execution. +class HmacPlugin( object ): + name = 'hmac' + api = 2 + + + def __init__( self, hmac_secret ): + self._hmac_secret = hmac_secret + self._logger = logging.getLogger( __name__ ) + + + def __call__( self, callback ): + def wrapper( *args, **kwargs ): + body = request.body.read() + if not utils.ContentHexHmacValid( body, + request.headers[ _HMAC_HEADER ], + self._hmac_secret ): + self._logger.info( 'Dropping request with bad HMAC.' ) + abort( httplib.UNAUTHORIZED, 'Unauthorized, received bad HMAC.') + return + body = callback( *args, **kwargs ) + response.headers[ _HMAC_HEADER ] = utils.CreateHexHmac( + body, self._hmac_secret ) + return body + return wrapper + diff --git a/python/ycm/server/ycmd.py b/python/ycm/server/ycmd.py index 34e66172..64b0f07f 100755 --- a/python/ycm/server/ycmd.py +++ b/python/ycm/server/ycmd.py @@ -27,10 +27,12 @@ import argparse import waitress import signal import os +import base64 from ycm import user_options_store from ycm import extra_conf_store from ycm import utils from ycm.server.watchdog_plugin import WatchdogPlugin +from ycm.server.hmac_plugin import HmacPlugin def YcmCoreSanityCheck(): if 'ycm_core' in sys.modules: @@ -103,6 +105,8 @@ def Main(): options = ( json.load( open( args.options_file, 'r' ) ) if args.options_file else user_options_store.DefaultOptions() ) + utils.RemoveIfExists( args.options_file ) + hmac_secret = base64.b64decode( options[ 'hmac_secret' ] ) user_options_store.SetAll( options ) # This ensures that ycm_core is not loaded before extra conf @@ -126,6 +130,7 @@ def Main(): handlers.UpdateUserOptions( options ) SetUpSignalHandler(args.stdout, args.stderr, args.keep_logfiles) handlers.app.install( WatchdogPlugin( args.idle_suicide_seconds ) ) + handlers.app.install( HmacPlugin( hmac_secret ) ) waitress.serve( handlers.app, host = args.host, port = args.port, diff --git a/python/ycm/utils.py b/python/ycm/utils.py index d9128716..095337c9 100644 --- a/python/ycm/utils.py +++ b/python/ycm/utils.py @@ -25,6 +25,8 @@ import functools import socket import stat import json +import hmac +import hashlib from distutils.spawn import find_executable import subprocess import collections @@ -212,3 +214,11 @@ def SafePopen( *args, **kwargs ): return subprocess.Popen( *args, **kwargs ) +def ContentHexHmacValid( content, hmac, hmac_secret ): + return hmac == CreateHexHmac( content, hmac_secret ) + + +def CreateHexHmac( content, hmac_secret ): + return hmac.new( hmac_secret, + msg = content, + digestmod = hashlib.sha256 ).hexdigest() diff --git a/python/ycm/youcompleteme.py b/python/ycm/youcompleteme.py index 448236eb..83849ce1 100644 --- a/python/ycm/youcompleteme.py +++ b/python/ycm/youcompleteme.py @@ -22,6 +22,7 @@ import vim import tempfile import json import signal +import base64 from subprocess import PIPE from ycm import vimsupport from ycm import utils @@ -58,6 +59,7 @@ os.environ['no_proxy'] = '127.0.0.1,localhost' # Ctrl-C in Vim. signal.signal( signal.SIGINT, signal.SIG_IGN ) +HMAC_SECRET_LENGTH = 16 NUM_YCMD_STDERR_LINES_ON_CRASH = 30 SERVER_CRASH_MESSAGE_STDERR_FILE = ( 'The ycmd server SHUT DOWN (restart with :YcmRestartServer). ' + @@ -80,16 +82,18 @@ class YouCompleteMe( object ): self._server_stderr = None self._server_popen = None self._filetypes_with_keywords_loaded = set() - self._temp_options_filename = None self._ycmd_keepalive = YcmdKeepalive() self._SetupServer() self._ycmd_keepalive.Start() def _SetupServer( self ): server_port = utils.GetUnusedLocalhostPort() + # The temp options file is deleted by ycmd during startup with tempfile.NamedTemporaryFile( delete = False ) as options_file: - self._temp_options_filename = options_file.name - json.dump( dict( self._user_options ), options_file ) + hmac_secret = os.urandom( HMAC_SECRET_LENGTH ) + options_dict = dict( self._user_options ) + options_dict[ 'hmac_secret' ] = base64.b64encode( hmac_secret ) + json.dump( options_dict, options_file ) options_file.flush() args = [ utils.PathToPythonInterpreter(), @@ -116,6 +120,7 @@ class YouCompleteMe( object ): self._server_popen = utils.SafePopen( args, stdout = PIPE, stderr = PIPE) BaseRequest.server_location = 'http://localhost:' + str( server_port ) + BaseRequest.hmac_secret = hmac_secret self._NotifyUserIfServerCrashed() @@ -148,7 +153,6 @@ class YouCompleteMe( object ): def _ServerCleanup( self ): if self._IsServerAlive(): self._server_popen.terminate() - utils.RemoveIfExists( self._temp_options_filename ) def RestartServer( self ):