cmd/derper: allow absent SNI when using manual certs and IP literal for hostname

Updates #11776

Change-Id: I81756415feb630da093833accc3074903ebd84a7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/derper/cert.go

@@ -8,6 +8,7 @@ import (
 	"crypto/x509"
 	"errors"
 	"fmt"
+	"net"
 	"net/http"
 	"path/filepath"
 	"regexp"
@@ -54,7 +55,8 @@ func certProviderByCertMode(mode, dir, hostname string) (certProvider, error) {
 
 type manualCertManager struct {
 	cert       *tls.Certificate
-	hostname string
+	hostname   string // hostname or IP address of server
+	noHostname bool   // whether hostname is an IP address
 }
 
 // NewManualCertManager returns a cert provider which read certificate by given hostname on create.
@@ -74,7 +76,11 @@ func NewManualCertManager(certdir, hostname string) (certProvider, error) {
 	if err := x509Cert.VerifyHostname(hostname); err != nil {
 		return nil, fmt.Errorf("cert invalid for hostname %q: %w", hostname, err)
 	}
-	return &manualCertManager{cert: &cert, hostname: hostname}, nil
+	return &manualCertManager{
+		cert:       &cert,
+		hostname:   hostname,
+		noHostname: net.ParseIP(hostname) != nil,
+	}, nil
 }
 
 func (m *manualCertManager) TLSConfig() *tls.Config {
@@ -88,7 +94,7 @@ func (m *manualCertManager) TLSConfig() *tls.Config {
 }
 
 func (m *manualCertManager) getCertificate(hi *tls.ClientHelloInfo) (*tls.Certificate, error) {
-	if hi.ServerName != m.hostname {
+	if hi.ServerName != m.hostname && !m.noHostname {
 		return nil, fmt.Errorf("cert mismatch with hostname: %q", hi.ServerName)
 	}
 

cmd/derper/cert_test.go

@@ -0,0 +1,97 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package main
+
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/tls"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"math/big"
+	"net"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// Verify that in --certmode=manual mode, we can use a bare IP address
+// as the --hostname and that GetCertificate will return it.
+func TestCertIP(t *testing.T) {
+	dir := t.TempDir()
+	const hostname = "1.2.3.4"
+
+	priv, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
+	serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ip := net.ParseIP(hostname)
+	if ip == nil {
+		t.Fatalf("invalid IP address %q", hostname)
+	}
+	template := &x509.Certificate{
+		SerialNumber: serialNumber,
+		Subject: pkix.Name{
+			Organization: []string{"Tailscale Test Corp"},
+		},
+		NotBefore: time.Now(),
+		NotAfter:  time.Now().Add(30 * 24 * time.Hour),
+
+		KeyUsage:              x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		BasicConstraintsValid: true,
+		IPAddresses:           []net.IP{ip},
+	}
+	derBytes, err := x509.CreateCertificate(rand.Reader, template, template, &priv.PublicKey, priv)
+	if err != nil {
+		t.Fatal(err)
+	}
+	certOut, err := os.Create(filepath.Join(dir, hostname+".crt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}); err != nil {
+		t.Fatalf("Failed to write data to cert.pem: %v", err)
+	}
+	if err := certOut.Close(); err != nil {
+		t.Fatalf("Error closing cert.pem: %v", err)
+	}
+
+	keyOut, err := os.OpenFile(filepath.Join(dir, hostname+".key"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
+	if err != nil {
+		t.Fatal(err)
+	}
+	privBytes, err := x509.MarshalPKCS8PrivateKey(priv)
+	if err != nil {
+		t.Fatalf("Unable to marshal private key: %v", err)
+	}
+	if err := pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: privBytes}); err != nil {
+		t.Fatalf("Failed to write data to key.pem: %v", err)
+	}
+	if err := keyOut.Close(); err != nil {
+		t.Fatalf("Error closing key.pem: %v", err)
+	}
+
+	cp, err := certProviderByCertMode("manual", dir, hostname)
+	if err != nil {
+		t.Fatal(err)
+	}
+	back, err := cp.TLSConfig().GetCertificate(&tls.ClientHelloInfo{
+		ServerName: "", // no SNI
+	})
+	if err != nil {
+		t.Fatalf("GetCertificate: %v", err)
+	}
+	if back == nil {
+		t.Fatalf("GetCertificate returned nil")
+	}
+}

cmd/derper/derper.go

@@ -58,7 +58,7 @@ var (
 	configPath  = flag.String("c", "", "config file path")
 	certMode    = flag.String("certmode", "letsencrypt", "mode for getting a cert. possible options: manual, letsencrypt")
 	certDir     = flag.String("certdir", tsweb.DefaultCertDir("derper-certs"), "directory to store LetsEncrypt certs, if addr's port is :443")
-	hostname    = flag.String("hostname", "derp.tailscale.com", "LetsEncrypt host name, if addr's port is :443")
+	hostname    = flag.String("hostname", "derp.tailscale.com", "LetsEncrypt host name, if addr's port is :443. When --certmode=manual, this can be an IP address to avoid SNI checks")
 	runSTUN     = flag.Bool("stun", true, "whether to run a STUN server. It will bind to the same IP (if any) as the --addr flag value.")
 	runDERP     = flag.Bool("derp", true, "whether to run a DERP server. The only reason to set this false is if you're decommissioning a server but want to keep its bootstrap DNS functionality still running.")
 

cmd/derper/derper_test.go

@@ -6,7 +6,6 @@ package main
 import (
 	"bytes"
 	"context"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -138,5 +137,4 @@ func TestTemplate(t *testing.T) {
 	if !strings.Contains(str, "Debug info") {
 		t.Error("Output is missing debug info")
 	}
-	fmt.Println(buf.String())
 }