[bug#32346] [PATCH 0/6] TOR via Unix domain SOCKS socket

[bug#32346] [PATCH 0/6] TOR via Unix domain SOCKS socket

[Chris Marusich]

Hi,

This patch series makes it possible to use our TOR service over a Unix
domain socket.  Specifically, after applying this patch series you can
configure the TOR service as follows:

    (service tor-service-type
             (tor-configuration (socks-socket-type 'unix)))

This configures the TOR daemon to run its SOCKS proxy on the Unix
domain socket "/var/run/tor/socks-sock" rather than localhost port
9050.  The SOCKS socket is made group-writable so that any member of
the "tor" group may connect to it.

Along the way, this patch series also adds system tests for TOR and
adds a tiny bit of functionality to the marionette test framework.

This patch series applies cleanly to commit
0d206dd054b74279838d44029c52a5f7663de189.  For each patch in the
series, I've verified that (1) Guix builds successfully and (2) both
the new "tor" system test and the existing "dhcpd" system test pass.
I ran the "dhcpd" system test as a sanity test to make sure I didn't
introduce a regression in existing system tests.  I'm currently
reconfiguring my GuixSD system to confirm that it works for real, but
it's going to take a long time for the reconfigure to succeed, so I'm
sending the patch series now.

Regarding hidden services, I haven't changed anything intentionally,
so it should continue to work as before.  I'll also test hidden
services, but that's also going to take more time because I've never
done it before, and I have to wait for my system to build many things.

I'll let you know if it all works after I reconfigure my system.  In
the meantime, please take a look at the patches.  Thank you!

Chris Marusich (6):
  services: tor: Add a system test.
  services: tor: Rename activation procedure.
  marionette: Add support for QEMU's "quit" command.
  marionette: Add wait-for-unix-socket.
  tests: tor: Add more test cases.
  services: tor: Make it easier to use UNIX sockets.

 doc/guix.texi               | 52 +++++++++++++++++---
 gnu/build/marionette.scm    | 28 ++++++++++-
 gnu/services/networking.scm | 34 ++++++++++---
 gnu/tests/networking.scm    | 97 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 196 insertions(+), 15 deletions(-)

-- 
2.18.0

[bug#32346] [PATCH 1/6] services: tor: Add a system test.

[Chris Marusich]

* gnu/services/networking.scm (tor-configuration->torrc): Set PidFile to
/var/run/tor/tor.pid in the base torrc configuration.
(tor-shepherd-service) <start>: Call make-forkexec-constructor/container with
a new #:pid-file argument to tell Shepherd where to find the PID file.  Add a
a new <file-system-mapping> to its existing #:mappings argument to share
/var/run/tor with the the container.
(tor-hidden-services-activation): Update docstring.  Create /var/run/tor and
set its permissions so only the tor user can access it.
* gnu/tests/networking.scm (%test-tor, %tor-os): New variables.
(run-tor-test): New procedure.
---
 gnu/services/networking.scm | 20 +++++++++++--
 gnu/tests/networking.scm    | 56 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index d5d0cf9d1..bd79e6589 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -7,6 +7,7 @@
 ;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
 ;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2018 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2018 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -612,6 +613,7 @@ demand.")))
 ### These lines were generated from your system configuration:
 User tor
 DataDirectory /var/lib/tor
+PidFile /var/run/tor/tor.pid
 Log notice syslog\n" port)
 
                 (for-each (match-lambda
@@ -665,12 +667,17 @@ HiddenServicePort ~a ~a~%"
                                             (writable? #t))
                                            (file-system-mapping
                                             (source "/dev/log") ;for syslog
-                                            (target source)))))
+                                            (target source))
+                                           (file-system-mapping
+                                            (source "/var/run/tor")
+                                            (target source)
+                                            (writable? #t)))
+                          #:pid-file "/var/run/tor/tor.pid"))
                 (stop #~(make-kill-destructor))
                 (documentation "Run the Tor anonymous network overlay."))))))))
 
 (define (tor-hidden-service-activation config)
-  "Return the activation gexp for SERVICES, a list of hidden services."
+  "Set up directories for TOR and its hidden services, if any."
   #~(begin
       (use-modules (guix build utils))
 
@@ -686,6 +693,15 @@ HiddenServicePort ~a ~a~%"
           ;; The daemon bails out if we give wider permissions.
           (chmod directory #o700)))
 
+      ;; Allow TOR to write its PID file.
+      (mkdir-p "/var/run/tor")
+      (chown "/var/run/tor" (passwd:uid %user) (passwd:gid %user))
+      ;; Set the group permissions to rw so that if the system administrator
+      ;; has specified UnixSocksGroupWritable=1 in their torrc file, members
+      ;; of the "tor" group will be able to use the SOCKS socket.
+      (chmod "/var/run/tor" #o750)
+
+      ;; Allow TOR to access the hidden services' directories.
       (mkdir-p "/var/lib/tor")
       (chown "/var/lib/tor" (passwd:uid %user) (passwd:gid %user))
       (chmod "/var/lib/tor" #o700)
diff --git a/gnu/tests/networking.scm b/gnu/tests/networking.scm
index 323679e7f..c9a4f5463 100644
--- a/gnu/tests/networking.scm
+++ b/gnu/tests/networking.scm
@@ -30,7 +30,7 @@
   #:use-module (gnu packages bash)
   #:use-module (gnu packages networking)
   #:use-module (gnu services shepherd)
-  #:export (%test-inetd %test-openvswitch %test-dhcpd))
+  #:export (%test-inetd %test-openvswitch %test-dhcpd %test-tor))
 
 (define %inetd-os
   ;; Operating system with 2 inetd services.
@@ -339,3 +339,57 @@ subnet 192.168.1.0 netmask 255.255.255.0 {
    (name "dhcpd")
    (description "Test a running DHCP daemon configuration.")
    (value (run-dhcpd-test))))
+
+\f
+;;;
+;;; Services related to TOR
+;;;
+
+(define %tor-os
+  (simple-operating-system
+   (tor-service)))
+
+(define (run-tor-test)
+  (define os
+    (marionette-operating-system %tor-os
+                                 #:imported-modules '((gnu services herd))
+                                 #:requirements '(tor)))
+
+  (define test
+    (with-imported-modules '((gnu build marionette))
+      #~(begin
+          (use-modules (gnu build marionette)
+                       (ice-9 popen)
+                       (ice-9 rdelim)
+                       (srfi srfi-64))
+
+          (define marionette
+            (make-marionette (list #$(virtual-machine os))))
+
+          (mkdir #$output)
+          (chdir #$output)
+
+          (test-begin "tor")
+
+          (test-assert "tor is alive"
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd)
+                             (srfi srfi-1))
+                (live-service-running
+                 (find (lambda (live)
+                         (memq 'tor
+                               (live-service-provision live)))
+                       (current-services))))
+             marionette))
+
+          (test-end)
+          (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
+
+  (gexp->derivation "tor-test" test))
+
+(define %test-tor
+  (system-test
+   (name "tor")
+   (description "Test a running TOR daemon configuration.")
+   (value (run-tor-test))))
-- 
2.18.0

[bug#32346] [PATCH 2/6] services: tor: Rename activation procedure.

[Chris Marusich]

* gnu/services/networking.scm: Rename the procedure
tor-hidden-service-activation to tor-activation.
---
 gnu/services/networking.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index bd79e6589..9523f97f6 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -676,7 +676,7 @@ HiddenServicePort ~a ~a~%"
                 (stop #~(make-kill-destructor))
                 (documentation "Run the Tor anonymous network overlay."))))))))
 
-(define (tor-hidden-service-activation config)
+(define (tor-activation config)
   "Set up directories for TOR and its hidden services, if any."
   #~(begin
       (use-modules (guix build utils))
@@ -721,7 +721,7 @@ HiddenServicePort ~a ~a~%"
                        (service-extension account-service-type
                                           (const %tor-accounts))
                        (service-extension activation-service-type
-                                          tor-hidden-service-activation)))
+                                          tor-activation)))
 
                 ;; This can be extended with hidden services.
                 (compose concatenate)
-- 
2.18.0

[bug#32346] [PATCH 3/6] marionette: Add support for QEMU's "quit" command.

[Chris Marusich]

* gnu/build/marionette.scm (marionette-control): Don't wait for the
monitor prompt when the command was "quit".
---
 gnu/build/marionette.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/build/marionette.scm b/gnu/build/marionette.scm
index bb018fc9c..61284b898 100644
--- a/gnu/build/marionette.scm
+++ b/gnu/build/marionette.scm
@@ -222,7 +222,8 @@ pcsys_monitor\")."
     (($ <marionette> _ _ monitor)
      (display command monitor)
      (newline monitor)
-     (wait-for-monitor-prompt monitor))))
+     ;; The "quit" command terminates QEMU immediately, with no output.
+     (unless (string=? command "quit") (wait-for-monitor-prompt monitor)))))
 
 (define* (marionette-screen-text marionette
                                  #:key
-- 
2.18.0

[bug#32346] [PATCH 4/6] marionette: Add wait-for-unix-socket.

[Chris Marusich]

* gnu/build/marionette.scm (wait-for-unix-socket): New variable.
---
 gnu/build/marionette.scm | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/gnu/build/marionette.scm b/gnu/build/marionette.scm
index 61284b898..3588bcbe6 100644
--- a/gnu/build/marionette.scm
+++ b/gnu/build/marionette.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2016, 2017, 2018 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -27,6 +28,7 @@
             marionette-eval
             wait-for-file
             wait-for-tcp-port
+            wait-for-unix-socket
             marionette-control
             marionette-screen-text
             wait-for-screen-text
@@ -214,6 +216,29 @@ MARIONETTE.  Raise an error on failure."
     ('failure
      (error "nobody's listening on port" port))))
 
+(define* (wait-for-unix-socket path marionette
+                                #:key (timeout 20))
+  "Wait for up to TIMEOUT seconds for PATH, a Unix domain socket, to accept
+connections in MARIONETTE.  Raise an error on failure."
+  (match (marionette-eval
+          `(begin
+             (let ((sock (socket PF_UNIX SOCK_STREAM 0)))
+               (let loop ((i 0))
+                 (catch 'system-error
+                   (lambda ()
+                     (connect sock AF_UNIX ,path)
+                     'success)
+                   (lambda args
+                     (if (< i ,timeout)
+                         (begin
+                           (sleep 1)
+                           (loop (+ 1 i)))
+                         'failure))))))
+          marionette)
+    ('success #t)
+    ('failure
+     (error "nobody's listening on unix domain socket" path))))
+
 (define (marionette-control command marionette)
   "Run COMMAND in the QEMU monitor of MARIONETTE.  COMMAND is a string such as
 \"sendkey ctrl-alt-f1\" or \"screendump foo.ppm\" (info \"(qemu-doc)
-- 
2.18.0

[bug#32346] [PATCH 5/6] tests: tor: Add more test cases.

[Chris Marusich]

* gnu/tests/networking.scm (%tor-os/unix-socks-socket): New variable.
(run-tor-test) <os/unix-socks-socket, marionette/unix-socks-socket>
<socket-directory>: New variables.
<"tor is alive">: Move common code from this test case...
<tor-is-alive?>: ...into this new procedure.
<"tor is listening", "tor is alive, even when using a SOCKS socket">
<"tor is listening, even when using a SOCKS socket">: New test cases.
---
 gnu/tests/networking.scm | 59 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 6 deletions(-)

diff --git a/gnu/tests/networking.scm b/gnu/tests/networking.scm
index c9a4f5463..b332ec3cf 100644
--- a/gnu/tests/networking.scm
+++ b/gnu/tests/networking.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
 ;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
+;;; Copyright © 2018 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -349,12 +350,29 @@ subnet 192.168.1.0 netmask 255.255.255.0 {
   (simple-operating-system
    (tor-service)))
 
+(define %tor-os/unix-socks-socket
+  (simple-operating-system
+   (service tor-service-type
+            (tor-configuration
+             (config-file
+              (plain-file "test-torrc"
+                          "\
+SocksPort unix:/var/run/tor/socks-sock
+UnixSocksGroupWritable 1
+")
+              )))))
+
 (define (run-tor-test)
   (define os
     (marionette-operating-system %tor-os
                                  #:imported-modules '((gnu services herd))
                                  #:requirements '(tor)))
 
+  (define os/unix-socks-socket
+    (marionette-operating-system %tor-os/unix-socks-socket
+                                 #:imported-modules '((gnu services herd))
+                                 #:requirements '(tor)))
+
   (define test
     (with-imported-modules '((gnu build marionette))
       #~(begin
@@ -366,12 +384,7 @@ subnet 192.168.1.0 netmask 255.255.255.0 {
           (define marionette
             (make-marionette (list #$(virtual-machine os))))
 
-          (mkdir #$output)
-          (chdir #$output)
-
-          (test-begin "tor")
-
-          (test-assert "tor is alive"
+          (define (tor-is-alive? marionette)
             (marionette-eval
              '(begin
                 (use-modules (gnu services herd)
@@ -383,6 +396,40 @@ subnet 192.168.1.0 netmask 255.255.255.0 {
                        (current-services))))
              marionette))
 
+          (mkdir #$output)
+          (chdir #$output)
+
+          (test-begin "tor")
+
+          ;; Test the usual TOR service.
+
+          (test-assert "tor is alive"
+            (tor-is-alive? marionette))
+
+          (test-assert "tor is listening"
+            (let ((default-port 9050))
+              (wait-for-tcp-port default-port marionette)))
+
+          ;; Don't run two VMs at once.
+          (marionette-control "quit" marionette)
+
+          ;; Test the TOR service using a SOCKS socket.
+
+          (let* ((socket-directory "/tmp/more-sockets")
+                 (_ (mkdir socket-directory))
+                 (marionette/unix-socks-socket
+                  (make-marionette
+                   (list #$(virtual-machine os/unix-socks-socket))
+                   ;; We can't use the same socket directory as the first
+                   ;; marionette.
+                   #:socket-directory socket-directory)))
+            (test-assert "tor is alive, even when using a SOCKS socket"
+              (tor-is-alive? marionette/unix-socks-socket))
+
+            (test-assert "tor is listening, even when using a SOCKS socket"
+              (wait-for-unix-socket "/var/run/tor/socks-sock"
+                                    marionette/unix-socks-socket)))
+
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
 
-- 
2.18.0

[bug#32346] [PATCH 6/6] services: tor: Make it easier to use UNIX sockets.

[Chris Marusich]

* doc/guix.texi (Networking Services): Document it, and mention that
tor-service is deprecated.
* gnu/services/networking.scm (<tor-configuration>) <socks-socket-type>:
New field.
(tor-configuration->torrc): When socks-socket-type is 'unix, set
SocksPort to UNIX domain socket /var/run/tor/socks-sock and set
UnixSocksGroupWritable to 1.
* gnu/tests/networking.scm (%tor-os/unix-socks-socket): Instead of using
a custom config file, just set socks-socket-type to 'unix.
---
 doc/guix.texi               | 52 ++++++++++++++++++++++++++++++++-----
 gnu/services/networking.scm | 10 +++++--
 gnu/tests/networking.scm    |  8 +-----
 3 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 080b091b3..c72b1e480 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -11445,16 +11445,54 @@ detailed discussion of each configuration field.
 @end deftp
 
 @cindex Tor
-@deffn {Scheme Procedure} tor-service [@var{config-file}] [#:tor @var{tor}]
-Return a service to run the @uref{https://torproject.org, Tor} anonymous
-networking daemon.
+@defvr {Scheme Variable} tor-service-type
+This is the type for a service that runs the @uref{https://torproject.org,
+Tor} anonymous networking daemon.  The service is configured using a
+@code{<tor-configuration>} record.  By default, the TOR daemon runs as the
+@code{tor} unprivileged user, which is a member of the @code{tor} group.
+
+@end defvr
 
-The daemon runs as the @code{tor} unprivileged user.  It is passed
-@var{config-file}, a file-like object, with an additional @code{User tor} line
-and lines for hidden services added via @code{tor-hidden-service}.  Run
-@command{man tor} for information about the configuration file.
+@deffn {Scheme Procedure} tor-service [@var{config-file}] [#:tor @var{tor}]
+This procedure is deprecated and will be removed in a future release.  Return
+a service of the @code{tor-service-type} type.  @var{config-file} and
+@var{tor} have the same meaning as in @code{<tor-configuration>}.
 @end deffn
 
+@deftp {Data Type} tor-configuration
+@table @asis
+@item @code{tor} (default: @code{tor})
+The package that provides the TOR daemon.  This package is expected to provide
+the daemon at @file{bin/tor} relative to its output directory.  The default
+package is the @uref{https://www.torproject.org, TOR Project's}
+implementation.
+@item @code{config-file} (default: @code{(plain-file "empty" "")})
+The configuration file to use.  It will be appended to a default configuration
+file, and the final configuration file will be passed to @code{tor} via its
+@code{-f} option.  This may be any ``file-like'' object (@pxref{G-Expressions,
+file-like objects}).  See @code{man tor} for details on the configuration file
+syntax.
+@item @code{hidden-services} (default: @code{'()})
+The list of @code{<hidden-service>} records to use.  For any hidden service
+you include in this list, appropriate configuration to enable the hidden
+service will be automatically added to the default configuration file.  You
+may conveniently create @code{<hidden-service>} records using the
+@code{tor-hidden-service} procedure described below.
+@item @code{socks-socket-type} (default: @code{'tcp})
+The default socket type that TOR should use for its SOCKS socket.  This must
+be either @code{'tcp} or @code{'unix}.  If it is @code{'tcp}, then by default
+TOR will listen on TCP port 9050 on the loopback interface (i.e., localhost).
+If it is @code{'unix}, then TOR will listen on the UNIX domain socket
+@file{/var/run/tor/socks-sock}, which will be made writable by members of the
+@code{tor} group.
+
+If you want to customize the SOCKS socket in more detail, leave
+@code{socks-socket-type} at its default value of @code{'tcp} and use
+@code{config-file} to override the default by providing your own
+@code{SocksPort} option.
+@end table
+@end deftp
+
 @cindex hidden service
 @deffn {Scheme Procedure} tor-hidden-service @var{name} @var{mapping}
 Define a new Tor @dfn{hidden service} called @var{name} and implementing
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 9523f97f6..a7f722807 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -577,7 +577,9 @@ demand.")))
   (config-file      tor-configuration-config-file
                     (default (plain-file "empty" "")))
   (hidden-services  tor-configuration-hidden-services
-                    (default '())))
+                    (default '()))
+  (socks-socket-type tor-configuration-socks-socket-type ; 'tcp or 'unix
+                     (default 'tcp)))
 
 (define %tor-accounts
   ;; User account and groups for Tor.
@@ -599,7 +601,7 @@ demand.")))
 (define (tor-configuration->torrc config)
   "Return a 'torrc' file for CONFIG."
   (match config
-    (($ <tor-configuration> tor config-file services)
+    (($ <tor-configuration> tor config-file services socks-socket-type)
      (computed-file
       "torrc"
       (with-imported-modules '((guix build utils))
@@ -615,6 +617,10 @@ User tor
 DataDirectory /var/lib/tor
 PidFile /var/run/tor/tor.pid
 Log notice syslog\n" port)
+                (when (eq? 'unix '#$socks-socket-type)
+                  (display "\
+SocksPort unix:/var/run/tor/socks-sock
+UnixSocksGroupWritable 1\n" port))
 
                 (for-each (match-lambda
                             ((service (ports hosts) ...)
diff --git a/gnu/tests/networking.scm b/gnu/tests/networking.scm
index b332ec3cf..06d6250b8 100644
--- a/gnu/tests/networking.scm
+++ b/gnu/tests/networking.scm
@@ -354,13 +354,7 @@ subnet 192.168.1.0 netmask 255.255.255.0 {
   (simple-operating-system
    (service tor-service-type
             (tor-configuration
-             (config-file
-              (plain-file "test-torrc"
-                          "\
-SocksPort unix:/var/run/tor/socks-sock
-UnixSocksGroupWritable 1
-")
-              )))))
+             (socks-socket-type 'unix)))))
 
 (define (run-tor-test)
   (define os
-- 
2.18.0

[bug#32346] [PATCH 0/6] TOR via Unix domain SOCKS socket

[Nils Gillmann]

Chris Marusich transcribed 2.1K bytes:
> Hi,
...
> Regarding hidden services, I haven't changed anything intentionally,
> so it should continue to work as before.  I'll also test hidden
> services, but that's also going to take more time because I've never
> done it before, and I have to wait for my system to build many things.

Good Work!

Now for the hidden-services..
Couldn't you achieve the effect you wanted without renaming
tor-hidden-services?

> I'll let you know if it all works after I reconfigure my system.  In
> the meantime, please take a look at the patches.  Thank you!
> 
> Chris Marusich (6):
>   services: tor: Add a system test.
>   services: tor: Rename activation procedure.
>   marionette: Add support for QEMU's "quit" command.
>   marionette: Add wait-for-unix-socket.
>   tests: tor: Add more test cases.
>   services: tor: Make it easier to use UNIX sockets.
> 
>  doc/guix.texi               | 52 +++++++++++++++++---
>  gnu/build/marionette.scm    | 28 ++++++++++-
>  gnu/services/networking.scm | 34 ++++++++++---
>  gnu/tests/networking.scm    | 97 ++++++++++++++++++++++++++++++++++++-
>  4 files changed, 196 insertions(+), 15 deletions(-)
> 
> -- 
> 2.18.0
> 
> 
> 
> 

[bug#32346] [PATCH 0/6] TOR via Unix domain SOCKS socket

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

Nils Gillmann <ng0@n0.is> writes:

> Good Work!

Thank you!  I hope it proves useful.

> Now for the hidden-services..
> Couldn't you achieve the effect you wanted without renaming
> tor-hidden-services?

Which patch are you referring to?  I don't think I've renamed
tor-hidden-services.

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#32346] [PATCH 0/6] TOR via Unix domain SOCKS socket

[Nils Gillmann]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

Chris Marusich transcribed 1.3K bytes:
> Nils Gillmann <ng0@n0.is> writes:
> 
> > Good Work!
> 
> Thank you!  I hope it proves useful.
> 
> > Now for the hidden-services..
> > Couldn't you achieve the effect you wanted without renaming
> > tor-hidden-services?
> 
> Which patch are you referring to?  I don't think I've renamed
> tor-hidden-services.
> 
> -- 
> Chris

Oh, I've read it again. My bad. Blame the heat :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#32346] [PATCH 0/6] TOR via Unix domain SOCKS socket

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

Chris Marusich <cmmarusich@gmail.com> writes:

> I'm currently reconfiguring my GuixSD system to confirm that it works
> for real. [...]  I'll also test hidden services.

I finished reconfiguring my system.  I can confirm that TOR works even
with a UNIX domain socket, and hidden services work the same as before.

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#32346] [PATCH 1/6] services: tor: Add a system test.

[Ludovic Courtès]

Hi Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

> * gnu/services/networking.scm (tor-configuration->torrc): Set PidFile to
> /var/run/tor/tor.pid in the base torrc configuration.
> (tor-shepherd-service) <start>: Call make-forkexec-constructor/container with
> a new #:pid-file argument to tell Shepherd where to find the PID file.  Add a
> a new <file-system-mapping> to its existing #:mappings argument to share
> /var/run/tor with the the container.
> (tor-hidden-services-activation): Update docstring.  Create /var/run/tor and
> set its permissions so only the tor user can access it.
> * gnu/tests/networking.scm (%test-tor, %tor-os): New variables.
> (run-tor-test): New procedure.

LGTM!  Great work.

> +      ;; Allow TOR to write its PID file.

Nitpick: I think the maintainers no longer consider the name an acronym
and write it as “Tor”, not “TOR”.

Thanks,
Ludo’.

[bug#32346] [PATCH 2/6] services: tor: Rename activation procedure.

[Ludovic Courtès]

Chris Marusich <cmmarusich@gmail.com> skribis:

> * gnu/services/networking.scm: Rename the procedure
> tor-hidden-service-activation to tor-activation.

OK!

[bug#32346] [PATCH 3/6] marionette: Add support for QEMU's "quit" command.

[Ludovic Courtès]

Chris Marusich <cmmarusich@gmail.com> skribis:

> * gnu/build/marionette.scm (marionette-control): Don't wait for the
> monitor prompt when the command was "quit".

LGTM!

[bug#32346] [PATCH 4/6] marionette: Add wait-for-unix-socket.

[Ludovic Courtès]

Chris Marusich <cmmarusich@gmail.com> skribis:

> * gnu/build/marionette.scm (wait-for-unix-socket): New variable.

[...]

> +(define* (wait-for-unix-socket path marionette

Super nitpick: s/path/file/ or s/path/file-name/, as per GNU
convention (where “path” means “search path”.)

Otherwise LGTM!

Ludo’.

[bug#32346] [PATCH 5/6] tests: tor: Add more test cases.

[Ludovic Courtès]

Chris Marusich <cmmarusich@gmail.com> skribis:

> * gnu/tests/networking.scm (%tor-os/unix-socks-socket): New variable.
> (run-tor-test) <os/unix-socks-socket, marionette/unix-socks-socket>
> <socket-directory>: New variables.
> <"tor is alive">: Move common code from this test case...
> <tor-is-alive?>: ...into this new procedure.
> <"tor is listening", "tor is alive, even when using a SOCKS socket">
> <"tor is listening, even when using a SOCKS socket">: New test cases.

Excellent, LGTM!

[bug#32346] [PATCH 6/6] services: tor: Make it easier to use UNIX sockets.

[Ludovic Courtès]

Chris Marusich <cmmarusich@gmail.com> skribis:

> * doc/guix.texi (Networking Services): Document it, and mention that
> tor-service is deprecated.
> * gnu/services/networking.scm (<tor-configuration>) <socks-socket-type>:
> New field.
> (tor-configuration->torrc): When socks-socket-type is 'unix, set
> SocksPort to UNIX domain socket /var/run/tor/socks-sock and set
> UnixSocksGroupWritable to 1.
> * gnu/tests/networking.scm (%tor-os/unix-socks-socket): Instead of using
> a custom config file, just set socks-socket-type to 'unix.

[...]

> +This is the type for a service that runs the @uref{https://torproject.org,
> +Tor} anonymous networking daemon.  The service is configured using a
> +@code{<tor-configuration>} record.  By default, the TOR daemon runs as the

Please s/TOR/Tor/ in this file.

> +@deftp {Data Type} tor-configuration
> +@table @asis
> +@item @code{tor} (default: @code{tor})
> +The package that provides the TOR daemon.  This package is expected to provide
> +the daemon at @file{bin/tor} relative to its output directory.  The default
> +package is the @uref{https://www.torproject.org, TOR Project's}
> +implementation.
> +@item @code{config-file} (default: @code{(plain-file "empty" "")})

You could skip a line between between each @item for clarity.

Otherwise LGTM.

Thank you for this nice patch series!

Ludo’.

bug#32346: [PATCH 1/6] services: tor: Add a system test.

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

Hi Ludo,

I've incorporated your feedback and committed this patch series as
3bcb305b98e02f6c9d98e7325813fc00f18f0e6c.  Details follow.

ludo@gnu.org (Ludovic Courtès) writes:

> Chris Marusich <cmmarusich@gmail.com> skribis:
>
>
> [...]
>
>> +      ;; Allow TOR to write its PID file.
>
> Nitpick: I think the maintainers no longer consider the name an acronym
> and write it as “Tor”, not “TOR”.

This is good to know.  In each patch that uses the incorrect name "TOR",
I've changed it to use the correct name "Tor".

>> +(define* (wait-for-unix-socket path marionette
>
> Super nitpick: s/path/file/ or s/path/file-name/, as per GNU
> convention (where “path” means “search path”.)

I've updated this to follow the convention.

>> +@deftp {Data Type} tor-configuration
>> +@table @asis
>> +@item @code{tor} (default: @code{tor})
>> +The package that provides the TOR daemon.  This package is expected
>> to provide
>> +the daemon at @file{bin/tor} relative to its output directory.  The default
>> +package is the @uref{https://www.torproject.org, TOR Project's}
>> +implementation.
>> +@item @code{config-file} (default: @code{(plain-file "empty" "")})
>
> You could skip a line between between each @item for clarity.

Good idea.  I've done this, too.

> Thank you for this nice patch series!

Thank you for the thoughtful review!

I appreciate your attention to detail.  If you hadn't told me, I
wouldn't have known about some of these things, such as the GNU
convention to use "file" or "file-name" instead of "path" when speaking
about UNIX domain sockets.  I'll remember these things going forward.

And with that, I will now close this patch!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]