Merge pull request #1021 from flit/bugfix/server_security

Default probe server to local only, plus associated cleanup.

Author: flit

docs/options.md

@@ -254,6 +254,15 @@ Whether to resume a halted target when disconnecting.
 Controls whether all 256 ADIv5 AP addresses will be probed.
 </td></tr>
 
+<tr><td>serve_local_only</td>
+<td>bool</td>
+<td>True</td>
+<td>
+When this option is True, the GDB server, probe server, semihosting telnet, and raw SWV server are only served
+on localhost, making them inaccessible across the network. Set to False to enable connecting to these ports
+from any machine on the network.
+</td></tr>
+
 <tr><td>smart_flash</td>
 <td>bool</td>
 <td>True</td>
@@ -372,15 +381,6 @@ Whether to use GDB syscalls for semihosting file access operations, or to have p
 operations. This is most useful if GDB is running on a remote system.
 </td></tr>
 
-<tr><td>serve_local_only</td>
-<td>bool</td>
-<td>True</td>
-<td>
-When this option is True, the GDB server and semihosting telnet ports are only served on localhost,
-making them inaccessible across the network. If False, you can connect to these ports from any
-machine that is on the same network.
-</td></tr>
-
 <tr><td>step_into_interrupt</td>
 <td>bool</td>
 <td>False</td>

pyocd/__main__.py

@@ -348,10 +348,13 @@ def build_parser(self):
             help="Send setting to DAPAccess layer.")
         serverParser.add_argument("-p", "--port", dest="port_number", type=int, default=None,
             help="Set the server's port number (default 5555).")
-        serverParser.add_argument("--local-only", dest="serve_local_only", default=False, action="store_true",
-            help="Allow remote TCP/IP connections (default is yes).")
+        serverParser.add_argument("--allow-remote", dest="serve_local_only", default=None, action="store_false",
+            help="Allow remote TCP/IP connections (default is no).")
+        serverParser.add_argument("--local-only", default=False, action="store_true",
+            help="Ignored and deprecated. Server is local only by default. Use --alow-remote to enable remote "
+                 "connections.")
         serverParser.add_argument("-u", "--uid", dest="unique_id",
-            help="Serve only the specified probe. Can be used multiple times.")
+            help="Serve the specified probe.")
         serverParser.set_defaults(verbose=0, quiet=0)
 
         self._parser = parser
@@ -882,7 +885,9 @@ def do_server(self):
         # probe, we don't set it in the session because we don't want the board, target, etc objects
         # to be created.
         session_options = convert_session_options(self._args.options)
-        session = Session(probe=None, options=session_options)
+        session = Session(probe=None,
+                serve_local_only=self._args.serve_local_only,
+                options=session_options)
 
         # The ultimate intent is to serve all available probes by default. For now we just serve
         # a single probe.

pyocd/core/options.py

@@ -91,6 +91,9 @@
         "Whether to run target on disconnect."),
     OptionInfo('scan_all_aps', bool, False,
         "Controls whether all 256 ADIv5 AP addresses will be probed. Default is False."),
+    OptionInfo('serve_local_only', bool, True,
+        "When this option is True, the GDB server, probe server, and semihosting telnet, and raw SWV "
+        "server are only served on localhost. Set to False to enable remote connections."),
     OptionInfo('smart_flash', bool, True,
         "If set to True, the flash loader will attempt to not program pages whose contents are not "
         "going to change by scanning target flash memory. A value of False will force all pages to "
@@ -127,9 +130,6 @@
         "semihosting will print to the console."),
     OptionInfo('semihost_use_syscalls', bool, False,
         "Whether to use GDB syscalls for semihosting file access operations."),
-    OptionInfo('serve_local_only', bool, True,
-        "When this option is True, the GDB server and semihosting telnet ports are only served on "
-        "localhost."),
     OptionInfo('step_into_interrupt', bool, False,
         "Enable interrupts when performing step operations."),
     OptionInfo('swv_clock', int, 1000000,

pyocd/gdbserver/gdbserver.py

@@ -167,8 +167,9 @@ def __init__(self, session, core=None):
         self.first_run_after_reset_or_flash = True
 
         self.abstract_socket = ListenerSocket(self.port, self.packet_size)
-        if self.serve_local_only:
-            self.abstract_socket.host = 'localhost'
+        if not self.serve_local_only:
+            # We really should be binding to explicit interfaces, not all available.
+            self.abstract_socket.host = ''
         self.abstract_socket.init()
         # Read back bound port in case auto-assigned (port 0)
         self.port = self.abstract_socket.port

pyocd/probe/tcp_probe_server.py

@@ -39,7 +39,7 @@ class DebugProbeServer(threading.Thread):
     can be terminated by calling the stop() method, which will also kill the server thread.
     """
 
-    def __init__(self, session, probe, port=None, serve_local_only=True):
+    def __init__(self, session, probe, port=None, serve_local_only=None):
         """! @brief Constructor.
         
         @param self The object.
@@ -52,8 +52,8 @@ def __init__(self, session, probe, port=None, serve_local_only=True):
             @ref pyocd.probe.shared_probe_proxy.SharedDebugProbeProxy "SharedDebugProbeProxy"
             then a new proxy is created to allow the probe to be shared by multiple connections.
         @param port The TCP port number. Defaults to the 'probeserver.port' option if not provided.
-        @param serve_local_only Boolean. Whether to restrict the server to be accessible only from
-            localhost.
+        @param serve_local_only Optional Boolean. Whether to restrict the server to be accessible only from
+            localhost. If not specified (set to None), then the 'serve_local_only' session option is used.
         """
         super(DebugProbeServer, self).__init__()
 
@@ -78,6 +78,10 @@ def __init__(self, session, probe, port=None, serve_local_only=True):
         else:
             self._port = port
 
+        # Default to the serve_local_only session option.
+        if serve_local_only is None:
+            serve_local_only = session.options.get('serve_local_only')
+        
         host = 'localhost' if serve_local_only else ''
         address = (host, self._port)
 

pyocd/trace/swv.py

@@ -159,6 +159,7 @@ def run(self):
 
         swv_raw_server = StreamServer(
                             self._session.options.get('swv_raw_port'),
+                            serve_local_only=self._session.options.get('serve_local_only'),
                             name="SWV raw",
                             is_read_only=True) \
                          if self._session.options.get('swv_raw_enable') else None

pyocd/utility/server.py

@@ -56,8 +56,9 @@ def __init__(self, port, serve_local_only=True, name=None, is_read_only=True, ex
         self._is_read_only = is_read_only
         self._abstract_socket = None
         self._abstract_socket = ListenerSocket(port, 4096)
-        if serve_local_only:
-            self._abstract_socket.host = 'localhost'
+        if not serve_local_only:
+            # We really should be binding to explicit interfaces, not all available.
+            self._abstract_socket.host = ''
         self._abstract_socket.init()
         self._port = self._abstract_socket.port
         self._buffer = bytearray()

pyocd/utility/sockets.py

@@ -24,7 +24,7 @@ def __init__(self, port, packet_size):
         self.listener = None
         self.conn = None
         self.port = port
-        self.host = ''
+        self.host = 'localhost'
 
     def init(self):
         if self.listener is None: