Pārlūkot izejas kodu

doctor: validate fronting domain TLS certificate

The fronting-domain step only opened a bare TCP connection, so a missing,
expired, untrusted or wrong-host certificate still reported a green check.
That is exactly the misleading result reported in #518.

After the TCP dial, perform a default crypto/tls handshake against the
fronting endpoint with ServerName set to the secret host. Standard
verification validates the chain against the system roots, checks the leaf
SAN against the secret host, and enforces the validity period in one step,
so expired/untrusted/wrong-host certificates surface as descriptive x509
errors.

The dial target still honors the domain-fronting.host override while SNI
stays the secret host, matching what domain fronting puts on the wire.

When proxy-protocol is enabled the listener expects a PROXY header before
the ClientHello, which doctor does not emit yet; the certificate probe is
skipped with an informational note instead of reporting a false negative.
doctor-tls-cert-check
Alexey Dolotov 3 nedēļas atpakaļ
vecāks
revīzija
d2722ef5c8
2 mainītis faili ar 257 papildinājumiem un 3 dzēšanām
  1. 83
    3
      internal/cli/doctor.go
  2. 174
    0
      internal/cli/doctor_test.go

+ 83
- 3
internal/cli/doctor.go Parādīt failu

@@ -2,6 +2,8 @@ package cli
2 2
 
3 3
 import (
4 4
 	"context"
5
+	"crypto/tls"
6
+	"crypto/x509"
5 7
 	"errors"
6 8
 	"fmt"
7 9
 	"maps"
@@ -66,6 +68,16 @@ var (
66 68
 	tplEFrontingDomain = template.Must(
67 69
 		template.New("").Parse("  ❌ {{ .address }}: {{ .error }}\n"),
68 70
 	)
71
+
72
+	tplOFrontingTLS = template.Must(
73
+		template.New("").Parse("  ✅ TLS certificate for {{ .host }} is valid\n"),
74
+	)
75
+	tplEFrontingTLS = template.Must(
76
+		template.New("").Parse("  ❌ TLS certificate for {{ .host }} is invalid: {{ .error }}\n"),
77
+	)
78
+	tplSFrontingTLS = template.Must(
79
+		template.New("").Parse("  ⏭ TLS certificate check skipped: proxy-protocol is enabled (the listener expects a PROXY header that mtg doctor does not send yet)\n"),
80
+	)
69 81
 )
70 82
 
71 83
 type Doctor struct {
@@ -339,13 +351,19 @@ func (d *Doctor) checkNetworkAddresses(ntw mtglib.Network, dc int, addresses []s
339 351
 }
340 352
 
341 353
 func (d *Doctor) checkFrontingDomain(ntw mtglib.Network) bool {
342
-	host := d.conf.Secret.Host
354
+	// SNI must always be the secret host: that is what domain fronting puts on
355
+	// the wire and what the certificate is issued for. The TCP target may be a
356
+	// different address when domain-fronting.host overrides it (in the
357
+	// sni-router setup it is an internal name like "web").
358
+	sniHost := d.conf.Secret.Host
359
+
360
+	dialHost := sniHost
343 361
 	if override := d.conf.GetDomainFrontingHost(); override != "" {
344
-		host = override
362
+		dialHost = override
345 363
 	}
346 364
 
347 365
 	port := d.conf.GetDomainFrontingPort(mtglib.DefaultDomainFrontingPort)
348
-	address := net.JoinHostPort(host, strconv.Itoa(int(port)))
366
+	address := net.JoinHostPort(dialHost, strconv.Itoa(int(port)))
349 367
 
350 368
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
351 369
 	defer cancel()
@@ -367,9 +385,71 @@ func (d *Doctor) checkFrontingDomain(ntw mtglib.Network) bool {
367 385
 		"address": address,
368 386
 	})
369 387
 
388
+	// With proxy-protocol enabled the fronting listener expects a PROXY header
389
+	// before the TLS ClientHello, so a bare TLS handshake would hang or be
390
+	// rejected and report a misleading failure. mtg doctor does not emit that
391
+	// header yet, so skip the certificate probe rather than print a false
392
+	// negative. See issue #518.
393
+	if d.conf.GetDomainFrontingProxyProtocol(false) {
394
+		tplSFrontingTLS.Execute(os.Stdout, nil) //nolint: errcheck
395
+		return true
396
+	}
397
+
398
+	// A default crypto/tls client handshake against the fronting endpoint with
399
+	// ServerName = secret host validates the whole certificate in one shot:
400
+	// chain against the system roots, leaf SAN against the secret host, and
401
+	// validity period. An expired / untrusted / wrong-host certificate all
402
+	// surface as descriptive x509 errors.
403
+	if err := probeFrontingTLS(ctx, dialer, address, sniHost, nil); err != nil {
404
+		tplEFrontingTLS.Execute(os.Stdout, map[string]any{ //nolint: errcheck
405
+			"host":  sniHost,
406
+			"error": err,
407
+		})
408
+		return false
409
+	}
410
+
411
+	tplOFrontingTLS.Execute(os.Stdout, map[string]any{ //nolint: errcheck
412
+		"host": sniHost,
413
+	})
414
+
370 415
 	return true
371 416
 }
372 417
 
418
+// probeFrontingTLS dials dialAddress over TCP and performs a TLS handshake
419
+// presenting sniHost as the SNI / ServerName. Verification is left at the
420
+// crypto/tls default (InsecureSkipVerify=false), so the handshake fails with a
421
+// descriptive x509 error if the certificate chain is untrusted, the leaf SAN
422
+// does not cover sniHost, or the certificate is expired/not-yet-valid.
423
+//
424
+// rootCAs overrides the trust anchors; it is nil in production (system roots)
425
+// and is only set by tests that need a self-signed anchor.
426
+func probeFrontingTLS(
427
+	ctx context.Context,
428
+	dialer *net.Dialer,
429
+	dialAddress string,
430
+	sniHost string,
431
+	rootCAs *x509.CertPool,
432
+) error {
433
+	conn, err := dialer.DialContext(ctx, "tcp", dialAddress)
434
+	if err != nil {
435
+		return fmt.Errorf("cannot dial %s: %w", dialAddress, err)
436
+	}
437
+	defer conn.Close() //nolint: errcheck
438
+
439
+	if deadline, ok := ctx.Deadline(); ok {
440
+		conn.SetDeadline(deadline) //nolint: errcheck
441
+	}
442
+
443
+	tlsConn := tls.Client(conn, &tls.Config{
444
+		ServerName: sniHost,
445
+		RootCAs:    rootCAs,
446
+		MinVersion: tls.VersionTLS12,
447
+	})
448
+	defer tlsConn.Close() //nolint: errcheck
449
+
450
+	return tlsConn.HandshakeContext(ctx)
451
+}
452
+
373 453
 func (d *Doctor) checkSecretHost(resolver *net.Resolver, ntw mtglib.Network) bool {
374 454
 	res := runSNICheck(context.Background(), resolver, d.conf, ntw)
375 455
 

+ 174
- 0
internal/cli/doctor_test.go Parādīt failu

@@ -0,0 +1,174 @@
1
+package cli
2
+
3
+import (
4
+	"context"
5
+	"crypto/ecdsa"
6
+	"crypto/elliptic"
7
+	"crypto/rand"
8
+	"crypto/tls"
9
+	"crypto/x509"
10
+	"crypto/x509/pkix"
11
+	"math/big"
12
+	"net"
13
+	"strings"
14
+	"testing"
15
+	"time"
16
+)
17
+
18
+// makeCert builds a self-signed leaf certificate valid for the supplied DNS
19
+// name (and IP, so dialing 127.0.0.1 still reaches the listener) plus a
20
+// matching tls.Config and an x509 pool that trusts it.
21
+func makeCert(t *testing.T, dnsName string, notAfter time.Time) (tls.Certificate, *x509.CertPool) {
22
+	t.Helper()
23
+
24
+	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
25
+	if err != nil {
26
+		t.Fatalf("generate key: %v", err)
27
+	}
28
+
29
+	tmpl := &x509.Certificate{
30
+		SerialNumber:          big.NewInt(1),
31
+		Subject:               pkix.Name{CommonName: dnsName},
32
+		NotBefore:             time.Now().Add(-time.Hour),
33
+		NotAfter:              notAfter,
34
+		KeyUsage:              x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
35
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
36
+		BasicConstraintsValid: true,
37
+		IsCA:                  true,
38
+		DNSNames:              []string{dnsName},
39
+		IPAddresses:           []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback},
40
+	}
41
+
42
+	der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key)
43
+	if err != nil {
44
+		t.Fatalf("create certificate: %v", err)
45
+	}
46
+
47
+	leaf, err := x509.ParseCertificate(der)
48
+	if err != nil {
49
+		t.Fatalf("parse certificate: %v", err)
50
+	}
51
+
52
+	pool := x509.NewCertPool()
53
+	pool.AddCert(leaf)
54
+
55
+	return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key, Leaf: leaf}, pool
56
+}
57
+
58
+// startTLSServer spins up a TLS listener that completes handshakes using cert
59
+// and returns its address. It is closed when the test finishes.
60
+func startTLSServer(t *testing.T, cert tls.Certificate) string {
61
+	t.Helper()
62
+
63
+	ln, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{
64
+		Certificates: []tls.Certificate{cert},
65
+		MinVersion:   tls.VersionTLS12,
66
+	})
67
+	if err != nil {
68
+		t.Fatalf("listen: %v", err)
69
+	}
70
+
71
+	t.Cleanup(func() { _ = ln.Close() })
72
+
73
+	go func() {
74
+		for {
75
+			conn, err := ln.Accept()
76
+			if err != nil {
77
+				return
78
+			}
79
+
80
+			go func() {
81
+				// Drive the handshake so the client side completes, then drop.
82
+				if tc, ok := conn.(*tls.Conn); ok {
83
+					_ = tc.Handshake()
84
+				}
85
+				_ = conn.Close()
86
+			}()
87
+		}
88
+	}()
89
+
90
+	return ln.Addr().String()
91
+}
92
+
93
+func TestProbeFrontingTLS_ValidCert(t *testing.T) {
94
+	cert, pool := makeCert(t, "front.example.org", time.Now().Add(24*time.Hour))
95
+	addr := startTLSServer(t, cert)
96
+
97
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
98
+	defer cancel()
99
+
100
+	err := probeFrontingTLS(ctx, &net.Dialer{}, addr, "front.example.org", pool)
101
+	if err != nil {
102
+		t.Fatalf("expected success, got error: %v", err)
103
+	}
104
+}
105
+
106
+func TestProbeFrontingTLS_WrongHost(t *testing.T) {
107
+	// Cert is for front.example.org, but we verify against other.example.org.
108
+	cert, pool := makeCert(t, "front.example.org", time.Now().Add(24*time.Hour))
109
+	addr := startTLSServer(t, cert)
110
+
111
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
112
+	defer cancel()
113
+
114
+	err := probeFrontingTLS(ctx, &net.Dialer{}, addr, "other.example.org", pool)
115
+	if err == nil {
116
+		t.Fatal("expected SAN-mismatch failure, got success")
117
+	}
118
+	if !strings.Contains(err.Error(), "x509") {
119
+		t.Fatalf("expected x509 verification error, got: %v", err)
120
+	}
121
+}
122
+
123
+func TestProbeFrontingTLS_UntrustedCA(t *testing.T) {
124
+	// Server cert is self-signed; we hand the client an empty pool that does
125
+	// not trust it. Default verification must reject.
126
+	cert, _ := makeCert(t, "front.example.org", time.Now().Add(24*time.Hour))
127
+	addr := startTLSServer(t, cert)
128
+
129
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
130
+	defer cancel()
131
+
132
+	err := probeFrontingTLS(ctx, &net.Dialer{}, addr, "front.example.org", x509.NewCertPool())
133
+	if err == nil {
134
+		t.Fatal("expected untrusted-CA failure, got success")
135
+	}
136
+	if !strings.Contains(err.Error(), "x509") {
137
+		t.Fatalf("expected x509 verification error, got: %v", err)
138
+	}
139
+}
140
+
141
+func TestProbeFrontingTLS_ExpiredCert(t *testing.T) {
142
+	// Same trust anchor, but the cert is already expired.
143
+	cert, pool := makeCert(t, "front.example.org", time.Now().Add(-time.Hour))
144
+	addr := startTLSServer(t, cert)
145
+
146
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
147
+	defer cancel()
148
+
149
+	err := probeFrontingTLS(ctx, &net.Dialer{}, addr, "front.example.org", pool)
150
+	if err == nil {
151
+		t.Fatal("expected expiry failure, got success")
152
+	}
153
+	if !strings.Contains(err.Error(), "expired") {
154
+		t.Fatalf("expected expiry error, got: %v", err)
155
+	}
156
+}
157
+
158
+func TestProbeFrontingTLS_OverrideDialDifferentFromSNI(t *testing.T) {
159
+	// Domain-fronting override: dial one address (the listener bound to
160
+	// 127.0.0.1), but verify the cert against the secret host name. The cert
161
+	// is issued for the secret host, so verification must pass even though the
162
+	// dial target is a bare IP:port.
163
+	cert, pool := makeCert(t, "secret.example.org", time.Now().Add(24*time.Hour))
164
+	addr := startTLSServer(t, cert) // e.g. 127.0.0.1:NNNNN
165
+
166
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
167
+	defer cancel()
168
+
169
+	// dial-target (addr, an IP:port) != SNI (secret.example.org)
170
+	err := probeFrontingTLS(ctx, &net.Dialer{}, addr, "secret.example.org", pool)
171
+	if err != nil {
172
+		t.Fatalf("expected success when dialing override addr with secret-host SNI, got: %v", err)
173
+	}
174
+}

Notiek ielāde…
Atcelt
Saglabāt