Failed experiment results in numerous nil pointer dereferences

internal/engine/experiment/smtp/smtp.go

@@ -92,7 +92,7 @@ type TestKeys struct {
 	Queries       []*model.ArchivalDNSLookupResult          `json:"queries"`
 	TCPConnect    []*model.ArchivalTCPConnectResult         `json:"tcp_connect"`
 	TLSHandshakes []*model.ArchivalTLSOrQUICHandshakeResult `json:"tls_handshakes"`
-	SMTPErrors    map[string][]*string                      `json:"smtp"`
+	Errors        map[string][]*string                      `json:"smtp"`
 	NoOpCounter   uint8                                     `json:"successful_noops"`
 	// Used for global failure (DNS resolution)
 	Failure string `json:"failure"`
@@ -117,9 +117,7 @@ func (m Measurer) ExperimentVersion() string {
 	return testVersion
 }
 
-// Manages sequential SMTP sessions to the same hostname (over different IPs)
+type TCPRunner struct {
-// don't use in parallel!
-type SMTPRunner struct {
 	trace     *measurexlite.Trace
 	logger    model.Logger
 	ctx       context.Context
@@ -127,17 +125,84 @@ type SMTPRunner struct {
 	tlsconfig *tls.Config
 	host      string
 	port      string
-	// addr is changed everytime SMTPRunner.conn(addr) is called
+}
+
+type TCPSession struct {
 	addr     string
+	port     string
+	runner   *TCPRunner
+	errors   []*string
+	tls      bool
+	raw_conn *net.Conn
+	tls_conn *net.Conn
 }
 
-func (r SMTPRunner) smtp_error(err error) {
+func (s TCPSession) Close() {
-	key := net.JoinHostPort(r.addr, r.port)
+	if s.tls {
-	// Key is initialized in conn() no need to check here
+        var conn = *s.tls_conn
-	r.tk.SMTPErrors[key] = append(r.tk.SMTPErrors[key], tracex.NewFailure(err))
+        conn.Close()
+		//*(s.tls_conn).Close()
+	} else {
+        var conn = *s.raw_conn
+        conn.Close()
+        //*(s.raw_conn).Close()
+    }
 }
 
-func (r SMTPRunner) resolve(host string) ([]string, bool) {
+func (s TCPSession) current_conn() net.Conn {
+	if s.tls {
+		return *s.tls_conn
+	} else {
+		return *s.raw_conn
+	}
+}
+
+func (r TCPRunner) conn(addr string, port string) (*TCPSession, bool) {
+	key := net.JoinHostPort(addr, port)
+	// Initialize errors
+    if r.tk.Errors == nil {
+        r.tk.Errors = make(map[string][]*string)
+    }
+	r.tk.Errors[key] = []*string{}
+
+	s := new(TCPSession)
+	if !s.conn(addr, port, r, r.tk.Errors[key]) {
+		return nil, false
+	}
+	return s, true
+}
+
+func (r TCPRunner) dial(addr string, port string) (net.Conn, error) {
+	dialer := r.trace.NewDialerWithoutResolver(r.logger)
+	conn, err := dialer.DialContext(r.ctx, "tcp", net.JoinHostPort(addr, port))
+	r.tk.TCPConnect = append(r.tk.TCPConnect, r.trace.TCPConnects()...)
+	return conn, err
+
+}
+
+func (s TCPSession) conn(addr string, port string, runner TCPRunner, errors []*string) bool {
+	// Initialize addr field and corresponding errors in TestKeys
+	s.addr = addr
+	s.port = port
+	s.tls = false
+    s.runner = &runner
+    s.errors = errors
+
+	conn, err := runner.dial(addr, port)
+	if err != nil {
+		s.error(err)
+		return false
+	}
+    s.raw_conn = &conn
+
+	return true
+}
+
+func (s TCPSession) error(err error) {
+	s.errors = append(s.errors, tracex.NewFailure(err))
+}
+
+func (r TCPRunner) resolve(host string) ([]string, bool) {
 	r.logger.Infof("Resolving DNS for %s", host)
 	resolver := r.trace.NewStdlibResolver(r.logger)
 	addrs, err := resolver.LookupHost(r.ctx, host)
@@ -151,77 +216,70 @@ func (r SMTPRunner) resolve(host string) ([]string, bool) {
 	return addrs, true
 }
 
-func (r SMTPRunner) conn(addr string) (net.Conn, bool) {
+func (s TCPSession) handshake() bool {
-	// Initialize addr field and corresponding errors in TestKeys
+	if s.tls {
-	r.addr = addr
+		// TLS already initialized...
-	if r.tk.SMTPErrors == nil {
+		return true
-		r.tk.SMTPErrors = make(map[string][]*string)
 	}
-	r.tk.SMTPErrors[net.JoinHostPort(addr, r.port)] = []*string{}
+	s.runner.logger.Infof("Starting TLS handshake with %s:%s", s.addr, s.port)
-
+	thx := s.runner.trace.NewTLSHandshakerStdlib(s.runner.logger)
-	dialer := r.trace.NewDialerWithoutResolver(r.logger)
+	tconn, _, err := thx.Handshake(s.runner.ctx, *s.raw_conn, s.runner.tlsconfig)
-	conn, err := dialer.DialContext(r.ctx, "tcp", net.JoinHostPort(r.addr, r.port))
+	s.runner.tk.TLSHandshakes = append(s.runner.tk.TLSHandshakes, s.runner.trace.FirstTLSHandshakeOrNil())
-	r.tk.TCPConnect = append(r.tk.TCPConnect, r.trace.TCPConnects()...)
 	if err != nil {
-		r.smtp_error(err)
+		s.error(err)
-		return nil, false
+		return false
-	}
-	return conn, true
 	}
 
-func (r SMTPRunner) handshake(conn net.Conn) (net.Conn, bool) {
+	s.tls = true
-	r.logger.Infof("Starting TLS handshake with %s:%s (%s)", r.host, r.port, r.addr)
+	s.tls_conn = &tconn
-	thx := r.trace.NewTLSHandshakerStdlib(r.logger)
+	s.runner.logger.Infof("Handshake succeeded")
-	tconn, _, err := thx.Handshake(r.ctx, conn, r.tlsconfig)
+	return true
-	r.tk.TLSHandshakes = append(r.tk.TLSHandshakes, r.trace.FirstTLSHandshakeOrNil())
-	if err != nil {
-		r.smtp_error(err)
-		return nil, false
-	}
-	r.logger.Infof("Handshake succeeded")
-	return tconn, true
 }
 
-func (r SMTPRunner) starttls(conn net.Conn, message string) (net.Conn, bool) {
+func (s TCPSession) starttls(message string) bool {
+	if s.tls {
+		// TLS already initialized...
+		return true
+	}
 	if message != "" {
-		r.logger.Infof("Asking for StartTLS upgrade")
+		s.runner.logger.Infof("Asking for StartTLS upgrade")
-		conn.Write([]byte(message))
+		s.current_conn().Write([]byte(message))
 	}
-	tconn, success := r.handshake(conn)
+	return s.handshake()
-	return tconn, success
 }
 
-func (r SMTPRunner) smtp(conn net.Conn, ehlo string, noop uint8) bool {
+func (s TCPSession) smtp(ehlo string, noop uint8) bool {
-	client, err := smtp.NewClient(conn, ehlo)
+	// Auto-choose plaintext/TCP session
+	client, err := smtp.NewClient(s.current_conn(), ehlo)
 	if err != nil {
-		r.smtp_error(err)
+		s.error(err)
 		return false
 	}
 	err = client.Hello(ehlo)
 	if err != nil {
-		r.smtp_error(err)
+		s.error(err)
 		return false
 	}
 
 	if noop > 0 {
-		r.logger.Infof("Trying to generate more no-op traffic")
+		s.runner.logger.Infof("Trying to generate more no-op traffic")
 		// TODO: noop counter per IP address
-		r.tk.NoOpCounter = 0
+		s.runner.tk.NoOpCounter = 0
-		for r.tk.NoOpCounter < noop {
+		for s.runner.tk.NoOpCounter < noop {
-			r.tk.NoOpCounter += 1
+			s.runner.tk.NoOpCounter += 1
-			r.logger.Infof("NoOp Iteration %d", r.tk.NoOpCounter)
+			s.runner.logger.Infof("NoOp Iteration %d", s.runner.tk.NoOpCounter)
 			err = client.Noop()
 			if err != nil {
-				r.smtp_error(err)
+				s.error(err)
 				break
 			}
 		}
 
-		if r.tk.NoOpCounter == noop {
+		if s.runner.tk.NoOpCounter == noop {
-			r.logger.Infof("Successfully generated no-op traffic")
+			s.runner.logger.Infof("Successfully generated no-op traffic")
 			return true
 		} else {
-			r.logger.Infof("Failed no-op traffic at iteration %d", r.tk.NoOpCounter)
+			s.runner.logger.Infof("Failed no-op traffic at iteration %d", s.runner.tk.NoOpCounter)
 			return false
 		}
 	}
@@ -255,7 +313,7 @@ func (m Measurer) Run(
 		ServerName:         config.host,
 	}
 
-	runner := SMTPRunner{
+	runner := TCPRunner{
 		trace:     trace,
 		logger:    log,
 		ctx:       ctx,
@@ -263,7 +321,6 @@ func (m Measurer) Run(
 		tlsconfig: &tlsconfig,
 		host:      config.host,
 		port:      config.port,
-		addr:      "",
 	}
 
 	// First resolve DNS
@@ -273,38 +330,34 @@ func (m Measurer) Run(
 	}
 
 	for _, addr := range addrs {
-		conn, success := runner.conn(addr)
+		tcp_session, success := runner.conn(addr, config.port)
-		if !success {
-			return nil
-		}
-		defer conn.Close()
-
-		if config.forced_tls {
-			// Direct TLS connection
-			tconn, success := runner.handshake(conn)
 		if !success {
 			continue
 		}
-			defer tconn.Close()
+		defer tcp_session.Close()
+
+		if config.forced_tls {
+			// Direct TLS connection
+			if !tcp_session.handshake() {
+				continue
+			}
 
 			// Try EHLO + NoOps
-			if !runner.smtp(tconn, "localhost", 10) {
+			if !tcp_session.smtp("localhost", 10) {
 				continue
 			}
 		} else {
 			// StartTLS... first try plaintext EHLO
-			if !runner.smtp(conn, "localhost", 0) {
+			if !tcp_session.smtp("localhost", 0) {
 				continue
 			}
 
 			// Upgrade via StartTLS and try EHLO + NoOps
-			tconn, success := runner.starttls(conn, "STARTTLS\n")
+			if !tcp_session.starttls("STARTTLS\n") {
-			if !success {
 				continue
 			}
-			defer tconn.Close()
 
-			if !runner.smtp(tconn, "localhost", 10) {
+			if !tcp_session.smtp("localhost", 10) {
 				continue
 			}
 		}