ssl: cover the management (br-private) IP in the generated identity

changelog.d/ssl-san-management-ip.changed.md

@@ -0,0 +1,10 @@
+- **The generated TLS identity now covers the router's management address.** The
+  admin board and LuCI are served on `br-private` (`network.private.ipaddr`), but
+  the self-signed identity only carried the hostname, its `.lan` alias and the
+  **LAN** IP — so reaching the UI by the **management** IP produced a hostname
+  mismatch on top of the expected self-signed warning. `routerSANs` (and the
+  coverage check) now include the management IP (de-duplicated), so
+  `https://<management-ip>` validates. The identity stays **self-signed** — no
+  public CA issues certs for `.lan` — so the browser warning is still expected;
+  a managed deployment should install its own CA-signed cert with
+  `tollgate ssl apply <cert> <key>`.

src/cmd/tollgate-cli/ssl.go

@@ -218,7 +218,7 @@ func sslApply(args []string) error {
 	// identity.
 	var applyErr error
 	if len(args) == 0 {
-		applyErr = sslApplySelfSigned(lanIP)
+		applyErr = sslApplySelfSigned(lanIP, privateIPFromUCI())
 	} else {
 		applyErr = sslApplyRealCert(args, lanIP)
 	}
@@ -244,7 +244,7 @@ func identityInstalled() bool {
 	return err == nil && !key.IsDir() && key.Size() > 0
 }
 
-func sslApplySelfSigned(lanIP string) error {
+func sslApplySelfSigned(lanIP, privateIP string) error {
 	hostname, err := uciGet("system.@system[0].hostname")
 	if err != nil || hostname == "" {
 		hostname = "TollGate"
@@ -258,7 +258,7 @@ func sslApplySelfSigned(lanIP string) error {
 		return fmt.Errorf("failed to generate RSA key: %w", err)
 	}
 
-	tmpl := selfSignedTemplate(hostname, lanIP)
+	tmpl := selfSignedTemplate(hostname, lanIP, privateIP)
 
 	certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key)
 	if err != nil {
@@ -733,30 +733,46 @@ func parseLanIP(value string) string {
 	return ip.String()
 }
 
+// privateIPFromUCI reads network.private.ipaddr (the br-private management
+// address) and returns the bare address, or "" when it is unset (a router that
+// has not run the private-network setup, or an older install). Best-effort: a
+// missing management address must not fail the identity.
+func privateIPFromUCI() string {
+	return parseLanIP(uciGetOrEmpty("network.private.ipaddr"))
+}
+
 // routerSANs is the identity this router must present: the configured hostname,
 // its <hostname>.lan alias (dnsmasq serves the system hostname in the 'lan'
-// zone) and the LAN address the admin listeners bind.
-func routerSANs(hostname, lanIP string) (dnsNames []string, ipAddresses []net.IP) {
+// zone), and the addresses the admin listeners bind — the LAN address AND the
+// MANAGEMENT (br-private) address. The board and LuCI live on the management
+// bridge, so an operator who reaches the UI by its management IP must not hit a
+// hostname mismatch on top of the (expected) self-signed warning.
+func routerSANs(hostname, lanIP, privateIP string) (dnsNames []string, ipAddresses []net.IP) {
 	if h := strings.TrimSpace(hostname); h != "" {
 		dnsNames = append(dnsNames, h+".lan", h)
 	}
-	if ip := net.ParseIP(parseLanIP(lanIP)); ip != nil {
-		ipAddresses = append(ipAddresses, ip)
+	seen := map[string]bool{}
+	for _, raw := range []string{lanIP, privateIP} {
+		ip := net.ParseIP(parseLanIP(raw))
+		if ip != nil && !seen[ip.String()] {
+			seen[ip.String()] = true
+			ipAddresses = append(ipAddresses, ip)
+		}
 	}
 	return dnsNames, ipAddresses
 }
 
 // selfSignedTemplate is the ONE certificate template this product generates;
 // `ssl apply` and the unattended setup path both go through it.
-func selfSignedTemplate(hostname, lanIP string) *x509.Certificate {
+func selfSignedTemplate(hostname, lanIP, privateIP string) *x509.Certificate {
 	domain := strings.TrimSpace(hostname)
 	if domain == "" {
 		domain = "TollGate"
 	}
 	if !strings.HasSuffix(domain, ".lan") {
 		domain += ".lan"
 	}
-	dnsNames, ipAddresses := routerSANs(hostname, lanIP)
+	dnsNames, ipAddresses := routerSANs(hostname, lanIP, privateIP)
 	serialNumber, _ := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128))
 	return &x509.Certificate{
 		SerialNumber: serialNumber,
@@ -783,6 +799,20 @@ func routerTLSNames() (hosts, ips []string) {
 	if ip := parseLanIP(uciGetOrEmpty("network.lan.ipaddr")); ip != "" {
 		ips = append(ips, ip)
 	}
+	// The MANAGEMENT (br-private) address too: the board and LuCI answer there,
+	// so a certificate that covers it IS a usable identity for this router.
+	if ip := privateIPFromUCI(); ip != "" {
+		taken := false
+		for _, existing := range ips {
+			if existing == ip {
+				taken = true
+				break
+			}
+		}
+		if !taken {
+			ips = append(ips, ip)
+		}
+	}
 	return hosts, ips
 }
 

src/cmd/tollgate-cli/ssl_coverage_test.go

@@ -117,7 +117,7 @@ func TestParseLanIP(t *testing.T) {
 // template with a nil address makes x509.CreateCertificate fail outright, so the
 // router ended up with no identity at all.
 func TestRouterSANsCoverHostnameAndLANIP(t *testing.T) {
-	dnsNames, ipAddrs := routerSANs(testRouterHostname, testRouterLANIPWithPrefix)
+	dnsNames, ipAddrs := routerSANs(testRouterHostname, testRouterLANIPWithPrefix, "")
 
 	wantDNS := []string{testRouterHostname + ".lan", testRouterHostname}
 	if len(dnsNames) != len(wantDNS) {
@@ -140,7 +140,7 @@ func TestRouterSANsCoverHostnameAndLANIP(t *testing.T) {
 	}
 
 	// An unparseable LAN IP must not smuggle a nil address into the template.
-	if _, ips := routerSANs(testRouterHostname, "garbage"); len(ips) != 0 {
+	if _, ips := routerSANs(testRouterHostname, "garbage", ""); len(ips) != 0 {
 		t.Errorf("IPAddresses = %v, want none for an unparseable LAN IP", ips)
 	}
 }
@@ -149,7 +149,7 @@ func TestRouterSANsCoverHostnameAndLANIP(t *testing.T) {
 // sslApplySelfSigned does and asks the question a browser asks: does this
 // identity validate the address it was reached on?
 func TestSelfSignedIdentityCoversTheRouter(t *testing.T) {
-	cert := parseTestCertPEM(t, writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix)))
+	cert := parseTestCertPEM(t, writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix, "")))
 
 	for _, name := range []string{testRouterHostname, testRouterHostname + ".lan", testRouterLANIP} {
 		if err := cert.VerifyHostname(name); err != nil {
@@ -161,6 +161,38 @@ func TestSelfSignedIdentityCoversTheRouter(t *testing.T) {
 	}
 }
 
+// TestSelfSignedIdentityCoversTheManagementIP pins the rc4 fix: the board and
+// LuCI live on the br-private management bridge, so the generated identity must
+// ALSO cover the management IP (network.private.ipaddr); otherwise an operator
+// who reaches the UI by its management IP gets a hostname mismatch on top of the
+// (expected) self-signed warning.
+func TestSelfSignedIdentityCoversTheManagementIP(t *testing.T) {
+	const privateIP = "10.46.60.1"
+
+	_, ipAddrs := routerSANs(testRouterHostname, testRouterLANIPWithPrefix, privateIP)
+	if len(ipAddrs) != 2 {
+		t.Fatalf("IPAddresses = %v, want the LAN IP and the management IP", ipAddrs)
+	}
+	if !ipAddrs[0].Equal(net.ParseIP(testRouterLANIP)) || !ipAddrs[1].Equal(net.ParseIP(privateIP)) {
+		t.Errorf("IPAddresses = %v, want [%s %s]", ipAddrs, testRouterLANIP, privateIP)
+	}
+
+	// De-duplicated when the management IP equals the LAN IP.
+	if _, ips := routerSANs(testRouterHostname, testRouterLANIP, testRouterLANIP); len(ips) != 1 {
+		t.Errorf("IPAddresses = %v, want one entry when the management IP == the LAN IP", ips)
+	}
+	// An unset management IP leaves the identity as it was.
+	if _, ips := routerSANs(testRouterHostname, testRouterLANIP, ""); len(ips) != 1 {
+		t.Errorf("IPAddresses = %v, want only the LAN IP when there is no management IP", ips)
+	}
+
+	// And the generated certificate validates the management IP.
+	cert := parseTestCertPEM(t, writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix, privateIP)))
+	if err := cert.VerifyHostname(privateIP); err != nil {
+		t.Errorf("generated identity does not cover the management IP %q: %v", privateIP, err)
+	}
+}
+
 // ------------------------------------------------------------------- coverage
 
 // TestCertCoverageRejectsTheVendorPlaceholder is the regression test for the
@@ -241,7 +273,7 @@ func TestCertCoverageWithoutRouterNames(t *testing.T) {
 // --------------------------------------------------------------- file reading
 
 func TestCertCoversRouterReadsTheCertificateFile(t *testing.T) {
-	good := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIP))
+	good := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIP, ""))
 
 	t.Run("covering certificate", func(t *testing.T) {
 		stubUCI(t, map[string]string{
@@ -293,7 +325,7 @@ func TestCertCoversRouterReadsTheCertificateFile(t *testing.T) {
 // script also computes: redirect_https is '1' only while the certificate
 // uhttpd.main presents covers this router.
 func TestApplyRedirectHTTPSTracksCoverage(t *testing.T) {
-	good := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIP))
+	good := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIP, ""))
 	placeholder := writeTestCertPEM(t, &x509.Certificate{
 		Subject:  pkix.Name{CommonName: "OpenWrt"},
 		DNSNames: []string{"OpenWrt"},

src/cmd/tollgate-cli/ssl_entry_owner_test.go

@@ -75,7 +75,7 @@ func TestCertPathFollowsTheEntryListener(t *testing.T) {
 // from have to live on the same section, or the entry serves HTTPS without a
 // redirect (or a redirect towards a listener that is not there).
 func TestRedirectIsWrittenOnTheEntryListener(t *testing.T) {
-	good := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIP))
+	good := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIP, ""))
 	stubUCI(t, map[string]string{
 		"system.@system[0].hostname": testRouterHostname,
 		"network.lan.ipaddr":         testRouterLANIPWithPrefix,

src/cmd/tollgate-cli/ssl_identity_lifecycle_test.go

@@ -259,7 +259,7 @@ func TestSSLCoversJSONContract(t *testing.T) {
 	}{
 		{
 			name:       "covering identity",
-			certPath:   writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIP)),
+			certPath:   writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIP, "")),
 			wantCovers: true,
 		},
 		{

src/cmd/tollgate-cli/ui_test.go

@@ -170,7 +170,7 @@ func TestReadEntryUIConfiguredFailsClosedToBoard(t *testing.T) {
 func TestUILinksBoardMapping(t *testing.T) {
 	root := redirectUIPaths(t)
 	writeEntryUIFixture(t, root, "board", "board")
-	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix))
+	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix, ""))
 	stubUCI(t, boardUCI(cert))
 
 	got := computeUILinks()
@@ -205,7 +205,7 @@ func TestUILinksBoardMapping(t *testing.T) {
 func TestUILinksLuciMapping(t *testing.T) {
 	root := redirectUIPaths(t)
 	writeEntryUIFixture(t, root, "luci", "")
-	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix))
+	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix, ""))
 	stubUCI(t, luciUCI(cert))
 
 	got := computeUILinks()
@@ -262,7 +262,7 @@ func TestUILinksNoCoveringIdentityIsNoLink(t *testing.T) {
 func TestUILinksNoTLSListenerIsNoLink(t *testing.T) {
 	root := redirectUIPaths(t)
 	writeEntryUIFixture(t, root, "board", "board")
-	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix))
+	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix, ""))
 	seed := boardUCI(cert)
 	// LuCI's section exists but serves HTTP only, as a half-converged or
 	// deliberately-cleartext secondary pair would.
@@ -289,7 +289,7 @@ func TestUILinksNoTLSListenerIsNoLink(t *testing.T) {
 func TestUILinksJSONShapeMatchesTheSPA(t *testing.T) {
 	root := redirectUIPaths(t)
 	writeEntryUIFixture(t, root, "board", "board")
-	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix))
+	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix, ""))
 	// The board is linked; LuCI is not (placeholder on its listener) — one
 	// payload exercising both halves of the contract.
 	seed := boardUCI(cert)
@@ -354,7 +354,7 @@ func TestUILinksJSONShapeMatchesTheSPA(t *testing.T) {
 func TestUILinksPlainOutputIsNotJSON(t *testing.T) {
 	root := redirectUIPaths(t)
 	writeEntryUIFixture(t, root, "board", "board")
-	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix))
+	cert := writeTestCertPEM(t, selfSignedTemplate(testRouterHostname, testRouterLANIPWithPrefix, ""))
 	stubUCI(t, boardUCI(cert))
 
 	out, err := captureStdout(t, func() error {