Skip to content

[BUG] TLS config builders can return (nil, nil), causing nil-pointer panics in listenTCP / dialTCP #101

Description

@AlexStocks

Summary

Both TLS config builders can return (nil, nil) — a nil *tls.Config and a nil error — and none of the call sites guard against that combination. The result is a nil-pointer panic far away from the root cause.

Bug 1: return nil, err with a nil errtransport/tls.go

ServerTlsConfigBuilder.BuildTlsConfig() (line 75-78):

certPool = x509.NewCertPool()
if ok := certPool.AppendCertsFromPEM(certPem); !ok {
    log.Error("failed to parse root certificate file")
    return nil, err      // <-- err is still nil here (ReadFile succeeded)
}

ClientTlsConfigBuilder.BuildTlsConfig() (line 107-111) has the identical pattern:

ok := clientCertPool.AppendCertsFromPEM(certBytes)
if !ok {
    log.Error("failed to parse root certificate")
    return nil, err      // <-- err is nil here as well
}

In both cases the parse failure is reported as success with a nil config.

Bug 2: call sites treat "no error" as "have config"

server.listenTCP()transport/server.go (line 205-218):

if s.sslEnabled {
    if sslConfig, buildTlsConfErr := s.tlsConfigBuilder.BuildTlsConfig(); buildTlsConfErr == nil && sslConfig != nil {
        streamListener, err = tls.Listen("tcp", s.addr, sslConfig)
    }
}
...
s.streamListener = streamListener
s.addr = s.streamListener.Addr().String()   // <-- panics: streamListener is nil

If the builder returns (nil, nil) (Bug 1) — or returns a real error — streamListener stays nil and err stays nil, so the function proceeds and panics on s.streamListener.Addr().

client.dialTCP()transport/client.go (line 164-172) has the same shape:

if c.sslEnabled {
    if sslConfig, buildTlsConfErr := c.tlsConfigBuilder.BuildTlsConfig(); buildTlsConfErr == nil && sslConfig != nil {
        conn, err = tls.DialWithDialer(d, "tcp", c.addr, sslConfig)
    }
}
if err == nil && gxnet.IsSameAddr(conn.RemoteAddr(), conn.LocalAddr()) {   // <-- conn is nil, err is nil => panic

When the builder fails, conn == nil && err == nil, and conn.RemoteAddr() dereferences a nil interface.

Note the builder error is also silently swallowed in both call sites (no log, no propagation), which makes misconfigured TLS deployments very hard to diagnose.

Suggested fix

  • In tls.go, return a real error: return nil, perrors.New("failed to parse root certificate file").
  • In listenTCP/dialTCP, handle buildTlsConfErr != nil || sslConfig == nil explicitly (propagate/log the error and abort the attempt) instead of falling through with nil values.

Found during a local code-quality review of master (445fabf).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions