ipn/localapi,tstest/natlab: fix debug derp TLS check for sha256-raw CertName
serveDebugDERPRegion built its TLS config with ServerName: cmp.Or(derpNode.CertName, derpNode.HostName), which for a "sha256-raw:<hex>" CertName passed the raw fingerprint to Go's stock verifier as a hostname; the handshake always failed with a hostname mismatch. This is the second half of #15579; the first half (tailscaled itself failing with "unexpected multiple certs presented") was fixed in Extract a tlsConfigForNode helper that mirrors derphttp.Client.tlsClient so that sha256-raw and domain-fronting CertName values are dispatched to tlsdial.SetConfigExpectedCertHash and tlsdial.SetConfigExpectedCert respectively, falling back to HostName when CertName is empty. The core fix here was originally written by @imnuke in #19965; that PR also added a unit test in ipn/localapi/debugderp_test.go which is replaced in this commit by a new vmtest that exercises the whole stack: vnet now serves a self-signed cert valid for each fake DERP node's HostName and exposes its SHA-256 fingerprint, and vmtest grows a new SelfSignedDERPCertPinning EnvOption that swaps the test DERP map's nodes to CertName="sha256-raw:<hex>" with InsecureForTests cleared. TestSelfSignedDERPHashPinning then stands up two hard-NAT'd nodes, has them communicate over DERP, and calls DebugDERPRegion on each. Before this fix the test fails with the exact x509 hostname-mismatch error from the original bug; after, it passes. Updates #15579 Change-Id: I61f38ffebc7ac5abc962639db1ae88f5cd8633b1 Co-authored-by: Nuke <nuke@imnuke.dev> Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
committed by
Brad Fitzpatrick
co-authored by
Nuke
parent
52400dc6f4
commit
c91b7188e8
@@ -15,6 +15,7 @@ import (
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"tailscale.com/derp/derphttp"
|
||||
@@ -22,11 +23,28 @@ import (
|
||||
"tailscale.com/net/netaddr"
|
||||
"tailscale.com/net/netns"
|
||||
"tailscale.com/net/stun"
|
||||
"tailscale.com/net/tlsdial"
|
||||
"tailscale.com/tailcfg"
|
||||
"tailscale.com/types/key"
|
||||
"tailscale.com/types/nettype"
|
||||
)
|
||||
|
||||
// tlsConfigForNode builds a *tls.Config for connecting to a DERP node,
|
||||
// mirroring the logic in derphttp.Client.tlsClient so that sha256-raw cert
|
||||
// pinning and domain-fronting CertName values are handled correctly.
|
||||
func tlsConfigForNode(node *tailcfg.DERPNode) *tls.Config {
|
||||
conf := tlsdial.Config(nil, nil)
|
||||
conf.ServerName = node.HostName
|
||||
if node.CertName != "" {
|
||||
if suf, ok := strings.CutPrefix(node.CertName, "sha256-raw:"); ok {
|
||||
tlsdial.SetConfigExpectedCertHash(conf, suf)
|
||||
} else {
|
||||
tlsdial.SetConfigExpectedCert(conf, node.CertName)
|
||||
}
|
||||
}
|
||||
return conf
|
||||
}
|
||||
|
||||
func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) {
|
||||
if !h.PermitWrite {
|
||||
http.Error(w, "debug access denied", http.StatusForbidden)
|
||||
@@ -100,9 +118,7 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) {
|
||||
defer conn.Close()
|
||||
|
||||
// Upgrade to TLS and verify that works properly.
|
||||
tlsConn := tls.Client(conn, &tls.Config{
|
||||
ServerName: cmp.Or(derpNode.CertName, derpNode.HostName),
|
||||
})
|
||||
tlsConn := tls.Client(conn, tlsConfigForNode(derpNode))
|
||||
if err := tlsConn.HandshakeContext(ctx); err != nil {
|
||||
st.Errors = append(st.Errors, fmt.Sprintf("Error upgrading connection to node %q @ %q to TLS over IPv4: %v", derpNode.HostName, addr, err))
|
||||
} else {
|
||||
@@ -119,12 +135,7 @@ func (h *Handler) serveDebugDERPRegion(w http.ResponseWriter, r *http.Request) {
|
||||
defer conn.Close()
|
||||
|
||||
// Upgrade to TLS and verify that works properly.
|
||||
tlsConn := tls.Client(conn, &tls.Config{
|
||||
ServerName: cmp.Or(derpNode.CertName, derpNode.HostName),
|
||||
// TODO(andrew-d): we should print more
|
||||
// detailed failure information on if/why TLS
|
||||
// verification fails
|
||||
})
|
||||
tlsConn := tls.Client(conn, tlsConfigForNode(derpNode))
|
||||
if err := tlsConn.HandshakeContext(ctx); err != nil {
|
||||
st.Errors = append(st.Errors, fmt.Sprintf("Error upgrading connection to node %q @ %q to TLS over IPv6: %v", derpNode.HostName, addr, err))
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user