From 4263f1d6789a65b6261eb6f94b653f14faaac293 Mon Sep 17 00:00:00 2001 From: night1rider Date: Thu, 23 Jul 2026 17:33:42 -0600 Subject: [PATCH 1/4] wolftls: add test requiring ServerName when peer verification is enabled --- wolftls/tls_test.go | 68 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/wolftls/tls_test.go b/wolftls/tls_test.go index 42baef2..f07dbf8 100644 --- a/wolftls/tls_test.go +++ b/wolftls/tls_test.go @@ -594,6 +594,74 @@ func TestVerifyHostnameMismatch(t *testing.T) { <-errc } +// TestVerifyFailsClosedWhenServerNameEmpty checks that a client with peer +// verification enabled (InsecureSkipVerify == false) but no ServerName is +// rejected before the handshake, matching crypto/tls. +// +// The server presents a certificate that chains to the trusted CA but is +// issued for example.com / 127.0.0.1. doHandshake only performs the +// certificate name check when ServerName is non-empty, so with an empty +// ServerName the name is never checked while the trusted chain is still +// accepted. crypto/tls rejects this configuration ("either ServerName or +// InsecureSkipVerify must be specified"); wolftls should do the same. +func TestVerifyFailsClosedWhenServerNameEmpty(t *testing.T) { + certPEM := loadFile(t, certPath("server-cert.pem")) + keyPEM := loadFile(t, certPath("server-key.pem")) + caPEM := loadFile(t, certPath("ca-cert.pem")) + + serverConfig := &Config{ + Certificates: []Certificate{{ + CertPEM: certPEM, + KeyPEM: keyPEM, + }}, + } + + // Verification ON, but no ServerName. + clientConfig := &Config{ + ServerName: "", + InsecureSkipVerify: false, + RootCAPEMs: [][]byte{caPEM}, + } + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + + errc := make(chan error, 1) + go func() { + conn, err := ln.Accept() + if err != nil { + errc <- err + return + } + tlsConn := Server(conn, serverConfig) + defer tlsConn.Close() + errc <- tlsConn.Handshake() + }() + + conn, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + t.Fatalf("dial: %v", err) + } + tlsConn := Client(conn, clientConfig) + defer tlsConn.Close() + + err = tlsConn.Handshake() + if err == nil { + t.Fatal("client handshake succeeded with verification enabled but " + + "empty ServerName — the certificate name was never checked. " + + "Expected the handshake to be rejected, as crypto/tls does.") + } + t.Logf("failed closed as expected: %v", err) + + // The client rejected the config before sending a ClientHello, so close + // the connection to unblock the server's handshake read, then drain it. + tlsConn.Close() + <-errc +} + func TestMinMaxVersion(t *testing.T) { certPEM := loadFile(t, certPath("server-cert.pem")) keyPEM := loadFile(t, certPath("server-key.pem")) From 5ba07f4247e0a24a2acb96b95a8fb7edcc1ad647 Mon Sep 17 00:00:00 2001 From: night1rider Date: Thu, 23 Jul 2026 17:33:42 -0600 Subject: [PATCH 2/4] wolftls: require ServerName when peer verification is enabled --- wolftls/conn.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/wolftls/conn.go b/wolftls/conn.go index 33e2a70..28565cb 100644 --- a/wolftls/conn.go +++ b/wolftls/conn.go @@ -169,6 +169,12 @@ func (c *Conn) doHandshake() error { return errors.New("wolftls: ServerName contains NUL byte") } + // A client with verification enabled must set ServerName; without it the + // certificate name is never checked, so reject rather than proceed. + if c.isClient && !c.config.InsecureSkipVerify && c.config.ServerName == "" { + return errors.New("wolftls: either ServerName or InsecureSkipVerify must be specified") + } + // Create CTX with version-flexible method if c.isClient { c.ctx = wolfSSL.WolfSSL_CTX_new_v23_client() From a87a967431a6e848446231937ce10a17e9293e86 Mon Sep 17 00:00:00 2001 From: night1rider Date: Tue, 25 Aug 2026 00:32:28 -0600 Subject: [PATCH 3/4] wolftls: note ServerName guard matches crypto/tls --- wolftls/conn.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/wolftls/conn.go b/wolftls/conn.go index 28565cb..2051679 100644 --- a/wolftls/conn.go +++ b/wolftls/conn.go @@ -170,7 +170,9 @@ func (c *Conn) doHandshake() error { } // A client with verification enabled must set ServerName; without it the - // certificate name is never checked, so reject rather than proceed. + // certificate name is never checked, so reject rather than proceed. This is + // enforced to match crypto/tls, which requires either ServerName or + // InsecureSkipVerify when verifying. if c.isClient && !c.config.InsecureSkipVerify && c.config.ServerName == "" { return errors.New("wolftls: either ServerName or InsecureSkipVerify must be specified") } From ca95409f1e83e4ad399189d76276484fe814b2ad Mon Sep 17 00:00:00 2001 From: night1rider Date: Tue, 25 Aug 2026 03:22:07 -0600 Subject: [PATCH 4/4] wolftls: document mandatory ServerName and harden guard tests Update Config.ServerName/InsecureSkipVerify docs for the fail-closed requirement, assert the guard's error text, and add a test that InsecureSkipVerify allows an empty ServerName. --- wolftls/config.go | 14 +++++++--- wolftls/tls_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/wolftls/config.go b/wolftls/config.go index d07c5ef..6b3e231 100644 --- a/wolftls/config.go +++ b/wolftls/config.go @@ -72,13 +72,19 @@ type Certificate struct { // Config structures a TLS connection's parameters. type Config struct { - // ServerName is the value sent in the SNI extension. For clients, - // it is also used for hostname verification unless InsecureSkipVerify is set. + // ServerName is the value sent in the SNI extension and, for clients, is + // used for hostname verification. A client that verifies the peer + // (InsecureSkipVerify == false) must set ServerName: otherwise the + // certificate name is never checked, so doHandshake rejects the connection + // before the handshake, matching crypto/tls. Set InsecureSkipVerify to + // connect without a ServerName. ServerName string // InsecureSkipVerify disables wolfSSL's built-in certificate verification. - // When true, the VerifyConnection callback (if set) is still called after - // the handshake so the caller can perform custom verification. + // It is also the alternative to setting ServerName: when false, a client + // must provide ServerName so the certificate name can be checked. When + // true, the VerifyConnection callback (if set) is still called after the + // handshake so the caller can perform custom verification. // // This determines how a client verifies the server, setting it on a // server is a no-op. diff --git a/wolftls/tls_test.go b/wolftls/tls_test.go index f07dbf8..b04c91c 100644 --- a/wolftls/tls_test.go +++ b/wolftls/tls_test.go @@ -654,6 +654,12 @@ func TestVerifyFailsClosedWhenServerNameEmpty(t *testing.T) { "empty ServerName — the certificate name was never checked. " + "Expected the handshake to be rejected, as crypto/tls does.") } + // Assert it is the fail-closed guard rejecting the config, not an unrelated + // failure, so the test keeps pinning the guard if the handshake path changes. + if !strings.Contains(err.Error(), + "either ServerName or InsecureSkipVerify must be specified") { + t.Fatalf("expected the ServerName-required guard error, got: %v", err) + } t.Logf("failed closed as expected: %v", err) // The client rejected the config before sending a ClientHello, so close @@ -662,6 +668,62 @@ func TestVerifyFailsClosedWhenServerNameEmpty(t *testing.T) { <-errc } +// A client with InsecureSkipVerify set may omit ServerName: the fail-closed +// guard must not fire and the handshake must complete. This pins the guard's +// !InsecureSkipVerify condition so it is not later simplified into rejecting +// every empty-ServerName client. +func TestInsecureSkipVerifyAllowsEmptyServerName(t *testing.T) { + certPEM := loadFile(t, certPath("server-cert.pem")) + keyPEM := loadFile(t, certPath("server-key.pem")) + + serverConfig := &Config{ + Certificates: []Certificate{{ + CertPEM: certPEM, + KeyPEM: keyPEM, + }}, + } + + // Verification OFF and no ServerName: allowed, guard must not fire. + clientConfig := &Config{ + ServerName: "", + InsecureSkipVerify: true, + } + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + + errc := make(chan error, 1) + go func() { + conn, err := ln.Accept() + if err != nil { + errc <- err + return + } + tlsConn := Server(conn, serverConfig) + defer tlsConn.Close() + errc <- tlsConn.Handshake() + }() + + conn, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + t.Fatalf("dial: %v", err) + } + tlsConn := Client(conn, clientConfig) + defer tlsConn.Close() + + if err := tlsConn.Handshake(); err != nil { + t.Fatalf("client handshake should succeed with InsecureSkipVerify and "+ + "empty ServerName, got: %v", err) + } + + if err := <-errc; err != nil { + t.Fatalf("server handshake failed: %v", err) + } +} + func TestMinMaxVersion(t *testing.T) { certPEM := loadFile(t, certPath("server-cert.pem")) keyPEM := loadFile(t, certPath("server-key.pem"))