verify TLS certificates against the server name
Derive the TLS server name from the configured address. tls.DialWithDialer was handed the raw address, so a server reached by a name that differs from its certificate could not be verified and SNI was never sent. The dial is now split into a plain TCP connect and an explicit tls.Client with the hostname filled in. Clone the caller's TLS configuration before modifying it. Reconnects and concurrent clients share one configuration value, and setting ServerName on it would leak across connections. Apply the dialer timeout to the handshake as well. net.Dialer.Timeout covers only the TCP connect, so a peer that accepts and then goes silent could block startup forever. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
3787ad4cd1
commit
1a6c13e8aa
+50
-1
@@ -3,6 +3,7 @@ package gumble
|
||||
import (
|
||||
"crypto/tls"
|
||||
"errors"
|
||||
"fmt"
|
||||
"math"
|
||||
"net"
|
||||
"runtime"
|
||||
@@ -101,6 +102,19 @@ func Dial(config *Config) (*Client, error) {
|
||||
return DialWithDialer(new(net.Dialer), config, nil)
|
||||
}
|
||||
|
||||
// tlsServerName returns the hostname portion of a Mumble server address for
|
||||
// TLS certificate verification and SNI.
|
||||
func tlsServerName(address string) (string, error) {
|
||||
host, _, err := net.SplitHostPort(address)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("gumble: derive TLS server name from %q: %w", address, err)
|
||||
}
|
||||
if host == "" {
|
||||
return "", fmt.Errorf("gumble: derive TLS server name from %q: empty host", address)
|
||||
}
|
||||
return host, nil
|
||||
}
|
||||
|
||||
// DialWithDialer connects to the Mumble server at the address given in config.
|
||||
//
|
||||
// The function returns after the connection has been established, the initial
|
||||
@@ -113,11 +127,46 @@ func Dial(config *Config) (*Client, error) {
|
||||
func DialWithDialer(dialer *net.Dialer, config *Config, tlsConfig *tls.Config) (*Client, error) {
|
||||
start := time.Now()
|
||||
|
||||
conn, err := tls.DialWithDialer(dialer, "tcp", config.Address, tlsConfig)
|
||||
rawConn, err := dialer.Dial("tcp", config.Address)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// tls.Client cannot infer a server name from an already-open connection.
|
||||
// Clone the caller's configuration before deriving it so reconnects and
|
||||
// concurrent clients do not mutate a shared configuration.
|
||||
if tlsConfig == nil {
|
||||
tlsConfig = &tls.Config{}
|
||||
} else {
|
||||
tlsConfig = tlsConfig.Clone()
|
||||
}
|
||||
if tlsConfig.ServerName == "" {
|
||||
serverName, err := tlsServerName(config.Address)
|
||||
if err != nil {
|
||||
rawConn.Close()
|
||||
return nil, err
|
||||
}
|
||||
tlsConfig.ServerName = serverName
|
||||
}
|
||||
conn := tls.Client(rawConn, tlsConfig)
|
||||
// net.Dialer.Timeout covers only the TCP dial. Apply the same bounded
|
||||
// deadline to TLS negotiation so a peer that accepts but never responds
|
||||
// cannot block startup indefinitely.
|
||||
if dialer.Timeout > 0 {
|
||||
if err := conn.SetDeadline(start.Add(dialer.Timeout)); err != nil {
|
||||
rawConn.Close()
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
if err := conn.Handshake(); err != nil {
|
||||
rawConn.Close()
|
||||
return nil, err
|
||||
}
|
||||
if err := conn.SetDeadline(time.Time{}); err != nil {
|
||||
rawConn.Close()
|
||||
return nil, err
|
||||
}
|
||||
|
||||
client := &Client{
|
||||
Conn: NewConn(conn),
|
||||
Config: config,
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
package gumble
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestTLSServerNameUsesAddressHost(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
address string
|
||||
want string
|
||||
}{
|
||||
{"mumble.example:64738", "mumble.example"},
|
||||
{"[2001:db8::1]:64738", "2001:db8::1"},
|
||||
} {
|
||||
got, err := tlsServerName(test.address)
|
||||
if err != nil {
|
||||
t.Errorf("tlsServerName(%q): %v", test.address, err)
|
||||
continue
|
||||
}
|
||||
if got != test.want {
|
||||
t.Errorf("tlsServerName(%q) = %q, want %q", test.address, got, test.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestTLSServerNameRejectsAddressWithoutHost(t *testing.T) {
|
||||
if _, err := tlsServerName(":64738"); err == nil {
|
||||
t.Fatal("tlsServerName accepted an empty host")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user