Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set server name only for the current broker #1701

Merged
merged 3 commits into from May 19, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 23 additions & 19 deletions broker.go
Expand Up @@ -162,29 +162,11 @@ func (b *Broker) Open(conf *Config) error {
atomic.StoreInt32(&b.opened, 0)
return
}

if conf.Net.TLS.Enable {
Logger.Printf("Using tls")
cfg := conf.Net.TLS.Config
if cfg == nil {
cfg = &tls.Config{}
}
// If no ServerName is set, infer the ServerName
// from the hostname we're connecting to.
// Gets the hostname as tls.DialWithDialer does it.
if cfg.ServerName == "" {
colonPos := strings.LastIndex(b.addr, ":")
if colonPos == -1 {
colonPos = len(b.addr)
}
hostname := b.addr[:colonPos]
cfg.ServerName = hostname
}
b.conn = tls.Client(b.conn, cfg)
b.conn = tls.Client(b.conn, validServerNameTLS(b.addr, conf.Net.TLS.Config))
}

b.conn = newBufConn(b.conn)

b.conf = conf

// Create or reuse the global metrics shared between brokers
Expand Down Expand Up @@ -1440,3 +1422,25 @@ func (b *Broker) registerCounter(name string) metrics.Counter {
b.registeredMetrics = append(b.registeredMetrics, nameForBroker)
return metrics.GetOrRegisterCounter(nameForBroker, b.conf.MetricRegistry)
}

func validServerNameTLS(addr string, conf *tls.Config) *tls.Config {
cfg := conf

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d1egoaz i think here we don't need separate variable and could use conf from args just checked if it nil

if cfg == nil {
cfg = &tls.Config{}
}
// If no ServerName is set, infer the ServerName
// from the hostname we're connecting to.
// Gets the hostname as tls.DialWithDialer does it.
if cfg.ServerName == "" {
colonPos := strings.LastIndex(addr, ":")
if colonPos == -1 {
colonPos = len(addr)
}
hostname := addr[:colonPos]
// Make a copy to avoid polluting argument or default.
c := cfg.Clone()
c.ServerName = hostname
cfg = c
}
return cfg
}
15 changes: 15 additions & 0 deletions client_tls_test.go
Expand Up @@ -210,3 +210,18 @@ func doListenerTLSTest(t *testing.T, expectSuccess bool, serverConfig, clientCon
}
}
}

func TestSetServerName(t *testing.T) {
if validServerNameTLS("kafka-server.domain.com", nil).ServerName != "kafka-server.domain.com" {
t.Fatal("Expected kafka-server.domain.com as tls.ServerName when tls config is nil")
}

if validServerNameTLS("kafka-server.domain.com", &tls.Config{}).ServerName != "kafka-server.domain.com" {
t.Fatal("Expected kafka-server.domain.com as tls.ServerName when tls config ServerName is not provided")
}

c := &tls.Config{ServerName: "kafka-server-other.domain.com"}
if validServerNameTLS("", c).ServerName != "kafka-server-other.domain.com" {
t.Fatal("Expected kafka-server-other.domain.com as tls.ServerName when tls config ServerName is provided")
}
}