c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(tracex): internally store just the raw certificate (#787)

Browse Source 
By just storing the raw certificate we simplify the internal data
structure we use. In turn, this enables us to write better unit tests
using github.com/google/go-cmp where we can construct the expected
result and compare with that. (Yeah, in principle we could also
construct the full certificate but I'm not sure it's worth the effort
since we basically only care about the raw certificate.)

The general idea here is to make tracex more tested. Once it's more
tested, I will create separate structs for each event, which is
something that measurex also does. Once that is done, we can start
ensuring that the code in measurex and the code in tracex do the
same thing in terms of storing observations. When also this is done,
we can then rewrite measurex to use tracex directly.

The overall goal is https://github.com/ooni/probe/issues/2035.
This commit is contained in:
Simone Basso 2022-06-02 11:07:02 +02:00 committed by GitHub
parent 83e3167ce2
commit 9354191b85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 59 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
internal/tracex/archival.go
View File

@ -5,7 +5,6 @@ package tracex
//
import (
"crypto/x509"
"errors"
"net"
"net/http"
@ -303,9 +302,9 @@ func NewTLSHandshakesList(begin time.Time, events []Event) (out []TLSHandshake)
return
}
func tlsMakePeerCerts(in []*x509.Certificate) (out []MaybeBinaryValue) {
func tlsMakePeerCerts(in [][]byte) (out []MaybeBinaryValue) {
for _, entry := range in {
out = append(out, MaybeBinaryValue{Value: string(entry.Raw)})
out = append(out, MaybeBinaryValue{Value: string(entry)})
}
return
}

19
internal/tracex/archival_test.go
View File

@ -2,7 +2,6 @@ package tracex
import (
"context"
"crypto/x509"
"errors"
"io"
"net/http"
@ -533,11 +532,10 @@ func TestNewTLSHandshakesList(t *testing.T) {
Proto: "tcp",
TLSCipherSuite: "SUITE",
TLSNegotiatedProto: "h2",
TLSPeerCerts: []*x509.Certificate{{
Raw: []byte("deadbeef"),
}, {
Raw: []byte("abad1dea"),
}},
TLSPeerCerts: [][]byte{
[]byte("deadbeef"),
[]byte("abad1dea"),
},
TLSServerName: "x.org",
TLSVersion: "TLSv1.3",
Time: begin.Add(55 * time.Millisecond),
@ -569,11 +567,10 @@ func TestNewTLSHandshakesList(t *testing.T) {
Proto: "quic",
TLSCipherSuite: "SUITE",
TLSNegotiatedProto: "h3",
TLSPeerCerts: []*x509.Certificate{{
Raw: []byte("deadbeef"),
}, {
Raw: []byte("abad1dea"),
}},
TLSPeerCerts: [][]byte{
[]byte("deadbeef"),
[]byte("abad1dea"),
},
TLSServerName: "x.org",
TLSVersion: "TLSv1.3",
Time: begin.Add(55 * time.Millisecond),

3
internal/tracex/event.go
View File

@ -5,7 +5,6 @@ package tracex
//
import (
"crypto/x509"
"errors"
"net/http"
"time"
@ -297,7 +296,7 @@ type EventValue struct {
TLSCipherSuite string `json:",omitempty"`
TLSNegotiatedProto string `json:",omitempty"`
TLSNextProtos []string `json:",omitempty"`
TLSPeerCerts []*x509.Certificate `json:",omitempty"`
TLSPeerCerts [][]byte `json:",omitempty"`
TLSVersion string `json:",omitempty"`
Time time.Time `json:",omitempty"`
Transport string `json:",omitempty"`

1
internal/tracex/quic.go
View File

@ -64,6 +64,7 @@ func (h *QUICDialerSaver) DialContext(ctx context.Context, network string,
NoTLSVerify: tlsCfg.InsecureSkipVerify,
Proto: network,
TLSNextProtos: tlsCfg.NextProtos,
TLSPeerCerts: [][]byte{},
TLSServerName: tlsCfg.ServerName,
Time: stop,
}})

6
internal/tracex/quic_test.go
View File

@ -59,7 +59,7 @@ func TestQUICDialerSaver(t *testing.T) {
if value.TLSNegotiatedProto != "h3" {
t.Fatal("invalid negotiated protocol")
}
if diff := cmp.Diff(value.TLSPeerCerts, []*x509.Certificate{}); diff != "" {
if diff := cmp.Diff(value.TLSPeerCerts, [][]byte{{1, 2, 3, 4}}); diff != "" {
t.Fatal(diff)
}
if value.TLSVersion != "TLSv1.3" {
@ -83,7 +83,9 @@ func TestQUICDialerSaver(t *testing.T) {
cs := quic.ConnectionState{}
cs.TLS.ConnectionState.CipherSuite = tls.TLS_RSA_WITH_RC4_128_SHA
cs.TLS.NegotiatedProtocol = "h3"
cs.TLS.PeerCertificates = []*x509.Certificate{}
cs.TLS.PeerCertificates = []*x509.Certificate{{
Raw: []byte{1, 2, 3, 4},
}}
cs.TLS.Version = tls.VersionTLS13
return cs
},

13
internal/tracex/tls.go
View File

@ -77,22 +77,25 @@ var _ model.TLSHandshaker = &TLSHandshakerSaver{}
// tlsPeerCerts returns the certificates presented by the peer regardless
// of whether the TLS handshake was successful
func tlsPeerCerts(state tls.ConnectionState, err error) []*x509.Certificate {
func tlsPeerCerts(state tls.ConnectionState, err error) (out [][]byte) {
var x509HostnameError x509.HostnameError
if errors.As(err, &x509HostnameError) {
// Test case: https://wrong.host.badssl.com/
return []*x509.Certificate{x509HostnameError.Certificate}
return [][]byte{x509HostnameError.Certificate.Raw}
}
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.
return []*x509.Certificate{x509UnknownAuthorityError.Cert}
return [][]byte{x509UnknownAuthorityError.Cert.Raw}
}
var x509CertificateInvalidError x509.CertificateInvalidError
if errors.As(err, &x509CertificateInvalidError) {
// Test case: https://expired.badssl.com/
return []*x509.Certificate{x509CertificateInvalidError.Cert}
return [][]byte{x509CertificateInvalidError.Cert.Raw}
}
return state.PeerCertificates
for _, cert := range state.PeerCertificates {
out = append(out, cert.Raw)
}
return
}

16
internal/tracex/tls_test.go
View File

@ -57,7 +57,7 @@ func TestTLSHandshakerSaver(t *testing.T) {
if value.TLSNegotiatedProto != "h2" {
t.Fatal("invalid negotiated protocol")
}
if diff := cmp.Diff(value.TLSPeerCerts, []*x509.Certificate{}); diff != "" {
if diff := cmp.Diff(value.TLSPeerCerts, [][]byte{{1, 2, 3, 4}}); diff != "" {
t.Fatal(diff)
}
if value.TLSVersion != "TLSv1.3" {
@ -79,7 +79,9 @@ func TestTLSHandshakerSaver(t *testing.T) {
returnedConnState := tls.ConnectionState{
CipherSuite: tls.TLS_RSA_WITH_RC4_128_SHA,
NegotiatedProtocol: "h2",
PeerCertificates: []*x509.Certificate{},
PeerCertificates: []*x509.Certificate{{
Raw: []byte{1, 2, 3, 4},
}},
Version: tls.VersionTLS13,
}
returnedConn := &mocks.TLSConn{
@ -200,7 +202,7 @@ func Test_tlsPeerCerts(t *testing.T) {
tests := []struct {
name string
args args
want []*x509.Certificate
want [][]byte
}{{
name: "no error",
args: args{
@ -208,7 +210,7 @@ func Test_tlsPeerCerts(t *testing.T) {
PeerCertificates: []*x509.Certificate{cert0},
},
},
want: []*x509.Certificate{cert0},
want: [][]byte{cert0.Raw},
}, {
name: "all empty",
args: args{},
@ -221,7 +223,7 @@ func Test_tlsPeerCerts(t *testing.T) {
Certificate: cert0,
},
},
want: []*x509.Certificate{cert0},
want: [][]byte{cert0.Raw},
}, {
name: "x509.UnknownAuthorityError",
args: args{
@ -230,7 +232,7 @@ func Test_tlsPeerCerts(t *testing.T) {
Cert: cert0,
},
},
want: []*x509.Certificate{cert0},
want: [][]byte{cert0.Raw},
}, {
name: "x509.CertificateInvalidError",
args: args{
@ -239,7 +241,7 @@ func Test_tlsPeerCerts(t *testing.T) {
Cert: cert0,
},
},
want: []*x509.Certificate{cert0},
want: [][]byte{cert0.Raw},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {