Merge pull request #8650 from freqtrade/feat/secure_keys

Better secure the user's exchange keys during runtime

freqtrade/constants.py

@@ -690,4 +690,6 @@ BidAsk = Literal['bid', 'ask']
 OBLiteral = Literal['asks', 'bids']
 
 Config = Dict[str, Any]
+# Exchange part of the configuration.
+ExchangeConfig = Dict[str, Any]
 IntOrInf = float

freqtrade/exchange/__init__.py

@@ -1,6 +1,6 @@
 # flake8: noqa: F401
 # isort: off
-from freqtrade.exchange.common import remove_credentials, MAP_EXCHANGE_CHILDCLASS
+from freqtrade.exchange.common import remove_exchange_credentials, MAP_EXCHANGE_CHILDCLASS
 from freqtrade.exchange.exchange import Exchange
 # isort: on
 from freqtrade.exchange.binance import Binance

freqtrade/exchange/common.py

@@ -4,7 +4,7 @@ import time
 from functools import wraps
 from typing import Any, Callable, Optional, TypeVar, cast, overload
 
-from freqtrade.constants import Config
+from freqtrade.constants import ExchangeConfig
 from freqtrade.exceptions import DDosProtection, RetryableOrderError, TemporaryError
 from freqtrade.mixins import LoggingMixin
 
@@ -89,18 +89,18 @@ EXCHANGE_HAS_OPTIONAL = [
 ]
 
 
-def remove_credentials(config: Config) -> None:
+def remove_exchange_credentials(exchange_config: ExchangeConfig, dry_run: bool) -> None:
     """
     Removes exchange keys from the configuration and specifies dry-run
     Used for backtesting / hyperopt / edge and utils.
     Modifies the input dict!
     """
-    if config.get('dry_run', False):
+    if dry_run:
-        config['exchange']['key'] = ''
+        exchange_config['key'] = ''
-        config['exchange']['apiKey'] = ''
+        exchange_config['apiKey'] = ''
-        config['exchange']['secret'] = ''
+        exchange_config['secret'] = ''
-        config['exchange']['password'] = ''
+        exchange_config['password'] = ''
-        config['exchange']['uid'] = ''
+        exchange_config['uid'] = ''
 
 
 def calculate_backoff(retrycount, max_retries):

freqtrade/exchange/exchange.py

@@ -20,16 +20,16 @@ from dateutil import parser
 from pandas import DataFrame, concat
 
 from freqtrade.constants import (DEFAULT_AMOUNT_RESERVE_PERCENT, NON_OPEN_EXCHANGE_STATES, BidAsk,
-                                 BuySell, Config, EntryExit, ListPairsWithTimeframes, MakerTaker,
+                                 BuySell, Config, EntryExit, ExchangeConfig,
-                                 OBLiteral, PairWithTimeframe)
+                                 ListPairsWithTimeframes, MakerTaker, OBLiteral, PairWithTimeframe)
 from freqtrade.data.converter import clean_ohlcv_dataframe, ohlcv_to_dataframe, trades_dict_to_list
 from freqtrade.enums import OPTIMIZE_MODES, CandleType, MarginMode, TradingMode
 from freqtrade.enums.pricetype import PriceType
 from freqtrade.exceptions import (DDosProtection, ExchangeError, InsufficientFundsError,
                                   InvalidOrderException, OperationalException, PricingError,
                                   RetryableOrderError, TemporaryError)
-from freqtrade.exchange.common import (API_FETCH_ORDER_RETRY_COUNT, remove_credentials, retrier,
+from freqtrade.exchange.common import (API_FETCH_ORDER_RETRY_COUNT, remove_exchange_credentials,
-                                       retrier_async)
+                                       retrier, retrier_async)
 from freqtrade.exchange.exchange_utils import (ROUND, ROUND_DOWN, ROUND_UP, CcxtModuleType,
                                                amount_to_contract_precision, amount_to_contracts,
                                                amount_to_precision, contracts_to_amount,
@@ -92,8 +92,8 @@ class Exchange:
         # TradingMode.SPOT always supported and not required in this list
     ]
 
-    def __init__(self, config: Config, *, validate: bool = True,
+    def __init__(self, config: Config, *, exchange_config: Optional[ExchangeConfig] = None,
-                 load_leverage_tiers: bool = False) -> None:
+                 validate: bool = True, load_leverage_tiers: bool = False) -> None:
         """
         Initializes this module with the given config,
         it does basic validation whether the specified exchange and pairs are valid.
@@ -131,13 +131,13 @@ class Exchange:
 
         # Holds all open sell orders for dry_run
         self._dry_run_open_orders: Dict[str, Any] = {}
-        remove_credentials(config)
 
         if config['dry_run']:
             logger.info('Instance is running with dry_run enabled')
         logger.info(f"Using CCXT {ccxt.__version__}")
-        exchange_config = config['exchange']
+        exchange_conf: Dict[str, Any] = exchange_config if exchange_config else config['exchange']
-        self.log_responses = exchange_config.get('log_responses', False)
+        remove_exchange_credentials(exchange_conf, config.get('dry_run', False))
+        self.log_responses = exchange_conf.get('log_responses', False)
 
         # Leverage properties
         self.trading_mode: TradingMode = config.get('trading_mode', TradingMode.SPOT)
@@ -152,8 +152,8 @@ class Exchange:
         self._ft_has = deep_merge_dicts(self._ft_has, deepcopy(self._ft_has_default))
         if self.trading_mode == TradingMode.FUTURES:
             self._ft_has = deep_merge_dicts(self._ft_has_futures, self._ft_has)
-        if exchange_config.get('_ft_has_params'):
+        if exchange_conf.get('_ft_has_params'):
-            self._ft_has = deep_merge_dicts(exchange_config.get('_ft_has_params'),
+            self._ft_has = deep_merge_dicts(exchange_conf.get('_ft_has_params'),
                                             self._ft_has)
             logger.info("Overriding exchange._ft_has with config params, result: %s", self._ft_has)
 
@@ -165,18 +165,18 @@ class Exchange:
 
         # Initialize ccxt objects
         ccxt_config = self._ccxt_config
-        ccxt_config = deep_merge_dicts(exchange_config.get('ccxt_config', {}), ccxt_config)
+        ccxt_config = deep_merge_dicts(exchange_conf.get('ccxt_config', {}), ccxt_config)
-        ccxt_config = deep_merge_dicts(exchange_config.get('ccxt_sync_config', {}), ccxt_config)
+        ccxt_config = deep_merge_dicts(exchange_conf.get('ccxt_sync_config', {}), ccxt_config)
 
-        self._api = self._init_ccxt(exchange_config, ccxt_kwargs=ccxt_config)
+        self._api = self._init_ccxt(exchange_conf, ccxt_kwargs=ccxt_config)
 
         ccxt_async_config = self._ccxt_config
-        ccxt_async_config = deep_merge_dicts(exchange_config.get('ccxt_config', {}),
+        ccxt_async_config = deep_merge_dicts(exchange_conf.get('ccxt_config', {}),
                                              ccxt_async_config)
-        ccxt_async_config = deep_merge_dicts(exchange_config.get('ccxt_async_config', {}),
+        ccxt_async_config = deep_merge_dicts(exchange_conf.get('ccxt_async_config', {}),
                                              ccxt_async_config)
         self._api_async = self._init_ccxt(
-            exchange_config, ccxt_async, ccxt_kwargs=ccxt_async_config)
+            exchange_conf, ccxt_async, ccxt_kwargs=ccxt_async_config)
 
         logger.info(f'Using Exchange "{self.name}"')
         self.required_candle_call_count = 1
@@ -189,7 +189,7 @@ class Exchange:
                 self._startup_candle_count, config.get('timeframe', ''))
 
         # Converts the interval provided in minutes in config to seconds
-        self.markets_refresh_interval: int = exchange_config.get(
+        self.markets_refresh_interval: int = exchange_conf.get(
             "markets_refresh_interval", 60) * 60
 
         if self.trading_mode != TradingMode.SPOT and load_leverage_tiers:

freqtrade/freqtradebot.py

@@ -13,7 +13,7 @@ from schedule import Scheduler
 
 from freqtrade import constants
 from freqtrade.configuration import validate_config_consistency
-from freqtrade.constants import BuySell, Config, LongShort
+from freqtrade.constants import BuySell, Config, ExchangeConfig, LongShort
 from freqtrade.data.converter import order_book_to_dataframe
 from freqtrade.data.dataprovider import DataProvider
 from freqtrade.edge import Edge
@@ -23,6 +23,7 @@ from freqtrade.exceptions import (DependencyException, ExchangeError, Insufficie
                                   InvalidOrderException, PricingError)
 from freqtrade.exchange import (ROUND_DOWN, ROUND_UP, timeframe_to_minutes, timeframe_to_next_date,
                                 timeframe_to_seconds)
+from freqtrade.exchange.common import remove_exchange_credentials
 from freqtrade.misc import safe_value_fallback, safe_value_fallback2
 from freqtrade.mixins import LoggingMixin
 from freqtrade.persistence import Order, PairLocks, Trade, init_db
@@ -63,6 +64,9 @@ class FreqtradeBot(LoggingMixin):
 
         # Init objects
         self.config = config
+        exchange_config: ExchangeConfig = deepcopy(config['exchange'])
+        # Remove credentials from original exchange config to avoid accidental credentail exposure
+        remove_exchange_credentials(config['exchange'], True)
 
         self.strategy: IStrategy = StrategyResolver.load_strategy(self.config)
 
@@ -70,7 +74,7 @@ class FreqtradeBot(LoggingMixin):
         validate_config_consistency(config)
 
         self.exchange = ExchangeResolver.load_exchange(
-            self.config, load_leverage_tiers=True)
+            self.config, exchange_config=exchange_config, load_leverage_tiers=True)
 
         init_db(self.config['db_url'])
 

freqtrade/resolvers/exchange_resolver.py

@@ -2,9 +2,10 @@
 This module loads custom exchanges
 """
 import logging
+from typing import Optional
 
 import freqtrade.exchange as exchanges
-from freqtrade.constants import Config
+from freqtrade.constants import Config, ExchangeConfig
 from freqtrade.exchange import MAP_EXCHANGE_CHILDCLASS, Exchange
 from freqtrade.resolvers import IResolver
 
@@ -19,8 +20,8 @@ class ExchangeResolver(IResolver):
     object_type = Exchange
 
     @staticmethod
-    def load_exchange(config: Config, validate: bool = True,
+    def load_exchange(config: Config, *, exchange_config: Optional[ExchangeConfig] = None,
-                      load_leverage_tiers: bool = False) -> Exchange:
+                      validate: bool = True, load_leverage_tiers: bool = False) -> Exchange:
         """
         Load the custom class from config parameter
         :param exchange_name: name of the Exchange to load
@@ -37,13 +38,14 @@ class ExchangeResolver(IResolver):
                 kwargs={
                     'config': config,
                     'validate': validate,
+                    'exchange_config': exchange_config,
                     'load_leverage_tiers': load_leverage_tiers}
             )
         except ImportError:
             logger.info(
                 f"No {exchange_name} specific subclass found. Using the generic class instead.")
         if not exchange:
-            exchange = Exchange(config, validate=validate)
+            exchange = Exchange(config, validate=validate, exchange_config=exchange_config,)
         return exchange
 
     @staticmethod

freqtrade/rpc/api_server/api_backtest.py

@@ -11,6 +11,7 @@ from freqtrade.configuration.config_validation import validate_config_consistenc
 from freqtrade.data.btanalysis import get_backtest_resultlist, load_and_merge_backtest_result
 from freqtrade.enums import BacktestState
 from freqtrade.exceptions import DependencyException, OperationalException
+from freqtrade.exchange.common import remove_exchange_credentials
 from freqtrade.misc import deep_merge_dicts
 from freqtrade.rpc.api_server.api_schemas import (BacktestHistoryEntry, BacktestRequest,
                                                   BacktestResponse)
@@ -38,6 +39,7 @@ async def api_start_backtest(  # noqa: C901
         raise HTTPException(status_code=500, detail="base64 encoded strategies are not allowed.")
 
     btconfig = deepcopy(config)
+    remove_exchange_credentials(btconfig['exchange'], True)
     settings = dict(bt_settings)
     if settings.get('freqai', None) is not None:
         settings['freqai'] = dict(settings['freqai'])

tests/exchange/test_exchange.py

@@ -20,7 +20,7 @@ from freqtrade.exchange import (Binance, Bittrex, Exchange, Kraken, amount_to_pr
                                 timeframe_to_minutes, timeframe_to_msecs, timeframe_to_next_date,
                                 timeframe_to_prev_date, timeframe_to_seconds)
 from freqtrade.exchange.common import (API_FETCH_ORDER_RETRY_COUNT, API_RETRY_COUNT,
-                                       calculate_backoff, remove_credentials)
+                                       calculate_backoff, remove_exchange_credentials)
 from freqtrade.exchange.exchange import amount_to_contract_precision
 from freqtrade.resolvers.exchange_resolver import ExchangeResolver
 from tests.conftest import (EXMS, generate_test_data_raw, get_mock_coro, get_patched_exchange,
@@ -137,16 +137,14 @@ def test_init(default_conf, mocker, caplog):
     assert log_has('Instance is running with dry_run enabled', caplog)
 
 
-def test_remove_credentials(default_conf, caplog) -> None:
+def test_remove_exchange_credentials(default_conf) -> None:
     conf = deepcopy(default_conf)
-    conf['dry_run'] = False
+    remove_exchange_credentials(conf['exchange'], False)
-    remove_credentials(conf)
 
     assert conf['exchange']['key'] != ''
     assert conf['exchange']['secret'] != ''
 
-    conf['dry_run'] = True
+    remove_exchange_credentials(conf['exchange'], True)
-    remove_credentials(conf)
     assert conf['exchange']['key'] == ''
     assert conf['exchange']['secret'] == ''
     assert conf['exchange']['password'] == ''

tests/test_freqtradebot.py

@@ -121,7 +121,7 @@ def test_order_dict(default_conf_usdt, mocker, runmode, caplog) -> None:
 
     freqtrade = FreqtradeBot(conf)
     if runmode == RunMode.LIVE:
-        assert not log_has_re(".*stoploss_on_exchange .* dry-run", caplog)
+        assert not log_has_re(r".*stoploss_on_exchange .* dry-run", caplog)
     assert freqtrade.strategy.order_types['stoploss_on_exchange']
 
     caplog.clear()
@@ -136,7 +136,7 @@ def test_order_dict(default_conf_usdt, mocker, runmode, caplog) -> None:
     }
     freqtrade = FreqtradeBot(conf)
     assert not freqtrade.strategy.order_types['stoploss_on_exchange']
-    assert not log_has_re(".*stoploss_on_exchange .* dry-run", caplog)
+    assert not log_has_re(r".*stoploss_on_exchange .* dry-run", caplog)
 
 
 def test_get_trade_stake_amount(default_conf_usdt, mocker) -> None:
@@ -149,6 +149,34 @@ def test_get_trade_stake_amount(default_conf_usdt, mocker) -> None:
     assert result == default_conf_usdt['stake_amount']
 
 
+@pytest.mark.parametrize('runmode', [
+    RunMode.DRY_RUN,
+    RunMode.LIVE
+])
+def test_load_strategy_no_keys(default_conf_usdt, mocker, runmode, caplog) -> None:
+    patch_RPCManager(mocker)
+    patch_exchange(mocker)
+    conf = deepcopy(default_conf_usdt)
+    conf['runmode'] = runmode
+    erm = mocker.patch('freqtrade.freqtradebot.ExchangeResolver.load_exchange')
+
+    freqtrade = FreqtradeBot(conf)
+    strategy_config = freqtrade.strategy.config
+    assert id(strategy_config['exchange']) == id(conf['exchange'])
+    # Keys have been removed and are not passed to the exchange
+    assert strategy_config['exchange']['key'] == ''
+    assert strategy_config['exchange']['secret'] == ''
+
+    assert erm.call_count == 1
+    ex_conf = erm.call_args_list[0][1]['exchange_config']
+    assert id(ex_conf) != id(conf['exchange'])
+    # Keys are still present
+    assert ex_conf['key'] != ''
+    assert ex_conf['key'] == default_conf_usdt['exchange']['key']
+    assert ex_conf['secret'] != ''
+    assert ex_conf['secret'] == default_conf_usdt['exchange']['secret']
+
+
 @pytest.mark.parametrize("amend_last,wallet,max_open,lsamr,expected", [
                         (False, 120, 2, 0.5, [60, None]),
                         (True, 120, 2, 0.5, [60, 58.8]),