Fix a batch of Tor-related issues

docs/Tor.md

@@ -4,25 +4,25 @@ Current supported version of Tor is 0.3.3.6 or higher.
 
 ### Installing Tor on your node
 
-#### Linux:
+#### Linux
 
 ```shell
 sudo apt install tor
 ```
 
-#### Mac OS X:
+#### Mac OS X
 
 ```shell
 brew install tor
 ```
 
-#### Windows:
+#### Windows
 
 [Download the "Expert Bundle"](https://www.torproject.org/download/tor/) from Tor's website and extract it to `C:\tor`.
 
 ### Configuring Tor
 
-#### Linux and Max OS X:
+#### Linux and Max OS X
 
 Eclair requires safe cookie authentication as well as SOCKS5 and control connections to be enabled.
 
@@ -37,7 +37,7 @@ ExitPolicy reject *:* # don't change this unless you really know what you are do
 
 Make sure eclair is allowed to read Tor's cookie file (typically `/var/run/tor/control.authcookie`).
 
-#### Windows:
+#### Windows
 
 On Windows, it is easier to use the password authentication mechanism.
 
@@ -60,19 +60,19 @@ ExitPolicy reject *:* # don't change this unless you really know what you are do
 
 ### Start Tor
 
-#### Linux:
+#### Linux
 
 ```shell
 sudo systemctl start tor
 ```
 
-#### Mac OS X:
+#### Mac OS X
 
 ```shell
 brew services start tor
 ```
 
-#### Windows:
+#### Windows
 
 Open a CMD with administrator access
 
@@ -84,6 +84,7 @@ tor --service install -options -f "c:\tor\Conf\torrc"
 ### Configure Tor hidden service
 
 To create a Tor hidden service endpoint simply set the `eclair.tor.enabled` parameter in `eclair.conf` to true.
+
 ```
 eclair.tor.enabled = true
 ```
@@ -94,16 +95,25 @@ eclair.tor.auth = safecookie
 # eclair.tor.password = ""      # Needed if you set auth to password
 ```
 
+:warning: The Tor control port is a trusted local interface and must not be exposed to other machines. Eclair sends its
+onion private key to the control port on every startup, so anything that can read that connection can impersonate your
+node's onion address. With `password` authentication, the Tor control protocol also gives eclair no way of verifying
+that it is really talking to Tor: the password is sent in cleartext to whatever process is listening on the control
+port. Eclair therefore refuses to use `password` authentication unless `eclair.tor.host` is a local address. If your Tor
+daemon runs on another host or in a separate container, use `safecookie`, which authenticates the Tor server.
+
 Eclair will automatically set up a hidden service endpoint and add its onion address to the `server.public-ips` list.
 You can see what onion address is assigned using `eclair-cli`:
 
 ```shell
 eclair-cli getinfo
 ```
-Eclair saves the Tor endpoint's private key in `~/.eclair/tor.dat`, so that it can recreate the endpoint address after 
-a restart. If you remove the private key Eclair will regenerate the endpoint address.   
-
+
+Eclair saves the Tor endpoint's private key in `~/.eclair/tor.dat`, so that it can recreate the endpoint address after
+a restart. If you remove the private key Eclair will regenerate the endpoint address.
+
 For increased privacy do not advertise your IP address in the `server.public-ips` list, and set your binding IP to `localhost`:
+
 ```
 eclair.server.binding-ip = "127.0.0.1"
 ```
@@ -118,11 +128,13 @@ You can always see your node's onion address using `getinfo` CLI command.
 
 ### Configure SOCKS5 proxy
 
-By default, all incoming connections will be established via Tor network, but all outgoing will be created via the 
+By default, all incoming connections will be established via Tor network, but all outgoing will be created via the
 clearnet. To route them through Tor you can use Tor's SOCKS5 proxy. Add this line in your `eclair.conf`:
+
 ```
 eclair.socks5.enabled = true
 ```
+
 You can use SOCKS5 proxy only for specific types of addresses. Use `eclair.socks5.use-for-ipv4`, `eclair.socks5.use-for-ipv6`
 or `eclair.socks5.use-for-tor` for fine-tuning.
 
@@ -132,14 +144,14 @@ To create a new Tor circuit for every connection, use `randomize-credentials` pa
 eclair.socks5.randomize-credentials = true
 ```
 
-:warning: Tor hidden service and SOCKS5 are independent options. You can use just one of them, but if you want to get most privacy 
+:warning: Tor hidden service and SOCKS5 are independent options. You can use just one of them, but if you want to get most privacy
 features from using Tor, use both.
 
 Note, that bitcoind should be configured to use Tor as well (https://en.bitcoin.it/wiki/Setting_up_a_Tor_hidden_service).
 
 ### Blockchain watchdogs
 
-Eclair version 0.5.0 introduced blockchain watchdogs, that fetch bitcoin headers from various sources in 
+Eclair version 0.5.0 introduced blockchain watchdogs, that fetch bitcoin headers from various sources in
 order to detect whether the node is being eclipsed. Eclair supports four sources at the moment:
 
 * blockchainheaders.net
@@ -166,6 +178,6 @@ eclair.blockchain-watchdog.sources = [
 
 Also, you can disable Tor for all watchdog sources altogether using:
 
-```s
+```
 eclair.socks5.use-for-watchdogs = false
 ```

docs/release-notes/eclair-vnext.md

@@ -8,6 +8,8 @@
 
 ### Configuration changes
 
+#### Gossip queries
+
 A `query_channel_range` message is a few dozen bytes to send, but answering one requires scanning our whole routing
 table and sending back megabytes of data. We now limit how many of them we accept from a given peer, which can be
 configured with:
@@ -29,6 +31,17 @@ eclair.router.sync.max-queries-per-sync = 2000
 At the default `channel-query-chunk-size` this covers 200 000 channels, which is several times the current size of the
 network.
 
+#### Tor configuration
+
+`eclair.tor.auth` now defaults to `safecookie` instead of `password`.
+
+Password authentication sends our tor control password in cleartext to whatever process is listening on the control
+port, without any way of verifying that it is really tor. It is now rejected when `eclair.tor.host` isn't a local
+address: if you run tor on another host or in a separate container, switch to `eclair.tor.auth = safecookie`, which
+authenticates the tor server, or move the control port to the host running eclair. Note that the onion private key is
+sent to the control port on every startup, so a remote control port exposes it to the network regardless of the
+authentication method used.
+
 ### API changes
 
 <insert changes>

eclair-core/src/main/resources/reference.conf

@@ -547,9 +547,9 @@ eclair {
 
   tor {
     enabled = false
-    auth = "password" // safecookie, password
-    password = "foobar" // used when auth=password
-    host = "127.0.0.1"
+    auth = "safecookie" // safecookie, password
+    password = "foobar" // used when auth=password, which requires host to be a local address
+    host = "127.0.0.1" // must be a local address unless auth=safecookie
     port = 9051
     private-key-file = "tor.dat"
     targets = [] // a list of address:port, for advanced use (e.g. to send traffic to front servers). See the tor man page for syntax details.

eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala

@@ -211,17 +211,14 @@ object NodeParams extends Logging {
   }
 
   private def writeSeedToFile(path: File, seed: ByteVector): Unit = {
-    Files.write(path.toPath, seed.toArray)
-    // Seed files derive the node identity key and all channel keys: they must never be readable by other local users.
-    setOwnerPermissions(path.toPath, "rw-------")
+    writeSecret(path.toPath, seed.toArray)
     logger.info(s"create new seed file: ${path.getCanonicalPath}")
   }
 
   private def migrateSeedFile(source: File, destination: File): Unit = {
     if (source.exists() && !destination.exists()) {
-      Files.copy(source.toPath, destination.toPath)
-      // The copied file inherits default umask permissions, so we restrict it to its owner like a freshly created seed.
-      setOwnerPermissions(destination.toPath, "rw-------")
+      val seed = readSeedFromFile(source)
+      writeSecret(destination.toPath, seed.toArray)
       logger.info(s"migrate seed file: ${source.getCanonicalPath} → ${destination.getCanonicalPath}")
     }
   }

eclair-core/src/main/scala/fr/acinq/eclair/package.scala

@@ -24,24 +24,54 @@ import fr.acinq.eclair.payment.relay.Relayer.RelayFees
 import scodec.Attempt
 import scodec.bits.{BitVector, ByteVector}
 
-import java.nio.file.Path
 import java.nio.file.attribute.PosixFilePermissions
+import java.nio.file.{Files, Path}
 
 package object eclair {
 
   val randomGen = new StrongRandom()
 
+  /**
+   * Create an empty file that only the current user can read, replacing it if it already exists. The permissions must
+   * be set when the file is created: if we set them after writing to the file, there is a window during which other
+   * users can read its content.
+   */
+  def createSecretFile(path: Path): Unit = {
+    // File attributes are ignored when the file already exists, so we start from a clean slate.
+    Files.deleteIfExists(path)
+    try {
+      // The permissions are passed to the underlying open(2) call, so the file never exists with broader permissions.
+      // This also fails if someone else concurrently created that file, instead of storing our secret in their file.
+      Files.createFile(path, PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")))
+    } catch {
+      case _: UnsupportedOperationException => Files.createFile(path) // non-POSIX file system (e.g. Windows)
+    }
+  }
+
+  /** Write a secret to a file that only the current user can read. */
+  def writeSecret(path: Path, secret: String): Unit = {
+    createSecretFile(path)
+    Files.writeString(path, secret) // the file already exists, so this doesn't change its permissions
+  }
+
+  /** Write a secret to a file that only the current user can read. */
+  def writeSecret(path: Path, secret: Array[Byte]): Unit = {
+    createSecretFile(path)
+    Files.write(path, secret) // the file already exists, so this doesn't change its permissions
+  }
+
   /**
    * Restrict access to a file or directory to its owner, e.g. "rw-------" for a secret file or "rwx------" for a
    * directory containing secrets. This is a no-op on file systems that don't support POSIX permissions (e.g. Windows),
    * where access control is handled differently.
    */
-  def setOwnerPermissions(path: Path, permissions: String): Unit =
+  def setOwnerPermissions(path: Path, permissions: String): Unit = {
     try {
-      java.nio.file.Files.setPosixFilePermissions(path, PosixFilePermissions.fromString(permissions))
+      Files.setPosixFilePermissions(path, PosixFilePermissions.fromString(permissions))
     } catch {
       case _: UnsupportedOperationException => () // non-POSIX file system (e.g. Windows)
     }
+  }
 
   def randomBytes(length: Int): ByteVector = {
     val buffer = new Array[Byte](length)

eclair-core/src/main/scala/fr/acinq/eclair/tor/TorProtocolHandler.scala

@@ -21,10 +21,11 @@ import akka.io.Tcp.Connected
 import akka.util.ByteString
 import fr.acinq.eclair.tor.TorProtocolHandler.Authentication
 import fr.acinq.eclair.wire.protocol.{NodeAddress, Tor3}
+import fr.acinq.eclair.writeSecret
 import scodec.bits.Bases.Alphabets
 import scodec.bits.ByteVector
 
-import java.nio.file.attribute.PosixFilePermissions
+import java.net.InetSocketAddress
 import java.nio.file.{Files, Path, Paths}
 import java.util
 import javax.crypto.Mac
@@ -35,22 +36,21 @@ import scala.util.Try
 case class TorException(private val msg: String) extends RuntimeException(s"Tor error: $msg")
 
 /**
-  * Created by rorp
-  *
-  * Specification: https://gitweb.torproject.org/torspec.git/tree/control-spec.txt
-  *
-  * @param authentication      Tor controller auth mechanism (password or safecookie)
-  * @param privateKeyPath      path to a file that contains a Tor private key
-  * @param virtualPort         port for the public hidden service (typically 9735)
-  * @param targets             address of our protected server (format [host:]port), 127.0.0.1:[[virtualPort]] if empty
-  * @param onionAdded          a Promise to track creation of the endpoint
-  */
+ * Created by rorp
+ *
+ * Specification: https://gitweb.torproject.org/torspec.git/tree/control-spec.txt
+ *
+ * @param authentication Tor controller auth mechanism (password or safecookie)
+ * @param privateKeyPath path to a file that contains a Tor private key
+ * @param virtualPort    port for the public hidden service (typically 9735)
+ * @param targets        address of our protected server (format [host:]port), 127.0.0.1:[[virtualPort]] if empty
+ * @param onionAdded     a Promise to track creation of the endpoint
+ */
 class TorProtocolHandler(authentication: Authentication,
                          privateKeyPath: Path,
                          virtualPort: Int,
                          targets: Seq[String],
-                         onionAdded: Option[Promise[NodeAddress]]
-                        ) extends Actor with Stash with ActorLogging {
+                         onionAdded: Option[Promise[NodeAddress]]) extends Actor with Stash with ActorLogging {
 
   import TorProtocolHandler._
 
@@ -59,12 +59,28 @@ class TorProtocolHandler(authentication: Authentication,
   private var address: Option[NodeAddress] = None
 
   override def receive: Receive = {
-    case Connected(_, _) =>
+    case Connected(remoteAddress, _) =>
+      checkControlAddress(remoteAddress)
       receiver = sender()
       sendCommand("PROTOCOLINFO 1")
       context become protocolInfo
   }
 
+  /**
+   * The tor control protocol doesn't let us authenticate the server when using password authentication: we would send
+   * our password in cleartext to whatever process is listening on the control port. And since ADD_ONION also sends our
+   * onion private key in cleartext, a remote control port exposes both secrets to the network. We thus only allow
+   * password authentication on a local control port, and require safecookie otherwise, which authenticates the server.
+   */
+  private def checkControlAddress(remoteAddress: InetSocketAddress): Unit = {
+    val isLocal = Option(remoteAddress.getAddress).exists(_.isLoopbackAddress)
+    authentication match {
+      case _: Password if !isLocal => throw TorException(s"cannot use password authentication with a remote control port ($remoteAddress): use safecookie instead")
+      case _ if !isLocal => log.warning("tor control port {} is not local: our onion private key will be sent in cleartext over the network", remoteAddress)
+      case _ => ()
+    }
+  }
+
   def protocolInfo: Receive = {
     case data: ByteString =>
       val res = parseResponse(readResponse(data))
@@ -134,10 +150,7 @@ class TorProtocolHandler(authentication: Authentication,
   private def processOnionResponse(res: Map[String, String]): String = {
     val serviceId = res.getOrElse("ServiceID", throw TorException("service ID not found"))
     val privateKey = res.get("PrivateKey")
-    privateKey.foreach { pk =>
-      writeString(privateKeyPath, pk)
-      setPermissions(privateKeyPath, "rw-------")
-    }
+    privateKey.foreach(pk => writeSecret(privateKeyPath, pk))
     serviceId
   }
 
@@ -158,15 +171,19 @@ class TorProtocolHandler(authentication: Authentication,
   }
 
   private def computeClientHash(serverHash: ByteVector, serverNonce: ByteVector, clientNonce: ByteVector, cookieFile: Path): ByteVector = {
-    if (serverHash.length != 32)
+    if (serverHash.length != 32) {
       throw TorException("invalid server hash length")
-    if (serverNonce.length != 32)
+    }
+    if (serverNonce.length != 32) {
       throw TorException("invalid server nonce length")
+    }
 
     val cookie = ByteVector.view(Files.readAllBytes(cookieFile))
+    if (cookie.length != 32) {
+      throw TorException("invalid server cookie length")
+    }
 
     val message = cookie ++ clientNonce ++ serverNonce
-
     val computedServerHash = hmacSHA256(ServerKey, message)
     if (computedServerHash != serverHash) {
       throw TorException("unexpected server hash")
@@ -226,13 +243,6 @@ object TorProtocolHandler {
 
   def writeString(path: Path, string: String): Unit = Files.write(path, util.Arrays.asList(string))
 
-  def setPermissions(path: Path, permissionString: String): Unit =
-    try {
-      Files.setPosixFilePermissions(path, PosixFilePermissions.fromString(permissionString))
-    } catch {
-      case _: UnsupportedOperationException => () // we are on windows
-    }
-
   def unquote(s: String): String = s
     .stripSuffix("\"")
     .stripPrefix("\"")