From a72cc7151c02ec4ba3b036a3b28eabc1e1cb9a8c Mon Sep 17 00:00:00 2001
From: DecFox <33030671+DecFox@users.noreply.github.com>
Date: Fri, 6 May 2022 14:39:54 +0530
Subject: [PATCH] tls_handshakes: add endpoint addresses to handshake list
 (#711)

* tls_handshakes: add IP addresses

* tls_handshakes: extract ip from tcp-connect

* tls_handshake: switched to trace event

* saver.go: get remoteAddr before handshake

Not sure whether this is strictly necessary, but I'd rather take the
remoteAddr before calling Handshake, just in case a future version
of the handshake closes the `conn`. In such a case, `conn.RemoteAddr`
would return `nil` and we would crash here.

This occurred to me while reading once again the diff before merging.

Co-authored-by: decfox <decfox>
Co-authored-by: Simone Basso <bassosimone@gmail.com>
---
 internal/engine/netx/archival/archival.go      | 1 +
 internal/engine/netx/archival/archival_test.go | 2 ++
 internal/engine/netx/tlsdialer/saver.go        | 2 ++
 internal/model/archival.go                     | 1 +
 4 files changed, 6 insertions(+)

diff --git a/internal/engine/netx/archival/archival.go b/internal/engine/netx/archival/archival.go
index c2aa62a..f99af94 100644
--- a/internal/engine/netx/archival/archival.go
+++ b/internal/engine/netx/archival/archival.go
@@ -314,6 +314,7 @@ func NewTLSHandshakesList(begin time.Time, events []trace.Event) []TLSHandshake
 			continue
 		}
 		out = append(out, TLSHandshake{
+			Address:            ev.Address,
 			CipherSuite:        ev.TLSCipherSuite,
 			Failure:            NewFailure(ev.Err),
 			NegotiatedProtocol: ev.TLSNegotiatedProto,
diff --git a/internal/engine/netx/archival/archival_test.go b/internal/engine/netx/archival/archival_test.go
index 5a5e630..3196aa7 100644
--- a/internal/engine/netx/archival/archival_test.go
+++ b/internal/engine/netx/archival/archival_test.go
@@ -526,6 +526,7 @@ func TestNewTLSHandshakesList(t *testing.T) {
 				Err:  websocket.ErrReadLimit,
 				Time: begin.Add(17 * time.Millisecond),
 			}, {
+				Address:            "131.252.210.176:443",
 				Name:               "tls_handshake_done",
 				Err:                io.EOF,
 				NoTLSVerify:        false,
@@ -542,6 +543,7 @@ func TestNewTLSHandshakesList(t *testing.T) {
 			}},
 		},
 		want: []archival.TLSHandshake{{
+			Address:            "131.252.210.176:443",
 			CipherSuite:        "SUITE",
 			Failure:            archival.NewFailure(io.EOF),
 			NegotiatedProtocol: "h2",
diff --git a/internal/engine/netx/tlsdialer/saver.go b/internal/engine/netx/tlsdialer/saver.go
index e7a50a6..117244b 100644
--- a/internal/engine/netx/tlsdialer/saver.go
+++ b/internal/engine/netx/tlsdialer/saver.go
@@ -29,9 +29,11 @@ func (h SaverTLSHandshaker) Handshake(
 		TLSServerName: config.ServerName,
 		Time:          start,
 	})
+	remoteAddr := conn.RemoteAddr().String()
 	tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config)
 	stop := time.Now()
 	h.Saver.Write(trace.Event{
+		Address:            remoteAddr,
 		Duration:           stop.Sub(start),
 		Err:                err,
 		Name:               "tls_handshake_done",
diff --git a/internal/model/archival.go b/internal/model/archival.go
index b140433..42e0e84 100644
--- a/internal/model/archival.go
+++ b/internal/model/archival.go
@@ -163,6 +163,7 @@ type ArchivalTCPConnectStatus struct {
 //
 // See https://github.com/ooni/spec/blob/master/data-formats/df-006-tlshandshake.md
 type ArchivalTLSOrQUICHandshakeResult struct {
+	Address            string                    `json:"address"`
 	CipherSuite        string                    `json:"cipher_suite"`
 	Failure            *string                   `json:"failure"`
 	NegotiatedProtocol string                    `json:"negotiated_protocol"`