cleanup(ArchivalTLSOrQUICHandshakeResult): use ArchivalBinaryData (oo…

…ni#1324)

This diff modifies model.ArchivalBinaryData to always use
ArchivalBinaryData for representing certificates rather than passing
through the more complex (and unneeded in this case!)
model.ArchivalMaybeBinaryData.

Closes ooni/probe#2165

Part of ooni/probe#2531

internal/legacy/measurex/archival.go

@@ -1,5 +1,11 @@
 package measurex
 
+//
+// Archival
+//
+// This file defines helpers to serialize to the OONI data format.
+//
+
 import (
 	"net"
 	"net/http"
@@ -8,12 +14,6 @@ import (
 	"time"
 )
 
-//
-// Archival
-//
-// This file defines helpers to serialize to the OONI data format.
-//
-
 //
 // BinaryData
 //

internal/legacy/tracex/archival.go

@@ -305,9 +305,9 @@ func NewTLSHandshakesList(begin time.Time, events []Event) (out []TLSHandshake)
 	return
 }
 
-func tlsMakePeerCerts(in [][]byte) (out []MaybeBinaryValue) {
+func tlsMakePeerCerts(in [][]byte) (out []model.ArchivalBinaryData) {
 	for _, entry := range in {
-		out = append(out, MaybeBinaryValue{Value: string(entry)})
+		out = append(out, model.ArchivalBinaryData(entry))
 	}
 	return
 }

internal/legacy/tracex/archival_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/gorilla/websocket"
+	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )
 
@@ -547,11 +548,10 @@ func TestNewTLSHandshakesList(t *testing.T) {
 			Failure:            NewFailure(io.EOF),
 			NegotiatedProtocol: "h2",
 			NoTLSVerify:        false,
-			PeerCertificates: []MaybeBinaryValue{{
-				Value: "deadbeef",
-			}, {
-				Value: "abad1dea",
-			}},
+			PeerCertificates: []model.ArchivalBinaryData{
+				model.ArchivalBinaryData("deadbeef"),
+				model.ArchivalBinaryData("abad1dea"),
+			},
 			ServerName: "x.org",
 			T:          0.055,
 			TLSVersion: "TLSv1.3",
@@ -582,11 +582,10 @@ func TestNewTLSHandshakesList(t *testing.T) {
 			Failure:            NewFailure(io.EOF),
 			NegotiatedProtocol: "h3",
 			NoTLSVerify:        false,
-			PeerCertificates: []MaybeBinaryValue{{
-				Value: "deadbeef",
-			}, {
-				Value: "abad1dea",
-			}},
+			PeerCertificates: []model.ArchivalBinaryData{
+				model.ArchivalBinaryData("deadbeef"),
+				model.ArchivalBinaryData("abad1dea"),
+			},
 			ServerName: "x.org",
 			T:          0.055,
 			TLSVersion: "TLSv1.3",

internal/measurexlite/quic_test.go

@@ -156,7 +156,7 @@ func TestNewQUICDialerWithoutResolver(t *testing.T) {
 				Failure:            &expectedFailure,
 				NegotiatedProtocol: "",
 				NoTLSVerify:        true,
-				PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+				PeerCertificates:   []model.ArchivalBinaryData{},
 				ServerName:         "dns.cloudflare.com",
 				T:                  time.Second.Seconds(),
 				Tags:               []string{"antani"},
@@ -336,7 +336,7 @@ func TestFirstQUICHandshake(t *testing.T) {
 			Failure:            nil,
 			NegotiatedProtocol: "",
 			NoTLSVerify:        true,
-			PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+			PeerCertificates:   []model.ArchivalBinaryData{},
 			ServerName:         "dns.cloudflare.com",
 			T:                  time.Second.Seconds(),
 			Tags:               []string{},
@@ -348,7 +348,7 @@ func TestFirstQUICHandshake(t *testing.T) {
 			Failure:            nil,
 			NegotiatedProtocol: "",
 			NoTLSVerify:        true,
-			PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+			PeerCertificates:   []model.ArchivalBinaryData{},
 			ServerName:         "dns.google.com",
 			T:                  time.Second.Seconds(),
 			Tags:               []string{},

internal/measurexlite/tls.go

@@ -99,47 +99,36 @@ func NewArchivalTLSOrQUICHandshakeResult(
 	}
 }
 
-// newArchivalBinaryData is a factory that adapts binary data to the
-// model.ArchivalMaybeBinaryData format.
-func newArchivalBinaryData(data []byte) model.ArchivalMaybeBinaryData {
-	// TODO(https://github.com/ooni/probe/issues/2165): we should actually extend the
-	// model's archival data format to have a pure-binary-data type for the cases in which
-	// we know in advance we're dealing with binary data.
-	return model.ArchivalMaybeBinaryData{
-		Value: string(data),
-	}
-}
-
 // TLSPeerCerts extracts the certificates either from the list of certificates
 // in the connection state or from the error that occurred.
 func TLSPeerCerts(
-	state tls.ConnectionState, err error) (out []model.ArchivalMaybeBinaryData) {
-	out = []model.ArchivalMaybeBinaryData{}
+	state tls.ConnectionState, err error) (out []model.ArchivalBinaryData) {
+	out = []model.ArchivalBinaryData{}
 
 	var x509HostnameError x509.HostnameError
 	if errors.As(err, &x509HostnameError) {
 		// Test case: https://wrong.host.badssl.com/
-		out = append(out, newArchivalBinaryData(x509HostnameError.Certificate.Raw))
+		out = append(out, model.ArchivalBinaryData(x509HostnameError.Certificate.Raw))
 		return
 	}
 
 	var x509UnknownAuthorityError x509.UnknownAuthorityError
 	if errors.As(err, &x509UnknownAuthorityError) {
 		// Test case: https://self-signed.badssl.com/. This error has
 		// never been among the ones returned by MK.
-		out = append(out, newArchivalBinaryData(x509UnknownAuthorityError.Cert.Raw))
+		out = append(out, model.ArchivalBinaryData(x509UnknownAuthorityError.Cert.Raw))
 		return
 	}
 
 	var x509CertificateInvalidError x509.CertificateInvalidError
 	if errors.As(err, &x509CertificateInvalidError) {
 		// Test case: https://expired.badssl.com/
-		out = append(out, newArchivalBinaryData(x509CertificateInvalidError.Cert.Raw))
+		out = append(out, model.ArchivalBinaryData(x509CertificateInvalidError.Cert.Raw))
 		return
 	}
 
 	for _, cert := range state.PeerCertificates {
-		out = append(out, newArchivalBinaryData(cert.Raw))
+		out = append(out, model.ArchivalBinaryData(cert.Raw))
 	}
 	return
 }

internal/measurexlite/tls_test.go

@@ -123,7 +123,7 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
 				Failure:            &expectedFailure,
 				NegotiatedProtocol: "",
 				NoTLSVerify:        true,
-				PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+				PeerCertificates:   []model.ArchivalBinaryData{},
 				ServerName:         "dns.cloudflare.com",
 				T:                  time.Second.Seconds(),
 				Tags:               []string{"antani"},
@@ -279,7 +279,7 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
 				Failure:            nil,
 				NegotiatedProtocol: "",
 				NoTLSVerify:        false,
-				PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+				PeerCertificates:   []model.ArchivalBinaryData{},
 				ServerName:         "dns.google",
 				T:                  time.Second.Seconds(),
 				Tags:               []string{},
@@ -293,7 +293,7 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
 			if len(got.PeerCertificates) != 2 {
 				t.Fatal("expected to see two certificates")
 			}
-			got.PeerCertificates = []model.ArchivalMaybeBinaryData{} // see above
+			got.PeerCertificates = []model.ArchivalBinaryData{} // see above
 			if diff := cmp.Diff(expected, got); diff != "" {
 				t.Fatal(diff)
 			}
@@ -366,7 +366,7 @@ func TestFirstTLSHandshake(t *testing.T) {
 			Failure:            nil,
 			NegotiatedProtocol: "",
 			NoTLSVerify:        true,
-			PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+			PeerCertificates:   []model.ArchivalBinaryData{},
 			ServerName:         "dns.cloudflare.com",
 			T:                  time.Second.Seconds(),
 			Tags:               []string{},
@@ -378,7 +378,7 @@ func TestFirstTLSHandshake(t *testing.T) {
 			Failure:            nil,
 			NegotiatedProtocol: "",
 			NoTLSVerify:        true,
-			PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+			PeerCertificates:   []model.ArchivalBinaryData{},
 			ServerName:         "dns.google.com",
 			T:                  time.Second.Seconds(),
 			Tags:               []string{},
@@ -400,7 +400,7 @@ func TestTLSPeerCerts(t *testing.T) {
 	tests := []struct {
 		name    string
 		args    args
-		wantOut []model.ArchivalMaybeBinaryData
+		wantOut []model.ArchivalBinaryData
 	}{{
 		name: "x509.HostnameError",
 		args: args{
@@ -411,9 +411,9 @@ func TestTLSPeerCerts(t *testing.T) {
 				},
 			},
 		},
-		wantOut: []model.ArchivalMaybeBinaryData{{
-			Value: "deadbeef",
-		}},
+		wantOut: []model.ArchivalBinaryData{
+			model.ArchivalBinaryData("deadbeef"),
+		},
 	}, {
 		name: "x509.UnknownAuthorityError",
 		args: args{
@@ -424,9 +424,9 @@ func TestTLSPeerCerts(t *testing.T) {
 				},
 			},
 		},
-		wantOut: []model.ArchivalMaybeBinaryData{{
-			Value: "deadbeef",
-		}},
+		wantOut: []model.ArchivalBinaryData{
+			model.ArchivalBinaryData("deadbeef"),
+		},
 	}, {
 		name: "x509.CertificateInvalidError",
 		args: args{
@@ -437,9 +437,9 @@ func TestTLSPeerCerts(t *testing.T) {
 				},
 			},
 		},
-		wantOut: []model.ArchivalMaybeBinaryData{{
-			Value: "deadbeef",
-		}},
+		wantOut: []model.ArchivalBinaryData{
+			model.ArchivalBinaryData("deadbeef"),
+		},
 	}, {
 		name: "successful case",
 		args: args{
@@ -452,11 +452,10 @@ func TestTLSPeerCerts(t *testing.T) {
 			},
 			err: nil,
 		},
-		wantOut: []model.ArchivalMaybeBinaryData{{
-			Value: "deadbeef",
-		}, {
-			Value: "abad1dea",
-		}},
+		wantOut: []model.ArchivalBinaryData{
+			model.ArchivalBinaryData("deadbeef"),
+			model.ArchivalBinaryData("abad1dea"),
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/model/archival.go

@@ -235,20 +235,20 @@ type ArchivalTCPConnectStatus struct {
 //
 // See https://github.com/ooni/spec/blob/master/data-formats/df-006-tlshandshake.md
 type ArchivalTLSOrQUICHandshakeResult struct {
-	Network            string                    `json:"network"`
-	Address            string                    `json:"address"`
-	CipherSuite        string                    `json:"cipher_suite"`
-	Failure            *string                   `json:"failure"`
-	SoError            *string                   `json:"so_error,omitempty"`
-	NegotiatedProtocol string                    `json:"negotiated_protocol"`
-	NoTLSVerify        bool                      `json:"no_tls_verify"`
-	PeerCertificates   []ArchivalMaybeBinaryData `json:"peer_certificates"`
-	ServerName         string                    `json:"server_name"`
-	T0                 float64                   `json:"t0,omitempty"`
-	T                  float64                   `json:"t"`
-	Tags               []string                  `json:"tags"`
-	TLSVersion         string                    `json:"tls_version"`
-	TransactionID      int64                     `json:"transaction_id,omitempty"`
+	Network            string               `json:"network"`
+	Address            string               `json:"address"`
+	CipherSuite        string               `json:"cipher_suite"`
+	Failure            *string              `json:"failure"`
+	SoError            *string              `json:"so_error,omitempty"`
+	NegotiatedProtocol string               `json:"negotiated_protocol"`
+	NoTLSVerify        bool                 `json:"no_tls_verify"`
+	PeerCertificates   []ArchivalBinaryData `json:"peer_certificates"`
+	ServerName         string               `json:"server_name"`
+	T0                 float64              `json:"t0,omitempty"`
+	T                  float64              `json:"t"`
+	Tags               []string             `json:"tags"`
+	TLSVersion         string               `json:"tls_version"`
+	TransactionID      int64                `json:"transaction_id,omitempty"`
 }
 
 //

internal/model/archival_test.go

@@ -597,9 +597,9 @@ func TestArchivalTLSOrQUICHandshakeResult(t *testing.T) {
 				SoError:            nil,
 				NegotiatedProtocol: "http/1.1",
 				NoTLSVerify:        false,
-				PeerCertificates: []model.ArchivalMaybeBinaryData{{
-					Value: string(archivalBinaryInput),
-				}},
+				PeerCertificates: []model.ArchivalBinaryData{
+					model.ArchivalBinaryData(archivalBinaryInput),
+				},
 				ServerName:    "dns.google",
 				T0:            1.0,
 				T:             2.0,
@@ -625,7 +625,7 @@ func TestArchivalTLSOrQUICHandshakeResult(t *testing.T) {
 				})(),
 				NegotiatedProtocol: "",
 				NoTLSVerify:        false,
-				PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+				PeerCertificates:   []model.ArchivalBinaryData{},
 				ServerName:         "dns.google",
 				T0:                 1.0,
 				T:                  2.0,
@@ -700,9 +700,9 @@ func TestArchivalTLSOrQUICHandshakeResult(t *testing.T) {
 				SoError:            nil,
 				NegotiatedProtocol: "http/1.1",
 				NoTLSVerify:        false,
-				PeerCertificates: []model.ArchivalMaybeBinaryData{{
-					Value: string(archivalBinaryInput),
-				}},
+				PeerCertificates: []model.ArchivalBinaryData{
+					model.ArchivalBinaryData(archivalBinaryInput),
+				},
 				ServerName:    "dns.google",
 				T0:            1.0,
 				T:             2.0,
@@ -728,7 +728,7 @@ func TestArchivalTLSOrQUICHandshakeResult(t *testing.T) {
 				})(),
 				NegotiatedProtocol: "",
 				NoTLSVerify:        false,
-				PeerCertificates:   []model.ArchivalMaybeBinaryData{},
+				PeerCertificates:   []model.ArchivalBinaryData{},
 				ServerName:         "dns.google",
 				T0:                 1.0,
 				T:                  2.0,