v0.26.5 minor doc update and code cleanup

README.md

@@ -115,8 +115,7 @@ authentication designs for the rationale and what is deliberately left out.
   exit codes, `lw auth`; also the `lw(1)` manual, [`doc/lw.1`](doc/lw.1)
 - [Operations](doc/operations.md): running, monitoring, tuning,
   troubleshooting
-- [To Do](doc/todo.md): planned work in priority order: packaging, smaller
+- [To Do](doc/todo.md): planned work: packaging
-  follow-ups
 
 A fully annotated configuration covering every option ships as
 [`config/logwisp.toml`](config/logwisp.toml).

doc/README.md

@@ -30,8 +30,7 @@ streams, or downstream LogWisp nodes.
 - [CLI](cli.md): flags, signals, exit codes, `lw auth`; also the `lw(1)`
   manual, [`lw.1`](lw.1)
 - [Operations](operations.md): running, monitoring, tuning, troubleshooting
-- [To Do](todo.md): planned work in priority order: packaging, smaller
+- [To Do](todo.md): planned work: packaging
-  follow-ups
 
 A fully annotated configuration covering every option lives at
 [`config/logwisp.toml`](../config/logwisp.toml).

doc/security.md

@@ -446,7 +446,10 @@ A listener needs no certificate files. With `self_signed = true`, or with
 `issuer_cert_file` and `issuer_key_file`, it makes an ECDSA P-256 key once per
 process and, at startup and every reload, a certificate for it valid 397 days
 (never past the issuer), for `hosts`, its `host` (unless a wildcard),
-`os.Hostname()`, `localhost`, `127.0.0.1` and `::1`.
+`os.Hostname()`, `localhost`, `127.0.0.1` and `::1`. Every such listener of
+one process shares that key, self-signed or issued, so one pin matches them
+all; a dialer that must tell two apart verifies by name with `ca_file`, or
+the listeners use `cert_file`.
 
 - **Self-signed.** No CA can vouch for it, so dialers pin its key:
   `pin_sha256`, which the listener logs at WARN on every start. A reload
@@ -461,8 +464,9 @@ process and, at startup and every reload, a certificate for it valid 397 days
 - **Pins.** A pin is the SHA-256 of the server's public key, not of its
   certificate, so it survives reissue. `pin_sha256` accepts several, `;`
   between them, for a rotation. It replaces chain, name and validity checks:
-  whoever holds the key is the server. `scram` dialers still bind every login
+  whoever holds the key is the server. A pinned dialer never resumes a TLS
-  to that certificate.
+  session, so every connection checks the pins, and a pin a reload drops
+  holds at once. `scram` dialers still bind every login to that certificate.
 
 ```bash
 lw tls ca --dir /etc/logwisp/pki                       # ca.crt, ca.key (0600)

doc/todo.md

@@ -60,14 +60,3 @@ identity, and both skeletons download the tagged source.
 5. Shell completion for bash, zsh and fish, installed by `make install`.
 6. A packaging CI job: build the AUR package in an Arch container and the port
    in a FreeBSD VM, run `lw --version` and `make image-check`.
-
-## 2. Follow-ups
-
-Smaller items found along the way, each independent of the others.
-
-1. TLS: the generated-certificate key is one per process, so a pin taken from a
-   `self_signed` listener also matches an `issuer` listener of the same
-   process; document it or key per listener. Pins are checked in
-   `VerifyPeerCertificate`, which Go skips on resumption: guard it if a dialer
-   ever keeps a `ClientSessionCache`.
-2. `core.ShutdownTimeout` is unused.

internal/core/const.go

@@ -41,8 +41,6 @@ const (
 
 	ServiceStatsUpdateInterval = 1 * time.Second
 
-	ShutdownTimeout = 10 * time.Second
-
 	// Bounds the flush of a network sink's queues at Stop
 	SinkFlushTimeout = 2 * time.Second
 

internal/tlsx/pki.go

@@ -35,7 +35,8 @@ const (
 )
 
 // processKey is the key of every generated listener certificate: reloads
-// reissue certificates and keep it, so a pin holds until lw restarts.
+// reissue certificates and keep it, so a pin holds until lw restarts, and
+// matches every such listener of the process, self-signed or issued.
 var processKey = sync.OnceValues(NewKey)
 
 func NewKey() (*ecdsa.PrivateKey, error) { return ecdsa.GenerateKey(elliptic.P256(), rand.Reader) }
@@ -188,7 +189,7 @@ func pin(spki []byte) string {
 
 // verifyPins replaces chain verification: the server's key must hash to one
 // of the ';'-separated pins. Its name and validity are not checked. Go skips
-// it on a resumed session, and no dialer keeps a ClientSessionCache.
+// it on a resumed session, which Client therefore disables.
 func verifyPins(s string) (func([][]byte, [][]*x509.Certificate) error, error) {
 	var pins [][]byte
 	for p := range strings.SplitSeq(s, ";") {

internal/tlsx/tlsx.go

@@ -112,7 +112,9 @@ func Client(o *config.TLSOptions, host string) (*tls.Config, error) {
 		if cfg.VerifyPeerCertificate, err = verifyPins(o.PinSHA256); err != nil {
 			return nil, err
 		}
-		cfg.InsecureSkipVerify = true // the pin check above replaces it
+		// The pin check replaces Go's; Go skips it on a resumed session, so a
+		// pinned dialer never resumes, and a pin dropped by a reload holds
+		cfg.InsecureSkipVerify, cfg.SessionTicketsDisabled = true, true
 	}
 	if o.CAFile != "" {
 		pool, err := loadPool(o.CAFile)

internal/tlsx/tlsx_test.go

@@ -1,6 +1,7 @@
 package tlsx
 
 import (
+	"crypto/tls"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"net"
@@ -84,6 +85,50 @@ func TestSelfSignedListenerIsVerifiedByItsPin(t *testing.T) {
 	}
 }
 
+// Go skips the pin check on a resumed session, so a pinned dialer never
+// resumes one, even given a session cache
+func TestPinnedDialerNeverResumes(t *testing.T) {
+	server, err := Server(&config.TLSOptions{Enabled: true, SelfSigned: true}, "127.0.0.1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	client, err := Client(&config.TLSOptions{Enabled: true, PinSHA256: PinSHA256(server.Certificates[0].Leaf)}, "127.0.0.1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	client.ClientSessionCache = tls.NewLRUClientSessionCache(1)
+	ln, err := net.Listen("tcp4", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ln.Close()
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			conn.SetDeadline(time.Now().Add(5 * time.Second))
+			tconn := tls.Server(conn, server)
+			tconn.Write([]byte{1}) // after the handshake and its session ticket
+			tconn.Close()
+		}
+	}()
+	for i := range 2 {
+		conn, err := tls.Dial("tcp4", ln.Addr().String(), client)
+		if err != nil {
+			t.Fatal(err)
+		}
+		conn.SetDeadline(time.Now().Add(5 * time.Second))
+		_, err = conn.Read(make([]byte, 1)) // stores a ticket, if one came
+		resumed := conn.ConnectionState().DidResume
+		conn.Close()
+		if err != nil || resumed {
+			t.Fatalf("connection %d: resumed %v, %v", i+1, resumed, err)
+		}
+	}
+}
+
 // An issuer-signed listener chains to the CA file, for its host, and expires
 // with its issuer; a certificate that is no CA cannot issue.
 func TestIssuedListenerCertificateChainsToTheIssuer(t *testing.T) {